diff --git a/crates/bin/pcli/tests/network_integration.rs b/crates/bin/pcli/tests/network_integration.rs index b84a660413..906696d3fc 100644 --- a/crates/bin/pcli/tests/network_integration.rs +++ b/crates/bin/pcli/tests/network_integration.rs @@ -172,15 +172,11 @@ fn transaction_send_from_addr_0_to_addr_1() { let tvp: ProtoTransactionView = serde_json::value::from_value(view_json).unwrap(); let tv: TransactionView = tvp.try_into().unwrap(); - // TODO: the first may no longer be a spend because of ordering changes. - // Let's not try to fix this at the moment. Later we can put a "canonical ordering" into the planner. - /* - // There will be a lot of ActionViews in the body... let's just check that one is a Spend. + assert!(matches!( &tv.body_view.action_views[0], penumbra_transaction::ActionView::Spend(_) )); - */ // Inspect the TransactionView and ensure that we can read the memo text. let mv = tv diff --git a/crates/core/transaction/src/action.rs b/crates/core/transaction/src/action.rs index 9cfdb8174a..0ce196f431 100644 --- a/crates/core/transaction/src/action.rs +++ b/crates/core/transaction/src/action.rs @@ -127,6 +127,36 @@ impl Action { } } } + + /// Canonical action ordering according to protobuf definitions + pub fn variant_index(&self) -> usize { + match self { + Action::Spend(_) => 1, + Action::Output(_) => 2, + Action::Swap(_) => 3, + Action::SwapClaim(_) => 4, + Action::ValidatorDefinition(_) => 16, + Action::IbcRelay(_) => 17, + Action::ProposalSubmit(_) => 18, + Action::ProposalWithdraw(_) => 19, + Action::ValidatorVote(_) => 20, + Action::DelegatorVote(_) => 21, + Action::ProposalDepositClaim(_) => 22, + Action::PositionOpen(_) => 30, + Action::PositionClose(_) => 31, + Action::PositionWithdraw(_) => 32, + Action::Delegate(_) => 40, + Action::Undelegate(_) => 41, + Action::UndelegateClaim(_) => 42, + Action::CommunityPoolSpend(_) => 50, + Action::CommunityPoolOutput(_) => 51, + Action::CommunityPoolDeposit(_) => 52, + Action::Ics20Withdrawal(_) => 200, + Action::ActionDutchAuctionSchedule(_) => 53, + Action::ActionDutchAuctionEnd(_) => 54, + Action::ActionDutchAuctionWithdraw(_) => 55, + } + } } impl IsAction for Action { diff --git a/crates/core/transaction/src/plan.rs b/crates/core/transaction/src/plan.rs index d3c12f6034..5935c292c0 100644 --- a/crates/core/transaction/src/plan.rs +++ b/crates/core/transaction/src/plan.rs @@ -48,6 +48,12 @@ pub struct TransactionPlan { } impl TransactionPlan { + /// Sort the actions in [`TransactionPlan`] by type, using the protobuf field number in the [`ActionPlan`]. + pub fn sort_actions(&mut self) { + self.actions + .sort_by_key(|action: &ActionPlan| action.variant_index()); + } + /// Computes the [`EffectHash`] for the [`Transaction`] described by this /// [`TransactionPlan`]. /// @@ -500,7 +506,7 @@ mod tests { let mut rng = OsRng; let memo_plaintext = MemoPlaintext::new(Address::dummy(&mut rng), "".to_string()).unwrap(); - let plan = TransactionPlan { + let mut plan: TransactionPlan = TransactionPlan { // Put outputs first to check that the auth hash // computation is not affected by plan ordering. actions: vec![ @@ -528,6 +534,9 @@ mod tests { memo: Some(MemoPlan::new(&mut OsRng, memo_plaintext.clone())), }; + // Sort actions within the transaction plan. + plan.sort_actions(); + println!("{}", serde_json::to_string_pretty(&plan).unwrap()); let plan_effect_hash = plan.effect_hash(fvk).unwrap(); diff --git a/crates/core/transaction/src/plan/action.rs b/crates/core/transaction/src/plan/action.rs index 2b7264f035..6f1edace55 100644 --- a/crates/core/transaction/src/plan/action.rs +++ b/crates/core/transaction/src/plan/action.rs @@ -168,6 +168,36 @@ impl ActionPlan { }) } + /// Canonical action plan ordering according to protobuf definitions + pub fn variant_index(&self) -> usize { + match self { + ActionPlan::Spend(_) => 1, + ActionPlan::Output(_) => 2, + ActionPlan::Swap(_) => 3, + ActionPlan::SwapClaim(_) => 4, + ActionPlan::ValidatorDefinition(_) => 16, + ActionPlan::IbcAction(_) => 17, + ActionPlan::ProposalSubmit(_) => 18, + ActionPlan::ProposalWithdraw(_) => 19, + ActionPlan::ValidatorVote(_) => 20, + ActionPlan::DelegatorVote(_) => 21, + ActionPlan::ProposalDepositClaim(_) => 22, + ActionPlan::PositionOpen(_) => 30, + ActionPlan::PositionClose(_) => 31, + ActionPlan::PositionWithdraw(_) => 32, + ActionPlan::Delegate(_) => 40, + ActionPlan::Undelegate(_) => 41, + ActionPlan::UndelegateClaim(_) => 42, + ActionPlan::CommunityPoolSpend(_) => 50, + ActionPlan::CommunityPoolOutput(_) => 51, + ActionPlan::CommunityPoolDeposit(_) => 52, + ActionPlan::Ics20Withdrawal(_) => 200, + ActionPlan::ActionDutchAuctionSchedule(_) => 53, + ActionPlan::ActionDutchAuctionEnd(_) => 54, + ActionPlan::ActionDutchAuctionWithdraw(_) => 55, + } + } + pub fn balance(&self) -> Balance { use ActionPlan::*; diff --git a/crates/core/transaction/src/plan/build.rs b/crates/core/transaction/src/plan/build.rs index 1980f92e15..0b687b017f 100644 --- a/crates/core/transaction/src/plan/build.rs +++ b/crates/core/transaction/src/plan/build.rs @@ -119,7 +119,6 @@ impl TransactionPlan { witness_data: &WitnessData, auth_data: &AuthorizationData, ) -> Result { - // TODO: stream progress updates // 1. Build each action. let actions = self .actions diff --git a/crates/view/src/planner/action_list.rs b/crates/view/src/planner/action_list.rs index b0c8b3cb2a..d866eaa692 100644 --- a/crates/view/src/planner/action_list.rs +++ b/crates/view/src/planner/action_list.rs @@ -64,6 +64,10 @@ impl ActionList { }; plan.populate_detection_data(rng, fmd_params.precision_bits.into()); + // Implement a canonical ordering to the actions within the transaction + // plan to reduce client distinguishability. + plan.sort_actions(); + Ok(plan) }