Adding additional logging when overflow occurs in calculate_stake_rew… #3783
Conversation
…ards No change in functional behavior
|
The Firedancer team maintains a line-for-line reimplementation of the |
There was a problem hiding this comment.
Thanks for your contribution! If you want logging with unwrap(), rather than adding all of the match statements, it's more idiomatic to use expect(&format!("My formatted message with {variable}")): https://doc.rust-lang.org/std/option/enum.Option.html#method.expect
How about making that change?
| // assume stake.stake() is right | ||
| // bootstrap means fully-vested stake at epoch 0 |
| #[test] | ||
| #[should_panic] | ||
| fn test_overflow_on_umul_calculate_rewards() { |
There was a problem hiding this comment.
Rather than creating two separate tests, how about parametrizing the stake passed to new_stake and using the test_case crate to combine the two tests?
Problem
Currently unwrap is used during calculate_stake_rewards, this lead to a lack of logging when an overflow occurs.
It might be better to take an action other than panic!, but it matches the current code and it's not clear to me what other action should be taken.
Summary of Changes
Additional logs during overflow calculations during calculate_stake_rewards
No change of functionality, validated through new unit tests.
Fixes #