Merge pull request #2013 from radixdlt/tests/transaction-structure-tests

tests: Added tests for each `SubintentStructureError`

radix-common/src/data/manifest/model/manifest_resource_assertion.rs

@@ -20,13 +20,25 @@ impl ManifestResourceConstraints {
     /// * Panics if the constraint isn't valid for the resource address
     /// * Panics if constraints have already been specified against the resource
     pub fn with(
-        mut self,
+        self,
         resource_address: ResourceAddress,
         constraint: ManifestResourceConstraint,
     ) -> Self {
         if !constraint.is_valid_for(&resource_address) {
             panic!("Constraint isn't valid for the resource address");
         }
+        self.with_unchecked(resource_address, constraint)
+    }
+
+    /// Unlike `with`, this does not validate the constraint.
+    ///
+    /// ## Panics
+    /// * Panics if constraints have already been specified against the resource
+    pub fn with_unchecked(
+        mut self,
+        resource_address: ResourceAddress,
+        constraint: ManifestResourceConstraint,
+    ) -> Self {
         let replaced = self
             .specified_resources
             .insert(resource_address, constraint);

radix-rust/src/macros.rs

@@ -46,7 +46,7 @@ macro_rules! assert_matches {
             )
         }
     };
-    ($expression:expr, $pattern:pat $(if $condition:expr)? $(,)? => $code:expr) => {
+    ($expression:expr, $pattern:pat $(if $condition:expr)? => $code:expr $(,)?) => {
         match $expression {
             $pattern $(if $condition)? => $code,
             ref expression => panic!(
@@ -67,7 +67,7 @@ macro_rules! assert_matches {
             )
         }
     };
-    ($expression:expr, $pattern:pat $(if $condition:expr)? $(,)? => $code:expr, $($arg:tt)+) => {
+    ($expression:expr, $pattern:pat $(if $condition:expr)? => $code:expr, $($arg:tt)+) => {
         match $expression {
             $pattern $(if $condition)? => $code,
             ref expression => panic!(

radix-transactions/src/builder/manifest_builder.rs

@@ -343,7 +343,9 @@ impl<M: BuildableManifest> ManifestBuilder<M> {
         self
     }
 
-    #[deprecated = "This should not be used apart from for test code purposefully constructing invalid manifests. Instead use the more-tailored instruction, or add_instruction_advanced."]
+    /// This should not be used apart from for test code purposefully constructing invalid manifests.
+    /// Instead use the more-tailored instruction, or add_instruction_advanced.
+    #[cfg(test)]
     pub fn add_raw_instruction_ignoring_all_side_effects(
         self,
         instruction: impl Into<M::Instruction>,

radix-transactions/src/errors.rs

@@ -3,11 +3,9 @@ use sbor::*;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum HeaderValidationError {
-    UnknownVersion(u8),
     InvalidEpochRange,
     InvalidTimestampRange,
     InvalidNetwork,
-    InvalidCostUnitLimit,
     InvalidTip,
     NoValidEpochRangeAcrossAllIntents,
     NoValidTimestampRangeAcrossAllIntents,

radix-transactions/src/manifest/generator.rs

@@ -2262,14 +2262,13 @@ pub fn generator_error_diagnostics(
         GeneratorErrorKind::ManifestBuildError(
             ManifestBuildError::PreallocatedAddressesUnsupportedByManifestType,
         ) => {
-            let title =
-                format!("preallocated addresses are not supported in this manifest version");
+            let title = format!("preallocated addresses are not supported in this manifest type");
             (title, "unsupported instruction".to_string())
         }
         GeneratorErrorKind::ManifestBuildError(
             ManifestBuildError::ChildSubintentsUnsupportedByManifestType,
         ) => {
-            let title = format!("child subintents are not supported in this manifest version");
+            let title = format!("child subintents are not supported in this manifest type");
             (title, "unsupported instruction".to_string())
         }
         GeneratorErrorKind::HeaderInstructionMustComeFirst => {

radix-transactions/src/manifest/manifest_instructions.rs

@@ -1341,6 +1341,14 @@ pub struct YieldToParent {
     pub args: ManifestValue,
 }
 
+impl YieldToParent {
+    pub fn empty() -> Self {
+        Self {
+            args: ManifestValue::unit(),
+        }
+    }
+}
+
 impl ManifestInstruction for YieldToParent {
     const IDENT: &'static str = "YIELD_TO_PARENT";
     const ID: u8 = INSTRUCTION_YIELD_TO_PARENT_DISCRIMINATOR;

radix-transactions/src/model/mod.rs

@@ -238,7 +238,7 @@ Enum<3u8>(
         assert_eq!(
             validated,
             Err(TransactionValidationError::PrepareError(
-                PrepareError::Other(format!("Unknown transaction payload discriminator byte: 4"))
+                PrepareError::UnexpectedTransactionDiscriminator { actual: Some(4) }
             ))
         )
     }

radix-transactions/src/model/preparation/decoder.rs

@@ -4,10 +4,10 @@ use sbor::*;
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub enum ValueType {
     Blob,
-    Message,
     Subintent,
-    ChildIntentConstraint,
-    IntentSignatures,
+    ChildSubintentSpecifier,
+    SubintentSignatureBatches,
+    // Too many signatures is captured at validation time
 }
 
 #[derive(Debug, Clone, Eq, PartialEq)]
@@ -22,7 +22,9 @@ pub enum PrepareError {
         max: usize,
     },
     LengthOverflow,
-    Other(String),
+    UnexpectedTransactionDiscriminator {
+        actual: Option<u8>,
+    },
 }
 
 impl From<DecodeError> for PrepareError {

radix-transactions/src/model/user_transaction.rs

@@ -248,9 +248,9 @@ impl PreparedTransaction for PreparedUserTransaction {
     ) -> Result<Self, PrepareError> {
         let offset = decoder.get_offset();
         let slice = decoder.get_input_slice();
-        let discriminator_byte = slice.get(offset + 1).ok_or(PrepareError::Other(
-            "Could not read transaction payload discriminator byte".to_string(),
-        ))?;
+        let discriminator_byte = slice
+            .get(offset + 1)
+            .ok_or(PrepareError::UnexpectedTransactionDiscriminator { actual: None })?;
 
         let prepared = match TransactionDiscriminator::from_repr(*discriminator_byte) {
             Some(TransactionDiscriminator::V1Notarized) => PreparedUserTransaction::V1(
@@ -260,9 +260,9 @@ impl PreparedTransaction for PreparedUserTransaction {
                 PreparedNotarizedTransactionV2::prepare_from_transaction_enum(decoder)?,
             ),
             _ => {
-                return Err(PrepareError::Other(format!(
-                    "Unknown transaction payload discriminator byte: {discriminator_byte}"
-                )))
+                return Err(PrepareError::UnexpectedTransactionDiscriminator {
+                    actual: Some(*discriminator_byte),
+                })
             }
         };
 

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

@@ -11,6 +11,12 @@ pub struct BlobsV1 {
     pub blobs: Vec<BlobV1>,
 }
 
+impl BlobsV1 {
+    pub fn none() -> Self {
+        Self { blobs: Vec::new() }
+    }
+}
+
 impl From<IndexMap<Hash, Vec<u8>>> for BlobsV1 {
     fn from(blobs: IndexMap<Hash, Vec<u8>>) -> Self {
         let blobs = blobs

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

@@ -35,6 +35,12 @@ impl ChildSubintentSpecifier {
     }
 }
 
+impl From<SubintentHash> for ChildSubintentSpecifier {
+    fn from(hash: SubintentHash) -> Self {
+        Self { hash }
+    }
+}
+
 /// A new-type representing the index of a referenced intent.
 /// The first few of these will be the children of the given intent.
 ///
@@ -86,7 +92,7 @@ impl TransactionPreparableFromValueBody for PreparedChildSubintentSpecifiersV2 {
         let (hashes, summary) =
             ConcatenatedDigest::prepare_from_sbor_array_value_body::<Vec<RawHash>>(
                 decoder,
-                ValueType::ChildIntentConstraint,
+                ValueType::ChildSubintentSpecifier,
                 max_child_subintents_per_intent,
             )?;
 

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

@@ -7,6 +7,18 @@ pub struct IntentSignaturesV2 {
     pub signatures: Vec<IntentSignatureV1>,
 }
 
+impl IntentSignaturesV2 {
+    pub fn none() -> Self {
+        Self {
+            signatures: Vec::new(),
+        }
+    }
+
+    pub fn new(signatures: Vec<IntentSignatureV1>) -> Self {
+        Self { signatures }
+    }
+}
+
 impl TransactionPartialPrepare for IntentSignaturesV2 {
     type Prepared = PreparedIntentSignaturesV2;
 }
@@ -34,7 +46,7 @@ impl TransactionPreparableFromValueBody for PreparedNonRootSubintentSignaturesV2
             Vec<PreparedIntentSignaturesV2>,
         >(
             decoder,
-            ValueType::IntentSignatures,
+            ValueType::SubintentSignatureBatches,
             max_subintents_per_transaction,
         )?;
 

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

@@ -84,6 +84,15 @@ impl BuildableManifest for TransactionManifestV2 {
 impl BuildableManifestSupportingChildren for TransactionManifestV2 {}
 
 impl TransactionManifestV2 {
+    pub fn empty() -> Self {
+        Self {
+            instructions: Default::default(),
+            blobs: Default::default(),
+            children: Default::default(),
+            object_names: ManifestObjectNames::Unknown,
+        }
+    }
+
     pub fn from_intent_core(intent: &IntentCoreV2) -> Self {
         Self {
             instructions: intent.instructions.to_vec(),
@@ -93,6 +102,18 @@ impl TransactionManifestV2 {
         }
     }
 
+    pub fn to_intent_core(self, header: IntentHeaderV2, message: MessageV2) -> IntentCoreV2 {
+        IntentCoreV2 {
+            header,
+            blobs: self.blobs.into(),
+            message,
+            instructions: self.instructions.into(),
+            children: ChildSubintentSpecifiersV2 {
+                children: self.children,
+            },
+        }
+    }
+
     pub fn for_intent(self) -> (InstructionsV2, BlobsV1, ChildSubintentSpecifiersV2) {
         (
             self.instructions.into(),

radix-transactions/src/validation/mod.rs

@@ -16,4 +16,4 @@ pub use transaction_structure_validator::*;
 pub use transaction_validation_configuration::*;
 pub use transaction_validator::*;
 #[cfg(test)]
-pub use validation_test_helpers::*;
+pub(crate) use validation_test_helpers::*;

radix-transactions/src/validation/signature_validator.rs

@@ -585,4 +585,112 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn too_many_signatures_should_be_rejected() {
+        fn validate_transaction(
+            root_signature_count: usize,
+            signature_counts: Vec<usize>,
+        ) -> Result<ValidatedNotarizedTransactionV2, TransactionValidationError> {
+            TransactionV2Builder::new_with_test_defaults()
+                .add_children(
+                    signature_counts
+                        .iter()
+                        .enumerate()
+                        .map(|(i, signature_count)| {
+                            create_leaf_partial_transaction(i as u64, *signature_count)
+                        }),
+                )
+                .add_manifest_calling_each_child_once()
+                .multi_sign(
+                    (0..root_signature_count)
+                        .into_iter()
+                        .map(|i| Secp256k1PrivateKey::from_u64((100 + i) as u64).unwrap()),
+                )
+                .default_notarize_and_validate()
+        }
+
+        assert_matches!(validate_transaction(1, vec![10]), Ok(_));
+        assert_matches!(
+            validate_transaction(1, vec![10, 20]),
+            Err(TransactionValidationError::SignatureValidationError(
+                TransactionValidationErrorLocation::NonRootSubintent(SubintentIndex(1), _),
+                SignatureValidationError::TooManySignatures {
+                    total: 20,
+                    limit: 16,
+                },
+            ))
+        );
+        assert_matches!(
+            validate_transaction(17, vec![0, 3]),
+            Err(TransactionValidationError::SignatureValidationError(
+                TransactionValidationErrorLocation::RootTransactionIntent(_),
+                SignatureValidationError::TooManySignatures {
+                    total: 17,
+                    limit: 16,
+                },
+            ))
+        );
+        assert_matches!(
+            validate_transaction(1, vec![10, 10, 10, 10, 10, 10, 10]),
+            Err(TransactionValidationError::SignatureValidationError(
+                TransactionValidationErrorLocation::AcrossTransaction,
+                SignatureValidationError::TooManySignatures {
+                    total: 72, // 70 from subintent, 1 from transaction intent, 1 from notarization
+                    limit: 64
+                },
+            ))
+        );
+    }
+
+    #[test]
+    fn test_incorrect_number_of_subintent_signature_batches() {
+        // CASE 1: Too fee signatures
+        let validator = TransactionValidator::new_for_latest_simulator();
+
+        let mut transaction = TransactionV2Builder::new_with_test_defaults()
+            .add_children(vec![PartialTransactionV2Builder::new_with_test_defaults()
+                .add_trivial_manifest()
+                .build()])
+            .add_manifest_calling_each_child_once()
+            .default_notarize()
+            .build_minimal_no_validate();
+
+        // Remove one signature batch
+        let removed_signature_batch = transaction
+            .signed_transaction_intent
+            .non_root_subintent_signatures
+            .by_subintent
+            .pop()
+            .unwrap();
+
+        assert_matches!(
+            transaction.prepare_and_validate(&validator),
+            Err(TransactionValidationError::SignatureValidationError(
+                TransactionValidationErrorLocation::AcrossTransaction,
+                SignatureValidationError::IncorrectNumberOfSubintentSignatureBatches
+            ))
+        );
+
+        // CASE 2: Too many signature batches
+        let mut transaction = TransactionV2Builder::new_with_test_defaults()
+            .add_trivial_manifest()
+            .default_notarize()
+            .build_minimal_no_validate();
+
+        // Add an extra signature batch
+        transaction
+            .signed_transaction_intent
+            .non_root_subintent_signatures
+            .by_subintent
+            .push(removed_signature_batch);
+
+        assert_matches!(
+            transaction.prepare_and_validate(&validator),
+            Err(TransactionValidationError::SignatureValidationError(
+                TransactionValidationErrorLocation::AcrossTransaction,
+                SignatureValidationError::IncorrectNumberOfSubintentSignatureBatches
+            ))
+        );
+    }
 }