Skip to content

Commit 23b5b06

Browse files
authored
copyediting (#93)
1 parent ca387a4 commit 23b5b06

File tree

1 file changed

+46
-46
lines changed

1 file changed

+46
-46
lines changed
Lines changed: 46 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
# Token integration checklist
22

3-
Follow this checklist when interacting with arbitrary tokens. Make sure you understand the risks associated with each item, and justify any exceptions to these rules.
3+
The following checklist provides recommendations for interactions with arbitrary tokens. Every unchecked item should be justified, and its associated risks, understood.
44

5-
For convenience, all Slither [utilities](https://github.com/crytic/slither#tools) can be run directly on a token address, such as:
5+
For convenience, all Slither [utilities](https://github.com/crytic/slither#tools) can be run directly on a token address, such as the following:
66

77
```bash
88
slither-check-erc 0xdac17f958d2ee523a2206206994597c13d831ec7 TetherToken --erc erc20
99
slither-check-erc 0x06012c8cf97BEaD5deAe237070F9587f8E7A266d KittyCore --erc erc721
1010
```
1111

12-
To follow this checklist, you'll want to have this output from Slither for the token:
12+
To follow this checklist, use the below output from Slither for the token:
1313

1414
```bash
1515
- slither-check-erc [target] [contractName] [optional: --erc ERC_NUMBER]
@@ -20,76 +20,76 @@ To follow this checklist, you'll want to have this output from Slither for the t
2020

2121
## General considerations
2222

23-
- [ ] **The contract has a security review.** Avoid interacting with contracts that lack a security review. Check the length of the assessment (aka “level of effort), the reputation of the security firm, and the number and severity of the findings.
23+
- [ ] **The contract has a security review.** Avoid interacting with contracts that lack a security review. Check the length of the assessment (i.e., the level of effort), the reputation of the security firm, and the number and severity of the findings.
2424
- [ ] **You have contacted the developers.** You may need to alert their team to an incident. Look for appropriate contacts on [blockchain-security-contacts](https://github.com/crytic/blockchain-security-contacts).
2525
- [ ] **They have a security mailing list for critical announcements.** Their team should advise users (like you!) when critical issues are found or when upgrades occur.
2626

2727
## Contract composition
2828

29-
- [ ] **The contract avoids unneeded complexity.** The token should be a simple contract; a token with complex code requires a higher standard of review. Use Slither’s [human-summary printer](https://github.com/crytic/slither/wiki/Printer-documentation#human-summary) to identify complex code.
30-
- [ ] **The contract uses SafeMath.** Contracts that do not use SafeMath require a higher standard of review. Inspect the contract by hand for SafeMath usage.
31-
- [ ] **The contract has only a few non–token-related functions.** Nontoken-related functions increase the likelihood of an issue in the contract. Use Slither’s [contract-summary printer](https://github.com/crytic/slither/wiki/Printer-documentation#contract-summary) to broadly review the code used in the contract.
32-
- [ ] **The token only has one address.** Tokens with multiple entry points for balance updates can break internal bookkeeping based on the address (e.g. `balances[token_address][msg.sender]` might not reflect the actual balance).
29+
- [ ] **The contract avoids unneeded complexity.** The token should be a simple contract; a token with complex code requires a higher standard of review. Use Slither’s [`human-summary` printer](https://github.com/crytic/slither/wiki/Printer-documentation#human-summary) to identify complex code.
30+
- [ ] **The contract uses `SafeMath`.** Contracts that do not use `SafeMath` require a higher standard of review. Inspect the contract by hand for `SafeMath` usage.
31+
- [ ] **The contract has only a few non–token-related functions.** Non-token-related functions increase the likelihood of an issue in the contract. Use Slither’s [`contract-summary` printer](https://github.com/crytic/slither/wiki/Printer-documentation#contract-summary) to broadly review the code used in the contract.
32+
- [ ] **The token only has one address.** Tokens with multiple entry points for balance updates can break internal bookkeeping based on the address (e.g., `balances[token_address][msg.sender]` may not reflect the actual balance).
3333

3434
## Owner privileges
3535

36-
- [ ] **The token is not upgradeable.** Upgradeable contracts might change their rules over time. Use Slither’s [human-summary printer](https://github.com/crytic/slither/wiki/Printer-documentation#contract-summary) to determine if the contract is upgradeable.
37-
- [ ] **The owner has limited minting capabilities.** Malicious or compromised owners can abuse minting capabilities. Use Slither’s [human-summary printer](https://github.com/crytic/slither/wiki/Printer-documentation#contract-summary) to review minting capabilities, and consider manually reviewing the code.
38-
- [ ] **The token is not pausable.** Malicious or compromised owners can trap contracts relying on pausable tokens. Identify pauseable code by hand.
36+
- [ ] **The token is not upgradeable.** Upgradeable contracts may change their rules over time. Use Slither’s [`human-summary` printer](https://github.com/crytic/slither/wiki/Printer-documentation#contract-summary) to determine whether the contract is upgradeable.
37+
- [ ] **The owner has limited minting capabilities.** Malicious or compromised owners can abuse minting capabilities. Use Slither’s [`human-summary` printer](https://github.com/crytic/slither/wiki/Printer-documentation#contract-summary) to review minting capabilities, and consider manually reviewing the code.
38+
- [ ] **The token is not pausable.** Malicious or compromised owners can trap contracts relying on pausable tokens. Identify pausable code by hand.
3939
- [ ] **The owner cannot blacklist the contract.** Malicious or compromised owners can trap contracts relying on tokens with a blacklist. Identify blacklisting features by hand.
40-
- [ ] **The team behind the token is known and can be held responsible for abuse.** Contracts with anonymous development teams, or that reside in legal shelters should require a higher standard of review.
40+
- [ ] **The team behind the token is known and can be held responsible for abuse.** Contracts with anonymous development teams or teams that reside in legal shelters require a higher standard of review.
4141

42-
## ERC-20 tokens
42+
## ERC20 tokens
4343

44-
### Suggested ERC-20 conformity checks
44+
### ERC20 conformity checks
4545

46-
Slither includes a utility, [slither-check-erc](https://github.com/crytic/slither/wiki/ERC-Conformance), that reviews the conformance of a token to many related ERC standards. Use slither-check-erc to review that:
46+
Slither includes a utility, [`slither-check-erc`](https://github.com/crytic/slither/wiki/ERC-Conformance), that reviews the conformance of a token to many related ERC standards. Use `slither-check-erc` to review the following:
4747

48-
- [ ] **Transfer and transferFrom return a boolean.** Several tokens do not return a boolean on these functions. As a result, their calls in the contract might fail.
49-
- [ ] **The name, decimals, and symbol functions are present if used.** These functions are optional in the ERC20 standard and might not be present.
50-
- [ ] **Decimals returns a uint8.** Several tokens incorrectly return a uint256. If this is the case, ensure the value returned is below 255.
48+
- [ ] **`Transfer` and `transferFrom` return a boolean.** Several tokens do not return a boolean on these functions. As a result, their calls in the contract might fail.
49+
- [ ] **The `name`, `decimals`, and `symbol` functions are present if used.** These functions are optional in the ERC20 standard and may not be present.
50+
- [ ] **`Decimals` returns a `uint8`.** Several tokens incorrectly return a `uint256`. In such cases, ensure that the value returned is below 255.
5151
- [ ] **The token mitigates the known [ERC20 race condition](https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729).** The ERC20 standard has a known ERC20 race condition that must be mitigated to prevent attackers from stealing tokens.
5252

53-
Slither includes a utility, [slither-prop](https://github.com/crytic/slither/wiki/Property-generation), that generates unit tests and security properties that can discover many common ERC flaws. Use slither-prop to review that:
53+
Slither includes a utility, [`slither-prop`](https://github.com/crytic/slither/wiki/Property-generation), that generates unit tests and security properties that can discover many common ERC flaws. Use slither-prop to review the following:
5454

55-
- [ ] **The contract passes all unit tests and security properties from slither-prop.** Run the generated unit tests, then check the properties with [Echidna](https://github.com/crytic/echidna) and [Manticore](https://manticore.readthedocs.io/en/latest/verifier.html).
55+
- [ ] **The contract passes all unit tests and security properties from `slither-prop`.** Run the generated unit tests and then check the properties with [Echidna](https://github.com/crytic/echidna) and [Manticore](https://manticore.readthedocs.io/en/latest/verifier.html).
5656

57-
### Risks in ERC-20 extensions
58-
Contracts might have different behaviors from their original ERC specification, manually review that:
57+
### Risks of ERC20 Extensions
58+
The behavior of certain contracts may differ from the original ERC specification. Conduct a manual review of the following conditions:
5959

60-
- [ ] **The token is not an ERC777 token and has no external function call in transfer and transferFrom.** External calls in the transfer functions can lead to reentrancies.
61-
- [ ] **Transfer and transferFrom should not take a fee.** Deflationary tokens can lead to unexpected behavior.
62-
- [ ] **Potential interest earned from the token is taken into account.** Some tokens distribute interest to token holders. This interest might be trapped in the contract if not taken into account.
60+
- [ ] **The token is not an ERC777 token and has no external function call in `transfer` or `transferFrom`.** External calls in the transfer functions can lead to reentrancies.
61+
- [ ] **`Transfer` and `transferFrom` should not take a fee.** Deflationary tokens can lead to unexpected behavior.
62+
- [ ] **Potential interest earned from the token is taken into account.** Some tokens distribute interest to token holders. This interest may be trapped in the contract if not taken into account.
6363

6464
### Token scarcity
6565

66-
Reviews for issues of token scarcity requires manual review. Check for these conditions:
66+
Reviews of token scarcity issues must be executed manually. Check for the following conditions:
6767

68-
- [ ] **No user owns most of the supply.** If a few users own most of the tokens, they can influence operations based on the token's repartition.
68+
- [ ] **The supply is owned by more than a few users.** If a few users own most of the tokens, they can influence operations based on the tokens’ repartition.
6969
- [ ] **The total supply is sufficient.** Tokens with a low total supply can be easily manipulated.
70-
- [ ] **The tokens are located in more than a few exchanges.** If all the tokens are in one exchange, a compromise of the exchange can compromise the contract relying on the token.
71-
- [ ] **Users understand the associated risks of large funds or flash loans.** Contracts relying on the token balance must carefully take in consideration attackers with large funds or attacks through flash loans.
72-
- [ ] **The token does not allow flash minting**. Flash minting can lead to substantial swings in the balance and the total supply, which neccessitate strict and comprehensive overflow checks in the operation of the token.
70+
- [ ] **The tokens are located in more than a few exchanges.** If all the tokens are in one exchange, a compromise of the exchange could compromise the contract relying on the token.
71+
- [ ] **Users understand the risks associated with a large amount of funds or flash loans.** Contracts relying on the token balance must account for attackers with a large amount of funds or attacks executed through flash loans.
72+
- [ ] **The token does not allow flash minting.** Flash minting can lead to substantial swings in the balance and the total supply, which necessitate strict and comprehensive overflow checks in the operation of the token.
7373

74-
## ERC-721 tokens
74+
## ERC721 tokens
7575

76-
### Suggested ERC-721 conformity checks
77-
Contracts might have different behavior from their original ERC specification. Manually review that:
76+
### ERC721 Conformity Checks
7877

79-
- [ ] **Transfering tokens to 0x0 reverts.** Several tokens allow transfering to 0x0, counting these as burned, however the ERC-721 standard requires a revert in such case.
80-
- [ ] **safeTransferFrom functions are implemented with the correct signature.** Several tokens do not implement these functions. As a result, transferring NFTs into a contract can result in loss of assets.
81-
- [ ] **The name, decimals, and symbol functions are present if used.** These functions are optional in the ERC721 standard and might not be present.
82-
- [ ] **If used, decimals should return uint8(0).** Other values are not valid.
83-
- [ ] **The name and symbol functions can return an empty string.** This is allowed by the standard.
84-
- [ ] **ownerOf reverts if the `tokenId` is invalid or it was already burned. This function cannot return 0x0**. This is required by the standard, but it is not always properly implemented.
85-
- [ ] **Transferring an NFT clears its approvals**. This is required by the standard.
86-
- [ ] **The token id from an NFT cannot be changed during its lifetime**. This is required by the standard.
78+
The behavior of certain contracts may differ from the original ERC specification. Conduct a manual review of the following conditions:
8779

88-
### Common risks to ERC-721
80+
- [ ] **Transfers of tokens to the 0x0 address revert.** Several tokens allow transfers to 0x0 and consider tokens transferred to that address to have been burned; however, the ERC721 standard requires that such transfers revert.
81+
- [ ] **`safeTransferFrom` functions are implemented with the correct signature.** Several token contracts do not implement these functions. A transfer of NFTs to one of those contracts can result in a loss of assets.
82+
- [ ] **The `name`, `decimals`, and `symbol` functions are present if used.** These functions are optional in the ERC721 standard and may not be present.
83+
- [ ] **If it is used, `decimals` returns a `uint8(0)`.** Other values are invalid.
84+
- [ ] **The `name` and `symbol` functions can return an empty string.** This behavior is allowed by the standard.
85+
- [ ] **The `ownerOf` function reverts if the `tokenId` is invalid or is set to a token that has already been burned.** The function cannot return 0x0. This behavior is required by the standard, but it is not always properly implemented.
86+
- [ ] **A transfer of an NFT clears its approvals.** This is required by the standard.
87+
- [ ] **The token ID of an NFT cannot be changed during its lifetime.** This is required by the standard.
8988

90-
ERC-721 contracts are exposed to the following risks when implementing:
89+
### Common Risks of the ERC721 Standard
9190

92-
- [ ] **The onERC721Received callback is commonly exploited.** External calls in the transfer functions can lead to reentrancies. This is particulary risky when the callback is not explicit. E.g., when [calling `safeMint`](https://www.paradigm.xyz/2021/08/the-dangers-of-surprising-code/).
93-
- [ ] **Minting safely transfers an NFT to a smart contract**. If there is a function to mint, it should properly handle minting new tokens to a smart contract (similarly to `safeTransferFrom`) to avoid loss of assets.
94-
- [ ] **Burning clears approvals**. If there is a function to burn, it should clear previous approvals.
91+
To mitigate the risks associated with ERC721 contracts, conduct a manual review of the following conditions:
9592

93+
- [ ] **The `onERC721Received` callback is taken into account.** External calls in the transfer functions can lead to reentrancies, especially when the callback is not explicit (e.g., in [`safeMint`](https://www.paradigm.xyz/2021/08/the-dangers-of-surprising-code/) calls).
94+
- [ ] **When an NFT is minted, it is safely transferred to a smart contract.** If there is a minting function, it should behave similarly to safeTransferFrom and properly handle the minting of new tokens to a smart contract. This will prevent a loss of assets.
95+
- [ ] **The burning of a token clears its approvals.** If there is a burning function, it should clear the token’s previous approvals.

0 commit comments

Comments
 (0)