Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Solidity good practices #20

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ This and the security risks of smart contracts that interact with other smart co
* **Convincibility**: Write code that allows other parties to be easily convinced that it works. Remember that more time is spent in reviews/audits/bug bounties than in actually writing the code.
* **Reusability**: Write code that is generic enough to avoid starting from scratch every time. This should not hinder the 3 previous priorities.
* Do not **over-engineer**. Over-engineering lowers security, increases gas costs, and decreases convincibility. [Kiss](https://en.wikipedia.org/wiki/KISS_principle) ♥.
* **Keep the code in one place as much as possible**. Don't use separate functions unless it saves on code duplication/gas costs. Inheritance from abstract contracts is usually not recommended as it might make it tedious for the reviewer to check all the inherited code when reviewing a contract. OpenZeppelin contracts are ok to use as they are deemed secure, although they are often over-engineered.
* It is easier for the reviewers to check the code they are used to so it is better to **be conservative with the code you write**. Having unusual code arhitectures or using new Solidity features is usually not recommended (e.g. custom errors), unless it provides a clear benefit over the altered difficulty of reviewing.
* Smart contracts should only do what is **required, no more, no less**. Do not abstract contracts into more generic contracts if the deployed version will only use a subset of the functionality. E.g. if you can only have 2 parties in your contract, only write code for 2 parties. Do not write code supporting an arbitrary number of parties and then set it to 2 at deployment. This is a stark contrast to traditional software where generic abstractions can save time in the long run. In smart contract development, there is no continuous deployments and the additional time spent in security practices that a generic contract requires far outweighs the potential benefit of its reusability.
* However, **abstraction** can be useful when contracts need to interact with contracts which are **not known in advance**, either because they won't be finished by the time of deployment or because they will be developed by other projects you don't have control over. [**ERC20**](https://github.com/ethereum/EIPs/issues/20) and [**ERC792**](https://github.com/ethereum/EIPs/issues/792) are good examples of this.
* Contrary to what the compiler warnings may tell you, **send** is generally more secure than **transfer** as **transfer** allows the recipient to block the call. Don't use **transfer** unless there is a good reason to.
* Contrary to what the compiler warnings may tell you, **send** is generally more secure than **transfer** as **transfer** allows the recipient to block the call. Don't use **transfer** unless there is a good reason to. Sending ETH using **send** is also more secure than **call** as well as it takes away some of the stress of possible frontrunning by limiting the gas.