Merging dev-3.0.0 changes into dev-3.1.0 (#750)

* Fixed CTM verifyTransfer bug (#736) * Fixed CTM canTransferBug * Added test case * Added comment for devs * Move some variables / functions to internal (#737) Move some variables / functions to internal
PolymathNetwork · Jul 16, 2019 · 1d567a6 · 1d567a6
1 parent c1e5e50
commit 1d567a6
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 17 deletions.
diff --git a/contracts/modules/TransferManager/CTM/CountTransferManager.sol b/contracts/modules/TransferManager/CTM/CountTransferManager.sol
@@ -33,12 +33,16 @@ contract CountTransferManager is CountTransferManagerStorage, TransferManager {
         external
         returns(Result)
     {
-        (Result success, ) = _verifyTransfer(_from, _to, _amount);
+        (Result success, ) = _verifyTransfer(_from, _to, _amount, securityToken.holderCount());
         return success;
     }
 
     /**
      * @notice Used to verify the transfer transaction and prevent a transfer if it passes the allowed amount of token holders
+     * @dev module.verifyTransfer is called by SecToken.canTransfer and does not receive the updated holderCount therefore
+     *      verifyTransfer has to manually account for pot. tokenholder changes (by mimicking TokenLib.adjustInvestorCount).
+     *      module.executeTransfer is called by SecToken.transfer|issue|others and receives an updated holderCount 
+     *      as sectoken calls TokenLib.adjustInvestorCount before executeTransfer.
      * @param _from Address of the sender
      * @param _to Address of the receiver
      * @param _amount Amount to send
@@ -53,20 +57,33 @@ contract CountTransferManager is CountTransferManagerStorage, TransferManager {
         view
         returns(Result, bytes32)
     {
-        return _verifyTransfer(_from, _to, _amount);
+        uint256 holderCount = securityToken.holderCount();
+        if (_amount != 0 && _from != _to) {
+            // Check whether receiver is a new token holder
+            if (_to != address(0) && securityToken.balanceOf(_to) == 0) {
+                holderCount++;
+            }
+            // Check whether sender is moving all of their tokens
+            if (_amount == securityToken.balanceOf(_from)) {
+                holderCount--;
+            }
+        }
+
+        return _verifyTransfer(_from, _to, _amount, holderCount);
     }
 
     function _verifyTransfer(
         address _from,
         address _to,
-        uint256 _amount
+        uint256 _amount,
+        uint256 _holderCount
     )
         internal
         view
         returns(Result, bytes32)
     {
         if (!paused) {
-            if (maxHolderCount < securityToken.holderCount()) {
+            if (maxHolderCount < _holderCount) {
                 // Allow transfers to existing maxHolders
                 if (securityToken.balanceOf(_to) != 0 || securityToken.balanceOf(_from) == _amount) {
                     return (Result.NA, bytes32(0));

diff --git a/contracts/tokens/SecurityToken.sol b/contracts/tokens/SecurityToken.sol
@@ -931,7 +931,7 @@ contract SecurityToken is ERC20, ReentrancyGuard, SecurityTokenStorage, IERC1594
      */
     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)) {
+        if (_isSuccess(reasonCode) && _value > allowance(_from, msg.sender)) {
             return (StatusCodes.code(StatusCodes.Status.InsufficientAllowance), bytes32(0));
         }
     }
@@ -971,7 +971,7 @@ contract SecurityToken is ERC20, ReentrancyGuard, SecurityTokenStorage, IERC1594
     {
         if (_partition == UNLOCKED) {
             (reasonCode, appStatusCode) = _canTransfer(_from, _to, _value, _data);
-            if (isSuccess(reasonCode)) {
+            if (_isSuccess(reasonCode)) {
                 uint256 beforeBalance = _balanceOfByPartition(LOCKED, _to, 0);
                 uint256 afterbalance = _balanceOfByPartition(LOCKED, _to, _value);
                 toPartition = _returnPartition(beforeBalance, afterbalance, _value);
@@ -1103,7 +1103,7 @@ contract SecurityToken is ERC20, ReentrancyGuard, SecurityTokenStorage, IERC1594
     * @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) {
+    function _isSuccess(byte status) internal pure returns (bool successful) {
         return (status & 0x0F) == 0x01;
     }
 }
diff --git a/contracts/tokens/SecurityTokenStorage.sol b/contracts/tokens/SecurityTokenStorage.sol
@@ -8,18 +8,18 @@ import "openzeppelin-solidity/contracts/token/ERC20/IERC20.sol";
 
 contract SecurityTokenStorage {
 
-    uint8 constant PERMISSION_KEY = 1;
-    uint8 constant TRANSFER_KEY = 2;
-    uint8 constant MINT_KEY = 3;
-    uint8 constant CHECKPOINT_KEY = 4;
-    uint8 constant BURN_KEY = 5;
-    uint8 constant DATA_KEY = 6;
-    uint8 constant WALLET_KEY = 7;
+    uint8 internal constant PERMISSION_KEY = 1;
+    uint8 internal constant TRANSFER_KEY = 2;
+    uint8 internal constant MINT_KEY = 3;
+    uint8 internal constant CHECKPOINT_KEY = 4;
+    uint8 internal constant BURN_KEY = 5;
+    uint8 internal constant DATA_KEY = 6;
+    uint8 internal constant WALLET_KEY = 7;
 
     bytes32 internal constant INVESTORSKEY = 0xdf3a8dd24acdd05addfc6aeffef7574d2de3f844535ec91e8e0f3e45dba96731; //keccak256(abi.encodePacked("INVESTORS"))
     bytes32 internal constant TREASURY = 0xaae8817359f3dcb67d050f44f3e49f982e0359d90ca4b5f18569926304aaece6; //keccak256(abi.encodePacked("TREASURY_WALLET"))
-    bytes32 public constant LOCKED = "LOCKED";
-    bytes32 public constant UNLOCKED = "UNLOCKED";
+    bytes32 internal constant LOCKED = "LOCKED";
+    bytes32 internal constant UNLOCKED = "UNLOCKED";
 
     //////////////////////////
     /// Document datastructure
@@ -57,7 +57,7 @@ contract SecurityTokenStorage {
     }
 
     //Naming scheme to match Ownable
-    address public _owner;
+    address internal _owner;
     address public tokenFactory;
     bool public initialized;
 

diff --git a/test/d_count_transfer_manager.js b/test/d_count_transfer_manager.js
@@ -311,6 +311,9 @@ contract("CountTransferManager", async (accounts) => {
             );
 
             await catchRevert(I_SecurityToken.issue(account_investor3, new BN(web3.utils.toWei("3", "ether")), "0x0", { from: token_owner }));
+            await catchRevert(I_SecurityToken.transfer(account_investor3, new BN(web3.utils.toWei("1", "ether")), { from: account_investor2 }));
+            let canTransfer = await I_SecurityToken.canTransfer(account_investor3, new BN(web3.utils.toWei("1", "ether")), "0x0", { from: account_investor2 });
+            assert.equal(canTransfer[0], "0x50"); //Transfer failure.
         });
 
         it("Should still be able to add to original token holders", async () => {