DePayFi / depay-evm-launchpad

🚀 An EVM smart contract to perform token whitelist, claiming and release in a launchpad style.
25 stars 12 forks source link

Improvements based on Audit#3 report #9

Closed 10xSebastian closed 3 years ago

10xSebastian commented 3 years ago

M01-Multiplication before division has been fixed as part of improvements for Audit#2 already -> HERE and it is still fixed in master -> HERE

We do not intend to act on L01-Infinite loop possibility for the multi-action method whitelistAddresses. As confirmed by the auditor, it's an owner function and the owner needs to make sure to not pass too many addresses if calling that function. We would also like to add, that a typical preflight of your transaction happening in most common wallets when computing estimateGas before sending a potentially infinite loop transaction to this method, will already fail in the wallet client, hence the risk of sending too many addresses to that function is almost non-existing for us.

We still do not intend to add events to "critical functions" as reported in L02-Critical functions lack events for the following reasons: For all the initialization functions, those are called by the initiator/owner only, which in our opinion does not require any events. People that want to interact with smart contract will look at it's state after it has been started and there are not potential state changes after the launchpad started, as end is an implicit state change based on time passing, which cant even trigger an event. While we believe claim could be the only considerable transaction and state change worth while emit an event for, we ask people that would like to listen to events triggered by that function to simply listen to the Transfer event that that method also gonna emit already because claim transfers launched tokens, users can listen to that event instead without any disadvantages.

N01-Make variables constant is not implementable as init requires the launched token to be present in the smart contract at the moment init is called, so it cannot be moved to a constructor, as the smart contract wouldn't exist upon initialization via constructor, hence there would also not be the possibility of having stored any launchedTokens within the smart contract as that's only possible after the smart contract has been stored on the blockchain. We also believe that onlyUninitialized takes care of those variables not being able to be set or changed. We are okay not optimizing that entire implementation for the only sake of saving the deployer some gas.

Improves N02-Avoid using the SafeMath library as Solidity implements it > 0.8.0 -> HERE

As confirmed with the other auditors N03-Use the latest solidity version while deploying is in fact 0.8.1 as 0.8.6. is not considered stable just yet.

Improves N04-The variable totalClaimable should be zero -> HERE

We plan to not act on N05-Unnecessary condition as pointed out in the audit, too, we prefer the explicit revert reason over the gas optimization.