Merge pull request #1994 from radixdlt/tweak/add-non-root-subintent-h…

…ashes-to-user-transaction-hashes

tweak: Include non root subintent hashes on `UserTransactionHashes`

radix-transactions/src/builder/transaction_builder.rs

@@ -963,7 +963,6 @@ impl TransactionV2Builder {
             raw,
             object_names,
             transaction_hashes: prepared.hashes(),
-            non_root_subintent_hashes: prepared.non_root_subintent_hashes().collect(),
         }
     }
 
@@ -985,7 +984,6 @@ impl TransactionV2Builder {
             raw,
             object_names,
             transaction_hashes: validated.prepared.hashes(),
-            non_root_subintent_hashes: validated.prepared.non_root_subintent_hashes().collect(),
         }
     }
 
@@ -1024,7 +1022,6 @@ pub struct DetailedNotarizedTransactionV2 {
     pub raw: RawNotarizedTransaction,
     pub object_names: TransactionObjectNames,
     pub transaction_hashes: UserTransactionHashes,
-    pub non_root_subintent_hashes: IndexSet<SubintentHash>,
 }
 
 impl IntoExecutable for DetailedNotarizedTransactionV2 {

radix-transactions/src/model/concepts.rs

@@ -98,6 +98,13 @@ pub trait HasNotarizedTransactionHash {
     fn notarized_transaction_hash(&self) -> NotarizedTransactionHash;
 }
 
+pub trait HasNonRootSubintentHashes {
+    /// ## Validity Note
+    /// Preparable but invalid transactions may contain non-root subintents with duplicate [`SubintentHash`]es.
+    /// Therefore we return a `Vec` instead of an `IndexSet` here.
+    fn non_root_subintent_hashes(&self) -> Vec<SubintentHash>;
+}
+
 define_raw_transaction_payload!(RawSubintent, TransactionPayloadKind::Other);
 define_wrapped_hash!(
     /// A hash of the subintent.

radix-transactions/src/model/ledger_transaction.rs

@@ -569,24 +569,104 @@ impl IntoExecutable for ValidatedLedgerTransaction {
     }
 }
 
+define_versioned! {
+    // `LedgerTransactionHashes` is used in the node's `VersionedCommittedTransactionIdentifiers`,
+    // so we add this here tp catch possible backwards compatibility with the node integrations,
+    // and ensure we have versioned models here ready to go for the node integration.
+    #[derive(Debug, Clone, ScryptoSbor)]
+    pub VersionedLedgerTransactionHashes(LedgerTransactionHashesVersions) {
+        previous_versions: [
+            1 => LedgerTransactionHashesV1: { updates_to: 2 },
+        ],
+        latest_version: {
+            2 => LedgerTransactionHashes = LedgerTransactionHashesV2,
+        },
+    },
+    outer_attributes: [
+        #[derive(ScryptoSborAssertion)]
+        #[sbor_assert(backwards_compatible(
+            bottlenose = "FILE:ledger_transaction_hashes_bottlenose.bin",
+            cuttlefish = "FILE:ledger_transaction_hashes_cuttlefish.bin"
+        ))]
+    ]
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, Sbor)]
-pub struct LedgerTransactionHashes {
+pub struct LedgerTransactionHashesV2 {
     pub ledger_transaction_hash: LedgerTransactionHash,
-    pub kinded: KindedTransactionHashes,
+    pub kinded: KindedTransactionHashesV2,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Sbor)]
+pub struct LedgerTransactionHashesV1 {
+    pub ledger_transaction_hash: LedgerTransactionHash,
+    pub kinded: KindedTransactionHashesV1,
+}
+
+impl From<LedgerTransactionHashesV1> for LedgerTransactionHashesV2 {
+    fn from(value: LedgerTransactionHashesV1) -> Self {
+        let LedgerTransactionHashesV1 {
+            ledger_transaction_hash,
+            kinded,
+        } = value;
+        LedgerTransactionHashesV2 {
+            ledger_transaction_hash,
+            kinded: kinded.into(),
+        }
+    }
 }
 
 impl LedgerTransactionHashes {
-    pub fn as_user(&self) -> Option<UserTransactionHashes> {
+    pub fn as_user(&self) -> Option<&UserTransactionHashes> {
         self.kinded.as_user()
     }
 }
 
+pub type KindedTransactionHashes = KindedTransactionHashesV2;
+
+impl From<KindedTransactionHashesV1> for KindedTransactionHashesV2 {
+    fn from(value: KindedTransactionHashesV1) -> Self {
+        match value {
+            KindedTransactionHashesV1::Genesis {
+                system_transaction_hash,
+            } => KindedTransactionHashesV2::Genesis {
+                system_transaction_hash,
+            },
+            KindedTransactionHashesV1::User(user_transaction_hashes_v1) => {
+                KindedTransactionHashesV2::User(user_transaction_hashes_v1.into())
+            }
+            KindedTransactionHashesV1::RoundUpdateV1 { round_update_hash } => {
+                KindedTransactionHashesV2::RoundUpdateV1 { round_update_hash }
+            }
+            KindedTransactionHashesV1::FlashV1 {
+                flash_transaction_hash,
+            } => KindedTransactionHashesV2::FlashV1 {
+                flash_transaction_hash,
+            },
+        }
+    }
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Sbor)]
+pub enum KindedTransactionHashesV1 {
+    Genesis {
+        system_transaction_hash: SystemTransactionHash,
+    },
+    User(#[sbor(flatten)] UserTransactionHashesV1),
+    RoundUpdateV1 {
+        round_update_hash: RoundUpdateTransactionHash,
+    },
+    FlashV1 {
+        flash_transaction_hash: FlashTransactionHash,
+    },
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, Sbor)]
-pub enum KindedTransactionHashes {
+pub enum KindedTransactionHashesV2 {
     Genesis {
         system_transaction_hash: SystemTransactionHash,
     },
-    User(#[sbor(flatten)] UserTransactionHashes),
+    User(#[sbor(flatten)] UserTransactionHashesV2),
     RoundUpdateV1 {
         round_update_hash: RoundUpdateTransactionHash,
     },
@@ -596,9 +676,9 @@ pub enum KindedTransactionHashes {
 }
 
 impl KindedTransactionHashes {
-    pub fn as_user(&self) -> Option<UserTransactionHashes> {
+    pub fn as_user(&self) -> Option<&UserTransactionHashes> {
         match self {
-            KindedTransactionHashes::User(user) => Some(*user),
+            KindedTransactionHashes::User(user) => Some(user),
             _ => None,
         }
     }

radix-transactions/src/model/user_transaction.rs

@@ -171,21 +171,12 @@ impl PreparedUserTransaction {
     }
 
     pub fn hashes(&self) -> UserTransactionHashes {
-        UserTransactionHashes {
-            transaction_intent_hash: self.transaction_intent_hash(),
-            signed_transaction_intent_hash: self.signed_transaction_intent_hash(),
-            notarized_transaction_hash: self.notarized_transaction_hash(),
+        match self {
+            PreparedUserTransaction::V1(t) => t.hashes(),
+            PreparedUserTransaction::V2(t) => t.hashes(),
         }
     }
 
-    pub fn non_root_subintent_hashes(&self) -> impl Iterator<Item = SubintentHash> + '_ {
-        let boxed_iterator: Box<dyn Iterator<Item = SubintentHash> + '_> = match self {
-            PreparedUserTransaction::V1(_) => Box::new(core::iter::empty()),
-            PreparedUserTransaction::V2(t) => Box::new(t.non_root_subintent_hashes()),
-        };
-        boxed_iterator
-    }
-
     pub fn validate(
         self,
         validator: &TransactionValidator,
@@ -224,6 +215,15 @@ impl HasNotarizedTransactionHash for PreparedUserTransaction {
     }
 }
 
+impl HasNonRootSubintentHashes for PreparedUserTransaction {
+    fn non_root_subintent_hashes(&self) -> Vec<SubintentHash> {
+        match self {
+            Self::V1(_) => Default::default(),
+            Self::V2(t) => t.non_root_subintent_hashes(),
+        }
+    }
+}
+
 impl HasSummary for PreparedUserTransaction {
     fn get_summary(&self) -> &Summary {
         match self {
@@ -303,6 +303,15 @@ impl HasNotarizedTransactionHash for ValidatedUserTransaction {
     }
 }
 
+impl HasNonRootSubintentHashes for ValidatedUserTransaction {
+    fn non_root_subintent_hashes(&self) -> Vec<SubintentHash> {
+        match self {
+            Self::V1(_) => Default::default(),
+            Self::V2(t) => t.non_root_subintent_hashes(),
+        }
+    }
+}
+
 impl IntoExecutable for ValidatedUserTransaction {
     type Error = core::convert::Infallible;
 
@@ -332,29 +341,49 @@ impl ValidatedUserTransaction {
     }
 
     pub fn hashes(&self) -> UserTransactionHashes {
-        UserTransactionHashes {
-            transaction_intent_hash: self.transaction_intent_hash(),
-            signed_transaction_intent_hash: self.signed_transaction_intent_hash(),
-            notarized_transaction_hash: self.notarized_transaction_hash(),
+        match self {
+            Self::V1(t) => t.hashes(),
+            Self::V2(t) => t.hashes(),
         }
     }
-
-    pub fn non_root_subintent_hashes(&self) -> impl Iterator<Item = SubintentHash> + '_ {
-        let boxed_iterator: Box<dyn Iterator<Item = SubintentHash> + '_> = match self {
-            ValidatedUserTransaction::V1(_) => Box::new(core::iter::empty()),
-            ValidatedUserTransaction::V2(t) => Box::new(t.prepared.non_root_subintent_hashes()),
-        };
-        boxed_iterator
-    }
 }
 
+pub type UserTransactionHashes = UserTransactionHashesV2;
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Sbor)]
-pub struct UserTransactionHashes {
+pub struct UserTransactionHashesV1 {
+    pub transaction_intent_hash: TransactionIntentHash,
+    pub signed_transaction_intent_hash: SignedTransactionIntentHash,
+    pub notarized_transaction_hash: NotarizedTransactionHash,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, Sbor)]
+pub struct UserTransactionHashesV2 {
     pub transaction_intent_hash: TransactionIntentHash,
+    /// ## Validity Note
+    /// Preparable but invalid transactions may contain non-root subintents with duplicate [`SubintentHash`]es.
+    /// Therefore we return a `Vec` instead of an `IndexSet` here.
+    pub non_root_subintent_hashes: Vec<SubintentHash>,
     pub signed_transaction_intent_hash: SignedTransactionIntentHash,
     pub notarized_transaction_hash: NotarizedTransactionHash,
 }
 
+impl From<UserTransactionHashesV1> for UserTransactionHashesV2 {
+    fn from(value: UserTransactionHashesV1) -> Self {
+        let UserTransactionHashesV1 {
+            transaction_intent_hash,
+            signed_transaction_intent_hash,
+            notarized_transaction_hash,
+        } = value;
+        UserTransactionHashesV2 {
+            transaction_intent_hash,
+            non_root_subintent_hashes: vec![],
+            signed_transaction_intent_hash,
+            notarized_transaction_hash,
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

radix-transactions/src/model/v1/notarized_transaction_v1.rs

@@ -81,6 +81,15 @@ impl PreparedNotarizedTransactionV1 {
         self.signed_intent.intent.header.inner.end_epoch_exclusive
     }
 
+    pub fn hashes(&self) -> UserTransactionHashes {
+        UserTransactionHashes {
+            transaction_intent_hash: self.transaction_intent_hash(),
+            signed_transaction_intent_hash: self.signed_transaction_intent_hash(),
+            notarized_transaction_hash: self.notarized_transaction_hash(),
+            non_root_subintent_hashes: Default::default(),
+        }
+    }
+
     pub fn validate(
         self,
         validator: &TransactionValidator,

radix-transactions/src/model/v1/validated_notarized_transaction.rs

@@ -29,6 +29,10 @@ impl HasNotarizedTransactionHash for ValidatedNotarizedTransactionV1 {
 
 #[allow(deprecated)]
 impl ValidatedNotarizedTransactionV1 {
+    pub fn hashes(&self) -> UserTransactionHashes {
+        self.prepared.hashes()
+    }
+
     pub fn create_executable(self) -> ExecutableTransaction {
         let intent = self.prepared.signed_intent.intent;
         let intent_hash = intent.transaction_intent_hash();

radix-transactions/src/model/v2/non_root_subintents_v2.rs

@@ -22,6 +22,14 @@ pub struct PreparedNonRootSubintentsV2 {
 
 impl_has_summary!(PreparedNonRootSubintentsV2);
 
+impl HasNonRootSubintentHashes for PreparedNonRootSubintentsV2 {
+    fn non_root_subintent_hashes(&self) -> Vec<SubintentHash> {
+        // This can be shorter than `self.subintents` if the transaction is invalid,
+        // but this is OK as per the definition on the trait.
+        self.subintents.iter().map(|s| s.subintent_hash()).collect()
+    }
+}
+
 impl TransactionPreparableFromValueBody for PreparedNonRootSubintentsV2 {
     fn prepare_from_value_body(decoder: &mut TransactionDecoder) -> Result<Self, PrepareError> {
         let max_subintents_per_transaction = decoder.settings().max_subintents_per_transaction;

radix-transactions/src/model/v2/notarized_transaction_v2.rs

@@ -122,20 +122,13 @@ impl PreparedNotarizedTransactionV2 {
             .unwrap()
     }
 
-    pub fn non_root_subintent_hashes(&self) -> impl Iterator<Item = SubintentHash> + '_ {
-        self.signed_intent
-            .transaction_intent
-            .non_root_subintents
-            .subintents
-            .iter()
-            .map(|s| s.subintent_hash())
-    }
-
     pub fn hashes(&self) -> UserTransactionHashes {
         UserTransactionHashes {
             transaction_intent_hash: self.transaction_intent_hash(),
             signed_transaction_intent_hash: self.signed_transaction_intent_hash(),
             notarized_transaction_hash: self.notarized_transaction_hash(),
+            // NOTE: If the NotarizedTransactionV2 is invalid, there might be duplicate hashes
+            non_root_subintent_hashes: self.non_root_subintent_hashes(),
         }
     }
 
@@ -192,3 +185,9 @@ impl HasNotarizedTransactionHash for PreparedNotarizedTransactionV2 {
         NotarizedTransactionHash::from_hash(self.summary.hash)
     }
 }
+
+impl HasNonRootSubintentHashes for PreparedNotarizedTransactionV2 {
+    fn non_root_subintent_hashes(&self) -> Vec<SubintentHash> {
+        self.signed_intent.non_root_subintent_hashes()
+    }
+}

radix-transactions/src/model/v2/signed_transaction_intent_v2.rs

@@ -50,3 +50,9 @@ impl HasSignedTransactionIntentHash for PreparedSignedTransactionIntentV2 {
         SignedTransactionIntentHash::from_hash(self.summary.hash)
     }
 }
+
+impl HasNonRootSubintentHashes for PreparedSignedTransactionIntentV2 {
+    fn non_root_subintent_hashes(&self) -> Vec<SubintentHash> {
+        self.transaction_intent.non_root_subintent_hashes()
+    }
+}

radix-transactions/src/model/v2/transaction_intent_v2.rs

@@ -48,3 +48,9 @@ impl HasTransactionIntentHash for PreparedTransactionIntentV2 {
         TransactionIntentHash::from_hash(self.summary.hash)
     }
 }
+
+impl HasNonRootSubintentHashes for PreparedTransactionIntentV2 {
+    fn non_root_subintent_hashes(&self) -> Vec<SubintentHash> {
+        self.non_root_subintents.non_root_subintent_hashes()
+    }
+}

radix-transactions/src/model/v2/validated_notarized_transaction_v2.rs

@@ -58,6 +58,12 @@ impl HasNotarizedTransactionHash for ValidatedNotarizedTransactionV2 {
     }
 }
 
+impl HasNonRootSubintentHashes for ValidatedNotarizedTransactionV2 {
+    fn non_root_subintent_hashes(&self) -> Vec<SubintentHash> {
+        self.prepared.non_root_subintent_hashes()
+    }
+}
+
 impl IntoExecutable for ValidatedNotarizedTransactionV2 {
     type Error = core::convert::Infallible;
 
@@ -70,6 +76,10 @@ impl IntoExecutable for ValidatedNotarizedTransactionV2 {
 }
 
 impl ValidatedNotarizedTransactionV2 {
+    pub fn hashes(&self) -> UserTransactionHashes {
+        self.prepared.hashes()
+    }
+
     pub fn create_executable(self) -> ExecutableTransaction {
         let transaction_intent = self.prepared.signed_intent.transaction_intent;
         let transaction_intent_hash = transaction_intent.transaction_intent_hash();