Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enable multiple coinbase utxos #5879

Merged
merged 6 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 14 additions & 18 deletions base_layer/core/src/transactions/aggregated_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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() {
Expand All @@ -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;
SWvheerden marked this conversation as resolved.
Show resolved Hide resolved
}
}
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);
}
Expand Down
14 changes: 2 additions & 12 deletions base_layer/core/src/transactions/coinbase_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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)
SWvheerden marked this conversation as resolved.
Show resolved Hide resolved
));
// test catches that coinbase count on the kernel is wrong
assert!(matches!(
tx_kernel_test.body.check_coinbase_output(
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
16 changes: 1 addition & 15 deletions base_layer/core/src/validation/block_body/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
SWvheerden marked this conversation as resolved.
Show resolved Hide resolved
))
));

let (block, _) = blockchain
.create_unmined_block(block_spec!("A2", parent: "GB", skip_coinbase: true,))
Expand Down
Loading