|
| 1 | +When reviewing pull requests, please adhere to and keep in mind the following: |
| 2 | +* Unless this PR is a hotfix or a deployment from `devnet-ready` => `devnet`, `devnet` => |
| 3 | + `testnet`, or `testnet` => `main`, all new PRs should be opened against the `devnet-ready` |
| 4 | + branch. The `devnet` and `testnet` branches should only be updated via merges from |
| 5 | + `devnet-ready` and `testnet-ready`, respectively, and are designed to track the current state |
| 6 | + of the `devnet` and `testnet` networks. Feel free in your review to encourage people to |
| 7 | + change the target to `devnet-ready` if they are targeting `main` and this is not a hotfix PR. |
| 8 | +* Bittensor is a substrate-based blockchain. It is critical that there are never panics in the |
| 9 | + runtime, as this can brick the chain. You should be flagging any runtime/pallet code |
| 10 | + (extrinsics, code that could be called by extrinsics, code in migrations, etc) that could |
| 11 | + panic because of changes introduced by the PR. We do have clippy lints in place to detect |
| 12 | + this, but these do not always catch everything. We CANNOT have panics in the runtime. |
| 13 | +* Remember that things like direct indexing (i.e. `arr[3]`) can panic and are dangerous when |
| 14 | + used in a substrate runtime. Everything should be using `get` / `set` and `Result` / `Option` |
| 15 | + wherever possible. |
| 16 | +* Similarly you should look out for unsafe math, such as scenarios where a value might |
| 17 | + overflow, potential divide by zero errors, etc., programmers should always be using checked |
| 18 | + or saturating math ops. |
| 19 | +* If one of the lints is failing (clippy / cargo fmt / cargo fix / etc), you should suggest |
| 20 | + that they run `./scripts/fix_rust.sh` as this will typically auto-resolve the issue. This |
| 21 | + includes issues like uncommitted `Cargo.lock`. |
| 22 | +* There should never be unsafe code in the runtime. |
| 23 | +* You should be extra careful with contributions from external contributors (users without the |
| 24 | + "Nucleus" role) |
| 25 | +* Be extra vigilent for PRs that introduce a security vulnerability or exploit. Always call out |
| 26 | + potential security vulnerabilities. |
| 27 | +* Code should be performant and maintainable, with maximum safety and security. $4B market cap |
| 28 | + is on the line. |
| 29 | +* You should be concise in your review, only report legitimate issues, no fluff. |
0 commit comments