Skip to content

Instantly share code, notes, and snippets.

@furusiyya
Created October 30, 2017 02:35
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save furusiyya/9672eda0a0fa387c078258432bb9fd0d to your computer and use it in GitHub Desktop.
Save furusiyya/9672eda0a0fa387c078258432bb9fd0d to your computer and use it in GitHub Desktop.

Specifications of a contract

  • A flat pricing with ETH: USD - $0.02 per token (50 tokens to 1 USD)
  • Coin rate should be $0.02 (can just use the live feed from www.oraclize.it or similar)
  • Token supply: 1 billion, selling 150 million e.g. $3M cap.
  • Token: 8 decimals
  • Multisig wallet for the proceeds, as per gnosis (as that doesn't seem to have had any issues vs. parity)
  • Ensure tokens are reserved and allocated by a certain date when they unlock to subscribers
  • No ETH cap: Anyone can buy as many tokens as they like until all tokens are sold
  • Sale ends when 150m tokens (out of 1bn) allocated to contract run out or a certain date is reached
  • Ideally the smart contract should verify in etherscan and fit into the token tracker
  • There will be no whitelisting of investors and no hard/soft cap.
  • There will be no refund

Audit

Crowdsale code is audited against above specifications. AllocatedCrowdsale.sol is considered as a root for the crowdsale so audit covers only following files:

  • AllocatedCrowdSale.sol
  • Crowdsale.sol
  • FinalizeAgent.sol
  • Haltable.sol
  • PricingStategy.sol
  • FlatPricing.sol
    Few contracts are imported from zeppline-solidity so commit 8e01dd1 of zeppelin is used in audit.

1. Unit tests

No unit test are provided which is a big red flag. I will recommend to write unit test and use of truffle.

2. Redundant code

Some unnecessary code is found that is not required according the specs so I will recommend to remove this to minimizing the contract size.

  • No Time Period for crowdsale
    Crowdsale will be open until all the tokens are not sold and their is no ether cap so following line of codes are unnecessary.
    • CrowdSale.sol:30
    • Crowdsale.sol:33
    • Crowdsale.sol:53
    • Crowdsale.sol:170
  • No refund
    There will be no refund in the crowdsale so following lines are unnecessary.
    • Crowdsale.sol:74
    • Crowdsale.sol:77
    • Crowdsale.sol:125
  • No whitelisting
    Anonymous investments will be acceptable so following lines of code can be removed.
    • CrowdSale.sol:83
    • Crowdsale.sol:89
    • Crowdsale.sol:92
    • Crowdsale.sol:101
    • Crowdsale.sol:104
    • Crowdsale.sol:128
    • Crowdsale.sol:131
    • Crowdsale.sol:281 to Crowdsale.sol:335
    • CrowdSale.sol:375 to CrowdSale.sol:415
    • CrowdSale.sol:476 to Crowdsale.sol:504
  • Unused variables
    • Remove unused variables from FlatPricing:29

3. Correction

After removing redundant code following corrections are required:

  • Remove _minimumFundingGoal, _baseEthCap from constructor parameter list of Crowdsale.sol and AllocatedCrowdsale.sol.
  • Rename investInternal(address receiver, uint128 customerId) to buy(address receiver) and make it public
  • Modify event Invested(address investor, uint weiAmount, uint tokenAmount, uint128 customerId) to event Invested(address investor, uint weiAmount, uint tokenAmount)
  • Remove PreFunding State from the Crowdsale.sol
  • Modify check condition of State.Success in Crowdsale.sol:538 to depends upon amount of token sold instead of minimum funding.

4. Calculation Error

Function calculatePrice() in FlatPricing does not calculate right price because of invalid multiplier value. Total decimal points in ether are 18 so we have to devide by 10 ** 10 instead of 10 ** 8.

5. Following preconditions and sanity checks are missing

  • At Crowdsale.sol constructor parameter _token
  • At AllocatedCrowdsale.sol constructor parameter _beneficiary
  • At preallocate() function parameters Crowdsale.sol:261
  • At setMultisig() function parameters Crowdsale.sol:461
  • At setEthToUsd() function parameters Crowdsale.sol:450
  • At setEthToUsd() function parameters Crowdsale.sol:450
  • At setEthToUsd() function parameters FlatPricing.sol:22
  • At calculatePrice() function parameters FlatPricing.sol:29
  • Restrict accessibility to external for the following functions
    • setPricingStrategy()
    • setEthToUsd()
    • setMultisig()
    • setEndsAt()
    • setFinalizeAgent()
    • finalize()
    • setMultisig()

Recommendations

  • Use of wallet
    Software wallets have pretty bad history so I would recommend hardware wallet. Multisig is secure until there is no bug in code but hardware wallet will always require physical interaction to initiate transaction.

Conclusions

One critical issue found regarding calculation mentioned in section 4. Crowdsale can go into invalid state or even unrecoverable state due to the absence of sanity checks at function arguments explained in section 5. Redundant code found that increases size of the contract so its a waste of gas(ethers). Right now contract is not complete according to the specs so after implementation of above changes please test it again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment