Skip to content

Commit

Permalink
docs: add to reviewers guide (#5788)
Browse files Browse the repository at this point in the history
Description
---
Adds two things to watch out for when reviewing code:
1. Tokio::select Biased
2. Semantic rust functions not handling result types

---------

Co-authored-by: Brian Pearce <[email protected]>
  • Loading branch information
SWvheerden and brianp authored Oct 5, 2023
1 parent f17ac6b commit 05fc2e9
Showing 1 changed file with 19 additions and 0 deletions.
19 changes: 19 additions & 0 deletions docs/src/reviewing_guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,25 @@ Tokio has many useful channels, and it can be difficult to know what to look out
2. When receiving events in a loop, Error::Closed should be used to break out of the loop, because the sender halves have dropped and no more events will be received
2. Sending to a broadcast `Sender` before there are any receivers (or if they have all dropped) will error. If this is happening on startup, it should be
handled gracefully and may succeed in future when a receiver is added.
3. When using `tokio::select` watch out for `biased` keyword as this can cause the loop to not run certain piece of code. `Biased` causes the loop to be polled in order of the code. If the first items polled will always be ready, the later items will never be run.

#### Rust function code
Watch out for functional rust code and ensure that they will not overflow. These functional code brackets don't always allow for error handling. See the following code snippet as an example:
```rust
fn sum_kernels(body: &AggregateBody, offset_with_fee: PedersenCommitment) -> KernelSum {
// Sum all kernel excesses and fees
body.kernels().iter().fold(
KernelSum {
fees: MicroMinotari(0),
sum: offset_with_fee,
},
|acc, val| KernelSum {
fees: acc.fees + val.fee,
sum: &acc.sum + &val.excess,
},
)
}
Here is a good example of nice semantic rust code, but the code has the potential to panic and overflow when counting up the fees from the kernels. Semantic RUST is the preferred way of writing code for Tari, it should not come at the determent of safe code.

#### Behind-the-scenes panics

Expand Down

0 comments on commit 05fc2e9

Please sign in to comment.