feat!: remove coinbases from weight calculations (#6738)

Description
---
removes coinbases from the weight calculations making calculations
regarding weight simpler.
Adds max coinbase constant, currently 1000. 

Motivation and Context
---
This will simplify the mining and verification of coinbases and weights,
by making coinbases not count towards block weight.

How Has This Been Tested?
---
unit tests
>

applications/minotari_app_grpc/proto/types.proto

@@ -143,4 +143,5 @@ message ConsensusConstants {
     repeated PermittedRangeProofs permitted_range_proof_types = 34;
     uint64 inflation_bips = 35;
     uint64 tail_epoch_length = 36;
+    uint64 max_block_coinbase_count = 37;
 }

applications/minotari_app_grpc/src/conversions/consensus_constants.rs

@@ -107,6 +107,7 @@ impl From<ConsensusConstants> for grpc::ConsensusConstants {
             future_time_limit: cc.ftl().as_u64(),
             difficulty_block_window: cc.difficulty_block_window(),
             max_block_transaction_weight: cc.max_block_transaction_weight(),
+            max_block_coinbase_count: cc.max_block_coinbase_count(),
             pow_algo_count: cc.pow_algo_count(),
             median_timestamp_count: u64::try_from(cc.median_timestamp_count()).unwrap_or(0),
             emission_initial: emission_initial.into(),

applications/minotari_node/src/commands/command/status.rs

@@ -89,7 +89,7 @@ impl CommandContext {
                     if mempool_stats.unconfirmed_weight == 0 {
                         0
                     } else {
-                        1 + mempool_stats.unconfirmed_weight / constants.max_block_weight_excluding_coinbases(1)?
+                        1 + mempool_stats.unconfirmed_weight / constants.max_block_transaction_weight()
                     },
                 ),
             );

applications/minotari_node/src/grpc/base_node_grpc_server.rs

@@ -954,15 +954,7 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer {
         let constants_weight = self
             .consensus_rules
             .consensus_constants(meta.best_block_height().saturating_add(1))
-            .max_block_weight_excluding_coinbases(request.coinbases.len())
-            .map_err(|e| {
-                warn!(
-                    target: LOG_TARGET,
-                    "Could not get new block template: {}",
-                    e.to_string()
-                );
-                obscure_error_if_true(report_error_flag, Status::internal(e.to_string()))
-            })?;
+            .max_block_transaction_weight();
         let asking_weight = if request.max_weight > constants_weight || request.max_weight == 0 {
             constants_weight
         } else {

base_layer/core/src/base_node/comms_interface/inbound_handlers.rs

@@ -294,9 +294,7 @@ where B: BlockchainBackend + 'static
                 header.version = constants.blockchain_version();
                 header.pow.pow_algo = request.algo;
 
-                let constants_weight = constants
-                    .max_block_weight_excluding_coinbases(1)
-                    .map_err(|e| CommsInterfaceError::InternalError(e.to_string()))?;
+                let constants_weight = constants.max_block_transaction_weight();
                 let asking_weight = if request.max_weight > constants_weight || request.max_weight == 0 {
                     constants_weight
                 } else {

base_layer/core/src/blocks/block.rs

@@ -108,6 +108,7 @@ impl Block {
             consensus_constants.coinbase_min_maturity(),
             factories,
             self.header.height,
+            consensus_constants.max_block_coinbase_count(),
         )?;
         Ok(())
     }

base_layer/core/src/consensus/consensus_constants.rs

@@ -28,17 +28,15 @@ use std::{
 use chrono::{DateTime, Duration, Utc};
 use tari_common::configuration::Network;
 use tari_common_types::epoch::VnEpoch;
-use tari_script::{script, OpcodeVersion};
+use tari_script::OpcodeVersion;
 use tari_utilities::epoch_time::EpochTime;
 
 use crate::{
-    borsh::SerializedSize,
     consensus::network::NetworkConsensus,
     proof_of_work::{Difficulty, PowAlgorithm},
     transactions::{
         tari_amount::{uT, MicroMinotari},
         transaction_components::{
-            OutputFeatures,
             OutputFeaturesVersion,
             OutputType,
             RangeProofType,
@@ -71,6 +69,8 @@ pub struct ConsensusConstants {
     difficulty_block_window: u64,
     /// Maximum transaction weight used for the construction of new blocks.
     max_block_transaction_weight: u64,
+    /// Maximum coinbases allowed in a block
+    max_block_coinbase_count: u64,
     /// This is how many blocks we use to count towards the median timestamp to ensure the block chain timestamp moves
     /// forward
     median_timestamp_count: usize,
@@ -231,26 +231,9 @@ impl ConsensusConstants {
         self.max_block_transaction_weight
     }
 
-    /// Maximum transaction weight used for the construction of new blocks. It leaves place for 1 kernel and 1 output
-    /// with default features, as well as the maximum possible value of the `coinbase_extra` field
-    pub fn max_block_weight_excluding_coinbases(&self, number_of_coinbases: usize) -> std::io::Result<u64> {
-        Ok(self.max_block_transaction_weight - self.calculate_n_output_kernel_weight(number_of_coinbases)?)
-    }
-
-    fn calculate_n_output_kernel_weight(&self, num_outputs: usize) -> std::io::Result<u64> {
-        let output_features = OutputFeatures { ..Default::default() };
-        let max_extra_size = self.coinbase_output_features_extra_max_length() as usize;
-
-        let features_and_scripts_size = self.transaction_weight.round_up_features_and_scripts_size(
-            output_features.get_serialized_size()? +
-                max_extra_size +
-                script!(PushPubKey(Box::default()))
-                    .map_err(|e| e.to_std_io_error())?
-                    .get_serialized_size()?,
-        );
-        Ok(self
-            .transaction_weight
-            .calculate(1, 0, num_outputs, features_and_scripts_size * num_outputs))
+    /// Maximum block coinbases used for construction of new blocks.
+    pub fn max_block_coinbase_count(&self) -> u64 {
+        self.max_block_coinbase_count
     }
 
     pub fn coinbase_output_features_extra_max_length(&self) -> u32 {
@@ -398,6 +381,7 @@ impl ConsensusConstants {
             future_time_limit: 540,
             difficulty_block_window,
             max_block_transaction_weight: 19500,
+            max_block_coinbase_count: 1000,
             median_timestamp_count: 11,
             emission_initial: 18_462_816_327 * uT,
             emission_decay: &ESMERALDA_DECAY_PARAMS,
@@ -463,6 +447,7 @@ impl ConsensusConstants {
             // adj. + 95% = 127,795 - this effectively targets ~2Mb blocks closely matching the previous 19500
             // weightings
             max_block_transaction_weight: 127_795,
+            max_block_coinbase_count: 1000,
             median_timestamp_count: 11,
             emission_initial: 5_538_846_115 * uT,
             emission_decay: &EMISSION_DECAY,
@@ -521,6 +506,7 @@ impl ConsensusConstants {
             future_time_limit: 540,
             difficulty_block_window: 90,
             max_block_transaction_weight: 127_795,
+            max_block_coinbase_count: 1000,
             median_timestamp_count: 11,
             emission_initial: ESMERALDA_INITIAL_EMISSION,
             emission_decay: &ESMERALDA_DECAY_PARAMS,
@@ -583,6 +569,7 @@ impl ConsensusConstants {
             future_time_limit: 540,
             difficulty_block_window: 90,
             max_block_transaction_weight: 127_795,
+            max_block_coinbase_count: 1000,
             median_timestamp_count: 11,
             emission_initial: INITIAL_EMISSION,
             emission_decay: &EMISSION_DECAY,
@@ -634,6 +621,7 @@ impl ConsensusConstants {
             future_time_limit: 540,
             difficulty_block_window: 90,
             max_block_transaction_weight: 127_795,
+            max_block_coinbase_count: 1000,
             median_timestamp_count: 11,
             emission_initial: INITIAL_EMISSION,
             emission_decay: &EMISSION_DECAY,
@@ -696,6 +684,7 @@ impl ConsensusConstants {
             future_time_limit: 540,
             difficulty_block_window,
             max_block_transaction_weight: 127_795,
+            max_block_coinbase_count: 1000,
             median_timestamp_count: 11,
             emission_initial: INITIAL_EMISSION,
             emission_decay: &EMISSION_DECAY,

base_layer/core/src/mempool/mempool_storage.rs

@@ -399,8 +399,7 @@ impl MempoolStorage {
         let target_weight = self
             .rules
             .consensus_constants(tip_height)
-            .max_block_weight_excluding_coinbases(1)
-            .map_err(|e| MempoolError::InternalError(e.to_string()))?;
+            .max_block_transaction_weight();
         let stats = self.unconfirmed_pool.get_fee_per_gram_stats(count, target_weight)?;
         Ok(stats)
     }

base_layer/core/src/transactions/aggregated_body.rs

@@ -294,6 +294,7 @@ impl AggregateBody {
         coinbase_min_maturity: u64,
         factories: &CryptoFactories,
         height: u64,
+        maximum_coinbase_count: u64,
     ) -> Result<(), TransactionError> {
         let mut coinbase_utxo_sum = Commitment::default();
         let mut coinbase_kernel = None;
@@ -317,6 +318,16 @@ impl AggregateBody {
             target: LOG_TARGET,
             "{} coinbases found in body.", coinbase_counter,
         );
+        if coinbase_counter > maximum_coinbase_count {
+            warn!(
+                target: LOG_TARGET,
+                "{} coinbases found in body. Only {} is permitted.", coinbase_counter, maximum_coinbase_count
+            );
+            return Err(TransactionError::TooManyCoinbaseKernels {
+                max: maximum_coinbase_count,
+                found: coinbase_counter,
+            });
+        }
 
         let mut coinbase_kernel_counter = 0; // there should be exactly 1 coinbase kernel as well
         for kernel in self.kernels() {
@@ -328,7 +339,7 @@ impl AggregateBody {
         if coinbase_kernel.is_none() || coinbase_kernel_counter != 1 {
             warn!(
                 target: LOG_TARGET,
-                "{} coinbase kernels found in body. Only a single coinbase kernel is permitted.", coinbase_counter,
+                "{} coinbase kernels found in body. Only a single coinbase kernel is permitted.", coinbase_kernel_counter,
             );
             return Err(TransactionError::MoreThanOneCoinbaseKernel);
         }

base_layer/core/src/transactions/coinbase_builder.rs

@@ -619,6 +619,7 @@ mod test {
                 rules.consensus_constants(0).coinbase_min_maturity(),
                 &factories,
                 42,
+                1,
             )
             .unwrap();
 
@@ -666,7 +667,8 @@ mod test {
                 block_reward,
                 rules.consensus_constants(0).coinbase_min_maturity(),
                 &factories,
-                42
+                42,
+                1
             ),
             Err(TransactionError::InvalidCoinbaseMaturity)
         ));
@@ -729,7 +731,8 @@ mod test {
                 block_reward,
                 rules.consensus_constants(0).coinbase_min_maturity(),
                 &factories,
-                42
+                42,
+                1
             ),
             Err(TransactionError::InvalidCoinbase)
         ));
@@ -758,7 +761,8 @@ mod test {
                 block_reward,
                 rules.consensus_constants(0).coinbase_min_maturity(),
                 &factories,
-                42
+                42,
+                1
             )
             .is_ok());
     }
@@ -899,7 +903,8 @@ mod test {
                 block_reward,
                 rules.consensus_constants(0).coinbase_min_maturity(),
                 &factories,
-                42
+                42,
+                2
             ),
             Err(TransactionError::MoreThanOneCoinbaseKernel)
         ));
@@ -983,6 +988,7 @@ mod test {
                 rules.consensus_constants(0).coinbase_min_maturity(),
                 &factories,
                 42,
+                2,
             )
             .unwrap();
         body1.verify_kernel_signatures().unwrap_err();
@@ -1050,8 +1056,131 @@ mod test {
                 rules.consensus_constants(0).coinbase_min_maturity(),
                 &factories,
                 42,
+                2,
             )
             .unwrap();
         body2.verify_kernel_signatures().unwrap();
     }
+
+    #[tokio::test]
+    #[allow(clippy::too_many_lines)]
+    #[allow(clippy::identity_op)]
+    async fn too_may_coinbases() {
+        let (builder, rules, factories, key_manager) = get_builder();
+        let p = TestParams::new(&key_manager).await;
+        // We just want some small amount here.
+        let missing_fee = rules.emission_schedule().block_reward(4200000) + (2 * uT);
+        let wallet_payment_address = TariAddress::default();
+        let builder = builder
+            .with_block_height(42)
+            .with_fees(1 * uT)
+            .with_commitment_mask_id(p.commitment_mask_key_id.clone())
+            .with_encryption_key_id(TariKeyId::default())
+            .with_sender_offset_key_id(p.sender_offset_key_id.clone())
+            .with_script_key_id(p.script_key_id.clone())
+            .with_script(push_pubkey_script(wallet_payment_address.public_spend_key()))
+            .with_range_proof_type(RangeProofType::RevealedValue);
+        let (tx1, wo1) = builder
+            .build(
+                rules.consensus_constants(0),
+                rules.emission_schedule(),
+                PaymentId::Empty,
+            )
+            .await
+            .unwrap();
+
+        // we calculate a duplicate tx here so that we can have a coinbase with the correct fee amount
+        let block_reward = rules.emission_schedule().block_reward(42) + missing_fee;
+        let builder = CoinbaseBuilder::new(key_manager.clone());
+        let builder = builder
+            .with_block_height(4200000)
+            .with_fees(1 * uT)
+            .with_commitment_mask_id(p.commitment_mask_key_id.clone())
+            .with_encryption_key_id(TariKeyId::default())
+            .with_sender_offset_key_id(p.sender_offset_key_id)
+            .with_script_key_id(p.script_key_id)
+            .with_script(push_pubkey_script(wallet_payment_address.public_spend_key()))
+            .with_range_proof_type(RangeProofType::RevealedValue);
+        let (tx2, wo2) = builder
+            .build(
+                rules.consensus_constants(0),
+                rules.emission_schedule(),
+                PaymentId::Empty,
+            )
+            .await
+            .unwrap();
+
+        let coinbase1 = tx1.body.outputs()[0].clone();
+        let coinbase2 = tx2.body.outputs()[0].clone();
+
+        let kernel_1 = tx1.body.kernels()[0].clone();
+        let kernel_2 = tx2.body.kernels()[0].clone();
+        let excess = &kernel_1.excess + &kernel_2.excess;
+
+        // lets create a new kernel with a correct signature
+        let new_nonce1 = key_manager
+            .get_next_key(TransactionKeyManagerBranch::KernelNonce.get_branch_key())
+            .await
+            .unwrap();
+        let new_nonce2 = key_manager
+            .get_next_key(TransactionKeyManagerBranch::KernelNonce.get_branch_key())
+            .await
+            .unwrap();
+        let nonce = &new_nonce1.pub_key + &new_nonce2.pub_key;
+        let kernel_message = TransactionKernel::build_kernel_signature_message(
+            &TransactionKernelVersion::get_current_version(),
+            kernel_1.fee,
+            kernel_1.lock_height,
+            &kernel_1.features,
+            &None,
+        );
+
+        let mut kernel_signature = key_manager
+            .get_partial_txo_kernel_signature(
+                &wo1.spending_key_id,
+                &new_nonce1.key_id,
+                &nonce,
+                excess.as_public_key(),
+                &TransactionKernelVersion::get_current_version(),
+                &kernel_message,
+                &kernel_1.features,
+                TxoStage::Output,
+            )
+            .await
+            .unwrap();
+        kernel_signature = &kernel_signature +
+            &key_manager
+                .get_partial_txo_kernel_signature(
+                    &wo2.spending_key_id,
+                    &new_nonce2.key_id,
+                    &nonce,
+                    excess.as_public_key(),
+                    &TransactionKernelVersion::get_current_version(),
+                    &kernel_message,
+                    &kernel_1.features,
+                    TxoStage::Output,
+                )
+                .await
+                .unwrap();
+        let kernel = KernelBuilder::new()
+            .with_fee(0.into())
+            .with_features(kernel_1.features)
+            .with_lock_height(kernel_1.lock_height)
+            .with_excess(&excess)
+            .with_signature(kernel_signature)
+            .build()
+            .unwrap();
+
+        let mut body = AggregateBody::new(Vec::new(), vec![coinbase1, coinbase2], vec![kernel]);
+        body.sort();
+
+        body.check_coinbase_output(
+            block_reward,
+            rules.consensus_constants(0).coinbase_min_maturity(),
+            &factories,
+            42,
+            1,
+        )
+        .unwrap_err();
+    }
 }