Skip to content

Commit 4b219dc

Browse files
authored
Added a pull request template (#881)
* Added a pull request template that includes a solidity review checklist * Defined low-level calls * Added another item to the "math" section * Addressed review feedback from @jrhea
1 parent 6aca70e commit 4b219dc

File tree

1 file changed

+64
-0
lines changed

1 file changed

+64
-0
lines changed

pull_request_template.md

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# Resolved Issues
2+
3+
# Description
4+
5+
# Review Checklists
6+
7+
Please check each item **before approving** the pull request. While going
8+
through the checklist, it is recommended to leave comments on items that are
9+
referenced in the checklist to make sure that they are reviewed. If there are
10+
multiple reviewers, copy the checklists into sections titled `## [Reviewer Name]`.
11+
If the PR doesn't touch Solidity and/or Rust, the corresponding checklist can
12+
be removed.
13+
14+
## [[Reviewer Name]]
15+
16+
### Solidity
17+
18+
- [ ] **Tokens**
19+
- [ ] Do all `approve` calls use `forceApprove`?
20+
- [ ] Do all `transfer` calls use `safeTransfer`?
21+
- [ ] Do all `transferFrom` calls use `msg.sender` as the `from` address?
22+
- [ ] If not, is the function access restricted to prevent unauthorized
23+
token spend?
24+
- [ ] **Low-level calls (`call`, `delegatecall`, `staticcall`, `transfer`, `send`)**
25+
- [ ] Is the returned `success` boolean checked to handle failed calls?
26+
- [ ] If using `delegatecall`, are there strict access controls on the
27+
addresses that can be called? It shouldn't be possible to `delegatecall`
28+
arbitrary addresses, so the list of possible targets should either be
29+
immutable or tightly controlled by an admin.
30+
- [ ] **Reentrancy**
31+
- [ ] Are functions that make external calls or transfer ether marked as `nonReentrant`?
32+
- [ ] If not, is there documentation that explains why reentrancy is
33+
not a concern or how it's mitigated?
34+
- [ ] **Gas Optimizations**
35+
- [ ] Is the logic as simple as possible?
36+
- [ ] Are the storage values that are used repeatedly cached in stack or
37+
memory variables?
38+
- [ ] If loops are used, are there guards in place to avoid out-of-gas
39+
issues?
40+
- [ ] **Visibility**
41+
- [ ] Are all `payable` functions restricted to avoid stuck ether?
42+
- [ ] **Math**
43+
- [ ] Is all of the arithmetic checked or guarded by if-statements that will
44+
catch underflows?
45+
- [ ] If `Safe` functions are altered, are potential underflows and
46+
overflows caught so that a failure flag can be thrown?
47+
- [ ] Are all of the rounding directions clearly documented?
48+
- [ ] **Testing**
49+
- [ ] Are there new or updated unit or integration tests?
50+
- [ ] Do the tests cover the happy paths?
51+
- [ ] Do the tests cover the unhappy paths?
52+
- [ ] Are there an adequate number of fuzz tests to ensure that we are
53+
covering the full input space?
54+
55+
### Rust
56+
57+
- [ ] **Testing**
58+
- [ ] Are there new or updated unit or integration tests?
59+
- [ ] Do the tests cover the happy paths?
60+
- [ ] Do the tests cover the unhappy paths?
61+
- [ ] Are there an adequate number of fuzz tests to ensure that we are
62+
covering the full input space?
63+
- [ ] If matching Solidity behavior, are there differential fuzz tests that
64+
ensure that Rust matches Solidity?

0 commit comments

Comments
 (0)