Fix canTransfer spec (#709)

* Fix spec

* Fix conflict

* Fix merge conflict

* Small fixes

* Revert package.json change

* Fix test case

Author: adamdossa

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

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);

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) {

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;

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;
+    }
 }

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() => {

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 }));
         });