Skip to content
This repository was archived by the owner on Jan 18, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 24 additions & 31 deletions contracts/core/exchange-wrappers/TakerWalletWrapper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol";
import { Authorizable } from "../../lib/Authorizable.sol";
import { LibBytes } from "../../external/0x/LibBytes.sol";
import { ITransferProxy } from "../interfaces/ITransferProxy.sol";
import { IVault } from "../interfaces/IVault.sol";
import { ERC20Wrapper } from "../../lib/ERC20Wrapper.sol";


/**
Expand All @@ -38,7 +38,6 @@ contract TakerWalletWrapper is
/* ============ State Variables ============ */

address public transferProxy;
address public vault;

/* ============ Constants ============ */

Expand All @@ -47,25 +46,27 @@ contract TakerWalletWrapper is
/* ============ Constructor ============ */

constructor(
address _transferProxy,
address _vault
address _transferProxy
)
public
{
transferProxy = _transferProxy;
vault = _vault;
}

/* ============ Public Functions ============ */

function exchange(
address _tradeOriginator,
address _taker,
uint _orderCount,
bytes _orderData
)
public
onlyAuthorized
returns(address[], uint256[])
{
address[] memory takerTokens = new address[](_orderCount);
uint256[] memory takerTokenAmounts = new uint256[](_orderCount);

uint256 scannedBytes = 32;
while (scannedBytes < _orderData.length) {

Expand All @@ -77,39 +78,31 @@ contract TakerWalletWrapper is
takerTokenAmount := mload(add(_orderData, add(scannedBytes, 32)))
}

executeTransfer(
// Transfer from taker's wallet to this wrapper
ITransferProxy(transferProxy).transfer(
takerToken,
takerTokenAmount,
_taker,
_tradeOriginator,
address(this)
);

// Ensure allowance of transfer from this wrapper to TransferProxy
ERC20Wrapper.ensureAllowance(
takerToken,
address(this),
transferProxy,
takerTokenAmount
);

// Record taker token and amount to return values
uint256 orderCount = scannedBytes >> 6;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work? Does it increment one at a time?

Copy link
Contributor

@asoong asoong Jul 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but this is particular to our workflow because we scan 64 bytes at at time, so you can think of it as counting from 0 starting on the 7th bit.

takerTokens[orderCount] = takerToken;
takerTokenAmounts[orderCount] = takerTokenAmount;

// Update scanned bytes with header and body lengths
scannedBytes = scannedBytes.add(TRANSFER_REQUEST_LENGTH);
}
}

/* ============ Private ============ */

function executeTransfer(
address _taker,
address _tradeOriginator,
address _takerToken,
uint256 _takerTokenAmount
)
private
{
ITransferProxy(transferProxy).transfer(
_takerToken,
_takerTokenAmount,
_taker,
vault
);

IVault(vault).incrementTokenOwner(
_tradeOriginator,
_takerToken,
_takerTokenAmount
);
return (takerTokens, takerTokenAmounts);
}
}
82 changes: 67 additions & 15 deletions contracts/core/extensions/CoreAccounting.sol
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ contract CoreAccounting is
external
isValidBatchTransaction(_tokenAddresses, _quantities)
{
// For each token and quantity pair, run deposit function
for (uint i = 0; i < _tokenAddresses.length; i++) {
deposit(
_tokenAddresses[i],
_quantities[i]
);
}
// Call internal batch deposit function
batchDepositInternal(
msg.sender,
msg.sender,
_tokenAddresses,
_quantities
);
}

/**
Expand Down Expand Up @@ -130,15 +130,8 @@ contract CoreAccounting is
isPositiveQuantity(_quantity)
{
// Call TransferProxy contract to transfer user tokens to Vault
ITransferProxy(state.transferProxyAddress).transfer(
_tokenAddress,
_quantity,
depositInternal(
msg.sender,
state.vaultAddress
);

// Call Vault contract to attribute deposited tokens to user
IVault(state.vaultAddress).incrementTokenOwner(
msg.sender,
_tokenAddress,
_quantity
Expand Down Expand Up @@ -171,4 +164,63 @@ contract CoreAccounting is
_quantity
);
}

/* ============ Internal Functions ============ */

/**
* Deposit any quantity of tokens into the vault.
*
* @param _tokenAddress The address of the ERC20 token
* @param _quantity The number of tokens to deposit
*/
function depositInternal(
address _from,
address _to,
address _tokenAddress,
uint _quantity
)
public
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be internal?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes...important catch. Imma push a fix immediately.

{
// Call TransferProxy contract to transfer user tokens to Vault
ITransferProxy(state.transferProxyAddress).transfer(
_tokenAddress,
_quantity,
_from,
state.vaultAddress
);

// Call Vault contract to attribute deposited tokens to user
IVault(state.vaultAddress).incrementTokenOwner(
_to,
_tokenAddress,
_quantity
);
}

