view: reorganize planner components

crates/view/src/planner/action_list.rs → crates/core/transaction/src/action_list.rs

@@ -1,18 +1,16 @@
 use anyhow::Result;
 use std::collections::BTreeMap;
 
+use crate::plan::MemoPlan;
+use crate::{gas::GasCost, TransactionParameters};
+use crate::{ActionPlan, TransactionPlan};
 use penumbra_asset::{asset, Balance};
 use penumbra_fee::{Fee, FeeTier, Gas, GasPrices};
 use penumbra_keys::Address;
 use penumbra_num::Amount;
 use penumbra_shielded_pool::{fmd, OutputPlan};
-use penumbra_transaction::plan::MemoPlan;
-use penumbra_transaction::{gas::GasCost, TransactionParameters};
-use penumbra_transaction::{ActionPlan, TransactionPlan};
 use rand_core::{CryptoRng, RngCore};
 
-use crate::SpendableNoteRecord;
-
 /// A list of planned actions to be turned into a TransactionPlan.
 ///
 /// A transaction is a bundle of actions plus auxiliary data (like a memo). A
@@ -230,46 +228,3 @@ impl ActionList {
             .retain(|_, output| output.value.amount > Amount::zero());
     }
 }
-
-/// Prioritize notes to spend to release value of a specific transaction.
-///
-/// Various logic is possible for note selection. Currently, this method
-/// prioritizes notes sent to a one-time address, then notes with the largest
-/// value:
-///
-/// - Prioritizing notes sent to one-time addresses optimizes for a future in
-/// which we implement DAGSync keyed by fuzzy message detection (which will not
-/// be able to detect notes sent to one-time addresses). Spending these notes
-/// immediately converts them into change notes, sent to the default address for
-/// the users' account, which are detectable.
-///
-/// - Prioritizing notes with the largest value optimizes for gas used by the
-/// transaction.
-///
-/// We may want to make note prioritization configurable in the future. For
-/// instance, a user might prefer a note prioritization strategy that harvested
-/// capital losses when possible, using cost basis information retained by the
-/// view server.
-pub fn prioritize_and_filter_spendable_notes(
-    records: Vec<SpendableNoteRecord>,
-) -> Vec<SpendableNoteRecord> {
-    let mut filtered = records
-        .into_iter()
-        .filter(|record| record.note.amount() > Amount::zero())
-        .collect::<Vec<_>>();
-
-    filtered.sort_by(|a, b| {
-        // Sort by whether the note was sent to an ephemeral address...
-        match (
-            a.address_index.is_ephemeral(),
-            b.address_index.is_ephemeral(),
-        ) {
-            (true, false) => std::cmp::Ordering::Less,
-            (false, true) => std::cmp::Ordering::Greater,
-            // ... then by largest amount.
-            _ => b.note.amount().cmp(&a.note.amount()),
-        }
-    });
-
-    filtered
-}

crates/core/transaction/src/lib.rs

@@ -26,24 +26,25 @@ mod transaction;
 mod witness_data;
 
 pub mod action;
+pub mod action_list;
 pub mod gas;
 pub mod memo;
 pub mod plan;
 pub mod view;
 
 pub use action::Action;
+pub use action_list::ActionList;
 pub use auth_data::AuthorizationData;
 pub use detection_data::DetectionData;
 pub use error::Error;
 pub use is_action::IsAction;
 pub use parameters::TransactionParameters;
+pub use penumbra_txhash as txhash;
 pub use plan::{ActionPlan, TransactionPlan};
 pub use transaction::{Transaction, TransactionBody};
 pub use view::{ActionView, MemoPlaintextView, MemoView, TransactionPerspective, TransactionView};
 pub use witness_data::WitnessData;
 
-pub use penumbra_txhash as txhash;
-
 /// A compatibility wrapper for trait implementations that are temporarily duplicated
 /// in multiple crates as an orphan rule work around until we finish splitting crates (#2288).
 pub struct Compat<'a, T>(pub &'a T);

crates/view/src/planner.rs

@@ -45,12 +45,9 @@ use penumbra_tct as tct;
 use penumbra_transaction::{
     memo::MemoPlaintext,
     plan::{ActionPlan, MemoPlan, TransactionPlan},
-    TransactionParameters,
+    ActionList, TransactionParameters,
 };
 
-mod action_list;
-use action_list::ActionList;
-
 /// A planner for a [`TransactionPlan`] that can fill in the required spends and change outputs upon
 /// finalization to make a transaction balance.
 pub struct Planner<R: RngCore + CryptoRng> {
@@ -455,6 +452,40 @@ impl<R: RngCore + CryptoRng> Planner<R> {
         self
     }
 
+    /// Prioritize notes to spend to release value of a specific transaction.
+    ///
+    /// Various logic is possible for note selection. Currently, this method
+    /// prioritizes notes sent to a one-time address, then notes with the largest
+    /// value:
+    ///
+    /// - Prioritizing notes sent to one-time addresses optimizes for a future in
+    /// which we implement DAGSync keyed by fuzzy message detection (which will not
+    /// be able to detect notes sent to one-time addresses). Spending these notes
+    /// immediately converts them into change notes, sent to the default address for
+    /// the users' account, which are detectable.
+    ///
+    /// - Prioritizing notes with the largest value optimizes for gas used by the
+    /// transaction.
+    ///
+    /// We may want to make note prioritization configurable in the future. For
+    /// instance, a user might prefer a note prioritization strategy that harvested
+    /// capital losses when possible, using cost basis information retained by the
+    /// view server.
+    fn prioritize_and_filter_spendable_notes(
+        &mut self,
+        records: Vec<SpendableNoteRecord>,
+    ) -> Vec<SpendableNoteRecord> {
+        // Filter out zero valued notes.
+        let mut filtered = records
+            .into_iter()
+            .filter(|record| record.note.amount() > Amount::zero())
+            .collect::<Vec<_>>();
+
+        filtered.sort_by(|a, b| b.note.amount().cmp(&a.note.amount()));
+
+        filtered
+    }
+
     /// Add spends and change outputs as required to balance the transaction, using the view service
     /// provided to supply the notes and other information.
     pub async fn plan<V: ViewClient>(
@@ -505,9 +536,7 @@ impl<R: RngCore + CryptoRng> Planner<R> {
                 .await?;
             notes_by_asset_id.insert(
                 required.asset_id,
-                // TODO: reorganize later, the important thing here is that this
-                // is separate from the Planner logic (will never be commonized)
-                action_list::prioritize_and_filter_spendable_notes(records),
+                self.prioritize_and_filter_spendable_notes(records),
             );
         }