From 9c7b189caa8618083ac70fc475b95ea4dd9bf68d Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Tue, 5 Mar 2024 16:29:37 -0500 Subject: [PATCH] =?UTF-8?q?view:=20=F0=9F=94=AD=20exhaustive=20pattern=20i?= =?UTF-8?q?n=20`Planner`=20reset?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/view/src/planner.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/crates/view/src/planner.rs b/crates/view/src/planner.rs index 83e16ea310..8d0fa0836c 100644 --- a/crates/view/src/planner.rs +++ b/crates/view/src/planner.rs @@ -673,12 +673,26 @@ impl Planner { 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) }