Best Practices
smart contract best practices
External Calls
Use caution when making external calls
Mark untrusted contracts
Avoid state changes after external calls
Don't use
transfer()orsend()Handle errors in external calls
Favor pull over push for external calls
Don't delegatecall to untrusted code
Force-feeding Ether
Beware of coding an invariant that strictly checks the balance of a contract.
An attacker can forcibly send ether to any account using selfdestruct and this cannot be prevented (not even with a fallback function that does a revert()).
Also, since contract addresses can be precomputed, ether can be sent to an address before the contract is deployed.
Speed Bumps
Speed bumps slow down actions, so that if malicious actions occur, there is time to recover.
struct RequestedWithdrawal {
uint amount;
uint time;
}
mapping (address => uint) private balances;
mapping (address => RequestedWithdrawal) private requestedWithdrawals;
uint constant withdrawalWaitPeriod = 28 days; // 4 weeks
function requestWithdrawal() public {
if (balances[msg.sender] > 0) {
uint amountToWithdraw = balances[msg.sender];
balances[msg.sender] = 0; // for simplicity, we withdraw everything;
// presumably, the deposit function prevents new deposits when withdrawals are in progress
requestedWithdrawals[msg.sender] = RequestedWithdrawal({
amount: amountToWithdraw,
time: block.timestamp
});
}
}
function withdraw() public {
if(requestedWithdrawals[msg.sender].amount > 0 && block.timestamp > requestedWithdrawals[msg.sender].time + withdrawalWaitPeriod) {
uint amountToWithdraw = requestedWithdrawals[msg.sender].amount;
requestedWithdrawals[msg.sender].amount = 0;
require(msg.sender.send(amountToWithdraw));
}
}Rate Limiting
Rate limiting halts or requires approval for substantial changes. E.g. limit deposit withdrawals, tokens issued by contract.
uint internal period; // how many blocks before limit resets
uint internal limit; // max ether to withdraw per period
uint internal currentPeriodEnd; // block which the current period ends at
uint internal currentPeriodAmount; // amount already withdrawn this period
constructor(uint _period, uint _limit) public {
period = _period;
limit = _limit;
currentPeriodEnd = block.number + period;
}
function withdraw(uint amount) public {
// Update period before proceeding
updatePeriod();
// Prevent overflow
uint totalAmount = currentPeriodAmount + amount;
require(totalAmount >= currentPeriodAmount, 'overflow');
// Disallow withdraws that exceed current rate limit
require(currentPeriodAmount + amount < limit, 'exceeds period limit');
currentPeriodAmount += amount;
msg.sender.transfer(amount);
}
function updatePeriod() internal {
if(currentPeriodEnd < block.number) {
currentPeriodEnd = block.number + period;
currentPeriodAmount = 0;
}
}Deployment
Contracts should have a substantial and prolonged testing period - before substantial money is put at risk. Ideally one should have:
A full test suite with 100% test coverage (or close to it)
Deploy on your own testnet
Deploy on the public testnet with substantial testing and bug bounties
Exhaustive testing should allow various players to interact with the contract at volume
Deploy on the mainnet in beta, with limits to the amount at risk
Automatic Deprecation
During testing, you can force an automatic deprecation by preventing any actions, after a certain time period. For example, an alpha contract may work for several weeks and then automatically shut down all actions, except for the final withdrawal.
modifier isActive() {
require(block.number <= SOME_BLOCK_NUMBER);
_;
}
function deposit() public isActive {
// some code
}
function withdraw() public {
// some code
}Restrict amount of ether per user/contract
Reduces risk in case of insecure contracts
Assert, Require, Revert
Enforce invariants with assert()
An assert guard triggers when an assertion fails. Assert guards should often be combined with other techniques, such as pausing the contract and allowing upgrades, else you may end up stuck, with an assertion that is always failing.
**Use assert(), require(), revert() properly
The assert function should only be used to test for internal errors, and to check invariants.
The **require** function should be used to ensure valid conditions, such as inputs, or contract state variables are met, or to validate return values from calls to external contracts.
Following this paradigm allows formal analysis tools to verify that the invalid opcode can never be reached: meaning no invariants in the code are violated and that the code is formally verified.
pragma solidity ^0.5.0;
contract Sharer {
function sendHalf(address payable addr) public payable returns (uint balance) {
// require() can have an optional message string
require(msg.value % 2 == 0, "Even value required.");
uint balanceBeforeTransfer = address(this).balance;
(bool success, ) = addr.call.value(msg.value / 2)("");
require(success);
// Since we reverted if the transfer failed, there should be
// no way for us to still have half of the money.
assert(address(this).balance == balanceBeforeTransfer - msg.value / 2); // used for internal error checking
return address(this).balance;
}
}Modifiers as Guards
The code inside a modifier is usually executed before the function body, so any state changes or external calls will violate the Checks-Effects-Interactions pattern. Moreover, these statements may also remain unnoticed by the developer, as the code for modifier may be far from the function declaration. For example, an external call in modifier can lead to the reentrancy attack:
contract Registry {
address owner;
function isVoter(address _addr) external returns(bool) {
// Code
}
}
contract Election {
Registry registry;
modifier isEligible(address _addr) {
require(registry.isVoter(_addr));
_;
}
function vote() isEligible(msg.sender) public {
// Code
}In this case, the Registry contract can make a reentrancy attack by calling Election.vote() inside isVoter().
Integer Division
All integer division rounds down to the nearest integer. If you need more precision, consider using a multiplier, or store both the numerator and denominator.
// bad
// Result is 2, all integer division rounds DOWN to the nearest integer
uint x = 5 / 2;
// good
// using a multiplier prevents rounding down, this multiplier needs to be accounted for when working with x in the future
uint multiplier = 10;
uint x = (5 * multiplier) / 2;
// good
// store the numerator and denominator to calculate the result off-chain
uint numerator = 5;
uint denominator = 2;Locking Pragmas
Contracts should be deployed with the same compiler version and flags that they have been tested the most with. Locking the pragma helps ensure that contracts do not accidentally get deployed using, for example, the latest compiler which may have higher risks of undiscovered bugs.
Event Monitoring
Events can be used to monitor the contract's activity after it is deployed. Transaction history itself may be insufficient, as message calls between contracts are not recorded in the blockchain.
Events also show input parameters, and can be used to trigger functions in the user interface. Events that were emitted stay in the blockchain along with the other contract data and are available for future audit.
Complex Inheritance
When a contract is deployed, the compiler will linearize the inheritance from right to left (after the keyword is the parents are listed from the most base-like to the most derived).
contract Final {
uint public a;
function Final(uint f) public {
a = f;
}
}
contract B is Final {
int public fee;
function B(uint f) Final(f) public {
}
function setFee() public {
fee = 3;
}
}
contract C is Final {
int public fee;
function C(uint f) Final(f) public {
}
function setFee() public {
fee = 5;
}
}
contract A is B, C {
function A() public B(3) C(5) {
setFee();
}
}The consequence of the linearization will yield a fee value of 5, since C is the most derived contract.
EXTCODESIZE Checks
Avoid using extcodesize to check for Externally Owned Accounts.
The following modifier (or a similar check) is often used to verify whether a call was made from an externally owned account (EOA) or a contract account:
modifier isNotContract(address _a) {
uint size;
assembly {
size := extcodesize(_a)
}
require(size == 0);
_;
}The idea is straightforward: if an address contains code, it's not an EOA but a contract account. However, a contract does not have source code available during construction. This means that while the constructor is running, it can make calls to other contracts, but extcodesize for its address returns zero. Below is a minimal example that shows how this check can be circumvented:
contract OnlyForEOA {
uint public flag;
// bad
modifier isNotContract(address _a){
uint len;
assembly { len := extcodesize(_a) }
require(len == 0);
_;
}
function setFlag(uint i) public isNotContract(msg.sender){
flag = i;
}
}
contract FakeEOA {
constructor(address _a) public {
OnlyForEOA c = OnlyForEOA(_a);
c.setFlag(1);
}
}Because contract addresses can be pre-computed, this check could also fail if it checks an address which is empty at block n, but which has a contract deployed to it at some block greater than n.
Last updated