From b37b6052df8190070c4f318c7d452d24ac5712fc Mon Sep 17 00:00:00 2001 From: satyam Date: Thu, 4 Oct 2018 11:30:07 +0530 Subject: [PATCH 1/6] move investors data and logic into the library --- contracts/libraries/TokenLib.sol | 51 ++++++++++++++++++++++++++++++ contracts/tokens/SecurityToken.sol | 43 +++++++------------------ migrations/2_deploy_contracts.js | 34 -------------------- test/o_security_token.js | 12 +++---- 4 files changed, 68 insertions(+), 72 deletions(-) diff --git a/contracts/libraries/TokenLib.sol b/contracts/libraries/TokenLib.sol index 8fa281d26..24c09f81c 100644 --- a/contracts/libraries/TokenLib.sol +++ b/contracts/libraries/TokenLib.sol @@ -1,9 +1,12 @@ pragma solidity ^0.4.24; import "../modules/PermissionManager/IPermissionManager.sol"; +import "./KindMath.sol"; library TokenLib { + using KindMath for uint256; + // Struct for module data struct ModuleData { bytes32 name; @@ -22,6 +25,15 @@ library TokenLib { uint256 value; } + struct InvestorDataStorage { + // List of investors (may not be pruned to remove old investors with current zero balances) + mapping (address => bool) investorListed; + // List of token holders + address[] investors; + // Total number of non-zero token holders + uint256 investorCount; + } + // Emit when Module get archived from the securityToken event ModuleArchived(uint8[] _types, address _module, uint256 _timestamp); // Emit when Module get unarchived from the securityToken @@ -148,5 +160,44 @@ library TokenLib { ); } + /** + * @notice keeps track of the number of non-zero token holders + * @param _from sender of transfer + * @param _to receiver of transfer + * @param _value value of transfer + */ + function adjustInvestorCount(InvestorDataStorage storage _investors, address _from, address _to, uint256 _value, uint256 _balanceTo, uint256 _balanceFrom) public { + if ((_value == 0) || (_from == _to)) { + return; + } + // Check whether receiver is a new token holder + if ((_balanceTo == 0) && (_to != address(0))) { + _investors.investorCount = (_investors.investorCount).add(1); + } + // Check whether sender is moving all of their tokens + if (_value == _balanceFrom) { + _investors.investorCount = (_investors.investorCount).sub(1); + } + //Also adjust investor list + if (!_investors.investorListed[_to] && (_to != address(0))) { + _investors.investors.push(_to); + _investors.investorListed[_to] = true; + } + + } + + /** + * @notice removes addresses with zero balances from the investors list + * @param _index Index in investor list at which to start removing zero balances + * NB - pruning this list will mean you may not be able to iterate over investors on-chain as of a historical checkpoint + */ + function pruneInvestors(InvestorDataStorage storage _investorData, uint256 _index) public { + _investorData.investorListed[_investorData.investors[_index]] = false; + _investorData.investors[_index] = _investorData.investors[_investorData.investors.length - 1]; + _investorData.investors.length--; + } + + + } diff --git a/contracts/tokens/SecurityToken.sol b/contracts/tokens/SecurityToken.sol index 9f232463b..27564e0e7 100644 --- a/contracts/tokens/SecurityToken.sol +++ b/contracts/tokens/SecurityToken.sol @@ -27,6 +27,8 @@ import "../libraries/TokenLib.sol"; contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, RegistryUpdater { using SafeMath for uint256; + TokenLib.InvestorDataStorage investorData; + // Use to hold the version struct SemanticVersion { uint8 major; @@ -50,12 +52,6 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr // Value of current checkpoint uint256 public currentCheckpointId; - // Total number of non-zero token holders - uint256 public investorCount; - - // List of token holders - address[] investors; - // Use to temporarily halt all transactions bool public transfersFrozen; @@ -86,9 +82,6 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr // Times at which each checkpoint was created uint256[] checkpointTimes; - // List of investors (may not be pruned to remove old investors with current zero balances) - mapping (address => bool) investorListed; - // Emit at the time when module get added event ModuleAdded( uint8[] _types, @@ -401,23 +394,7 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr * @param _value value of transfer */ function _adjustInvestorCount(address _from, address _to, uint256 _value) internal { - if ((_value == 0) || (_from == _to)) { - return; - } - // Check whether receiver is a new token holder - if ((balanceOf(_to) == 0) && (_to != address(0))) { - investorCount = investorCount.add(1); - } - // Check whether sender is moving all of their tokens - if (_value == balanceOf(_from)) { - investorCount = investorCount.sub(1); - } - //Also adjust investor list - if (!investorListed[_to] && (_to != address(0))) { - investors.push(_to); - investorListed[_to] = true; - } - + TokenLib.adjustInvestorCount(investorData, _from, _to, _value, balanceOf(_to), balanceOf(_from)); } /** @@ -427,11 +404,9 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr * NB - pruning this list will mean you may not be able to iterate over investors on-chain as of a historical checkpoint */ function pruneInvestors(uint256 _start, uint256 _iters) external onlyOwner { - for (uint256 i = _start; i < Math.min256(_start.add(_iters), investors.length); i++) { - if ((i < investors.length) && (balanceOf(investors[i]) == 0)) { - investorListed[investors[i]] = false; - investors[i] = investors[investors.length - 1]; - investors.length--; + for (uint256 i = _start; i < Math.min256(_start.add(_iters), investorData.investors.length); i++) { + if ((i < investorData.investors.length) && (balanceOf(investorData.investors[i]) == 0)) { + TokenLib.pruneInvestors(investorData, i); } } } @@ -442,7 +417,11 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr * @return length */ function getInvestors() external view returns(address[]) { - return investors; + return investorData.investors; + } + + function getInvestorCount() external view returns(uint256) { + return investorData.investorCount; } /** diff --git a/migrations/2_deploy_contracts.js b/migrations/2_deploy_contracts.js index fd2c27bb0..7b1feeece 100644 --- a/migrations/2_deploy_contracts.js +++ b/migrations/2_deploy_contracts.js @@ -133,40 +133,6 @@ module.exports = function (deployer, network, accounts) { }).then(() => { // Link libraries return deployer.link(TokenLib, STFactory); - // }).then(() => { - // return deployer.link(VersionUtils, SecurityTokenRegistry); - // }).then(() => { - // return deployer.link(VersionUtils, GeneralTransferManagerFactory); - // }).then(() => { - // return deployer.link(VersionUtils, GeneralPermissionManagerFactory); - // }).then(() => { - // return deployer.link(VersionUtils, PercentageTransferManagerFactory); - // }).then(() => { - // return deployer.link(VersionUtils, USDTieredSTOProxyFactory); - // }).then(() => { - // return deployer.link(VersionUtils, CountTransferManagerFactory); - // }).then(() => { - // return deployer.link(VersionUtils, EtherDividendCheckpointFactory); - // }).then(() => { - // return deployer.link(VersionUtils, ERC20DividendCheckpointFactory); - // }).then(() => { - // return deployer.link(VersionUtils, ManualApprovalTransferManagerFactory); - // }).then(() => { - // return deployer.link(VersionUtils, CappedSTOFactory); - // }).then(() => { - // return deployer.link(VersionUtils, USDTieredSTOFactory); - // }).then(() => { - // return deployer.link(Util, CappedSTOFactory); - // }).then(() => { - // return deployer.link(Util, USDTieredSTOFactory); - // }).then(() => { - // return deployer.link(Util, CountTransferManagerFactory); - // }).then(() => { - // return deployer.link(Util, PercentageTransferManagerFactory); - // }).then(() => { - // return deployer.link(Util, SecurityTokenRegistry); - // }).then(() => { - // return deployer.link(Util, STFactory); }).then(() => { // A) Deploy the ModuleRegistry Contract (It contains the list of verified ModuleFactory) return deployer.deploy(ModuleRegistry, {from: PolymathAccount}); diff --git a/test/o_security_token.js b/test/o_security_token.js index 68b3d6816..b6abc9b20 100644 --- a/test/o_security_token.js +++ b/test/o_security_token.js @@ -1154,7 +1154,7 @@ contract('SecurityToken', accounts => { it("Should force burn the tokens - value too high", async ()=> { let errorThrown = false; await I_GeneralTransferManager.changeAllowAllBurnTransfers(true, {from : token_owner}); - let currentInvestorCount = await I_SecurityToken.investorCount(); + let currentInvestorCount = await I_SecurityToken.getInvestorCount.call(); let currentBalance = await I_SecurityToken.balanceOf(account_temp); try { let tx = await I_SecurityToken.forceBurn(account_temp, currentBalance + web3.utils.toWei("500", "ether"), "", { from: account_controller }); @@ -1168,7 +1168,7 @@ contract('SecurityToken', accounts => { it("Should force burn the tokens - wrong caller", async ()=> { let errorThrown = false; await I_GeneralTransferManager.changeAllowAllBurnTransfers(true, {from : token_owner}); - let currentInvestorCount = await I_SecurityToken.investorCount(); + let currentInvestorCount = await I_SecurityToken.getInvestorCount.call(); let currentBalance = await I_SecurityToken.balanceOf(account_temp); try { let tx = await I_SecurityToken.forceBurn(account_temp, currentBalance, "", { from: token_owner }); @@ -1181,13 +1181,13 @@ contract('SecurityToken', accounts => { }); it("Should burn the tokens", async ()=> { - let currentInvestorCount = await I_SecurityToken.investorCount(); + let currentInvestorCount = await I_SecurityToken.getInvestorCount.call(); let currentBalance = await I_SecurityToken.balanceOf(account_temp); // console.log(currentInvestorCount.toString(), currentBalance.toString()); let tx = await I_SecurityToken.forceBurn(account_temp, currentBalance, "", { from: account_controller }); // console.log(tx.logs[0].args._value.toNumber(), currentBalance.toNumber()); assert.equal(tx.logs[0].args._value.toNumber(), currentBalance.toNumber()); - let newInvestorCount = await I_SecurityToken.investorCount(); + let newInvestorCount = await I_SecurityToken.getInvestorCount.call(); // console.log(newInvestorCount.toString()); assert.equal(newInvestorCount.toNumber() + 1, currentInvestorCount.toNumber(), "Investor count drops by one"); }); @@ -1298,13 +1298,13 @@ contract('SecurityToken', accounts => { let sender = account_investor1; let receiver = account_investor2; - let start_investorCount = await I_SecurityToken.investorCount.call(); + let start_investorCount = await I_SecurityToken.getInvestorCount.call(); let start_balInv1 = await I_SecurityToken.balanceOf.call(account_investor1); let start_balInv2 = await I_SecurityToken.balanceOf.call(account_investor2); let tx = await I_SecurityToken.forceTransfer(account_investor1, account_investor2, web3.utils.toWei("10", "ether"), "reason", {from: account_controller}); - let end_investorCount = await I_SecurityToken.investorCount.call(); + let end_investorCount = await I_SecurityToken.getInvestorCount.call(); let end_balInv1 = await I_SecurityToken.balanceOf.call(account_investor1); let end_balInv2 = await I_SecurityToken.balanceOf.call(account_investor2); From 11f6e60f28b4e2f10f21c4c2ebf18c4b5ec4142d Mon Sep 17 00:00:00 2001 From: satyam Date: Thu, 4 Oct 2018 11:33:19 +0530 Subject: [PATCH 2/6] minor naming fix --- contracts/libraries/TokenLib.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contracts/libraries/TokenLib.sol b/contracts/libraries/TokenLib.sol index 24c09f81c..691b71068 100644 --- a/contracts/libraries/TokenLib.sol +++ b/contracts/libraries/TokenLib.sol @@ -166,22 +166,22 @@ library TokenLib { * @param _to receiver of transfer * @param _value value of transfer */ - function adjustInvestorCount(InvestorDataStorage storage _investors, address _from, address _to, uint256 _value, uint256 _balanceTo, uint256 _balanceFrom) public { + function adjustInvestorCount(InvestorDataStorage storage _investorData, address _from, address _to, uint256 _value, uint256 _balanceTo, uint256 _balanceFrom) public { if ((_value == 0) || (_from == _to)) { return; } // Check whether receiver is a new token holder if ((_balanceTo == 0) && (_to != address(0))) { - _investors.investorCount = (_investors.investorCount).add(1); + _investorData.investorCount = (_investorData.investorCount).add(1); } // Check whether sender is moving all of their tokens if (_value == _balanceFrom) { - _investors.investorCount = (_investors.investorCount).sub(1); + _investorData.investorCount = (_investorData.investorCount).sub(1); } //Also adjust investor list - if (!_investors.investorListed[_to] && (_to != address(0))) { - _investors.investors.push(_to); - _investors.investorListed[_to] = true; + if (!_investorData.investorListed[_to] && (_to != address(0))) { + _investorData.investors.push(_to); + _investorData.investorListed[_to] = true; } } From 2b50c164d39ef27546a15a712d418e0d2f7861be Mon Sep 17 00:00:00 2001 From: satyam Date: Thu, 4 Oct 2018 12:33:37 +0530 Subject: [PATCH 3/6] add some improvements --- contracts/interfaces/ISecurityToken.sol | 5 +++++ contracts/libraries/TokenLib.sol | 6 +++++- contracts/tokens/SecurityToken.sol | 3 +++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/contracts/interfaces/ISecurityToken.sol b/contracts/interfaces/ISecurityToken.sol index a9c3b05a8..4cf950308 100644 --- a/contracts/interfaces/ISecurityToken.sol +++ b/contracts/interfaces/ISecurityToken.sol @@ -245,4 +245,9 @@ interface ISecurityToken { * @notice Use to get the version of the securityToken */ function getVersion() external view returns(uint8[]); + + /** + * @notice gets the investor count + */ + function getInvestorCount() external view returns(uint256) } diff --git a/contracts/libraries/TokenLib.sol b/contracts/libraries/TokenLib.sol index 691b71068..7aaa2e7f8 100644 --- a/contracts/libraries/TokenLib.sol +++ b/contracts/libraries/TokenLib.sol @@ -162,9 +162,12 @@ library TokenLib { /** * @notice keeps track of the number of non-zero token holders + * @param _investorData Date releated to investor metrics * @param _from sender of transfer * @param _to receiver of transfer * @param _value value of transfer + * @param _balanceTo balance of the _to address + * @param _balanceFrom balance of the _from address */ function adjustInvestorCount(InvestorDataStorage storage _investorData, address _from, address _to, uint256 _value, uint256 _balanceTo, uint256 _balanceFrom) public { if ((_value == 0) || (_from == _to)) { @@ -188,7 +191,8 @@ library TokenLib { /** * @notice removes addresses with zero balances from the investors list - * @param _index Index in investor list at which to start removing zero balances + * @param _investorData Date releated to investor metrics + * @param _index Index in investor list * NB - pruning this list will mean you may not be able to iterate over investors on-chain as of a historical checkpoint */ function pruneInvestors(InvestorDataStorage storage _investorData, uint256 _index) public { diff --git a/contracts/tokens/SecurityToken.sol b/contracts/tokens/SecurityToken.sol index 895075941..65256a914 100644 --- a/contracts/tokens/SecurityToken.sol +++ b/contracts/tokens/SecurityToken.sol @@ -420,6 +420,9 @@ contract SecurityToken is StandardToken, DetailedERC20, ReentrancyGuard, Registr return investorData.investors; } + /** + * @notice gets the investor count + */ function getInvestorCount() external view returns(uint256) { return investorData.investorCount; } From d5e0a7f03fd094d1cc42169da3182b3f406d4cf2 Mon Sep 17 00:00:00 2001 From: Mudit Gupta Date: Thu, 4 Oct 2018 13:03:16 +0530 Subject: [PATCH 4/6] added a missing semi-colon --- contracts/interfaces/ISecurityToken.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/interfaces/ISecurityToken.sol b/contracts/interfaces/ISecurityToken.sol index 4cf950308..09ffd2bb9 100644 --- a/contracts/interfaces/ISecurityToken.sol +++ b/contracts/interfaces/ISecurityToken.sol @@ -249,5 +249,5 @@ interface ISecurityToken { /** * @notice gets the investor count */ - function getInvestorCount() external view returns(uint256) + function getInvestorCount() external view returns(uint256); } From 7e2859be63aac7dfeefe235baed97677a37281a9 Mon Sep 17 00:00:00 2001 From: satyam Date: Thu, 4 Oct 2018 17:24:06 +0530 Subject: [PATCH 5/6] minor fix --- contracts/modules/TransferManager/CountTransferManager.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/modules/TransferManager/CountTransferManager.sol b/contracts/modules/TransferManager/CountTransferManager.sol index 274dae2d8..565a03033 100644 --- a/contracts/modules/TransferManager/CountTransferManager.sol +++ b/contracts/modules/TransferManager/CountTransferManager.sol @@ -28,7 +28,7 @@ contract CountTransferManager is ITransferManager { /// @notice Used to verify the transfer transaction according to the rule implemented in the trnasfer managers function verifyTransfer(address /* _from */, address _to, uint256 /* _amount */, bool /* _isTransfer */) public returns(Result) { if (!paused) { - if (maxHolderCount < ISecurityToken(securityToken).investorCount()) { + if (maxHolderCount < ISecurityToken(securityToken).getInvestorCount()) { // Allow transfers to existing maxHolders if (ISecurityToken(securityToken).balanceOf(_to) != 0) { return Result.NA; From 6d8bcee90d038d96bc0f344b6735b21e2b2ce95d Mon Sep 17 00:00:00 2001 From: satyam Date: Thu, 4 Oct 2018 18:29:42 +0530 Subject: [PATCH 6/6] remove investorCount --- contracts/interfaces/ISecurityToken.sol | 6 ------ 1 file changed, 6 deletions(-) diff --git a/contracts/interfaces/ISecurityToken.sol b/contracts/interfaces/ISecurityToken.sol index 09ffd2bb9..b0cbd22b1 100644 --- a/contracts/interfaces/ISecurityToken.sol +++ b/contracts/interfaces/ISecurityToken.sol @@ -116,12 +116,6 @@ interface ISecurityToken { */ function investors(uint256 _index) external view returns (address); - /** - * @notice gets the number of investors - * @return count of investors - */ - function investorCount() external view returns (uint256); - /** * @notice allows the owner to withdraw unspent POLY stored by them on the ST. * @dev Owner can transfer POLY to the ST which will be used to pay for modules that require a POLY fee.