From d29910d700df0f338bfa8b490d7d1dab11d79375 Mon Sep 17 00:00:00 2001 From: Mudit Gupta Date: Fri, 25 Jan 2019 11:31:31 +0530 Subject: [PATCH 1/4] Initial optimization --- contracts/modules/STO/USDTieredSTO.sol | 71 +++++++++-------------- contracts/storage/USDTieredSTOStorage.sol | 10 ++-- 2 files changed, 34 insertions(+), 47 deletions(-) diff --git a/contracts/modules/STO/USDTieredSTO.sol b/contracts/modules/STO/USDTieredSTO.sol index 800651f93..d41920965 100644 --- a/contracts/modules/STO/USDTieredSTO.sol +++ b/contracts/modules/STO/USDTieredSTO.sol @@ -6,17 +6,16 @@ import "../../interfaces/IOracle.sol"; import "../../RegistryUpdater.sol"; import "../../libraries/DecimalMath.sol"; import "openzeppelin-solidity/contracts/math/SafeMath.sol"; -import "openzeppelin-solidity/contracts/utils/ReentrancyGuard.sol"; import "../../storage/USDTieredSTOStorage.sol"; /** * @title STO module for standard capped crowdsale */ -contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { +contract USDTieredSTO is USDTieredSTOStorage, STO { using SafeMath for uint256; - string public constant POLY_ORACLE = "PolyUsdOracle"; - string public constant ETH_ORACLE = "EthUsdOracle"; + string internal constant POLY_ORACLE = "PolyUsdOracle"; + string internal constant ETH_ORACLE = "EthUsdOracle"; //////////// // Events // @@ -200,10 +199,12 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { ) internal { - require(_tokensPerTierTotal.length > 0, "No tiers provided"); require( - _ratePerTier.length == _tokensPerTierTotal.length && _ratePerTierDiscountPoly.length == _tokensPerTierTotal.length && _tokensPerTierDiscountPoly.length == _tokensPerTierTotal.length, - "Tier data length mismatch" + _tokensPerTierTotal.length > 0 && + _ratePerTier.length == _tokensPerTierTotal.length && + _ratePerTierDiscountPoly.length == _tokensPerTierTotal.length && + _tokensPerTierDiscountPoly.length == _tokensPerTierTotal.length, + "Invalid Input" ); delete tiers; for (uint256 i = 0; i < _ratePerTier.length; i++) { @@ -253,7 +254,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { * @notice Reserve address must be whitelisted to successfully finalize */ function finalize() public onlyOwner { - require(!isFinalized, "STO is already finalized"); + require(!isFinalized, "STO already finalized"); isFinalized = true; uint256 tempReturned; uint256 tempSold; @@ -266,7 +267,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { tiers[i].mintedTotal = tiers[i].tokenTotal; } } - require(ISecurityToken(securityToken).mint(reserveWallet, tempReturned), "Error in minting"); + require(ISecurityToken(securityToken).mint(reserveWallet, tempReturned), "Minting Failed"); emit ReserveTokenMint(msg.sender, reserveWallet, tempReturned, currentTier); finalAmountReturned = tempReturned; totalTokensSold = tempSold; @@ -437,7 +438,6 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { FundRaiseType _fundRaiseType ) internal - nonReentrant whenNotPaused returns(uint256 spentUSD, uint256 spentValue) { @@ -485,10 +485,10 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { returns(uint256 netInvestedUSD) { require(isOpen(), "STO not open"); - require(_investmentValue > 0, "No funds were sent"); + require(_investmentValue > 0, "No funds sent"); // Check for minimum investment - require(investedUSD.add(investorInvestedUSD[_beneficiary]) >= minimumInvestmentUSD, "Total investment < minimumInvestmentUSD"); + require(investedUSD.add(investorInvestedUSD[_beneficiary]) >= minimumInvestmentUSD, "Investment < min"); netInvestedUSD = investedUSD; // Check for non-accredited cap if (investors[_beneficiary].accredited == uint8(0)) { @@ -565,7 +565,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { purchasedTokens = maximumTokens; } if (purchasedTokens > 0) { - require(ISecurityToken(securityToken).mint(_beneficiary, purchasedTokens), "Error in minting"); + require(ISecurityToken(securityToken).mint(_beneficiary, purchasedTokens), "Mint failed"); emit TokenPurchase(msg.sender, _beneficiary, purchasedTokens, spentUSD, _tierPrice, _tier); } } @@ -579,15 +579,8 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { * @return bool Whether the STO is accepting investments */ function isOpen() public view returns(bool) { - if (isFinalized) - return false; - /*solium-disable-next-line security/no-block-members*/ - if (now < startTime) - return false; - /*solium-disable-next-line security/no-block-members*/ - if (now >= endTime) - return false; - if (capReached()) + /*solium-disable-next-line security/no-block-members*/ + if (isFinalized || now < startTime || now >= endTime || capReached()) return false; return true; } @@ -613,10 +606,11 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { } else if (_fundRaiseType == FundRaiseType.POLY) { return IOracle(_getOracle(bytes32("POLY"), bytes32("USD"))).getPrice(); } else if (_fundRaiseType == FundRaiseType.SC) { - return 1 * 10**18; - } else { - revert("Incorrect funding"); - } + return 10**18; + } //else { it's a view function so reverting and returning 0 rate is same for outside EVM world. + // Non-view functions that call this verify the fundraisetype first. + // revert("Invalid funding"); + // } } /** @@ -626,8 +620,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { * @return uint256 Value in USD */ function convertToUSD(FundRaiseType _fundRaiseType, uint256 _amount) public view returns(uint256) { - uint256 rate = getRate(_fundRaiseType); - return DecimalMath.mul(_amount, rate); + return DecimalMath.mul(_amount, getRate(_fundRaiseType)); } /** @@ -637,8 +630,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { * @return uint256 Value in ETH or POLY */ function convertFromUSD(FundRaiseType _fundRaiseType, uint256 _amount) public view returns(uint256) { - uint256 rate = getRate(_fundRaiseType); - return DecimalMath.div(_amount, rate); + return DecimalMath.div(_amount, getRate(_fundRaiseType)); } /** @@ -648,20 +640,17 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { function getTokensSold() public view returns (uint256) { if (isFinalized) return totalTokensSold; - else - return getTokensMinted(); + return getTokensMinted(); } /** * @notice Return the total no. of tokens minted * @return uint256 Total number of tokens minted */ - function getTokensMinted() public view returns (uint256) { - uint256 tokensMinted; + function getTokensMinted() public view returns (uint256 tokensMinted) { for (uint256 i = 0; i < tiers.length; i++) { tokensMinted = tokensMinted.add(tiers[i].mintedTotal); } - return tokensMinted; } /** @@ -669,12 +658,10 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { * param _fundRaiseType The fund raising currency (e.g. ETH, POLY, SC) to calculate sold tokens for * @return uint256 Total number of tokens sold for ETH */ - function getTokensSoldFor(FundRaiseType _fundRaiseType) public view returns (uint256) { - uint256 tokensSold; + function getTokensSoldFor(FundRaiseType _fundRaiseType) public view returns (uint256 tokensSold) { for (uint256 i = 0; i < tiers.length; i++) { tokensSold = tokensSold.add(tiers[i].minted[uint8(_fundRaiseType)]); } - return tokensSold; } /** @@ -683,7 +670,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { * @return uint256[] array of minted tokens in each fund raise type */ function getTokensMintedByTier(uint256 _tier) public view returns(uint256[] memory) { - require(_tier < tiers.length, "Invalid tier"); + //require(_tier < tiers.length, "Invalid tier"); It will revert due to array out of bounds anyway uint256[] memory tokensMinted = new uint256[](3); tokensMinted[0] = tiers[_tier].minted[uint8(FundRaiseType.ETH)]; tokensMinted[1] = tiers[_tier].minted[uint8(FundRaiseType.POLY)]; @@ -697,7 +684,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { * @return uint256 Total number of tokens sold in the tier */ function getTokensSoldByTier(uint256 _tier) public view returns (uint256) { - require(_tier < tiers.length, "Incorrect tier"); + //require(_tier < tiers.length, "Invalid tier"); It will revert due to array out of bounds anyway uint256 tokensSold; tokensSold = tokensSold.add(tiers[_tier].minted[uint8(FundRaiseType.ETH)]); tokensSold = tokensSold.add(tiers[_tier].minted[uint8(FundRaiseType.POLY)]); @@ -724,8 +711,8 @@ contract USDTieredSTO is USDTieredSTOStorage, STO, ReentrancyGuard { /** * @notice Return the permissions flag that are associated with STO */ - function getPermissions() public view returns(bytes32[] memory) { - bytes32[] memory allPermissions = new bytes32[](0); + function getPermissions() public view returns(bytes32[] memory allPermissions) { + //bytes32[] memory allPermissions = new bytes32[](0); return allPermissions; } diff --git a/contracts/storage/USDTieredSTOStorage.sol b/contracts/storage/USDTieredSTOStorage.sol index a06721ac4..080aef1eb 100644 --- a/contracts/storage/USDTieredSTOStorage.sol +++ b/contracts/storage/USDTieredSTOStorage.sol @@ -38,22 +38,22 @@ contract USDTieredSTOStorage { mapping(bytes32 => mapping(bytes32 => string)) oracleKeys; - IERC20 public usdToken; + //IERC20 public usdToken; not needed, must've been left out in merge. // Determine whether users can invest on behalf of a beneficiary - bool public allowBeneficialInvestments = false; + bool public allowBeneficialInvestments; // Whether or not the STO has been finalized bool public isFinalized; // Address where ETH, POLY & DAI funds are delivered - address payable public wallet; + //address payable public wallet; Already defined in STOStorage // Address of issuer reserve wallet for unsold tokens address public reserveWallet; // List of stable coin addresses - address[] public usdTokens; + address[] internal usdTokens; //there is a getUsdTokens getter // Current tier uint256 public currentTier; @@ -74,7 +74,7 @@ contract USDTieredSTOStorage { mapping (address => Investor) public investors; // List of active stable coin addresses - mapping (address => bool) public usdTokenEnabled; + mapping (address => bool) internal usdTokenEnabled; // List of all addresses that have been added as accredited or non-accredited without // the default limit From 24bd577e489e5794a62e50b2c68621a1fc7f40b2 Mon Sep 17 00:00:00 2001 From: Mudit Gupta Date: Fri, 25 Jan 2019 12:23:05 +0530 Subject: [PATCH 2/4] Tests fixed --- test/p_usd_tiered_sto.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/p_usd_tiered_sto.js b/test/p_usd_tiered_sto.js index c41150ad0..4661fb043 100644 --- a/test/p_usd_tiered_sto.js +++ b/test/p_usd_tiered_sto.js @@ -378,7 +378,7 @@ contract("USDTieredSTO", async (accounts) => { _reserveWallet[stoId], "Incorrect _reserveWallet in config" ); - assert.equal(await I_USDTieredSTO_Array[stoId].usdTokens.call(0), _usdToken[stoId][0], "Incorrect _usdToken in config"); + assert.equal((await I_USDTieredSTO_Array[stoId].getUsdTokens())[0], _usdToken[stoId][0], "Incorrect _usdToken in config"); assert.equal( await I_USDTieredSTO_Array[stoId].getNumberOfTiers(), _tokensPerTierTotal[stoId].length, @@ -567,7 +567,7 @@ contract("USDTieredSTO", async (accounts) => { _reserveWallet[stoId], "Incorrect _reserveWallet in config" ); - assert.equal(await I_USDTieredSTO_Array[stoId].usdTokens.call(0), _usdToken[stoId][0], "Incorrect _usdToken in config"); + assert.equal((await I_USDTieredSTO_Array[stoId].getUsdTokens())[0], _usdToken[stoId][0], "Incorrect _usdToken in config"); assert.equal( await I_USDTieredSTO_Array[stoId].getNumberOfTiers(), _tokensPerTierTotal[stoId].length, @@ -965,7 +965,7 @@ contract("USDTieredSTO", async (accounts) => { "0x0000000000000000000003000000000000000000", "STO Configuration doesn't set as expected" ); - assert.equal((await I_USDTieredSTO_Array[stoId].usdTokens.call(0)), accounts[3], "STO Configuration doesn't set as expected"); + assert.equal((await I_USDTieredSTO_Array[stoId].getUsdTokens())[0], accounts[3], "STO Configuration doesn't set as expected"); }); it("Should fail to change config after endTime", async () => { @@ -3868,7 +3868,7 @@ contract("USDTieredSTO", async (accounts) => { let init_WalletETHBal = new BN(await web3.eth.getBalance(WALLET)); let init_WalletPOLYBal = await I_PolyToken.balanceOf(WALLET); - let tokensToMint = (await I_USDTieredSTO_Array[stoId].buyWithPOLY.call(ACCREDITED1, investment_POLY))[2]; + let tokensToMint = (await I_USDTieredSTO_Array[stoId].buyWithPOLY.call(ACCREDITED1, investment_POLY, {from: ACCREDITED1}))[2]; // Buy With POLY let tx2 = await I_USDTieredSTO_Array[stoId].buyWithPOLY(ACCREDITED1, investment_POLY, { From ed06731bad0c0e11fda54ab1101137b14f935c38 Mon Sep 17 00:00:00 2001 From: Mudit Gupta Date: Fri, 25 Jan 2019 12:25:18 +0530 Subject: [PATCH 3/4] comments removed --- contracts/modules/STO/USDTieredSTO.sol | 8 +------- contracts/storage/USDTieredSTOStorage.sol | 7 +------ 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/contracts/modules/STO/USDTieredSTO.sol b/contracts/modules/STO/USDTieredSTO.sol index d41920965..1e232a5ee 100644 --- a/contracts/modules/STO/USDTieredSTO.sol +++ b/contracts/modules/STO/USDTieredSTO.sol @@ -607,10 +607,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO { return IOracle(_getOracle(bytes32("POLY"), bytes32("USD"))).getPrice(); } else if (_fundRaiseType == FundRaiseType.SC) { return 10**18; - } //else { it's a view function so reverting and returning 0 rate is same for outside EVM world. - // Non-view functions that call this verify the fundraisetype first. - // revert("Invalid funding"); - // } + } } /** @@ -670,7 +667,6 @@ contract USDTieredSTO is USDTieredSTOStorage, STO { * @return uint256[] array of minted tokens in each fund raise type */ function getTokensMintedByTier(uint256 _tier) public view returns(uint256[] memory) { - //require(_tier < tiers.length, "Invalid tier"); It will revert due to array out of bounds anyway uint256[] memory tokensMinted = new uint256[](3); tokensMinted[0] = tiers[_tier].minted[uint8(FundRaiseType.ETH)]; tokensMinted[1] = tiers[_tier].minted[uint8(FundRaiseType.POLY)]; @@ -684,7 +680,6 @@ contract USDTieredSTO is USDTieredSTOStorage, STO { * @return uint256 Total number of tokens sold in the tier */ function getTokensSoldByTier(uint256 _tier) public view returns (uint256) { - //require(_tier < tiers.length, "Invalid tier"); It will revert due to array out of bounds anyway uint256 tokensSold; tokensSold = tokensSold.add(tiers[_tier].minted[uint8(FundRaiseType.ETH)]); tokensSold = tokensSold.add(tiers[_tier].minted[uint8(FundRaiseType.POLY)]); @@ -712,7 +707,6 @@ 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[](0); return allPermissions; } diff --git a/contracts/storage/USDTieredSTOStorage.sol b/contracts/storage/USDTieredSTOStorage.sol index 080aef1eb..267df4f58 100644 --- a/contracts/storage/USDTieredSTOStorage.sol +++ b/contracts/storage/USDTieredSTOStorage.sol @@ -38,22 +38,17 @@ contract USDTieredSTOStorage { mapping(bytes32 => mapping(bytes32 => string)) oracleKeys; - //IERC20 public usdToken; not needed, must've been left out in merge. - // Determine whether users can invest on behalf of a beneficiary bool public allowBeneficialInvestments; // Whether or not the STO has been finalized bool public isFinalized; - // Address where ETH, POLY & DAI funds are delivered - //address payable public wallet; Already defined in STOStorage - // Address of issuer reserve wallet for unsold tokens address public reserveWallet; // List of stable coin addresses - address[] internal usdTokens; //there is a getUsdTokens getter + address[] internal usdTokens; // Current tier uint256 public currentTier; From dee447ea1838b098df2ec4de930524590988d472 Mon Sep 17 00:00:00 2001 From: Adam Dossa Date: Fri, 25 Jan 2019 07:52:36 -0400 Subject: [PATCH 4/4] Make some functions external --- contracts/modules/STO/USDTieredSTO.sol | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/modules/STO/USDTieredSTO.sol b/contracts/modules/STO/USDTieredSTO.sol index 1e232a5ee..757ea39c4 100644 --- a/contracts/modules/STO/USDTieredSTO.sol +++ b/contracts/modules/STO/USDTieredSTO.sol @@ -201,8 +201,8 @@ contract USDTieredSTO is USDTieredSTOStorage, STO { { require( _tokensPerTierTotal.length > 0 && - _ratePerTier.length == _tokensPerTierTotal.length && - _ratePerTierDiscountPoly.length == _tokensPerTierTotal.length && + _ratePerTier.length == _tokensPerTierTotal.length && + _ratePerTierDiscountPoly.length == _tokensPerTierTotal.length && _tokensPerTierDiscountPoly.length == _tokensPerTierTotal.length, "Invalid Input" ); @@ -655,7 +655,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO { * param _fundRaiseType The fund raising currency (e.g. ETH, POLY, SC) to calculate sold tokens for * @return uint256 Total number of tokens sold for ETH */ - function getTokensSoldFor(FundRaiseType _fundRaiseType) public view returns (uint256 tokensSold) { + function getTokensSoldFor(FundRaiseType _fundRaiseType) external view returns (uint256 tokensSold) { for (uint256 i = 0; i < tiers.length; i++) { tokensSold = tokensSold.add(tiers[i].minted[uint8(_fundRaiseType)]); } @@ -666,7 +666,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO { * param _tier The tier to return minted tokens for * @return uint256[] array of minted tokens in each fund raise type */ - function getTokensMintedByTier(uint256 _tier) public view returns(uint256[] memory) { + function getTokensMintedByTier(uint256 _tier) external view returns(uint256[] memory) { uint256[] memory tokensMinted = new uint256[](3); tokensMinted[0] = tiers[_tier].minted[uint8(FundRaiseType.ETH)]; tokensMinted[1] = tiers[_tier].minted[uint8(FundRaiseType.POLY)]; @@ -679,7 +679,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO { * param _tier The tier to calculate sold tokens for * @return uint256 Total number of tokens sold in the tier */ - function getTokensSoldByTier(uint256 _tier) public view returns (uint256) { + function getTokensSoldByTier(uint256 _tier) external view returns (uint256) { uint256 tokensSold; tokensSold = tokensSold.add(tiers[_tier].minted[uint8(FundRaiseType.ETH)]); tokensSold = tokensSold.add(tiers[_tier].minted[uint8(FundRaiseType.POLY)]); @@ -691,7 +691,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO { * @notice Return the total no. of tiers * @return uint256 Total number of tiers */ - function getNumberOfTiers() public view returns (uint256) { + function getNumberOfTiers() external view returns (uint256) { return tiers.length; } @@ -699,7 +699,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO { * @notice Return the usd tokens accepted by the STO * @return address[] usd tokens */ - function getUsdTokens() public view returns (address[] memory) { + function getUsdTokens() external view returns (address[] memory) { return usdTokens; } @@ -722,7 +722,7 @@ contract USDTieredSTO is USDTieredSTOStorage, STO { * @return Amount of tokens sold. * @return Array of bools to show if funding is allowed in ETH, POLY, SC respectively */ - function getSTODetails() public view returns(uint256, uint256, uint256, uint256[] memory, uint256[] memory, uint256, uint256, uint256, bool[] memory) { + function getSTODetails() external view returns(uint256, uint256, uint256, uint256[] memory, uint256[] memory, uint256, uint256, uint256, bool[] memory) { uint256[] memory cap = new uint256[](tiers.length); uint256[] memory rate = new uint256[](tiers.length); for(uint256 i = 0; i < tiers.length; i++) {