From b6e59214e460845b3174f9dc90631d18371ffc53 Mon Sep 17 00:00:00 2001 From: bweick Date: Tue, 24 Jul 2018 18:13:57 -0700 Subject: [PATCH 1/4] Contract styling fixes. --- contracts/core/SetToken.sol | 63 ++++++- contracts/core/SetTokenFactory.sol | 33 ++-- contracts/core/TransferProxy.sol | 19 +-- contracts/core/Vault.sol | 39 ++--- .../exchange-wrappers/TakerWalletWrapper.sol | 27 ++- contracts/core/extensions/CoreAccounting.sol | 158 +++++++++--------- .../extensions/CoreExchangeDispatcher.sol | 4 +- contracts/core/extensions/CoreFactory.sol | 29 ++-- contracts/core/extensions/CoreInternal.sol | 62 ++++--- contracts/core/extensions/CoreIssuance.sol | 146 ++++++++-------- .../core/extensions/CoreIssuanceOrder.sol | 70 ++++++-- contracts/core/interfaces/ICore.sol | 64 +++---- contracts/core/interfaces/ICoreAccounting.sol | 10 +- contracts/core/interfaces/ICoreIssuance.sol | 10 +- contracts/core/interfaces/IExchange.sol | 8 +- contracts/core/interfaces/ISetFactory.sol | 15 +- contracts/core/interfaces/ISetToken.sol | 32 ++++ contracts/core/interfaces/ITransferProxy.sol | 6 +- contracts/core/interfaces/IVault.sol | 18 +- contracts/core/lib/CoreSharedModifiers.sol | 12 +- contracts/core/lib/CoreState.sol | 82 +++++++-- contracts/core/lib/ExchangeHandler.sol | 1 + contracts/core/lib/OrderLibrary.sol | 29 +++- .../coreIssuanceOrderScenarios.spec.ts | 1 - 24 files changed, 597 insertions(+), 341 deletions(-) diff --git a/contracts/core/SetToken.sol b/contracts/core/SetToken.sol index 80e169805..5b0dc233a 100644 --- a/contracts/core/SetToken.sol +++ b/contracts/core/SetToken.sol @@ -18,8 +18,8 @@ pragma solidity 0.4.24; import { DetailedERC20 } from "zeppelin-solidity/contracts/token/ERC20/DetailedERC20.sol"; -import { StandardToken } from "zeppelin-solidity/contracts/token/ERC20/StandardToken.sol"; import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; +import { StandardToken } from "zeppelin-solidity/contracts/token/ERC20/StandardToken.sol"; import { ISetFactory } from "./interfaces/ISetFactory.sol"; @@ -97,6 +97,7 @@ contract SetToken is } modifier isPositive(uint _quantity) { + // Require quantity passed is greater than 0 require( _quantity > 0, ZERO_QUANTITY @@ -105,7 +106,9 @@ contract SetToken is } modifier isValidDestination(address _to) { + // Confirm address is not null require(_to != address(0)); + // Confirm address is not this address require(_to != address(this)); _; } @@ -117,11 +120,12 @@ contract SetToken is * * As looping operations are expensive, checking for duplicates will be on the onus of the application developer * - * @param _components address[] A list of component address which you want to include - * @param _units uint[] A list of quantities in gWei of each component (corresponds to the {Set} of _components) - * @param _naturalUnit uint The minimum multiple of Sets that can be issued or redeeemed - * @param _name string The Set's name - * @param _symbol string the Set's symbol + * @param _factory A list of component address which you want to include + * @param _components A list of component address which you want to include + * @param _units A list of quantities in gWei of each component (corresponds to the {Set} of _components) + * @param _naturalUnit The minimum multiple of Sets that can be issued or redeeemed + * @param _name The Set's name + * @param _symbol The Set's symbol */ constructor( address _factory, @@ -172,6 +176,7 @@ contract SetToken is // add component to isComponent mapping isComponent[keccak256(abi.encodePacked(currentComponent))] = true; + // Add component data to components struct array components.push(Component({ address_: currentComponent, unit_: currentUnits @@ -226,34 +231,51 @@ contract SetToken is isCore isPositive(_quantity) { + // Require user has tokens to burn require(balances[_from] >= _quantity); + // Update token balance of user balances[_from] = balances[_from].sub(_quantity); + + // Update total supply of Set Token totalSupply_ = totalSupply_.sub(_quantity); + // Emit a transfer log with to address being 0 indicating burn emit Transfer(_from, address(0), _quantity); } - /* ============ Getters ============ */ - + /* + * Get addresses of all components in the Set + * + * @return componentAddresses Array of component tokens + */ function getComponents() public view returns(address[]) { address[] memory componentAddresses = new address[](components.length); + + // Iterate through components and get address of each component for (uint16 i = 0; i < components.length; i++) { componentAddresses[i] = components[i].address_; } return componentAddresses; } + /* + * Get units of all tokens in Set + * + * @return units Array of component units + */ function getUnits() public view returns(uint[]) { uint[] memory units = new uint[](components.length); + + // Iterate through components and get units of each component for (uint16 i = 0; i < components.length; i++) { units[i] = components[i].unit_; } @@ -262,6 +284,13 @@ contract SetToken is /* ============ Transfer Overrides ============ */ + /* + * ERC20 like transfer function but checks destination is valid + * + * @param _to The address to send Set to + * @param _value The number of Sets to send + * @return bool True on successful transfer + */ function transfer( address _to, uint256 _value @@ -270,9 +299,18 @@ contract SetToken is isValidDestination(_to) returns (bool) { + // Use inherited transfer function return super.transfer(_to, _value); } + /* + * ERC20 like transferFrom function but checks destination is valid + * + * @param _from The address to send Set from + * @param _to The address to send Set to + * @param _value The number of Sets to send + * @return bool True on successful transfer + */ function transferFrom( address _from, address _to, @@ -282,11 +320,18 @@ contract SetToken is isValidDestination(_to) returns (bool) { + // Use inherited transferFrom function return super.transferFrom(_from, _to, _value); } - /* ============ Private Helpers ============ */ + /* ============ Internal Functions ============ */ + /* + * Checks to make sure token is component of Set + * + * @param _tokenAddress Address of token being checked + * @return bool True if token is component of Set + */ function tokenIsComponent( address _tokenAddress ) diff --git a/contracts/core/SetTokenFactory.sol b/contracts/core/SetTokenFactory.sol index e4386f0a4..42df7af19 100644 --- a/contracts/core/SetTokenFactory.sol +++ b/contracts/core/SetTokenFactory.sol @@ -16,8 +16,8 @@ pragma solidity 0.4.24; -import { SetToken } from "./SetToken.sol"; import { Authorizable } from "../lib/Authorizable.sol"; +import { SetToken } from "./SetToken.sol"; /** @@ -36,8 +36,6 @@ contract SetTokenFactory // Address of the Core contract address public core; - /* ============ No Constructor ============ */ - /* ============ Setter Functions ============ */ /** @@ -46,13 +44,13 @@ contract SetTokenFactory * @param _coreAddress The address of deployed core contract */ function setCoreAddress( - address _coreAddress + address _core ) external onlyOwner { // Commit passed address to vaultAddress state variable - core = _coreAddress; + core = _core; } /* ============ Public Functions ============ */ @@ -61,12 +59,12 @@ contract SetTokenFactory * Deploys a new SetToken contract. * Can only be called by authorized core contracts. * - * @param _components address[] The address of component tokens - * @param _units uint[] The units of each component token - * @param _naturalUnit uint The minimum unit to be issued or redeemed - * @param _name string The name of the new Set - * @param _symbol string The symbol of the new Set - * @return setToken address The address of the newly created SetToken + * @param _components The address of component tokens + * @param _units The units of each component token + * @param _naturalUnit The minimum unit to be issued or redeemed + * @param _name The name of the new Set + * @param _symbol The symbol of the new Set + * @return setToken The address of the newly created SetToken */ function create( address[] _components, @@ -77,9 +75,16 @@ contract SetTokenFactory ) external onlyAuthorized - returns - (address) + returns (address) { - return new SetToken(this, _components, _units, _naturalUnit, _name, _symbol); + // Create a new SetToken contract + return new SetToken( + this, + _components, + _units, + _naturalUnit, + _name, + _symbol + ); } } diff --git a/contracts/core/TransferProxy.sol b/contracts/core/TransferProxy.sol index 59c5a02ef..808729d78 100644 --- a/contracts/core/TransferProxy.sol +++ b/contracts/core/TransferProxy.sol @@ -16,9 +16,8 @@ pragma solidity 0.4.24; - -import { Authorizable } from "../lib/Authorizable.sol"; import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; +import { Authorizable } from "../lib/Authorizable.sol"; import { ERC20Wrapper } from "../lib/ERC20Wrapper.sol"; @@ -26,8 +25,8 @@ import { ERC20Wrapper } from "../lib/ERC20Wrapper.sol"; * @title TransferProxy * @author Set Protocol * - * The proxy contract is responsible for updating token balances to assist with issuance - * and filling issuance orders. + * The transferProxy contract is responsible for moving tokens through the system to + * assist with issuance and filling issuance orders. */ contract TransferProxy is @@ -35,21 +34,19 @@ contract TransferProxy is { using SafeMath for uint256; - /* ============ No Constructor ============ */ - /* ============ External Functions ============ */ /** * Transfers tokens from an address (that has set allowance on the proxy). * Can only be called by authorized core contracts. * - * @param _tokenAddress The address of the ERC20 token + * @param _token The address of the ERC20 token * @param _quantity The number of tokens to transfer * @param _from The address to transfer from * @param _to The address to transfer to */ function transfer( - address _tokenAddress, + address _token, uint _quantity, address _from, address _to @@ -59,13 +56,13 @@ contract TransferProxy is { // Retrieve current balance of token for the receiver uint existingBalance = ERC20Wrapper.balanceOf( - _tokenAddress, + _token, _to ); // Call specified ERC20 contract to transfer tokens (via proxy). ERC20Wrapper.transferFrom( - _tokenAddress, + _token, _from, _to, _quantity @@ -73,7 +70,7 @@ contract TransferProxy is // Get new balance of transferred token for receiver uint newBalance = ERC20Wrapper.balanceOf( - _tokenAddress, + _token, _to ); diff --git a/contracts/core/Vault.sol b/contracts/core/Vault.sol index 2e9d95d47..2f892b871 100644 --- a/contracts/core/Vault.sol +++ b/contracts/core/Vault.sol @@ -17,8 +17,8 @@ pragma solidity 0.4.24; -import { Authorizable } from "../lib/Authorizable.sol"; import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; +import { Authorizable } from "../lib/Authorizable.sol"; import { ERC20Wrapper } from "../lib/ERC20Wrapper.sol"; @@ -69,20 +69,18 @@ contract Vault is _; } - /* ============ No Constructor ============ */ - - /* ============ Public Functions ============ */ + /* ============ External Functions ============ */ /* * Withdraws user's unassociated tokens to user account. Can only be * called by authorized core contracts. * - * @param _tokenAddress The address of the ERC20 token + * @param _token The address of the ERC20 token * @param _to The address to transfer token to * @param _quantity The number of tokens to transfer */ function withdrawTo( - address _tokenAddress, + address _token, address _to, uint _quantity ) @@ -93,22 +91,23 @@ contract Vault is { // Retrieve current balance of token for the vault uint existingVaultBalance = ERC20Wrapper.balanceOf( - _tokenAddress, + _token, this ); // Call specified ERC20 token contract to transfer tokens from Vault to user ERC20Wrapper.transfer( - _tokenAddress, + _token, _to, _quantity ); // Verify transfer quantity is reflected in balance uint newVaultBalance = ERC20Wrapper.balanceOf( - _tokenAddress, + _token, this ); + // Check to make sure current balances are as expected require(newVaultBalance == existingVaultBalance.sub(_quantity)); } @@ -117,12 +116,12 @@ contract Vault is * only be called by authorized core contracts. * * @param _owner The address of the token owner - * @param _tokenAddress The address of the ERC20 token + * @param _token The address of the ERC20 token * @param _quantity The number of tokens to attribute to owner */ function incrementTokenOwner( address _owner, - address _tokenAddress, + address _token, uint _quantity ) external @@ -130,7 +129,7 @@ contract Vault is isNonZero(_quantity) { // Increment balances state variable adding _quantity to user's token amount - balances[_tokenAddress][_owner] = balances[_tokenAddress][_owner].add(_quantity); + balances[_token][_owner] = balances[_token][_owner].add(_quantity); } /* @@ -138,12 +137,12 @@ contract Vault is * be called by authorized core contracts. * * @param _owner The address of the token owner - * @param _tokenAddress The address of the ERC20 token + * @param _token The address of the ERC20 token * @param _quantity The number of tokens to deattribute to owner */ function decrementTokenOwner( address _owner, - address _tokenAddress, + address _token, uint _quantity ) external @@ -152,31 +151,29 @@ contract Vault is { // Require that user has enough unassociated tokens to withdraw tokens or issue Set require( - balances[_tokenAddress][_owner] >= _quantity, + balances[_token][_owner] >= _quantity, INSUFFICIENT_BALANCE ); // Decrement balances state variable subtracting _quantity to user's token amount - balances[_tokenAddress][_owner] = balances[_tokenAddress][_owner].sub(_quantity); + balances[_token][_owner] = balances[_token][_owner].sub(_quantity); } - /* ============ Getter Functions ============ */ - /* * Get balance of particular contract for owner. * * @param _owner The address of the token owner - * @param _tokenAddress The address of the ERC20 token + * @param _token The address of the ERC20 token */ function getOwnerBalance( address _owner, - address _tokenAddress + address _token ) external view returns (uint256) { // Return owners token balance - return balances[_tokenAddress][_owner]; + return balances[_token][_owner]; } } diff --git a/contracts/core/exchange-wrappers/TakerWalletWrapper.sol b/contracts/core/exchange-wrappers/TakerWalletWrapper.sol index 46bdcc0ce..cd15bb49a 100644 --- a/contracts/core/exchange-wrappers/TakerWalletWrapper.sol +++ b/contracts/core/exchange-wrappers/TakerWalletWrapper.sol @@ -19,9 +19,9 @@ pragma experimental "ABIEncoderV2"; 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 { ERC20Wrapper } from "../../lib/ERC20Wrapper.sol"; +import { ITransferProxy } from "../interfaces/ITransferProxy.sol"; +import { LibBytes } from "../../external/0x/LibBytes.sol"; /** @@ -35,26 +35,41 @@ contract TakerWalletWrapper is { using SafeMath for uint256; - /* ============ State Variables ============ */ - - address public transferProxy; - /* ============ Constants ============ */ uint256 constant TRANSFER_REQUEST_LENGTH = 64; + /* ============ State Variables ============ */ + + address public transferProxy; + /* ============ Constructor ============ */ + /** + * Sets the transferProxy address for the contract + * + * @param _transferProxy Address of current transferProxy + */ constructor( address _transferProxy ) public { + // Set transferProxy address transferProxy = _transferProxy; } /* ============ Public Functions ============ */ + /** + * Parses taker wallet orders and transfers tokens from taker's wallet + * + * @param _taker Taker wallet address + * @param _orderCount Amount of orders in exchange request + * @param _orderData Encoded taker wallet order data + * @return takerTokens Array of token addresses executed in orders + * @return takerTokenAmounts Array of token amounts executed in orders + */ function exchange( address _taker, uint _orderCount, diff --git a/contracts/core/extensions/CoreAccounting.sol b/contracts/core/extensions/CoreAccounting.sol index 6487db0ea..70fdd6ea7 100644 --- a/contracts/core/extensions/CoreAccounting.sol +++ b/contracts/core/extensions/CoreAccounting.sol @@ -47,10 +47,10 @@ contract CoreAccounting is /* ============ Modifiers ============ */ // Confirm that all inputs are valid for batch transactions - modifier isValidBatchTransaction(address[] _tokenAddresses, uint[] _quantities) { + modifier isValidBatchTransaction(address[] _tokens, uint[] _quantities) { // Confirm an empty _addresses array is not passed require( - _tokenAddresses.length > 0, + _tokens.length > 0, ADDRESSES_MISSING ); @@ -62,78 +62,32 @@ contract CoreAccounting is // Confirm there is one quantity for every token address require( - _tokenAddresses.length == _quantities.length, + _tokens.length == _quantities.length, BATCH_INPUT_MISMATCH ); _; } - /* ============ Public Functions ============ */ + /* ============ External Functions ============ */ /** - * Deposit multiple tokens to the vault. Quantities should be in the - * order of the addresses of the tokens being deposited. + * Deposit any quantity of tokens into the vault and attribute to sender. * - * @param _tokenAddresses Array of the addresses of the ERC20 tokens - * @param _quantities Array of the number of tokens to deposit - */ - function batchDeposit( - address[] _tokenAddresses, - uint[] _quantities - ) - external - isValidBatchTransaction(_tokenAddresses, _quantities) - { - // Call internal batch deposit function - batchDepositInternal( - msg.sender, - msg.sender, - _tokenAddresses, - _quantities - ); - } - - /** - * Withdraw multiple tokens from the vault. Quantities should be in the - * order of the addresses of the tokens being withdrawn. - * - * @param _tokenAddresses Array of the addresses of the ERC20 tokens - * @param _quantities Array of the number of tokens to withdraw - */ - function batchWithdraw( - address[] _tokenAddresses, - uint[] _quantities - ) - external - isValidBatchTransaction(_tokenAddresses, _quantities) - { - // For each token and quantity pair, run withdraw function - for (uint i = 0; i < _tokenAddresses.length; i++) { - withdraw( - _tokenAddresses[i], - _quantities[i] - ); - } - } - - /** - * Deposit any quantity of tokens into the vault. - * - * @param _tokenAddress The address of the ERC20 token + * @param _token The address of the ERC20 token * @param _quantity The number of tokens to deposit */ function deposit( - address _tokenAddress, + address _token, uint _quantity ) - public + external isPositiveQuantity(_quantity) { - // Call TransferProxy contract to transfer user tokens to Vault + // Call internal deposit function depositInternal( msg.sender, msg.sender, - _tokenAddress, + _token, _quantity ); } @@ -141,58 +95,106 @@ contract CoreAccounting is /** * Withdraw a quantity of tokens from the vault. * - * @param _tokenAddress The address of the ERC20 token + * @param _token The address of the ERC20 token * @param _quantity The number of tokens to withdraw */ function withdraw( - address _tokenAddress, + address _token, uint _quantity ) - public + external { // Call Vault contract to deattribute tokens to user - IVault(state.vaultAddress).decrementTokenOwner( + IVault(state.vault).decrementTokenOwner( msg.sender, - _tokenAddress, + _token, _quantity ); // Call Vault to withdraw tokens from Vault to user - IVault(state.vaultAddress).withdrawTo( - _tokenAddress, + IVault(state.vault).withdrawTo( + _token, msg.sender, _quantity ); } + /** + * Deposit multiple tokens to the vault and attribute to sender. + * Quantities should be in the order of the addresses of the tokens being deposited. + * + * @param _tokens Array of the addresses of the ERC20 tokens + * @param _quantities Array of the number of tokens to deposit + */ + function batchDeposit( + address[] _tokens, + uint[] _quantities + ) + external + isValidBatchTransaction(_tokens, _quantities) + { + // Call internal batch deposit function + batchDepositInternal( + msg.sender, + msg.sender, + _tokens, + _quantities + ); + } + + /** + * Withdraw multiple tokens from the vault. Quantities should be in the + * order of the addresses of the tokens being withdrawn. + * + * @param _tokens Array of the addresses of the ERC20 tokens + * @param _quantities Array of the number of tokens to withdraw + */ + function batchWithdraw( + address[] _tokens, + uint[] _quantities + ) + external + isValidBatchTransaction(_tokens, _quantities) + { + // For each token and quantity pair, run withdraw function + for (uint i = 0; i < _tokens.length; i++) { + withdraw( + _tokens[i], + _quantities[i] + ); + } + } + /* ============ Internal Functions ============ */ /** * Deposit any quantity of tokens into the vault. * - * @param _tokenAddress The address of the ERC20 token + * @param _from Address depositing token + * @param _to Address to credit for deposit + * @param _token Address of token being deposited * @param _quantity The number of tokens to deposit */ function depositInternal( address _from, address _to, - address _tokenAddress, + address _token, uint _quantity ) internal { // Call TransferProxy contract to transfer user tokens to Vault - ITransferProxy(state.transferProxyAddress).transfer( - _tokenAddress, + ITransferProxy(state.transferProxy).transfer( + _token, _quantity, _from, - state.vaultAddress + state.vault ); // Call Vault contract to attribute deposited tokens to user - IVault(state.vaultAddress).incrementTokenOwner( + IVault(state.vault).incrementTokenOwner( _to, - _tokenAddress, + _token, _quantity ); } @@ -201,24 +203,26 @@ contract CoreAccounting is * 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 + * @param _from Address depositing tokens + * @param _to Address to credit for deposits + * @param _tokens Addresses of tokens being deposited + * @param _quantities The quantities of tokens to deposit */ function batchDepositInternal( address _from, address _to, - address[] _tokenAddresses, + address[] _tokens, uint[] _quantities ) internal - isValidBatchTransaction(_tokenAddresses, _quantities) + isValidBatchTransaction(_tokens, _quantities) { - // For each token and quantity pair, run deposit function - for (uint i = 0; i < _tokenAddresses.length; i++) { + // For each token and quantity pair, run depositInternal function + for (uint i = 0; i < _tokens.length; i++) { depositInternal( _from, _to, - _tokenAddresses[i], + _tokens[i], _quantities[i] ); } diff --git a/contracts/core/extensions/CoreExchangeDispatcher.sol b/contracts/core/extensions/CoreExchangeDispatcher.sol index 007b512bc..be62c7abe 100644 --- a/contracts/core/extensions/CoreExchangeDispatcher.sol +++ b/contracts/core/extensions/CoreExchangeDispatcher.sol @@ -40,10 +40,10 @@ contract CoreExchangeDispatcher is address _exchange ); - /* ============ Setter Functions ============ */ + /* ============ External Functions ============ */ /** - * Register exchange address into mapping of exchanges + * Register exchange address into mapping of exchanges * * @param _exchangeId Enumeration of exchange * @param _exchange Exchange address to set diff --git a/contracts/core/extensions/CoreFactory.sol b/contracts/core/extensions/CoreFactory.sol index 3671b8b9f..8b6cc9bda 100644 --- a/contracts/core/extensions/CoreFactory.sol +++ b/contracts/core/extensions/CoreFactory.sol @@ -26,7 +26,7 @@ import { ISetFactory } from "../interfaces/ISetFactory.sol"; * @title Core Factory * @author Set Protocol * - * The CoreCreate contract contains public set token operations + * The CoreFactory contract contains Set Token creation operations */ contract CoreFactory is CoreState, @@ -43,7 +43,7 @@ contract CoreFactory is event SetTokenCreated( address indexed _setTokenAddress, - address _factoryAddress, + address _factory, address[] _components, uint[] _units, uint _naturalUnit, @@ -52,21 +52,21 @@ contract CoreFactory is ); - /* ============ Public Functions ============ */ + /* ============ External Functions ============ */ /** * Deploys a new Set Token and adds it to the valid list of SetTokens * - * @param _factoryAddress address The address of the Factory to create from - * @param _components address[] The address of component tokens - * @param _units uint[] The units of each component token - * @param _naturalUnit uint The minimum unit to be issued or redeemed - * @param _name string The name of the new Set - * @param _symbol string The symbol of the new Set - * @return setTokenAddress address The address of the new Set + * @param _factory The address of the Factory to create from + * @param _components The address of component tokens + * @param _units The units of each component token + * @param _naturalUnit The minimum unit to be issued or redeemed + * @param _name The name of the new Set + * @param _symbol The symbol of the new Set + * @return setTokenAddress The address of the new Set */ function create( - address _factoryAddress, + address _factory, address[] _components, uint[] _units, uint _naturalUnit, @@ -74,11 +74,11 @@ contract CoreFactory is string _symbol ) external - isValidFactory(_factoryAddress) + isValidFactory(_factory) returns (address) { // Create the Set - address newSetTokenAddress = ISetFactory(_factoryAddress).create( + address newSetTokenAddress = ISetFactory(_factory).create( _components, _units, _naturalUnit, @@ -92,9 +92,10 @@ contract CoreFactory is // Add Set to the array of tracked Sets state.setTokens.push(newSetTokenAddress); + // Emit Set Token creation log emit SetTokenCreated( newSetTokenAddress, - _factoryAddress, + _factory, _components, _units, _naturalUnit, diff --git a/contracts/core/extensions/CoreInternal.sol b/contracts/core/extensions/CoreInternal.sol index c6e783484..b80a208e0 100644 --- a/contracts/core/extensions/CoreInternal.sol +++ b/contracts/core/extensions/CoreInternal.sol @@ -25,76 +25,84 @@ import { CoreState } from "../lib/CoreState.sol"; * @title Core Internal * @author Set Protocol * - * The CoreInternal contract contains methods to alter state that tracks contract - * addresses that need to interact with Core. + * The CoreInternal contract contains methods to alter state of variables that track + * Core dependency addresses. */ contract CoreInternal is Ownable, CoreState, CoreModifiers { - /* ============ Setter Functions ============ */ + /* ============ External Functions ============ */ /** * Set vaultAddress. Can only be set by owner of Core. * - * @param _vaultAddress The address of the Vault + * @param _vault The address of the Vault */ function setVaultAddress( - address _vaultAddress + address _vault ) external onlyOwner { // Commit passed address to vaultAddress state variable - state.vaultAddress = _vaultAddress; + state.vault = _vault; } /** * Set transferProxyAddress. Can only be set by owner of Core. * - * @param _transferProxyAddress The address of the TransferProxy + * @param _transferProxy The address of the TransferProxy */ function setTransferProxyAddress( - address _transferProxyAddress + address _transferProxy ) external onlyOwner { // Commit passed address to transferProxyAddress state variable - state.transferProxyAddress = _transferProxyAddress; + state.transferProxy = _transferProxy; } /** - * Add a factory to the mapping of tracked factories. + * Add a factory to the mapping of tracked factories. Can only be set by + * owner of Core. * - * @param _factoryAddress The address of the SetTokenFactory to enable + * @param _factory The address of the SetTokenFactory to enable */ function enableFactory( - address _factoryAddress + address _factory ) external onlyOwner { - state.validFactories[_factoryAddress] = true; - state.factories.push(_factoryAddress); + // Mark as true in validFactories mapping + state.validFactories[_factory] = true; + + // Add to factories array + state.factories.push(_factory); } /** - * Disable a factory in the mapping of tracked factories. + * Disable a factory in the mapping of tracked factories. Can only be disabled + * by owner of Core. * - * @param _factoryAddress The address of the SetTokenFactory to disable + * @param _factory The address of the SetTokenFactory to disable */ function disableFactory( - address _factoryAddress + address _factory ) external onlyOwner - isValidFactory(_factoryAddress) + isValidFactory(_factory) { - state.validFactories[_factoryAddress] = false; + // Mark as false in validFactories mapping + state.validFactories[_factory] = false; + + // Find and remove factory from factories array for (uint256 i = 0; i < state.factories.length; i++) { - if (state.factories[i] == _factoryAddress) { + if (state.factories[i] == _factory) { state.factories[i] = state.factories[state.factories.length - 1]; state.factories.length -= 1; break; @@ -103,20 +111,24 @@ contract CoreInternal is } /** - * Disable a set token in the mapping of tracked set tokens. + * Disable a set token in the mapping of tracked set tokens. Can only + * be disables by owner of Core. * - * @param _setAddress The address of the SetToken to remove + * @param _set The address of the SetToken to disable */ function disableSet( - address _setAddress + address _set ) external onlyOwner - isValidSet(_setAddress) + isValidSet(_set) { + // Mark as false in validSet mapping state.validSets[_setAddress] = false; + + // Find and remove from setTokens array for (uint256 i = 0; i < state.setTokens.length; i++) { - if (state.setTokens[i] == _setAddress) { + if (state.setTokens[i] == _set) { state.setTokens[i] = state.setTokens[state.setTokens.length - 1]; state.setTokens.length -= 1; break; diff --git a/contracts/core/extensions/CoreIssuance.sol b/contracts/core/extensions/CoreIssuance.sol index 7944daabd..62c970c7e 100644 --- a/contracts/core/extensions/CoreIssuance.sol +++ b/contracts/core/extensions/CoreIssuance.sol @@ -28,7 +28,8 @@ import { IVault } from "../interfaces/IVault.sol"; * @title Core Issuance * @author Set Protocol * - * The CoreIssuance contract contains public set token operations + * The CoreIssuance contract contains function related to issuing and + * redeeming Sets. */ contract CoreIssuance is CoreState, @@ -45,53 +46,59 @@ contract CoreIssuance is uint _quantity ); - /* ============ Public Functions ============ */ + /* ============ External Functions ============ */ /** * Exchanges components for Set Tokens * - * @param _setAddress Address of set to issue + * @param _set Address of set to issue * @param _quantity Quantity of set to issue */ function issue( - address _setAddress, + address _set, uint _quantity ) external - isValidSet(_setAddress) + isValidSet(_set) isPositiveQuantity(_quantity) - isNaturalUnitMultiple(_quantity, _setAddress) + isNaturalUnitMultiple(_quantity, _set) { // Run issueInternal - issueInternal(msg.sender, _setAddress, _quantity); + issueInternal( + msg.sender, + _set, + _quantity + ); } /** * Function to convert Set Tokens into underlying components * - * @param _setAddress The address of the Set token + * @param _set The address of the Set token * @param _quantity The number of tokens to redeem */ function redeem( - address _setAddress, + address _set, uint _quantity ) external - isValidSet(_setAddress) + isValidSet(_set) isPositiveQuantity(_quantity) - isNaturalUnitMultiple(_quantity, _setAddress) + isNaturalUnitMultiple(_quantity, _set) { // Burn the Set token (thereby decrementing the SetToken balance) - ISetToken(_setAddress).burn(msg.sender, _quantity); + ISetToken(_set).burn(msg.sender, _quantity); - uint naturalUnit = ISetToken(_setAddress).naturalUnit(); + // Fetch Set token properties + uint naturalUnit = ISetToken(_set).naturalUnit(); + address[] memory components = ISetToken(_set).getComponents(); + uint[] memory units = ISetToken(_set).getUnits(); // Transfer the underlying tokens to the corresponding token balances - address[] memory components = ISetToken(_setAddress).getComponents(); - uint[] memory units = ISetToken(_setAddress).getUnits(); for (uint16 i = 0; i < components.length; i++) { address currentComponent = components[i]; + // Calculate redeemable amount of tokens uint tokenValue = calculateTransferValue( units[i], naturalUnit, @@ -99,14 +106,14 @@ contract CoreIssuance is ); // Decrement the Set amount - IVault(state.vaultAddress).decrementTokenOwner( - _setAddress, + IVault(state.vault).decrementTokenOwner( + _set, currentComponent, tokenValue ); // Increment the component amount - IVault(state.vaultAddress).incrementTokenOwner( + IVault(state.vault).incrementTokenOwner( msg.sender, currentComponent, tokenValue @@ -122,27 +129,27 @@ contract CoreIssuance is * allows you to optionally specify which component tokens to transfer * back to the user. The rest will remain in the vault under the users' addresses. * - * @param _setAddress The address of the Set token + * @param _set The address of the Set token * @param _quantity The number of tokens to redeem * @param _toWithdraw Mask of indexes of tokens to withdraw */ function redeemAndWithdraw( - address _setAddress, + address _set, uint _quantity, uint _toWithdraw ) external - isValidSet(_setAddress) + isValidSet(_set) isPositiveQuantity(_quantity) - isNaturalUnitMultiple(_quantity, _setAddress) + isNaturalUnitMultiple(_quantity, _set) { // Burn the Set token (thereby decrementing the SetToken balance) - ISetToken(_setAddress).burn(msg.sender, _quantity); + ISetToken(_set).burn(msg.sender, _quantity); // Fetch Set token properties - uint naturalUnit = ISetToken(_setAddress).naturalUnit(); - address[] memory components = ISetToken(_setAddress).getComponents(); - uint[] memory units = ISetToken(_setAddress).getUnits(); + uint naturalUnit = ISetToken(_set).naturalUnit(); + address[] memory components = ISetToken(_set).getComponents(); + uint[] memory units = ISetToken(_set).getUnits(); // Loop through and decrement vault balances for the set, withdrawing if requested for (uint i = 0; i < components.length; i++) { @@ -154,8 +161,8 @@ contract CoreIssuance is ); // Decrement the component amount owned by the Set - IVault(state.vaultAddress).decrementTokenOwner( - _setAddress, + IVault(state.vault).decrementTokenOwner( + _set, components[i], componentQuantity ); @@ -166,14 +173,14 @@ contract CoreIssuance is // Transfer to user if component is included in _toWithdraw if ((_toWithdraw & componentBitIndex) != 0) { // Call Vault to withdraw tokens from Vault to user - IVault(state.vaultAddress).withdrawTo( + IVault(state.vault).withdrawTo( components[i], msg.sender, componentQuantity ); } else { // Otherwise, increment the component amount for the user - IVault(state.vaultAddress).incrementTokenOwner( + IVault(state.vault).incrementTokenOwner( msg.sender, components[i], componentQuantity @@ -182,47 +189,26 @@ contract CoreIssuance is } } - /* ============ Private Functions ============ */ - - /** - * Function to calculate the transfer value of a component given quantity of Set - * - * @param _componentUnits The units of the component token - * @param _naturalUnit The natural unit of the Set token - * @param _quantity The number of tokens being redeem - */ - function calculateTransferValue( - uint _componentUnits, - uint _naturalUnit, - uint _quantity - ) - pure - internal - returns (uint) - { - return _quantity.div(_naturalUnit).mul(_componentUnits); - } - /* ============ Internal Functions ============ */ /** * Exchanges components for Set Tokens, accepting any owner * * @param _owner Address to issue set to - * @param _setAddress Address of set to issue + * @param _set Address of set to issue * @param _quantity Quantity of set to issue */ function issueInternal( address _owner, - address _setAddress, + address _set, uint _quantity ) internal { - // Fetch set token components - address[] memory components = ISetToken(_setAddress).getComponents(); - // Fetch set token component units - uint[] memory units = ISetToken(_setAddress).getUnits(); + // Fetch set token properties + uint naturalUnit = ISetToken(_set).naturalUnit(); + address[] memory components = ISetToken(_set).getComponents(); + uint[] memory units = ISetToken(_set).getUnits(); // Inspect vault for required component quantity for (uint16 i = 0; i < components.length; i++) { @@ -232,15 +218,19 @@ contract CoreIssuance is // Calculate required component quantity uint requiredComponentQuantity = calculateTransferValue( unit, - ISetToken(_setAddress).naturalUnit(), + naturalUnit, _quantity ); // Fetch component quantity in vault - uint vaultBalance = IVault(state.vaultAddress).getOwnerBalance(_owner, component); + uint vaultBalance = IVault(state.vault).getOwnerBalance( + _owner, + component + ); + if (vaultBalance >= requiredComponentQuantity) { // Decrement vault balance by the required component quantity - IVault(state.vaultAddress).decrementTokenOwner( + IVault(state.vault).decrementTokenOwner( _owner, component, requiredComponentQuantity @@ -248,7 +238,7 @@ contract CoreIssuance is } else { // User has less than required amount, decrement the vault by full balance if (vaultBalance > 0) { - IVault(state.vaultAddress).decrementTokenOwner( + IVault(state.vault).decrementTokenOwner( _owner, component, vaultBalance @@ -259,30 +249,52 @@ contract CoreIssuance is uint amountToDeposit = requiredComponentQuantity.sub(vaultBalance); // Transfer the remainder component quantity required to vault - ITransferProxy(state.transferProxyAddress).transfer( + ITransferProxy(state.transferProxy).transfer( component, requiredComponentQuantity.sub(vaultBalance), _owner, - state.vaultAddress + state.vault ); // Log transfer of component from issuer wallet emit IssuanceComponentDeposited( - _setAddress, + _set, component, amountToDeposit ); } // Increment the vault balance of the set token for the component - IVault(state.vaultAddress).incrementTokenOwner( - _setAddress, + IVault(state.vault).incrementTokenOwner( + _set, component, requiredComponentQuantity ); } // Issue set token - ISetToken(_setAddress).mint(_owner, _quantity); + ISetToken(_set).mint( + _owner, + _quantity + ); + } + + /** + * Function to calculate the transfer value of a component given quantity of Set + * + * @param _componentUnits The units of the component token + * @param _naturalUnit The natural unit of the Set token + * @param _quantity The number of tokens being redeem + */ + function calculateTransferValue( + uint _componentUnits, + uint _naturalUnit, + uint _quantity + ) + pure + internal + returns (uint) + { + return _quantity.div(_naturalUnit).mul(_componentUnits); } } diff --git a/contracts/core/extensions/CoreIssuanceOrder.sol b/contracts/core/extensions/CoreIssuanceOrder.sol index 1e583069f..9c36c4904 100644 --- a/contracts/core/extensions/CoreIssuanceOrder.sol +++ b/contracts/core/extensions/CoreIssuanceOrder.sol @@ -26,9 +26,9 @@ 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 { ISetToken } from "../interfaces/ISetToken.sol"; import { ITransferProxy } from "../interfaces/ITransferProxy.sol"; import { IVault } from "../interfaces/IVault.sol"; -import { ISetToken } from "../interfaces/ISetToken.sol"; import { LibBytes } from "../../external/0x/LibBytes.sol"; import { OrderLibrary } from "../lib/OrderLibrary.sol"; @@ -112,6 +112,7 @@ contract CoreIssuanceOrder is ) external { + // Create IssuanceOrder struct OrderLibrary.IssuanceOrder memory order = OrderLibrary.IssuanceOrder({ setAddress: _addresses[0], makerAddress: _addresses[1], @@ -152,7 +153,11 @@ contract CoreIssuanceOrder is ); // Settle Order - settleOrder(order, _fillQuantity, _orderData); + settleOrder( + order, + _fillQuantity, + _orderData + ); // Issue Set issueInternal( @@ -181,6 +186,7 @@ contract CoreIssuanceOrder is external isPositiveQuantity(_cancelQuantity) { + // Create IssuanceOrder struct OrderLibrary.IssuanceOrder memory order = OrderLibrary.IssuanceOrder({ setAddress: _addresses[0], makerAddress: _addresses[1], @@ -219,6 +225,7 @@ contract CoreIssuanceOrder is // Tally cancel in orderCancels mapping state.orderCancels[order.orderHash] = state.orderCancels[order.orderHash].add(canceledAmount); + // Emit cancel order event emit LogCancel( order.setAddress, order.makerAddress, @@ -237,7 +244,9 @@ contract CoreIssuanceOrder is * header represents a batch of orders for a particular exchange (0x, KNC, taker). Additional * information such as makerToken is encoded so it can be used to facilitate exchange orders * - * @param _orderData Bytes array containing the exchange orders to execute + * @param _orderData Bytes array containing the exchange orders to execute + * @param _makerAddress Issuance order maker address + * @return makerTokenUsed Amount of maker token used to execute orders */ function executeExchangeOrders( bytes _orderData, @@ -249,7 +258,7 @@ contract CoreIssuanceOrder is uint256 scannedBytes; uint256 makerTokenUsed; while (scannedBytes < _orderData.length) { - // Read the next exchange order header + // Read and parse the next exchange order header bytes memory headerData = LibBytes.slice( _orderData, scannedBytes, @@ -277,7 +286,7 @@ contract CoreIssuanceOrder is ); // Transfer header.makerTokenAmount to Exchange Wrapper - ITransferProxy(state.transferProxyAddress).transfer( + ITransferProxy(state.transferProxy).transfer( header.makerTokenAddress, header.makerTokenAmount, _makerAddress, @@ -348,6 +357,14 @@ contract CoreIssuanceOrder is ); } + /** + * Calculate and send tokens to taker and relayer + * + * @param _order IssuanceOrder object containing order params + * @param _fillQuantity Quantity of Set to be filled + * @param _requiredMakerTokenAmount Max amount of maker token available to fill orders + * @param _makerTokenUsed Amount of maker token used to fill order + */ function settleAccounts( OrderLibrary.IssuanceOrder _order, uint _fillQuantity, @@ -360,7 +377,7 @@ contract CoreIssuanceOrder is uint toTaker = _requiredMakerTokenAmount.sub(_makerTokenUsed); // Send left over maker token balance to taker - ITransferProxy(state.transferProxyAddress).transfer( + ITransferProxy(state.transferProxy).transfer( _order.makerToken, toTaker, _order.makerAddress, @@ -368,22 +385,27 @@ contract CoreIssuanceOrder is ); // Calculate fees required - uint requiredFees = OrderLibrary.getPartialAmount(_order.relayerTokenAmount, _fillQuantity, _order.quantity); + uint requiredFees = OrderLibrary.getPartialAmount( + _order.relayerTokenAmount, + _fillQuantity, + _order.quantity + ); //Send fees to relayer - ITransferProxy(state.transferProxyAddress).transfer( + ITransferProxy(state.transferProxy).transfer( _order.relayerToken, requiredFees, _order.makerAddress, _order.relayerAddress ); - ITransferProxy(state.transferProxyAddress).transfer( + ITransferProxy(state.transferProxy).transfer( _order.relayerToken, requiredFees, msg.sender, _order.relayerAddress ); + // Emit fill order event emit LogFill( _order.setAddress, _order.makerAddress, @@ -398,6 +420,14 @@ contract CoreIssuanceOrder is ); } + /** + * Check exchange orders acquire correct amount of tokens. Settle accounts for taker + * and relayer. + * + * @param _order IssuanceOrder object containing order params + * @param _fillQuantity Quantity of Set to be filled + * @param _orderData Bytestring encoding all exchange order data + */ function settleOrder( OrderLibrary.IssuanceOrder _order, uint _fillQuantity, @@ -416,35 +446,47 @@ contract CoreIssuanceOrder is uint[] memory requiredBalances = new uint[](_order.requiredComponents.length); // Calculate amount of maker token required - uint requiredMakerTokenAmount = OrderLibrary.getPartialAmount(_order.makerTokenAmount, _fillQuantity, _order.quantity); + uint requiredMakerTokenAmount = OrderLibrary.getPartialAmount( + _order.makerTokenAmount, + _fillQuantity, + _order.quantity + ); // Calculate amount of component tokens required to issue for (uint16 i = 0; i < _order.requiredComponents.length; i++) { // Get current vault balances - uint tokenBalance = IVault(state.vaultAddress).getOwnerBalance( + uint tokenBalance = IVault(state.vault).getOwnerBalance( _order.makerAddress, _order.requiredComponents[i] ); // Amount of component tokens to be added to Vault - uint requiredAddition = OrderLibrary.getPartialAmount(_order.requiredComponentAmounts[i], _fillQuantity, _order.quantity); + uint requiredAddition = OrderLibrary.getPartialAmount( + _order.requiredComponentAmounts[i], + _fillQuantity, + _order.quantity + ); // Required vault balances after exchange order executed requiredBalances[i] = tokenBalance.add(requiredAddition); } // Execute exchange orders - uint makerTokenAmountUsed = executeExchangeOrders(_orderData, _order.makerAddress); + uint makerTokenAmountUsed = executeExchangeOrders( + _orderData, + _order.makerAddress + ); // 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( + uint currentBal = IVault(state.vault).getOwnerBalance( _order.makerAddress, _order.requiredComponents[i] ); require(currentBal >= requiredBalances[i]); } + // Settle relayer and taker accounts settleAccounts( _order, _fillQuantity, diff --git a/contracts/core/interfaces/ICore.sol b/contracts/core/interfaces/ICore.sol index 6b283abe2..73f8f5614 100644 --- a/contracts/core/interfaces/ICore.sol +++ b/contracts/core/interfaces/ICore.sol @@ -29,61 +29,61 @@ interface ICore { /** * Set vaultAddress. Can only be set by owner of Core. * - * @param _vaultAddress The address of the Vault + * @param _vault The address of the Vault */ function setVaultAddress( - address _vaultAddress + address _vault ) external; /** * Set transferProxyAddress. Can only be set by owner of Core. * - * @param _transferProxyAddress The address of the TransferProxy + * @param _transferProxy The address of the TransferProxy */ function setTransferProxyAddress( - address _transferProxyAddress + address _transferProxy ) external; /** * Add a factory to the mapping of tracked factories. * - * @param _factoryAddress The address of the SetTokenFactory to enable + * @param _factory The address of the SetTokenFactory to enable */ function enableFactory( - address _factoryAddress + address _factory ) external; /** * Disable a factory in the mapping of tracked factories. * - * @param _factoryAddress The address of the SetTokenFactory to disable + * @param _factory The address of the SetTokenFactory to disable */ function disableFactory( - address _factoryAddress + address _factory ) external; /** * Disable a set token in the mapping of tracked set tokens. * - * @param _setAddress The address of the SetToken to remove + * @param _set The address of the SetToken to remove */ function disableSet( - address _setAddress + address _set ) external; /** - * Issue + * Exchanges components for Set Tokens * - * @param _setAddress Address of set to issue - * @param _quantity Quantity of set to issue. Should be multiple of natural unit. + * @param _set Address of set to issue + * @param _quantity Quantity of set to issue */ function issue( - address _setAddress, + address _set, uint _quantity ) external; @@ -91,11 +91,11 @@ interface ICore { /** * Function to convert Set Tokens into underlying components * - * @param _setAddress The address of the Set token + * @param _set The address of the Set token * @param _quantity The number of tokens to redeem. Should be multiple of natural unit. */ function redeem( - address _setAddress, + address _set, uint _quantity ) external; @@ -104,11 +104,11 @@ interface ICore { * 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 _tokens Array of the addresses of the ERC20 tokens * @param _quantities Array of the number of tokens to deposit */ function batchDeposit( - address[] _tokenAddresses, + address[] _tokens, uint[] _quantities ) external; @@ -117,11 +117,11 @@ interface ICore { * Withdraw multiple tokens from the vault. Quantities should be in the * order of the addresses of the tokens being withdrawn. * - * @param _tokenAddresses Array of the addresses of the ERC20 tokens + * @param _tokens Array of the addresses of the ERC20 tokens * @param _quantities Array of the number of tokens to withdraw */ function batchWithdraw( - address[] _tokenAddresses, + address[] _tokens, uint[] _quantities ) external; @@ -129,11 +129,11 @@ interface ICore { /** * Deposit any quantity of tokens into the vault. * - * @param _tokenAddress The address of the ERC20 token + * @param _token The address of the ERC20 token * @param _quantity The number of tokens to deposit */ function deposit( - address _tokenAddress, + address _token, uint _quantity ) public; @@ -141,11 +141,11 @@ interface ICore { /** * Withdraw a quantity of tokens from the vault. * - * @param _tokenAddress The address of the ERC20 token + * @param _token The address of the ERC20 token * @param _quantity The number of tokens to withdraw */ function withdraw( - address _tokenAddress, + address _token, uint _quantity ) public; @@ -153,16 +153,16 @@ interface ICore { /** * Deploys a new Set Token and adds it to the valid list of SetTokens * - * @param _factoryAddress address The address of the Factory to create from - * @param _components address[] The address of component tokens - * @param _units uint[] The units of each component token - * @param _naturalUnit uint The minimum unit to be issued or redeemed - * @param _name string The name of the new Set - * @param _symbol string The symbol of the new Set - * @return setTokenAddress address The address of the new Set + * @param _factory The address of the Factory to create from + * @param _components The address of component tokens + * @param _units The units of each component token + * @param _naturalUnit The minimum unit to be issued or redeemed + * @param _name The name of the new Set + * @param _symbol The symbol of the new Set + * @return setTokenAddress The address of the new Set */ function create( - address _factoryAddress, + address _factory, address[] _components, uint[] _units, uint _naturalUnit, diff --git a/contracts/core/interfaces/ICoreAccounting.sol b/contracts/core/interfaces/ICoreAccounting.sol index 7a7ddbb83..1819eabe9 100644 --- a/contracts/core/interfaces/ICoreAccounting.sol +++ b/contracts/core/interfaces/ICoreAccounting.sol @@ -26,17 +26,21 @@ pragma solidity 0.4.24; */ contract ICoreAccounting { + /* ============ Internal Functions ============ */ + /** * 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 + * @param _from Address depositing tokens + * @param _to Address to credit for deposits + * @param _tokens Addresses of tokens being deposited + * @param _quantities The quantities of tokens to deposit */ function batchDepositInternal( address _from, address _to, - address[] _tokenAddresses, + address[] _tokens, uint[] _quantities ) internal; diff --git a/contracts/core/interfaces/ICoreIssuance.sol b/contracts/core/interfaces/ICoreIssuance.sol index db7b7ad7d..cab806904 100644 --- a/contracts/core/interfaces/ICoreIssuance.sol +++ b/contracts/core/interfaces/ICoreIssuance.sol @@ -26,16 +26,18 @@ pragma solidity 0.4.24; */ contract ICoreIssuance { + /* ============ Internal Functions ============ */ + /** - * Issue internally. Can define who to issue to. + * Exchanges components for Set Tokens, accepting any owner * - * @param _owner Address to issue set to - * @param _setAddress Address of set to issue + * @param _owner Address to issue set to + * @param _set Address of set to issue * @param _quantity Quantity of set to issue */ function issueInternal( address _owner, - address _setAddress, + address _set, uint _quantity ) internal; diff --git a/contracts/core/interfaces/IExchange.sol b/contracts/core/interfaces/IExchange.sol index dead4e5eb..c0fe9129d 100644 --- a/contracts/core/interfaces/IExchange.sol +++ b/contracts/core/interfaces/IExchange.sol @@ -21,14 +21,16 @@ pragma solidity 0.4.24; * @title IExchange * @author Set Protocol * - * Interface for executing an order with an exchange + * Interface for executing an order with an exchange wrapper */ interface IExchange { + /* ============ External Functions ============ */ + /** - * Exchange some amount of takerToken for makerToken. + * Exchange some amount of makerToken for takerToken. * - * @param _taker Issuance order filler + * @param _taker Issuance order taker * @param _orderCount Expected number of orders to execute * @param _orderData Arbitrary bytes data for any information to pass to the exchange */ diff --git a/contracts/core/interfaces/ISetFactory.sol b/contracts/core/interfaces/ISetFactory.sol index 20097b517..8bb3801ba 100644 --- a/contracts/core/interfaces/ISetFactory.sol +++ b/contracts/core/interfaces/ISetFactory.sol @@ -21,12 +21,23 @@ pragma solidity 0.4.24; * @title ISetFactory * @author Set Protocol * - * The ITransferProxy interface provides operability for authorized contracts + * The ISetFactory interface provides operability for authorized contracts * to interact with SetTokenFactory */ interface ISetFactory { - function core() external returns (address); + /* ============ External Functions ============ */ + + /** + * Deploys a new Set Token and adds it to the valid list of SetTokens + * + * @param _components The address of component tokens + * @param _units The units of each component token + * @param _naturalUnit The minimum unit to be issued or redeemed + * @param _name The name of the new Set + * @param _symbol The symbol of the new Set + * @return setTokenAddress The address of the new Set + */ function create( address[] _components, uint[] _units, diff --git a/contracts/core/interfaces/ISetToken.sol b/contracts/core/interfaces/ISetToken.sol index 4b62f59ea..8b239d9ca 100644 --- a/contracts/core/interfaces/ISetToken.sol +++ b/contracts/core/interfaces/ISetToken.sol @@ -24,24 +24,56 @@ pragma solidity 0.4.24; * SetToken contract from another contract. */ interface ISetToken { + + /* ============ External Functions ============ */ + + /* + * Get natural unit of Set + * + * @return uint Natural unit of Set + */ function naturalUnit() external returns (uint); + /* + * Get addresses of all components in the Set + * + * @return componentAddresses Array of component tokens + */ function getComponents() external returns(address[]); + /* + * Get units of all tokens in Set + * + * @return units Array of component units + */ function getUnits() external returns(uint[]); + /* + * Mint set token for given address. + * Can only be called by authorized contracts. + * + * @param _issuer The address of the issuing account + * @param _quantity The number of sets to attribute to issuer + */ function mint( address _issuer, uint _quantity ) external; + /* + * Burn set token for given address. + * Can only be called by authorized contracts. + * + * @param _from The address of the redeeming account + * @param _quantity The number of sets to burn from redeemer + */ function burn( address _from, uint _quantity diff --git a/contracts/core/interfaces/ITransferProxy.sol b/contracts/core/interfaces/ITransferProxy.sol index 453606317..f41038b2c 100644 --- a/contracts/core/interfaces/ITransferProxy.sol +++ b/contracts/core/interfaces/ITransferProxy.sol @@ -25,17 +25,19 @@ pragma solidity 0.4.24; */ interface ITransferProxy { + /* ============ External Functions ============ */ + /** * Transfers tokens from an address (that has set allowance on the proxy). * Can only be called by authorized core contracts. * - * @param _tokenAddress The address of the ERC20 token + * @param _token The address of the ERC20 token * @param _quantity The number of tokens to transfer * @param _from The address to transfer from * @param _to The address to transfer to */ function transfer( - address _tokenAddress, + address _token, uint _quantity, address _from, address _to diff --git a/contracts/core/interfaces/IVault.sol b/contracts/core/interfaces/IVault.sol index 4f782c86a..65eb5d8c3 100644 --- a/contracts/core/interfaces/IVault.sol +++ b/contracts/core/interfaces/IVault.sol @@ -29,12 +29,12 @@ interface IVault { * Withdraws user's unassociated tokens to user account. Can only be * called by authorized core contracts. * - * @param _tokenAddress The address of the ERC20 token + * @param _token The address of the ERC20 token * @param _to The address to transfer token to * @param _quantity The number of tokens to transfer */ function withdrawTo( - address _tokenAddress, + address _token, address _to, uint _quantity ) @@ -45,12 +45,12 @@ interface IVault { * only be called by authorized core contracts. * * @param _owner The address of the token owner - * @param _tokenAddress The address of the ERC20 token + * @param _token The address of the ERC20 token * @param _quantity The number of tokens to attribute to owner */ function incrementTokenOwner( address _owner, - address _tokenAddress, + address _token, uint _quantity ) external; @@ -60,12 +60,12 @@ interface IVault { * be called by authorized core contracts. * * @param _owner The address of the token owner - * @param _tokenAddress The address of the ERC20 token + * @param _token The address of the ERC20 token * @param _quantity The number of tokens to deattribute to owner */ function decrementTokenOwner( address _owner, - address _tokenAddress, + address _token, uint _quantity ) external; @@ -73,12 +73,12 @@ interface IVault { /* * Get balance of particular contract for owner. * - * @param _owner The address of the token owner - * @param _tokenAddress The address of the ERC20 token + * @param _owner The address of the token owner + * @param _token The address of the ERC20 token */ function getOwnerBalance( address _owner, - address _tokenAddress + address _token ) external returns (uint256); diff --git a/contracts/core/lib/CoreSharedModifiers.sol b/contracts/core/lib/CoreSharedModifiers.sol index 33cfc7c2d..603eb6ea4 100644 --- a/contracts/core/lib/CoreSharedModifiers.sol +++ b/contracts/core/lib/CoreSharedModifiers.sol @@ -50,27 +50,27 @@ contract CoreModifiers is } // Verify Factory is linked to Core - modifier isValidFactory(address _factoryAddress) { + modifier isValidFactory(address _factory) { require( - state.validFactories[_factoryAddress], + state.validFactories[_factory], INVALID_FACTORY ); _; } // Verify set was created by core and is enabled - modifier isValidSet(address _setAddress) { + modifier isValidSet(address _set) { require( - state.validSets[_setAddress], + state.validSets[_set], INVALID_SET ); _; } // Validate quantity is multiple of natural unit - modifier isNaturalUnitMultiple(uint _quantity, address _setToken) { + modifier isNaturalUnitMultiple(uint _quantity, address _set) { require( - _quantity % ISetToken(_setToken).naturalUnit() == 0, + _quantity % ISetToken(_set).naturalUnit() == 0, INVALID_QUANTITY ); _; diff --git a/contracts/core/lib/CoreState.sol b/contracts/core/lib/CoreState.sol index c2c8af1f4..82315a5ad 100644 --- a/contracts/core/lib/CoreState.sol +++ b/contracts/core/lib/CoreState.sol @@ -33,10 +33,10 @@ contract CoreState { mapping(uint8 => address) exchanges; // Address of the TransferProxy contract - address transferProxyAddress; + address transferProxy; // Address of the Vault contract - address vaultAddress; + address vault; // Mapping of tracked SetToken factories mapping(address => bool) validFactories; @@ -63,7 +63,15 @@ contract CoreState { /* ============ Public Getters ============ */ - function exchanges(uint8 _exchangeId) + /** + * Return address belonging to given exchangeId. + * + * @param _exchangeId ExchangeId number + * @return address Address belonging to given exchangeId + */ + function exchanges( + uint8 _exchangeId + ) public view returns(address) @@ -71,23 +79,41 @@ contract CoreState { return state.exchanges[_exchangeId]; } - function transferProxyAddress() + /** + * Return transferProxy address. + * + * @return address transferProxy address + */ + function transferProxy() public view returns(address) { - return state.transferProxyAddress; + return state.transferProxy; } - function vaultAddress() + /** + * Return vault address + * + * @return address vault address + */ + function vault() public view returns(address) { - return state.vaultAddress; + return state.vault; } - function validFactories(address _factory) + /** + * Return boolean indicating if address is valid factory. + * + * @param _factory Factory address + * @return bool Boolean indicating if enabled factory + */ + function validFactories( + address _factory + ) public view returns(bool) @@ -95,6 +121,11 @@ contract CoreState { return state.validFactories[_factory]; } + /** + * Return array of all enabled factories. + * + * @return address[] Array of enabled factories + */ function factories() public view @@ -103,7 +134,15 @@ contract CoreState { return state.factories; } - function validSets(address _set) + /** + * Return boolean indicating if address is valid Set. + * + * @param _set Set address + * @return bool Boolean indicating if valid Set + */ + function validSets( + address _set + ) public view returns(bool) @@ -111,6 +150,11 @@ contract CoreState { return state.validSets[_set]; } + /** + * Return array of all valid Set Tokens. + * + * @return address[] Array of valid Set Tokens + */ function setTokens() public view @@ -119,7 +163,15 @@ contract CoreState { return state.setTokens; } - function orderFills(bytes32 _orderHash) + /** + * Return amount of Issuance Order already filled + * + * @param _orderHash Issuance Order orderHash + * @return uint Amount of Issuance Order filled + */ + function orderFills( + bytes32 _orderHash + ) public view returns(uint) @@ -127,7 +179,15 @@ contract CoreState { return state.orderFills[_orderHash]; } - function orderCancels(bytes32 _orderHash) + /** + * Return amount of Issuance Order already canceled + * + * @param _orderHash Issuance Order orderHash + * @return uint Amount of Issuance Order canceled + */ + function orderCancels( + bytes32 _orderHash + ) public view returns(uint) diff --git a/contracts/core/lib/ExchangeHandler.sol b/contracts/core/lib/ExchangeHandler.sol index 50a14f534..8865487e5 100644 --- a/contracts/core/lib/ExchangeHandler.sol +++ b/contracts/core/lib/ExchangeHandler.sol @@ -53,6 +53,7 @@ library ExchangeHandler { { ExchangeHeader memory header; + // Create ExchangeHeader struct assembly { mstore(header, mload(add(_headerData, 32))) // exchange mstore(add(header, 32), mload(add(_headerData, 64))) // orderCount diff --git a/contracts/core/lib/OrderLibrary.sol b/contracts/core/lib/OrderLibrary.sol index 72ecc2302..a54394690 100644 --- a/contracts/core/lib/OrderLibrary.sol +++ b/contracts/core/lib/OrderLibrary.sol @@ -23,7 +23,7 @@ import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; * @title OrderLibrary * @author Set Protocol * - * The Order Library contains functions for checking validation and hashing of Orders. + * The Order Library contains functions for checking validation and hashing of Issuance Orders. * */ @@ -144,22 +144,35 @@ library OrderLibrary { return recAddress == _signerAddress; } + /** + * Checks for rounding errors and returns value of potential partial amounts of a principal + * + * @param _prinicpal Number fractional amount is derived from + * @param _numerator Numerator of fraction + * @param _denominator Denominator of fraction + * @return uint256 Fractional amount of principal calculated + */ function getPartialAmount( - uint principal, - uint numerator, - uint denominator + uint _principal, + uint _numerator, + uint _denominator ) internal returns (uint256) { - uint remainder = mulmod(principal, numerator, denominator); + // Get remainder of partial amount (if 0 not a partial amount) + uint remainder = mulmod(_principal, _numerator, _denominator); + + // Return if not a partial amount if (remainder == 0) { - return principal.mul(numerator).div(denominator); + return _principal.mul(_numerator).div(_denominator); } - uint errPercentageTimes1000000 = remainder.mul(1000000).div(numerator.mul(principal)); + // Calculate error percentage + uint errPercentageTimes1000000 = remainder.mul(1000000).div(_numerator.mul(_principal)); + // Require error percentage is less than 0.1% require(errPercentageTimes1000000 < 1000, ROUNDING_ERROR_TOO_LARGE); - return principal.mul(numerator).div(denominator); + return _principal.mul(_numerator).div(_denominator); } } diff --git a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts index 86dcb4b36..721a2eefa 100644 --- a/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts +++ b/test/contracts/core/extensions/coreIssuanceOrderScenarios.spec.ts @@ -46,7 +46,6 @@ BigNumberSetup.configure(); ChaiSetup.configure(); const { expect } = chai; - import { assertTokenBalance, } from '../../../../utils/tokenAssertions'; From 0adeb30ec4e0275b9d82350fe0d343ec57e9cdc0 Mon Sep 17 00:00:00 2001 From: bweick Date: Tue, 24 Jul 2018 22:23:38 -0700 Subject: [PATCH 2/4] Cleaning up errors caused by styling --- contracts/core/SetTokenFactory.sol | 2 +- contracts/core/extensions/CoreAccounting.sol | 2 +- contracts/core/extensions/CoreInternal.sol | 2 +- contracts/core/interfaces/ICore.sol | 2 +- contracts/core/interfaces/ISetFactory.sol | 7 +++++++ contracts/core/lib/OrderLibrary.sol | 2 +- test/contracts/core/extensions/coreInternal.spec.ts | 4 ++-- 7 files changed, 14 insertions(+), 7 deletions(-) diff --git a/contracts/core/SetTokenFactory.sol b/contracts/core/SetTokenFactory.sol index 42df7af19..bfd7fdfa0 100644 --- a/contracts/core/SetTokenFactory.sol +++ b/contracts/core/SetTokenFactory.sol @@ -41,7 +41,7 @@ contract SetTokenFactory /** * Set core. Can only be set by owner of SetTokenFactory's owner. * - * @param _coreAddress The address of deployed core contract + * @param _core The address of deployed core contract */ function setCoreAddress( address _core diff --git a/contracts/core/extensions/CoreAccounting.sol b/contracts/core/extensions/CoreAccounting.sol index 70fdd6ea7..0ba5ea1ba 100644 --- a/contracts/core/extensions/CoreAccounting.sol +++ b/contracts/core/extensions/CoreAccounting.sol @@ -102,7 +102,7 @@ contract CoreAccounting is address _token, uint _quantity ) - external + public { // Call Vault contract to deattribute tokens to user IVault(state.vault).decrementTokenOwner( diff --git a/contracts/core/extensions/CoreInternal.sol b/contracts/core/extensions/CoreInternal.sol index b80a208e0..b85bd6ab9 100644 --- a/contracts/core/extensions/CoreInternal.sol +++ b/contracts/core/extensions/CoreInternal.sol @@ -124,7 +124,7 @@ contract CoreInternal is isValidSet(_set) { // Mark as false in validSet mapping - state.validSets[_setAddress] = false; + state.validSets[_set] = false; // Find and remove from setTokens array for (uint256 i = 0; i < state.setTokens.length; i++) { diff --git a/contracts/core/interfaces/ICore.sol b/contracts/core/interfaces/ICore.sol index 73f8f5614..0c9974eaf 100644 --- a/contracts/core/interfaces/ICore.sol +++ b/contracts/core/interfaces/ICore.sol @@ -136,7 +136,7 @@ interface ICore { address _token, uint _quantity ) - public; + external; /** * Withdraw a quantity of tokens from the vault. diff --git a/contracts/core/interfaces/ISetFactory.sol b/contracts/core/interfaces/ISetFactory.sol index 8bb3801ba..5bebd11ed 100644 --- a/contracts/core/interfaces/ISetFactory.sol +++ b/contracts/core/interfaces/ISetFactory.sol @@ -28,6 +28,13 @@ interface ISetFactory { /* ============ External Functions ============ */ + /** + * Return core address + * + * @return address core address + */ + function core() external returns (address); + /** * Deploys a new Set Token and adds it to the valid list of SetTokens * diff --git a/contracts/core/lib/OrderLibrary.sol b/contracts/core/lib/OrderLibrary.sol index a54394690..7f1f2fc4c 100644 --- a/contracts/core/lib/OrderLibrary.sol +++ b/contracts/core/lib/OrderLibrary.sol @@ -147,7 +147,7 @@ library OrderLibrary { /** * Checks for rounding errors and returns value of potential partial amounts of a principal * - * @param _prinicpal Number fractional amount is derived from + * @param _principal Number fractional amount is derived from * @param _numerator Numerator of fraction * @param _denominator Denominator of fraction * @return uint256 Fractional amount of principal calculated diff --git a/test/contracts/core/extensions/coreInternal.spec.ts b/test/contracts/core/extensions/coreInternal.spec.ts index b48bd763f..450a4218b 100644 --- a/test/contracts/core/extensions/coreInternal.spec.ts +++ b/test/contracts/core/extensions/coreInternal.spec.ts @@ -82,7 +82,7 @@ contract('CoreInternal', accounts => { it('sets vault address correctly', async () => { await subject(); - const storedVaultAddress = await core.vaultAddress.callAsync(); + const storedVaultAddress = await core.vault.callAsync(); expect(storedVaultAddress).to.eql(vault.address); }); @@ -117,7 +117,7 @@ contract('CoreInternal', accounts => { it('sets transfer proxy address correctly', async () => { await subject(); - const storedTransferProxyAddress = await core.transferProxyAddress.callAsync(); + const storedTransferProxyAddress = await core.transferProxy.callAsync(); expect(storedTransferProxyAddress).to.eql(transferProxy.address); }); From 495b5652f53ee7143519685c131b54e39dfba703 Mon Sep 17 00:00:00 2001 From: bweick Date: Tue, 24 Jul 2018 22:35:39 -0700 Subject: [PATCH 3/4] Updated expected log for new variable name. --- utils/contract_logs/core.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/contract_logs/core.ts b/utils/contract_logs/core.ts index 3d4ba2330..01a5181d9 100644 --- a/utils/contract_logs/core.ts +++ b/utils/contract_logs/core.ts @@ -15,7 +15,7 @@ interface CreateLogArgs { export function SetTokenCreated( _coreAddress: Address, _setTokenAddress: Address, - _factoryAddress: Address, + _factory: Address, _components: Address[], _units: BigNumber[], _naturalUnit: BigNumber, @@ -27,7 +27,7 @@ export function SetTokenCreated( address: _coreAddress, args: { _setTokenAddress, - _factoryAddress, + _factory, _components, _units, _naturalUnit, From bf90a229c3af22b317e671763658610a5776e6d9 Mon Sep 17 00:00:00 2001 From: bweick Date: Wed, 25 Jul 2018 11:01:38 -0700 Subject: [PATCH 4/4] Added ASCII table giving practical example of vault balances mapping. --- contracts/core/Vault.sol | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/contracts/core/Vault.sol b/contracts/core/Vault.sol index 2f892b871..b5ee7c3f1 100644 --- a/contracts/core/Vault.sol +++ b/contracts/core/Vault.sol @@ -44,7 +44,17 @@ contract Vault is /* ============ State Variables ============ */ - // Mapping of token address to map of owner address to balance. + // Mapping of token address to map of owner or Set address to balance. + // Example of mapping below: + // +--------------+---------------------+--------+ + // | TokenAddress | Set OR User Address | Amount | + // +--------------+---------------------+--------+ + // | TokenA | User 0x123 | 500 | + // | | User 0xABC | 300 | + // | | Set 0x456 | 1000 | + // | TokenB | User 0xDEF | 100 | + // | | Set 0xSET | 700 | + // +--------------+---------------------+--------+ mapping (address => mapping (address => uint256)) public balances;