feat: only coinbase output features may have metadata set, and is of …

…limited size; ref #4908 (#4960)

Description
---
Added validation to OutputFeatures which, for now, makes sure that only coinbase output features may have metadata set.

Motivation and Context
---
Remove Metadata from Output features: metadata is a remnant of the previous DAN implementation. It should be removed from output features

#4908

How Has This Been Tested?
---
unit tests

base_layer/core/src/blocks/block.rs

@@ -121,9 +121,19 @@ impl Block {
         Ok(())
     }
 
+    /// Run through the outputs of the block and check that
+    /// 1. only coinbase outputs may have metadata set,
+    /// 1. coinbase metadata length does not exceed its limit
+    pub fn check_output_features(&self, consensus_constants: &ConsensusConstants) -> Result<(), BlockValidationError> {
+        self.body
+            .check_output_features(consensus_constants.coinbase_output_features_metadata_max_length())?;
+
+        Ok(())
+    }
+
     /// Checks that all STXO rules (maturity etc) and kernel heights are followed
     pub fn check_spend_rules(&self) -> Result<(), BlockValidationError> {
-        self.body.check_stxo_rules(self.header.height)?;
+        self.body.check_utxo_rules(self.header.height)?;
         if self.body.max_kernel_timelock() > self.header.height {
             return Err(BlockValidationError::MaturityError);
         }

base_layer/core/src/consensus/consensus_constants.rs

@@ -97,6 +97,8 @@ pub struct ConsensusConstants {
     permitted_output_types: &'static [OutputType],
     /// How long does it take to timeout validator node registration
     validator_node_timeout: u64,
+    /// Coinbase outputs are allowed to have metadata, but it has the following length limit
+    coinbase_output_features_metadata_max_length: usize,
 }
 
 // todo: remove this once OutputFeaturesVersion is removed in favor of just TransactionOutputVersion
@@ -209,6 +211,10 @@ impl ConsensusConstants {
         self.transaction_weight.calculate(1, 0, 1, metadata_size)
     }
 
+    pub fn coinbase_output_features_metadata_max_length(&self) -> usize {
+        self.coinbase_output_features_metadata_max_length
+    }
+
     /// The amount of PoW algorithms used by the Tari chain.
     pub fn get_pow_algo_count(&self) -> u64 {
         self.proof_of_work.len() as u64
@@ -333,6 +339,7 @@ impl ConsensusConstants {
             kernel_version_range,
             permitted_output_types: OutputType::all(),
             validator_node_timeout: 100,
+            coinbase_output_features_metadata_max_length: 64,
         }]
     }
 
@@ -374,6 +381,7 @@ impl ConsensusConstants {
             kernel_version_range,
             permitted_output_types: Self::current_permitted_output_types(),
             validator_node_timeout: 0,
+            coinbase_output_features_metadata_max_length: 64,
         }]
     }
 
@@ -419,6 +427,7 @@ impl ConsensusConstants {
             // igor is the first network to support the new output types
             permitted_output_types: OutputType::all(),
             validator_node_timeout: 100,
+            coinbase_output_features_metadata_max_length: 64,
         }]
     }
 
@@ -470,6 +479,7 @@ impl ConsensusConstants {
                 kernel_version_range: kernel_version_range.clone(),
                 permitted_output_types: Self::current_permitted_output_types(),
                 validator_node_timeout: 0,
+                coinbase_output_features_metadata_max_length: 64,
             },
             ConsensusConstants {
                 effective_from_height: 23000,
@@ -494,6 +504,7 @@ impl ConsensusConstants {
                 kernel_version_range,
                 permitted_output_types: Self::current_permitted_output_types(),
                 validator_node_timeout: 0,
+                coinbase_output_features_metadata_max_length: 64,
             },
         ]
     }
@@ -542,6 +553,7 @@ impl ConsensusConstants {
             kernel_version_range,
             permitted_output_types: Self::current_permitted_output_types(),
             validator_node_timeout: 50,
+            coinbase_output_features_metadata_max_length: 64,
         };
 
         vec![consensus_constants_1]
@@ -586,6 +598,7 @@ impl ConsensusConstants {
             kernel_version_range,
             permitted_output_types: Self::current_permitted_output_types(),
             validator_node_timeout: 0,
+            coinbase_output_features_metadata_max_length: 64,
         }]
     }
 

base_layer/core/src/transactions/aggregated_body.rs

@@ -325,8 +325,29 @@ impl AggregateBody {
         Ok(())
     }
 
