diff --git a/base_layer/core/src/transactions/aggregated_body.rs b/base_layer/core/src/transactions/aggregated_body.rs index 3472bb3da1..d92cd44542 100644 --- a/base_layer/core/src/transactions/aggregated_body.rs +++ b/base_layer/core/src/transactions/aggregated_body.rs @@ -27,7 +27,7 @@ use std::{ use borsh::{BorshDeserialize, BorshSerialize}; use log::*; use serde::{Deserialize, Serialize}; -use tari_common_types::types::PrivateKey; +use tari_common_types::types::{Commitment, PrivateKey}; use tari_crypto::commitment::HomomorphicCommitmentFactory; use crate::transactions::{ @@ -240,7 +240,7 @@ impl AggregateBody { factories: &CryptoFactories, height: u64, ) -> Result<(), TransactionError> { - let mut coinbase_utxo = None; + let mut coinbase_utxo_sum = Commitment::default(); let mut coinbase_kernel = None; let mut coinbase_counter = 0; // there should be exactly 1 coinbase for utxo in self.outputs() { @@ -250,45 +250,41 @@ impl AggregateBody { warn!(target: LOG_TARGET, "Coinbase {} found with maturity set too low", utxo); return Err(TransactionError::InvalidCoinbaseMaturity); } - coinbase_utxo = Some(utxo); + coinbase_utxo_sum = &coinbase_utxo_sum + &utxo.commitment; } } - if coinbase_counter > 1 { - warn!( - target: LOG_TARGET, - "{} coinbases found in body. Only a single coinbase is permitted.", coinbase_counter, - ); - return Err(TransactionError::MoreThanOneCoinbase); - } - if coinbase_utxo.is_none() || coinbase_counter == 0 { + if coinbase_counter == 0 { return Err(TransactionError::NoCoinbase); } - let coinbase_utxo = coinbase_utxo.expect("coinbase_utxo: none checked"); + debug!( + target: LOG_TARGET, + "{} coinbases found in body.", coinbase_counter, + ); - let mut coinbase_counter = 0; // there should be exactly 1 coinbase kernel as well + let mut coinbase_kernel_counter = 0; // there should be exactly 1 coinbase kernel as well for kernel in self.kernels() { if kernel.features.contains(KernelFeatures::COINBASE_KERNEL) { - coinbase_counter += 1; + coinbase_kernel_counter += 1; coinbase_kernel = Some(kernel); } } - if coinbase_kernel.is_none() || coinbase_counter != 1 { + 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, ); - return Err(TransactionError::MoreThanOneCoinbase); + return Err(TransactionError::MoreThanOneCoinbaseKernel); } let coinbase_kernel = coinbase_kernel.expect("coinbase_kernel: none checked"); let rhs = &coinbase_kernel.excess + &factories.commitment.commit_value(&PrivateKey::default(), reward.0); - if rhs != coinbase_utxo.commitment { + if rhs != coinbase_utxo_sum { warn!( target: LOG_TARGET, - "Coinbase {} amount validation failed", coinbase_utxo + "Coinbase amount validation failed" ); return Err(TransactionError::InvalidCoinbase); } diff --git a/base_layer/core/src/transactions/coinbase_builder.rs b/base_layer/core/src/transactions/coinbase_builder.rs index 9ac6b9f19b..41430a1cad 100644 --- a/base_layer/core/src/transactions/coinbase_builder.rs +++ b/base_layer/core/src/transactions/coinbase_builder.rs @@ -616,7 +616,7 @@ mod test { tx.offset = tx.offset + offset; tx.body.sort(); - // lets add duplciate coinbase kernel + // lets add duplicate coinbase kernel let mut coinbase2 = tx2.body.outputs()[0].clone(); coinbase2.features = OutputFeatures::default(); let coinbase_kernel2 = tx2.body.kernels()[0].clone(); @@ -625,16 +625,6 @@ mod test { tx_kernel_test.body.sort(); - // test catches that coinbase count on the utxo is wrong - assert!(matches!( - tx.body.check_coinbase_output( - block_reward, - rules.consensus_constants(0).coinbase_min_maturity(), - &factories, - 42 - ), - Err(TransactionError::MoreThanOneCoinbase) - )); // test catches that coinbase count on the kernel is wrong assert!(matches!( tx_kernel_test.body.check_coinbase_output( @@ -643,7 +633,7 @@ mod test { &factories, 42 ), - Err(TransactionError::MoreThanOneCoinbase) + Err(TransactionError::MoreThanOneCoinbaseKernel) )); // testing that "block" is still valid let body_validator = AggregateBodyInternalConsistencyValidator::new(false, rules, factories); diff --git a/base_layer/core/src/transactions/transaction_components/error.rs b/base_layer/core/src/transactions/transaction_components/error.rs index e1cbf3c14d..f7bf4d72a4 100644 --- a/base_layer/core/src/transactions/transaction_components/error.rs +++ b/base_layer/core/src/transactions/transaction_components/error.rs @@ -51,8 +51,8 @@ pub enum TransactionError { InvalidCoinbase, #[error("Invalid coinbase maturity in body")] InvalidCoinbaseMaturity, - #[error("More than one coinbase in body")] - MoreThanOneCoinbase, + #[error("More than one coinbase kernel in body")] + MoreThanOneCoinbaseKernel, #[error("No coinbase in body")] NoCoinbase, #[error("Input maturity not reached")] diff --git a/base_layer/core/src/validation/block_body/test.rs b/base_layer/core/src/validation/block_body/test.rs index fb0375c7ab..d2feef8c70 100644 --- a/base_layer/core/src/validation/block_body/test.rs +++ b/base_layer/core/src/validation/block_body/test.rs @@ -191,7 +191,7 @@ async fn it_checks_the_coinbase_reward() { } #[tokio::test] -async fn it_checks_exactly_one_coinbase() { +async fn it_allows_multiple_coinbases() { let (blockchain, validator) = setup(true); let (mut block, coinbase) = blockchain.create_unmined_block(block_spec!("A1", parent: "GB")).await; @@ -212,20 +212,6 @@ async fn it_checks_exactly_one_coinbase() { .body .add_output(coinbase_output.to_transaction_output(&blockchain.km).await.unwrap()); block.body.sort(); - let block = blockchain.mine_block("GB", block, Difficulty::min()); - - let err = { - // `MutexGuard` cannot be held across an `await` point - let txn = blockchain.db().db_read_access().unwrap(); - let err = validator.validate_body(&*txn, block.block()).unwrap_err(); - err - }; - assert!(matches!( - err, - ValidationError::BlockError(BlockValidationError::TransactionError( - TransactionError::MoreThanOneCoinbase - )) - )); let (block, _) = blockchain .create_unmined_block(block_spec!("A2", parent: "GB", skip_coinbase: true,))