From 95be0769613e2881fa58a4e8856ebbc6acd4102e Mon Sep 17 00:00:00 2001 From: Adam Dossa Date: Fri, 14 Jun 2019 09:37:53 -0400 Subject: [PATCH] Fix canTransfer spec (#709) * Fix spec * Fix conflict * Fix merge conflict * Small fixes * Revert package.json change * Fix test case --- contracts/interfaces/ISecurityToken.sol | 30 ++++++++-------- contracts/interfaces/token/IERC1594.sol | 4 +-- contracts/libraries/TokenLib.sol | 12 +++---- .../modules/STO/USDTiered/USDTieredSTO.sol | 2 +- contracts/tokens/SecurityToken.sol | 34 +++++++++++-------- test/j_manual_approval_transfer_manager.js | 14 +++++--- test/o_security_token.js | 4 +-- 7 files changed, 55 insertions(+), 45 deletions(-) diff --git a/contracts/interfaces/ISecurityToken.sol b/contracts/interfaces/ISecurityToken.sol index 67a2dc2f8..a517073ee 100644 --- a/contracts/interfaces/ISecurityToken.sol +++ b/contracts/interfaces/ISecurityToken.sol @@ -19,7 +19,19 @@ interface ISecurityToken { event Transfer(address indexed from, address indexed to, uint256 value); event Approval(address indexed owner, address indexed spender, uint256 value); - // Emit at the time when module get added + /** + * @notice Transfers of securities may fail for a number of reasons. So this function will used to understand the + * cause of failure by getting the byte value. Which will be the ESC that follows the EIP 1066. ESC can be mapped + * with a reson string to understand the failure cause, table of Ethereum status code will always reside off-chain + * @param _to address The address which you want to transfer to + * @param _value uint256 the amount of tokens to be transferred + * @param _data The `bytes _data` allows arbitrary data to be submitted alongside the transfer. + * @return byte Ethereum status code (ESC) + * @return bytes32 Application specific reason code + */ + function canTransfer(address _to, uint256 _value, bytes calldata _data) external view returns (byte statusCode, bytes32 reasonCode); + + // Emit at the time when module get added event ModuleAdded( uint8[] _types, bytes32 indexed _name, @@ -114,19 +126,6 @@ interface ISecurityToken { */ function initialize(address _getterDelegate) external; - /** - * @notice Transfers of securities may fail for a number of reasons. So this function will used to understand the - * cause of failure by getting the byte value. Which will be the ESC that follows the EIP 1066. ESC can be mapped - * with a reson string to understand the failure cause, table of Ethereum status code will always reside off-chain - * @param _to address The address which you want to transfer to - * @param _value uint256 the amount of tokens to be transferred - * @param _data The `bytes _data` allows arbitrary data to be submitted alongside the transfer. - * @return bool It signifies whether the transaction will be executed or not. - * @return byte Ethereum status code (ESC) - * @return bytes32 Application specific reason code - */ - function canTransfer(address _to, uint256 _value, bytes calldata _data) external view returns (bool isExecuted, byte statusCode, bytes32 reasonCode); - /** * @notice The standard provides an on-chain function to determine whether a transfer will succeed, * and return details indicating the reason if the transfer is not valid. @@ -158,11 +157,10 @@ interface ISecurityToken { * @param _to address The address which you want to transfer to * @param _value uint256 the amount of tokens to be transferred * @param _data The `bytes _data` allows arbitrary data to be submitted alongside the transfer. - * @return bool It signifies whether the transaction will be executed or not. * @return byte Ethereum status code (ESC) * @return bytes32 Application specific reason code */ - function canTransferFrom(address _from, address _to, uint256 _value, bytes calldata _data) external view returns (bool isExecuted, byte statusCode, bytes32 reasonCode); + function canTransferFrom(address _from, address _to, uint256 _value, bytes calldata _data) external view returns (byte statusCode, bytes32 reasonCode); /** * @notice Used to attach a new document to the contract, or update the URI or hash of an existing attached document diff --git a/contracts/interfaces/token/IERC1594.sol b/contracts/interfaces/token/IERC1594.sol index 79cf13830..d74743c80 100644 --- a/contracts/interfaces/token/IERC1594.sol +++ b/contracts/interfaces/token/IERC1594.sol @@ -19,8 +19,8 @@ interface IERC1594 { function redeemFrom(address _tokenHolder, uint256 _value, bytes calldata _data) external; // Transfer Validity - function canTransfer(address _to, uint256 _value, bytes calldata _data) external view returns (bool, byte, bytes32); - function canTransferFrom(address _from, address _to, uint256 _value, bytes calldata _data) external view returns (bool, byte, bytes32); + function canTransfer(address _to, uint256 _value, bytes calldata _data) external view returns (byte, bytes32); + function canTransferFrom(address _from, address _to, uint256 _value, bytes calldata _data) external view returns (byte, bytes32); // Issuance / Redemption Events event Issued(address indexed _operator, address indexed _to, uint256 _value, bytes _data); diff --git a/contracts/libraries/TokenLib.sol b/contracts/libraries/TokenLib.sol index 8355a3dff..bf350edc5 100644 --- a/contracts/libraries/TokenLib.sol +++ b/contracts/libraries/TokenLib.sol @@ -463,22 +463,22 @@ library TokenLib { ) external pure - returns (bool, byte, bytes32) + returns (byte, bytes32) { if (!success) - return (false, StatusCodes.code(StatusCodes.Status.TransferFailure), appCode); + return (StatusCodes.code(StatusCodes.Status.TransferFailure), appCode); if (balanceOfFrom < value) - return (false, StatusCodes.code(StatusCodes.Status.InsufficientBalance), bytes32(0)); + return (StatusCodes.code(StatusCodes.Status.InsufficientBalance), bytes32(0)); if (to == address(0)) - return (false, StatusCodes.code(StatusCodes.Status.InvalidReceiver), bytes32(0)); + return (StatusCodes.code(StatusCodes.Status.InvalidReceiver), bytes32(0)); // Balance overflow can never happen due to totalsupply being a uint256 as well // else if (!KindMath.checkAdd(balanceOf(_to), _value)) - // return (false, 0x50, bytes32(0)); + // return (0x50, bytes32(0)); - return (true, StatusCodes.code(StatusCodes.Status.TransferSuccess), bytes32(0)); + return (StatusCodes.code(StatusCodes.Status.TransferSuccess), bytes32(0)); } function _getKey(bytes32 _key1, address _key2) internal pure returns(bytes32) { diff --git a/contracts/modules/STO/USDTiered/USDTieredSTO.sol b/contracts/modules/STO/USDTiered/USDTieredSTO.sol index ab99ec905..8283ca349 100644 --- a/contracts/modules/STO/USDTiered/USDTieredSTO.sol +++ b/contracts/modules/STO/USDTiered/USDTieredSTO.sol @@ -720,7 +720,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO { * @notice Return the permissions flag that are associated with STO */ function getPermissions() public view returns(bytes32[] memory allPermissions) { - bytes32[] memory allPermissions = new bytes32[](2); + allPermissions = new bytes32[](2); allPermissions[0] = OPERATOR; allPermissions[1] = ADMIN; return allPermissions; diff --git a/contracts/tokens/SecurityToken.sol b/contracts/tokens/SecurityToken.sol index 26ee2b7ef..e4a5ffa7f 100644 --- a/contracts/tokens/SecurityToken.sol +++ b/contracts/tokens/SecurityToken.sol @@ -911,11 +911,10 @@ contract SecurityToken is ERC20, ReentrancyGuard, SecurityTokenStorage, IERC1594 * @param _to address The address which you want to transfer to * @param _value uint256 the amount of tokens to be transferred * @param _data The `bytes _data` allows arbitrary data to be submitted alongside the transfer. - * @return bool It signifies whether the transaction will be executed or not. * @return byte Ethereum status code (ESC) * @return bytes32 Application specific reason code */ - function canTransfer(address _to, uint256 _value, bytes calldata _data) external view returns (bool, byte, bytes32) { + function canTransfer(address _to, uint256 _value, bytes calldata _data) external view returns (byte, bytes32) { return _canTransfer(msg.sender, _to, _value, _data); } @@ -927,22 +926,21 @@ contract SecurityToken is ERC20, ReentrancyGuard, SecurityTokenStorage, IERC1594 * @param _to address The address which you want to transfer to * @param _value uint256 the amount of tokens to be transferred * @param _data The `bytes _data` allows arbitrary data to be submitted alongside the transfer. - * @return bool It signifies whether the transaction will be executed or not. * @return byte Ethereum status code (ESC) * @return bytes32 Application specific reason code */ - function canTransferFrom(address _from, address _to, uint256 _value, bytes calldata _data) external view returns (bool success, byte reasonCode, bytes32 appCode) { - (success, reasonCode, appCode) = _canTransfer(_from, _to, _value, _data); - if (success && _value > allowance(_from, msg.sender)) { - return (false, StatusCodes.code(StatusCodes.Status.InsufficientAllowance), bytes32(0)); + function canTransferFrom(address _from, address _to, uint256 _value, bytes calldata _data) external view returns (byte reasonCode, bytes32 appCode) { + (reasonCode, appCode) = _canTransfer(_from, _to, _value, _data); + if (isSuccess(reasonCode) && _value > allowance(_from, msg.sender)) { + return (StatusCodes.code(StatusCodes.Status.InsufficientAllowance), bytes32(0)); } } - function _canTransfer(address _from, address _to, uint256 _value, bytes memory _data) internal view returns (bool, byte, bytes32) { + function _canTransfer(address _from, address _to, uint256 _value, bytes memory _data) internal view returns (byte, bytes32) { bytes32 appCode; bool success; if (_value % granularity != 0) { - return (false, StatusCodes.code(StatusCodes.Status.TransferFailure), bytes32(0)); + return (StatusCodes.code(StatusCodes.Status.TransferFailure), bytes32(0)); } (success, appCode) = TokenLib.verifyTransfer(modules[TRANSFER_KEY], modulesToData, _from, _to, _value, _data, transfersFrozen); return TokenLib.canTransfer(success, appCode, _to, _value, balanceOf(_from)); @@ -969,17 +967,16 @@ contract SecurityToken is ERC20, ReentrancyGuard, SecurityTokenStorage, IERC1594 ) external view - returns (byte esc, bytes32 appStatusCode, bytes32 toPartition) + returns (byte reasonCode, bytes32 appStatusCode, bytes32 toPartition) { if (_partition == UNLOCKED) { - bool success; - (success, esc, appStatusCode) = _canTransfer(_from, _to, _value, _data); - if (success) { + (reasonCode, appStatusCode) = _canTransfer(_from, _to, _value, _data); + if (isSuccess(reasonCode)) { uint256 beforeBalance = _balanceOfByPartition(LOCKED, _to, 0); uint256 afterbalance = _balanceOfByPartition(LOCKED, _to, _value); toPartition = _returnPartition(beforeBalance, afterbalance, _value); } - return (esc, appStatusCode, toPartition); + return (reasonCode, appStatusCode, toPartition); } return (StatusCodes.code(StatusCodes.Status.TransferFailure), bytes32(0), bytes32(0)); } @@ -1100,4 +1097,13 @@ contract SecurityToken is ERC20, ReentrancyGuard, SecurityTokenStorage, IERC1594 emit OwnershipTransferred(_owner, newOwner); _owner = newOwner; } + + /** + * @dev Check if a status code represents success (ie: 0x*1) + * @param status Binary ERC-1066 status code + * @return successful A boolean representing if the status code represents success + */ + function isSuccess(byte status) public pure returns (bool successful) { + return (status & 0x0F) == 0x01; + } } diff --git a/test/j_manual_approval_transfer_manager.js b/test/j_manual_approval_transfer_manager.js index e14823479..53bd88dc4 100644 --- a/test/j_manual_approval_transfer_manager.js +++ b/test/j_manual_approval_transfer_manager.js @@ -16,6 +16,10 @@ const Web3 = require("web3"); let BN = Web3.utils.BN; const web3 = new Web3(new Web3.providers.HttpProvider("http://localhost:8545")); // Hardcoded development port +const SUCCESS_CODE = 0x51; +const FAILURE_CODE = 0x50; + + contract("ManualApprovalTransferManager", accounts => { // Accounts Variable declaration let account_polymath; @@ -396,18 +400,20 @@ contract("ManualApprovalTransferManager", accounts => { from: account_investor1 } ); - console.log(JSON.stringify(verified[0])); - assert.equal(verified[0], true); + // console.log(JSON.stringify(verified[0])); + assert.equal(verified[0], SUCCESS_CODE); verified = await I_SecurityToken.canTransfer.call(account_investor4, web3.utils.toWei("4", "ether"), "0x0", { from: account_investor1 }); - assert.equal(verified[0], false); + // console.log(JSON.stringify(verified[0])); + assert.equal(verified[0], FAILURE_CODE); verified = await I_SecurityToken.canTransfer.call(account_investor4, web3.utils.toWei("1", "ether"), "0x0", { from: account_investor1 }); - assert.equal(verified[0], true); + // console.log(JSON.stringify(verified[0])); + assert.equal(verified[0], SUCCESS_CODE); }); it("Should fail to sell the tokens more than the allowance", async() => { diff --git a/test/o_security_token.js b/test/o_security_token.js index 85368aac9..3cee1f442 100644 --- a/test/o_security_token.js +++ b/test/o_security_token.js @@ -525,6 +525,7 @@ contract("SecurityToken", async (accounts) => { let tx = await I_SecurityToken.removeModule(I_GeneralTransferManager.address, { from: token_owner }); assert.equal(tx.logs[0].args._types[0], transferManagerKey); assert.equal(tx.logs[0].args._module, I_GeneralTransferManager.address); + await revertToSnapshot(key); }); @@ -666,8 +667,7 @@ contract("SecurityToken", async (accounts) => { it("Should Fail in transferring the token from one whitelist investor 1 to non whitelist investor 2", async () => { let _canTransfer = await I_SecurityToken.canTransfer.call(account_investor2, new BN(10).mul(new BN(10).pow(new BN(18))), "0x0", {from: account_investor1}); - assert.isFalse(_canTransfer[0]); - assert.equal(_canTransfer[1], 0x50); + assert.equal(_canTransfer[0], 0x50); await catchRevert(I_SecurityToken.transfer(account_investor2, new BN(10).mul(new BN(10).pow(new BN(18))), { from: account_investor1 })); });