-    /// This function will check all stxo to ensure that feature flags where followed
-    pub fn check_stxo_rules(&self, height: u64) -> Result<(), TransactionError> {
+    pub fn check_output_features(&self, max_coinbase_metadata_size: usize) -> Result<(), TransactionError> {
+        for output in self.outputs() {
+            // This field should be optional for coinbases (mining pools and
+            // other merge mined coins can use it), but it should be empty for non-coinbases
+            if !output.is_coinbase() && !output.features.metadata.is_empty() {
+                return Err(TransactionError::NonCoinbaseHasOutputFeaturesMetadata);
+            }
+
+            // For coinbases, the maximum length should be 64 bytes (2x hashes),
+            // so that arbitrary data cannot be included
+            if output.is_coinbase() && output.features.metadata.len() > max_coinbase_metadata_size {
+                return Err(TransactionError::InvalidOutputFeaturesMetadataSize {
+                    len: output.features.metadata.len(),
+                    max: max_coinbase_metadata_size,
+                });
+            }
+        }
+
+        Ok(())
+    }
+
+    /// This function will check all UTXO to ensure that feature flags where followed
+    pub fn check_utxo_rules(&self, height: u64) -> Result<(), TransactionError> {
         for input in self.inputs() {
             if input.features()?.maturity > height {
                 warn!(

base_layer/core/src/transactions/transaction_components/error.rs

@@ -71,6 +71,10 @@ pub enum TransactionError {
     InvalidCommitteeLength { len: usize, max: usize },
     #[error("Missing validator node signature")]
     MissingValidatorNodeSignature,
+    #[error("Only coinbase outputs may have metadata")]
+    NonCoinbaseHasOutputFeaturesMetadata,
+    #[error("Metadata size is {len} but the maximum is {max}")]
+    InvalidOutputFeaturesMetadataSize { len: usize, max: usize },
 }
 
 impl From<CovenantError> for TransactionError {

base_layer/core/src/transactions/transaction_components/test.rs

@@ -294,10 +294,10 @@ fn check_timelocks() {
     tx.body.add_input(input.clone());
     tx.body.add_kernel(kernel.clone());
     assert!(matches!(
-        tx.body.check_stxo_rules(1),
+        tx.body.check_utxo_rules(1),
         Err(TransactionError::InputMaturity)
     ));
-    tx.body.check_stxo_rules(5).unwrap();
+    tx.body.check_utxo_rules(5).unwrap();
 
     assert_eq!(tx.max_input_maturity(), 5);
     assert_eq!(tx.max_kernel_timelock(), 2);

base_layer/core/src/validation/block_validators/orphan.rs

@@ -34,6 +34,7 @@ use crate::{
             check_coinbase_output,
             check_kernel_lock_height,
             check_maturity,
+            check_output_features,
             check_permitted_output_types,
             check_sorting_and_duplicates,
             check_total_burned,
@@ -71,6 +72,7 @@ impl OrphanValidation for OrphanBlockValidator {
     /// 1. Is the accounting correct?
     fn validate(&self, block: &Block) -> Result<(), ValidationError> {
         let height = block.header.height;
+
         if height == 0 {
             warn!(target: LOG_TARGET, "Attempt to validate genesis block");
             return Err(ValidationError::ValidatingGenesis);
@@ -86,49 +88,36 @@ impl OrphanValidation for OrphanBlockValidator {
         } else {
             format!("block #{}", height)
         };
-        trace!(target: LOG_TARGET, "Validating {}", block_id);
 
         let constants = self.rules.consensus_constants(height);
+
         validate_versions(&block.body, constants)?;
-        trace!(target: LOG_TARGET, "SV - Block body versions are ok for {} ", &block_id);
         check_block_weight(block, constants)?;
-        trace!(target: LOG_TARGET, "SV - Block weight is ok for {} ", &block_id);
-
-        trace!(
-            target: LOG_TARGET,
-            "Checking duplicate inputs and outputs on {}",
-            block_id
-        );
         check_sorting_and_duplicates(&block.body)?;
-        trace!(
-            target: LOG_TARGET,
-            "SV - No duplicate inputs / outputs for {} ",
-            &block_id
-        );
+
         for output in block.body.outputs() {
             check_permitted_output_types(constants, output)?;
         }
-        trace!(target: LOG_TARGET, "SV - Permitted output type ok for {} ", &block_id);
+
         check_total_burned(&block.body)?;
-        trace!(target: LOG_TARGET, "SV - Burned outputs ok for {} ", &block_id);
 
         // Check that the inputs are are allowed to be spent
         check_maturity(height, block.body.inputs())?;
         check_kernel_lock_height(height, block.body.kernels())?;
-        trace!(target: LOG_TARGET, "SV - Output constraints are ok for {} ", &block_id);
+        check_output_features(block, &self.rules)?;
         check_coinbase_output(block, &self.rules, &self.factories)?;
-        trace!(target: LOG_TARGET, "SV - Coinbase output is ok for {} ", &block_id);
         check_accounting_balance(
             block,
             &self.rules,
             self.bypass_range_proof_verification,
             &self.factories,
         )?;
-        trace!(target: LOG_TARGET, "SV - accounting balance correct for {}", &block_id);
+
         debug!(
             target: LOG_TARGET,
             "{} has PASSED stateless VALIDATION check.", &block_id
         );
+
         Ok(())
     }
 }

base_layer/core/src/validation/helpers.rs

@@ -300,6 +300,12 @@ pub fn check_coinbase_output(
         .map_err(ValidationError::from)
 }
 
+pub fn check_output_features(block: &Block, rules: &ConsensusManager) -> Result<(), ValidationError> {
+    block
+        .check_output_features(rules.consensus_constants(block.header.height))
+        .map_err(ValidationError::from)
+}
+
 pub fn is_all_unique_and_sorted<'a, I: IntoIterator<Item = &'a T>, T: PartialOrd + 'a>(items: I) -> bool {
     let mut items = items.into_iter();
     let prev_item = items.next();