proto: transaction format cleanup

This commit:

- Aligns the `TransactionPlan` with the `Transaction` data structure
- Moves the `fee` into the `TransactionParameters`

Now the transaction body consists solely of:

- a list of `Action`s describing changes to the chain state
- a set of `TransactionParameters` describing under what conditions those changes can be applied
- a set of `DetectionData` (extensible) for assisting with transaction detection
- a set of `MemoData` with encrypted information about the transaction's purpose.

crates/bin/pcli/src/command/view/tx.rs

@@ -433,7 +433,13 @@ impl TxCmd {
 
             metadata_table.add_row(vec![
                 "Transaction Fee",
-                &tx_info.view.body_view.fee.value().format(&asset_cache),
+                &tx_info
+                    .view
+                    .body_view
+                    .transaction_parameters
+                    .fee
+                    .value()
+                    .format(&asset_cache),
             ]);
 
             let memo_view = tx_info.view.body_view.memo_view;

crates/core/app/src/action_handler/transaction.rs

@@ -113,8 +113,8 @@ mod tests {
     use penumbra_shielded_pool::{Note, OutputPlan, SpendPlan};
     use penumbra_tct as tct;
     use penumbra_transaction::{
-        plan::{CluePlan, TransactionPlan},
-        WitnessData,
+        plan::{CluePlan, DetectionDataPlan, TransactionPlan},
+        TransactionParameters, WitnessData,
     };
     use rand_core::OsRng;
 
@@ -151,16 +151,20 @@ mod tests {
         // Add a single spend and output to the transaction plan such that the
         // transaction balances.
         let plan = TransactionPlan {
-            expiry_height: 0,
-            fee: Fee::default(),
-            chain_id: "".into(),
+            transaction_parameters: TransactionParameters {
+                expiry_height: 0,
+                fee: Fee::default(),
+                chain_id: "".into(),
+            },
             actions: vec![
                 SpendPlan::new(&mut OsRng, note, auth_path.position()).into(),
                 SpendPlan::new(&mut OsRng, note2, auth_path2.position()).into(),
                 OutputPlan::new(&mut OsRng, value, *test_keys::ADDRESS_1).into(),
             ],
-            clue_plans: vec![CluePlan::new(&mut OsRng, *test_keys::ADDRESS_1, 1)],
-            memo_plan: None,
+            detection_data: DetectionDataPlan {
+                clue_plans: vec![CluePlan::new(&mut OsRng, *test_keys::ADDRESS_1, 1)],
+            },
+            memo_data: None,
         };
 
         // Build the transaction.
@@ -213,15 +217,17 @@ mod tests {
         // Add a single spend and output to the transaction plan such that the
         // transaction balances.
         let plan = TransactionPlan {
-            expiry_height: 0,
-            fee: Fee::default(),
-            chain_id: "".into(),
+            transaction_parameters: TransactionParameters {
+                expiry_height: 0,
+                fee: Fee::default(),
+                chain_id: "".into(),
+            },
             actions: vec![
                 SpendPlan::new(&mut OsRng, note, auth_path.position()).into(),
                 OutputPlan::new(&mut OsRng, value, *test_keys::ADDRESS_1).into(),
             ],
-            clue_plans: vec![],
-            memo_plan: None,
+            detection_data: DetectionDataPlan { clue_plans: vec![] },
+            memo_data: None,
         };
 
         // Build the transaction.

crates/core/app/src/action_handler/transaction/stateful.rs

@@ -75,7 +75,13 @@ pub(super) async fn fee_greater_than_base_fee<S: StateRead>(
 
     let transaction_base_price = current_gas_prices.price(&transaction.gas_cost());
 
-    if transaction.transaction_body().fee.amount() >= transaction_base_price {
+    if transaction
+        .transaction_body()
+        .transaction_parameters
+        .fee
+        .amount()
+        >= transaction_base_price
+    {
         Ok(())
     } else {
         Err(anyhow::anyhow!(

crates/core/app/src/tests/spend.rs

@@ -7,7 +7,6 @@ use decaf377_rdsa::SigningKey;
 use penumbra_asset::Value;
 use penumbra_chain::{component::StateWriteExt, EffectHash, TransactionContext};
 use penumbra_compact_block::component::CompactBlockManager;
-use penumbra_fee::Fee;
 use penumbra_keys::{test_keys, PayloadKey};
 use penumbra_num::Amount;
 use penumbra_sct::component::SourceContext;
@@ -249,7 +248,6 @@ async fn spend_duplicate_nullifier_same_transaction() {
             penumbra_transaction::Action::Output(output),
         ],
         transaction_parameters: TransactionParameters::default(),
-        fee: Fee::from_staking_token_amount(0u64.into()),
         detection_data: None,
         memo: None,
     };

crates/core/transaction/src/detection_data.rs

@@ -0,0 +1,37 @@
+use anyhow::Error;
+use decaf377_fmd::Clue;
+use penumbra_proto::core::transaction::v1alpha1 as pbt;
+use penumbra_proto::DomainType;
+
+/// Detection data used by a detection server using Fuzzy Message Detection.
+///
+/// Only present if outputs are present.
+#[derive(Clone, Debug, Default)]
+pub struct DetectionData {
+    pub fmd_clues: Vec<Clue>,
+}
+
+impl DomainType for DetectionData {
+    type Proto = pbt::DetectionData;
+}
+
+impl TryFrom<pbt::DetectionData> for DetectionData {
+    type Error = Error;
+
+    fn try_from(proto: pbt::DetectionData) -> anyhow::Result<Self, Self::Error> {
+        let fmd_clues = proto
+            .fmd_clues
+            .into_iter()
+            .map(|x| x.try_into())
+            .collect::<Result<Vec<Clue>, Error>>()?;
+        Ok(DetectionData { fmd_clues })
+    }
+}
+
+impl From<DetectionData> for pbt::DetectionData {
+    fn from(msg: DetectionData) -> Self {
+        let fmd_clues = msg.fmd_clues.into_iter().map(|x| x.into()).collect();
+
+        pbt::DetectionData { fmd_clues }
+    }
+}

crates/core/transaction/src/effect_hash.rs

@@ -22,7 +22,7 @@ use penumbra_shielded_pool::{output, spend, Ics20Withdrawal};
 use penumbra_stake::{validator, Delegate, Undelegate, UndelegateClaimBody};
 
 use crate::{
-    memo::MemoCiphertext, plan::TransactionPlan, transaction::DetectionData, Action, Transaction,
+    memo::MemoCiphertext, plan::TransactionPlan, Action, DetectionData, Transaction,
     TransactionBody, TransactionParameters,
 };
 
@@ -57,7 +57,6 @@ impl TransactionBody {
 
         // Hash the fixed data of the transaction body.
         state.update(self.transaction_parameters.effect_hash().as_bytes());
-        state.update(self.fee.effect_hash().as_bytes());
         if self.memo.is_some() {
             let memo_ciphertext = self.memo.clone();
             state.update(
@@ -104,18 +103,13 @@ impl TransactionPlan {
         let mut state = create_personalized_state(&pbt::TransactionBody::type_url());
 
         // Hash the fixed data of the transaction body.
-        let tx_params = TransactionParameters {
-            chain_id: self.chain_id.clone(),
-            expiry_height: self.expiry_height,
-        };
-        state.update(tx_params.effect_hash().as_bytes());
-        state.update(self.fee.effect_hash().as_bytes());
+        state.update(self.transaction_parameters.effect_hash().as_bytes());
 
         // Hash the memo and save the memo key for use with outputs later.
         let mut memo_key: Option<PayloadKey> = None;
-        if self.memo_plan.is_some() {
+        if self.memo_data.is_some() {
             let memo_plan = self
-                .memo_plan
+                .memo_data
                 .clone()
                 .expect("memo_plan must be present in TransactionPlan");
             let memo_ciphertext = memo_plan.memo().expect("can compute ciphertext");
@@ -124,14 +118,8 @@ impl TransactionPlan {
         }
 
         // Hash the detection data.
-        if !self.clue_plans.is_empty() {
-            let detection_data = DetectionData {
-                fmd_clues: self
-                    .clue_plans
-                    .iter()
-                    .map(|clue_plan| clue_plan.clue())
-                    .collect(),
-            };
+        if !self.detection_data.clue_plans.is_empty() {
+            let detection_data = self.detection_data.detection_data();
             state.update(detection_data.effect_hash().as_bytes());
         }
 
@@ -453,8 +441,8 @@ mod tests {
 
     use crate::{
         memo::MemoPlaintext,
-        plan::{CluePlan, MemoPlan, TransactionPlan},
-        WitnessData,
+        plan::{CluePlan, DetectionDataPlan, MemoPlan, TransactionPlan},
+        TransactionParameters, WitnessData,
     };
 
     /// This isn't an exhaustive test, but we don't currently have a
@@ -525,9 +513,6 @@ mod tests {
             text: "".to_string(),
         };
         let plan = TransactionPlan {
-            expiry_height: 0,
-            fee: Fee::default(),
-            chain_id: "penumbra-test".to_string(),
             // Put outputs first to check that the auth hash
             // computation is not affected by plan ordering.
             actions: vec![
@@ -544,8 +529,15 @@ mod tests {
                 SpendPlan::new(&mut OsRng, note1, 1u64.into()).into(),
                 SwapPlan::new(&mut OsRng, swap_plaintext).into(),
             ],
-            clue_plans: vec![CluePlan::new(&mut OsRng, addr, 1)],
-            memo_plan: Some(MemoPlan::new(&mut OsRng, memo_plaintext.clone()).unwrap()),
+            transaction_parameters: TransactionParameters {
+                expiry_height: 0,
+                fee: Fee::default(),
+                chain_id: "penumbra-test".to_string(),
+            },
+            detection_data: DetectionDataPlan {
+                clue_plans: vec![CluePlan::new(&mut OsRng, addr, 1)],
+            },
+            memo_data: Some(MemoPlan::new(&mut OsRng, memo_plaintext.clone()).unwrap()),
         };
 
         println!("{}", serde_json::to_string_pretty(&plan).unwrap());

crates/core/transaction/src/lib.rs

@@ -17,10 +17,12 @@
 
 mod auth_data;
 mod auth_hash;
+mod detection_data;
 mod effect_hash;
 mod error;
 mod id;
 mod is_action;
+mod parameters;
 mod transaction;
 mod witness_data;
 
@@ -33,12 +35,14 @@ pub mod view;
 pub use action::Action;
 pub use auth_data::AuthorizationData;
 pub use auth_hash::{AuthHash, AuthorizingData};
+pub use detection_data::DetectionData;
 pub use effect_hash::EffectingData;
 pub use error::Error;
 pub use id::Id;
 pub use is_action::IsAction;
+pub use parameters::TransactionParameters;
 pub use plan::ActionPlan;
-pub use transaction::{Transaction, TransactionBody, TransactionParameters};
+pub use transaction::{Transaction, TransactionBody};
 pub use view::{ActionView, MemoPlaintextView, MemoView, TransactionPerspective, TransactionView};
 pub use witness_data::WitnessData;
 

crates/core/transaction/src/parameters.rs

@@ -0,0 +1,41 @@
+use anyhow::Error;
+use penumbra_fee::Fee;
+use penumbra_proto::core::transaction::v1alpha1 as pbt;
+use penumbra_proto::DomainType;
+
+/// Parameters determining when the transaction should be accepted to the chain.
+#[derive(Clone, Debug, Default)]
+pub struct TransactionParameters {
+    pub expiry_height: u64,
+    pub chain_id: String,
+    pub fee: Fee,
+}
+
+impl DomainType for TransactionParameters {
+    type Proto = pbt::TransactionParameters;
+}
+
+impl TryFrom<pbt::TransactionParameters> for TransactionParameters {
+    type Error = Error;
+
+    fn try_from(proto: pbt::TransactionParameters) -> anyhow::Result<Self, Self::Error> {
+        Ok(TransactionParameters {
+            expiry_height: proto.expiry_height,
+            chain_id: proto.chain_id,
+            fee: proto
+                .fee
+                .ok_or_else(|| anyhow::anyhow!("transaction parameters missing fee"))?
+                .try_into()?,
+        })
+    }
+}
+
+impl From<TransactionParameters> for pbt::TransactionParameters {
+    fn from(msg: TransactionParameters) -> Self {
+        pbt::TransactionParameters {
+            expiry_height: msg.expiry_height,
+            chain_id: msg.chain_id,
+            fee: Some(msg.fee.into()),
+        }
+    }
+}