Simplify migration implementation using change address

This approach avoids the issue of dealing with fees outside the planner.
It does this by eschewing trying to figure out how much to output,
and instead focusing on what to spend (everything).
Then, the change address is set to the desired destination, allowing the
planner to automatically move the funds as needed, minus fees.

crates/bin/pcli/src/command/migrate.rs

@@ -1,8 +1,6 @@
 use crate::App;
 use anyhow::{Context, Result};
-use penumbra_fee::FeeTier;
-use penumbra_fee::GasPrices;
-use penumbra_keys::{keys::AddressIndex, FullViewingKey};
+use penumbra_keys::FullViewingKey;
 use penumbra_proto::view::v1::GasPricesRequest;
 use penumbra_view::ViewClient;
 use penumbra_wallet::plan::Planner;
@@ -23,7 +21,7 @@ pub enum MigrateCmd {
 impl MigrateCmd {
     #[tracing::instrument(skip(self, app))]
     pub async fn exec(&self, app: &mut App) -> Result<()> {
-        let gas_prices: GasPrices = app
+        let gas_prices = app
             .view
             .as_mut()
             .context("view service must be initialized")?
@@ -44,19 +42,15 @@ impl MigrateCmd {
 
                 let mut planner = Planner::new(OsRng);
 
-                let memo = format!("Migrating balance from {} to {}", source_fvk, dest_fvk);
-
-                let (source_address, _) =
-                    FullViewingKey::payment_address(&source_fvk, AddressIndex::new(0));
-
                 let (dest_address, _) = FullViewingKey::payment_address(
                     &FullViewingKey::from_str(&to[..])?,
-                    AddressIndex::new(0),
+                    Default::default(),
                 );
 
                 planner
                     .set_gas_prices(gas_prices)
-                    .set_fee_tier(FeeTier::default());
+                    .set_fee_tier(Default::default())
+                    .change_address(dest_address);
 
                 // Return all unspent notes from the view service
                 let notes = app
@@ -66,92 +60,22 @@ impl MigrateCmd {
                     .unspent_notes_by_account_and_asset()
                     .await?;
 
-                // Get all remaining note values after filtering out notes already spent by the planner
-                let note_values = notes.iter().flat_map(|(_, notes_by_asset)| {
-                    notes_by_asset.iter().map(|(asset, notes)| {
-                        let sum: u128 = notes
-                            .iter()
-                            .map(|record| u128::from(record.note.amount()))
-                            .sum();
-
-                        asset.value(sum.into())
-                    })
-                });
-
-                // Add all note values to the planner
-                note_values.clone().for_each(|value| {
-                    planner.output(value, dest_address.clone());
-                });
-
-                let fee =
-                    planner.compute_fee_estimate(&gas_prices, &FeeTier::default(), &source_address);
-
-                let asset_cache = app.view().assets().await?;
-
-                println!(
-                    "Estimated fee for sending total balance: {:?}",
-                    fee.0.format(&asset_cache)
-                );
-
-                // Update relevant note values to be less the estimated fee
-
-                let mut note_values_2 = Vec::new();
-
-                note_values.for_each(|value| {
-                    if value.asset_id == fee.0.asset_id {
-                        note_values_2.push(penumbra_asset::Value {
-                            asset_id: value.asset_id,
-                            // HACK: we are doubling the subtraction of the estimated fee,
-                            // as a temporary workaround for the fee estimation being inaccurate,
-                            // oddly by about 1.9x.
-                            amount: value
-                                .amount
-                                .checked_sub(&fee.0.amount)
-                                .expect("estimated fee is higher than remaining balance 1/2")
-                                .checked_sub(&fee.0.amount)
-                                .expect("estimated fee is higher than remaining balance 2/2"),
-                        });
-                    } else {
-                        note_values_2.push(penumbra_asset::Value {
-                            asset_id: value.asset_id,
-                            amount: value.amount,
-                        });
+                for notes in notes.into_values() {
+                    for notes in notes.into_values() {
+                        for note in notes {
+                            planner.spend(note.note, note.position);
+                        }
                     }
-                });
-
-                let mut planner_2 = Planner::new(OsRng);
-
-                planner_2
-                    .set_gas_prices(gas_prices)
-                    .set_fee_tier(FeeTier::default());
+                }
 
-                // Add all note values to the second planner
-                note_values_2.clone().into_iter().for_each(|value| {
-                    planner_2.output(value, dest_address.clone());
-                });
-
-                // defaulting to index 0 (the default account) here means that this operation
-                // will fail if the default account has insufficient funds for fees
-                // a more general solution could be finding the first/lowest-indexed account with sufficient funds
-                // and spending fees from that account
-                let address_index = AddressIndex::new(0);
-
-                let fee2 = planner_2.compute_fee_estimate(
-                    &gas_prices,
-                    &FeeTier::default(),
-                    &source_address,
-                );
-                println!(
-                    "Estimated fee 2/2 for sending total balance: {:?}",
-                    fee2.0.format(&asset_cache)
-                );
-                let plan = planner_2
+                let memo = format!("Migrating balance from {} to {}", source_fvk, dest_fvk);
+                let plan = planner
                     .memo(memo)
                     .plan(
                         app.view
                             .as_mut()
                             .context("view service must be initialized")?,
-                        address_index,
+                        Default::default(),
                     )
                     .await
                     .context("can't build send transaction")?;

crates/core/transaction/src/action_list.rs

@@ -29,10 +29,6 @@ pub struct ActionList {
 }
 
 impl ActionList {
-    /// Returns the transaction fee as currently estimated.
-    pub fn fee(&self) -> Fee {
-        self.fee
-    }
     /// Returns true if the resulting transaction would require a memo.
     pub fn requires_memo(&self) -> bool {
         let has_change_outputs = !self.change_outputs.is_empty();
@@ -128,7 +124,7 @@ impl ActionList {
     /// While the gas cost can be computed exactly, the base fee can only be
     /// estimated, because the actual base fee paid by the transaction will
     /// depend on the gas prices at the time it's accepted on-chain.
-    pub fn compute_fee_estimate(&self, gas_prices: &GasPrices, fee_tier: &FeeTier) -> Fee {
+    fn compute_fee_estimate(&self, gas_prices: &GasPrices, fee_tier: &FeeTier) -> Fee {
         let base_fee = gas_prices.fee(&self.gas_cost());
         base_fee.apply_tier(*fee_tier)
     }

crates/view/src/planner.rs

@@ -96,24 +96,6 @@ impl<R: RngCore + CryptoRng> Planner<R> {
         }
     }
 
-    /// Compute a fee estimate for the transaction.
-
-    pub fn compute_fee_estimate(
-        &mut self,
-        gas_prices: &GasPrices,
-        fee_tier: &FeeTier,
-        change_address: &Address,
-    ) -> Fee {
-        self.action_list.refresh_fee_and_change(
-            &mut self.rng,
-            gas_prices,
-            fee_tier,
-            change_address,
-        );
-
-        self.action_list.fee()
-    }
-
     /// Add an arbitrary action to the planner.
     pub fn action<A: Into<ActionPlan>>(&mut self, action: A) -> &mut Self {
         self.action_list.push(action);