Skip to content

Commit

Permalink
view: 🔭 exhaustive pattern in Planner reset
Browse files Browse the repository at this point in the history
see #3955.

`Planner::plan_with_spendable_and_votable_notes()` resets all of its
fields, **except for** `fee_tier`.

this does not address that "bug", per conversation in discord:

> **kate [penumbra]** i noticed that Planner::plan_with_spendable_and_votable_notes() does not clear the fee tier field. i believe that is a dormant bug. would a PR to fix that be welcomed?
> **hdevalence [penumbra]** i'd hold off on it, i would prefer to throw that code away
> **hdevalence [penumbra]** there should be no clearing of fields at all, for instance

to prevent other such bugs creeping up in the meantime, we add an
_exhaustive_ pattern destructuring the `Planner`, so that future fields
added to the planner must be contended with.
  • Loading branch information
cratelyn committed Mar 5, 2024
1 parent 98c42d8 commit 9c7b189
Showing 1 changed file with 20 additions and 6 deletions.
26 changes: 20 additions & 6 deletions crates/view/src/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -673,12 +673,26 @@ impl<R: RngCore + CryptoRng> Planner<R> {

tracing::debug!(plan = ?self.plan, "finished balancing transaction");

// Clear the planner and pull out the plan to return
self.balance = Balance::zero();
self.vote_intents = BTreeMap::new();
self.ibc_actions = Vec::new();
self.gas_prices = GasPrices::zero();
let plan = mem::take(&mut self.plan);
// Clear the planner and pull out the plan that we will return.
//
// NB: we exhaustively destructure `Self` here so that we'll be sure to reset
// fields added in the future.
let Self {
balance,
vote_intents,
plan,
ibc_actions,
gas_prices,
// we do not need to do anything with the rng.
rng: _,
// fee tiers aren't currently reset when the planner generates a plan. see #3955.
fee_tier: _,
} = self;
*balance = Balance::zero();
*vote_intents = BTreeMap::new();
*ibc_actions = Vec::new();
*gas_prices = GasPrices::zero();
let plan = mem::take(plan);

Ok(plan)
}
Expand Down

0 comments on commit 9c7b189

Please sign in to comment.