/**
* Deposit multiple tokens to the vault. Quantities should be in the
* order of the addresses of the tokens being deposited.
*
* @param _tokenAddresses Array of the addresses of the ERC20 tokens
* @param _quantities Array of the number of tokens to deposit
*/
function batchDepositInternal(
address _from,
address _to,
address[] _tokenAddresses,
uint[] _quantities
)
internal
isValidBatchTransaction(_tokenAddresses, _quantities)
{
// For each token and quantity pair, run deposit function
for (uint i = 0; i < _tokenAddresses.length; i++) {
depositInternal(
_from,
_to,
_tokenAddresses[i],
_quantities[i]
);
}
}
}
45 changes: 33 additions & 12 deletions contracts/core/extensions/CoreIssuanceOrder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol";
import { CoreModifiers } from "../lib/CoreSharedModifiers.sol";
import { CoreState } from "../lib/CoreState.sol";
import { ExchangeHandler } from "../lib/ExchangeHandler.sol";
import { ICoreAccounting } from "../interfaces/ICoreAccounting.sol";
import { ICoreIssuance } from "../interfaces/ICoreIssuance.sol";
import { IExchange } from "../interfaces/IExchange.sol";
import { ITransferProxy } from "../interfaces/ITransferProxy.sol";
Expand All @@ -42,6 +43,7 @@ import { OrderLibrary } from "../lib/OrderLibrary.sol";
*/
contract CoreIssuanceOrder is
ICoreIssuance,
ICoreAccounting,
CoreState,
CoreModifiers
{
Expand All @@ -50,7 +52,7 @@ contract CoreIssuanceOrder is

/* ============ Constants ============ */

uint256 constant EXCHANGE_HEADER_LENGTH = 128;
uint256 constant EXCHANGE_HEADER_LENGTH = 160;

string constant INVALID_CANCEL_ORDER = "Only maker can cancel order.";
string constant INVALID_EXCHANGE = "Exchange does not exist.";
Expand Down Expand Up @@ -151,11 +153,11 @@ contract CoreIssuanceOrder is
settleOrder(order, _fillQuantity, _orderData);

//Issue Set
// issueInternal(
// order.makerAddress,
// order.setAddress,
// _fillQuantity
// );
issueInternal(
order.makerAddress,
order.setAddress,
_fillQuantity
);
}

/**
Expand Down Expand Up @@ -264,7 +266,7 @@ contract CoreIssuanceOrder is

// Read the order body based on header order length info
uint256 exchangeDataLength = header.totalOrdersLength.add(EXCHANGE_HEADER_LENGTH);
bytes memory orderBody = LibBytes.slice(
bytes memory bodyData = LibBytes.slice(
_orderData,
scannedBytes.add(EXCHANGE_HEADER_LENGTH),
scannedBytes.add(exchangeDataLength)
Expand All @@ -278,13 +280,28 @@ contract CoreIssuanceOrder is
exchange
);

//Call Exchange
//IExchange(header.exchange).exchange(orderBody);
// Call Exchange
address[] memory componentFillTokens = new address[](header.orderCount);
uint[] memory componentFillAmounts = new uint[](header.orderCount);
(componentFillTokens, componentFillAmounts) = IExchange(exchange).exchange(
msg.sender,
header.orderCount,
bodyData
);

// Transfer component tokens from wrapper to vault
batchDepositInternal(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow that's really nice!

exchange,
_makerAddress,
componentFillTokens,
componentFillAmounts
);

// Update scanned bytes with header and body lengths
scannedBytes = scannedBytes.add(exchangeDataLength);
makerTokenUsed += header.makerTokenAmount;
}

return makerTokenUsed;
}

Expand Down Expand Up @@ -415,18 +432,22 @@ contract CoreIssuanceOrder is

// Execute exchange orders
uint makerTokenAmountUsed = executeExchangeOrders(_orderData, _order.makerAddress);
require(makerTokenAmountUsed <= requiredMakerTokenAmount);

// Check that maker's component tokens in Vault have been incremented correctly
for (i = 0; i < _order.requiredComponents.length; i++) {
uint currentBal = IVault(state.vaultAddress).getOwnerBalance(
_order.makerAddress,
_order.requiredComponents[i]
);
//require(currentBal >= requiredBalances[i]);
require(currentBal >= requiredBalances[i]);
}

settleAccounts(_order, _fillQuantity, requiredMakerTokenAmount, makerTokenAmountUsed);
settleAccounts(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have the logic to send remaining makerTokens to the taker/arb yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's in the settle accounts function itself

_order,
_fillQuantity,
requiredMakerTokenAmount,
makerTokenAmountUsed
);

// Tally fill in orderFills mapping
state.orderFills[_order.orderHash] = state.orderFills[_order.orderHash].add(_fillQuantity);
Expand Down
43 changes: 43 additions & 0 deletions contracts/core/interfaces/ICoreAccounting.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
Copyright 2018 Set Labs Inc.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

pragma solidity 0.4.24;


/**
* @title ICoreIssuance
* @author Set Protocol
*
* The ICoreIssuance Contract defines all the functions exposed in the CoreIssuance
* extension.
*/
contract ICoreAccounting {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little weird to be using an abstract class here instead of interface (same for ICoreIssuance)...I think solidity throws warnings if methods in interfaces aren't external which wouldn't work here though but I wonder if we should be splitting up the naming of interfaces from abstract classes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we can't use internal functions in libraries so we have to define it as a contract. Which unfortunately makes us have to be more careful with inheritance when the Core inherits the IssuanceOrder extension lest we overwrite the actual logic of the batchDepositInternal function. If we want to name it something else we can do that since it isn't technically an interface thought it is intended to act as such.


/**
* Deposit multiple tokens to the vault. Quantities should be in the
* order of the addresses of the tokens being deposited.
*
* @param _tokenAddresses Array of the addresses of the ERC20 tokens
* @param _quantities Array of the number of tokens to deposit
*/
function batchDepositInternal(
address _from,
address _to,
address[] _tokenAddresses,
uint[] _quantities
)
internal;
}
9 changes: 6 additions & 3 deletions contracts/core/interfaces/IExchange.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,15 @@ interface IExchange {
/**
* Exchange some amount of takerToken for makerToken.
*
* @param _orderData Arbitrary bytes data for any information to pass to the exchange
* @return uint256 The amount of makerToken received
* @param _taker Issuance order filler
* @param _orderCount Expected number of orders to execute
* @param _orderData Arbitrary bytes data for any information to pass to the exchange
*/
function exchange(
address _taker,
uint _orderCount,
bytes _orderData
)
external
returns (uint256);
returns (address[], uint256[]);
}
Loading