Skip to content

Commit

Permalink
feat(core)!: restrict output types to only those permitted by consens…
Browse files Browse the repository at this point in the history
…us (#4467)

Description
---
Adds a new transaction and block validation that enforces allow-listed `OutputType`s

Motivation and Context
---
As more `OutputType`s are added, they should only be permitted by consensus from a known height to allow for gradual network upgrades.

How Has This Been Tested?
---
New unit test for orphan validator.
All output types are permitted for localnet/cucumber tests
  • Loading branch information
sdbondi authored Aug 18, 2022
1 parent 9fc2dd2 commit a481a06
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 13 deletions.
30 changes: 27 additions & 3 deletions base_layer/core/src/consensus/consensus_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use crate::{
transaction_components::{
OutputFeatures,
OutputFeaturesVersion,
OutputType,
TransactionInputVersion,
TransactionKernelVersion,
TransactionOutputVersion,
Expand Down Expand Up @@ -85,11 +86,13 @@ pub struct ConsensusConstants {
/// Maximum byte size of TariScript
max_script_byte_size: usize,
/// Range of valid transaction input versions
pub(crate) input_version_range: RangeInclusive<TransactionInputVersion>,
input_version_range: RangeInclusive<TransactionInputVersion>,
/// Range of valid transaction output (and features) versions
pub(crate) output_version_range: OutputVersionRange,
output_version_range: OutputVersionRange,
/// Range of valid transaction kernel versions
pub(crate) kernel_version_range: RangeInclusive<TransactionKernelVersion>,
kernel_version_range: RangeInclusive<TransactionKernelVersion>,
/// An allowlist of output types
permitted_output_types: &'static [OutputType],
}

// todo: remove this once OutputFeaturesVersion is removed in favor of just TransactionOutputVersion
Expand Down Expand Up @@ -277,6 +280,11 @@ impl ConsensusConstants {
&self.kernel_version_range
}

/// Returns the permitted OutputTypes
pub fn permitted_output_types(&self) -> &[OutputType] {
self.permitted_output_types
}

pub fn localnet() -> Vec<Self> {
let difficulty_block_window = 90;
let mut algos = HashMap::new();
Expand Down Expand Up @@ -313,6 +321,7 @@ impl ConsensusConstants {
input_version_range,
output_version_range,
kernel_version_range,
permitted_output_types: OutputType::all(),
}]
}

Expand Down Expand Up @@ -352,6 +361,7 @@ impl ConsensusConstants {
input_version_range,
output_version_range,
kernel_version_range,
permitted_output_types: Self::current_permitted_output_types(),
}]
}

Expand Down Expand Up @@ -394,6 +404,7 @@ impl ConsensusConstants {
input_version_range,
output_version_range,
kernel_version_range,
permitted_output_types: Self::current_permitted_output_types(),
}]
}

Expand Down Expand Up @@ -443,6 +454,7 @@ impl ConsensusConstants {
input_version_range: input_version_range.clone(),
output_version_range: output_version_range.clone(),
kernel_version_range: kernel_version_range.clone(),
permitted_output_types: Self::current_permitted_output_types(),
},
ConsensusConstants {
effective_from_height: 23000,
Expand All @@ -465,6 +477,7 @@ impl ConsensusConstants {
input_version_range,
output_version_range,
kernel_version_range,
permitted_output_types: Self::current_permitted_output_types(),
},
]
}
Expand Down Expand Up @@ -512,6 +525,7 @@ impl ConsensusConstants {
input_version_range,
output_version_range,
kernel_version_range,
permitted_output_types: Self::current_permitted_output_types(),
}]
}

Expand Down Expand Up @@ -552,8 +566,13 @@ impl ConsensusConstants {
input_version_range,
output_version_range,
kernel_version_range,
permitted_output_types: Self::current_permitted_output_types(),
}]
}

const fn current_permitted_output_types() -> &'static [OutputType] {
&[OutputType::Coinbase, OutputType::Standard, OutputType::Burn]
}
}

static EMISSION_DECAY: [u64; 5] = [22, 23, 24, 26, 27];
Expand Down Expand Up @@ -628,6 +647,11 @@ impl ConsensusConstantsBuilder {
self
}

pub fn with_permitted_output_types(mut self, permitted_output_types: &'static [OutputType]) -> Self {
self.consensus.permitted_output_types = permitted_output_types;
self
}

pub fn build(self) -> ConsensusConstants {
self.consensus
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ impl OutputType {
pub fn from_byte(value: u8) -> Option<Self> {
FromPrimitive::from_u8(value)
}

pub const fn all() -> &'static [Self] {
&[OutputType::Standard, OutputType::Coinbase, OutputType::Burn]
}
}

impl Default for OutputType {
Expand Down Expand Up @@ -107,5 +111,7 @@ mod tests {
fn it_converts_from_byte_to_output_type() {
assert_eq!(OutputType::from_byte(0), Some(OutputType::Standard));
assert_eq!(OutputType::from_byte(1), Some(OutputType::Coinbase));
assert_eq!(OutputType::from_byte(2), Some(OutputType::Burn));
assert_eq!(OutputType::from_byte(255), None);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ impl<B: BlockchainBackend + 'static> BlockValidator<B> {
.into()
}

#[allow(clippy::too_many_lines)]
fn start_output_validation(
&self,
header: &BlockHeader,
Expand All @@ -372,11 +373,12 @@ impl<B: BlockchainBackend + 'static> BlockValidator<B> {
.map(|outputs| {
let range_proof_prover = self.factories.range_proof.clone();
let db = self.db.inner().clone();
let max_script_size = self.rules.consensus_constants(height).get_max_script_byte_size();
let constants = self.rules.consensus_constants(height).clone();
task::spawn_blocking(move || {
let db = db.db_read_access()?;
let mut aggregate_sender_offset = PublicKey::default();
let mut commitment_sum = Commitment::default();
let max_script_size = constants.get_max_script_byte_size();
let mut coinbase_index = None;
debug!(
target: LOG_TARGET,
Expand All @@ -400,6 +402,7 @@ impl<B: BlockchainBackend + 'static> BlockValidator<B> {
aggregate_sender_offset = aggregate_sender_offset + &output.sender_offset_public_key;
}

helpers::check_permitted_output_types(&constants, output)?;
helpers::check_tari_script_byte_size(&output.script, max_script_size)?;
output.verify_metadata_signature()?;
helpers::check_not_duplicate_txo(&*db, output)?;
Expand Down
8 changes: 7 additions & 1 deletion base_layer/core/src/validation/block_validators/orphan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use crate::{
check_coinbase_output,
check_kernel_lock_height,
check_maturity,
check_permitted_output_types,
check_sorting_and_duplicates,
check_total_burned,
},
Expand Down Expand Up @@ -86,7 +87,8 @@ impl OrphanValidation for OrphanBlockValidator {
};
trace!(target: LOG_TARGET, "Validating {}", block_id);

check_block_weight(block, self.rules.consensus_constants(height))?;
let constants = self.rules.consensus_constants(height);
check_block_weight(block, constants)?;
trace!(target: LOG_TARGET, "SV - Block weight is ok for {} ", &block_id);

trace!(
Expand All @@ -100,6 +102,10 @@ impl OrphanValidation for OrphanBlockValidator {
"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);

Expand Down
27 changes: 26 additions & 1 deletion base_layer/core/src/validation/block_validators/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ mod body_only {

mod orphan_validator {
use super::*;
use crate::txn_schema;
use crate::{transactions::transaction_components::OutputType, txn_schema};

#[test]
fn it_rejects_zero_conf_double_spends() {
Expand Down Expand Up @@ -330,4 +330,29 @@ mod orphan_validator {
let err = validator.validate(&unmined).unwrap_err();
assert!(matches!(err, ValidationError::UnsortedOrDuplicateInput));
}

#[test]
fn it_rejects_unpermitted_output_types() {
let rules = ConsensusManager::builder(Network::LocalNet)
.add_consensus_constants(
ConsensusConstantsBuilder::new(Network::LocalNet)
.with_permitted_output_types(&[OutputType::Coinbase])
.with_coinbase_lockheight(0)
.build(),
)
.build();
let mut blockchain = TestBlockchain::create(rules.clone());
let validator = OrphanBlockValidator::new(rules, false, CryptoFactories::default());
let (_, coinbase) = blockchain.append(block_spec!("1", parent: "GB")).unwrap();

let schema = txn_schema!(from: vec![coinbase], to: vec![201 * T]);
let (tx, _) = schema_to_transaction(&[schema]);

let transactions = tx.into_iter().map(|b| Arc::try_unwrap(b).unwrap()).collect::<Vec<_>>();

let (unmined, _) = blockchain.create_unmined_block(block_spec!("2", parent: "1", transactions: transactions));
let err = validator.validate(&unmined).unwrap_err();
unpack_enum!(ValidationError::OutputTypeNotPermitted { output_type } = err);
assert_eq!(output_type, OutputType::Standard);
}
}
2 changes: 2 additions & 0 deletions base_layer/core/src/validation/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ pub enum ValidationError {
},
#[error("Contains Invalid Burn: {0}")]
InvalidBurnError(String),
#[error("Output type '{output_type}' is not permitted")]
OutputTypeNotPermitted { output_type: OutputType },
}

// ChainStorageError has a ValidationError variant, so to prevent a cyclic dependency we use a string representation in
Expand Down
22 changes: 20 additions & 2 deletions base_layer/core/src/validation/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,15 +460,17 @@ pub fn check_input_is_utxo<B: BlockchainBackend>(db: &B, input: &TransactionInpu
}

/// This function checks:
/// 1. the byte size of TariScript does not exceed the maximum
/// 2. that the outputs do not already exist in the UTxO set.
/// 1. that the output type is permitted
/// 2. the byte size of TariScript does not exceed the maximum
/// 3. that the outputs do not already exist in the UTxO set.
pub fn check_outputs<B: BlockchainBackend>(
db: &B,
constants: &ConsensusConstants,
body: &AggregateBody,
) -> Result<(), ValidationError> {
let max_script_size = constants.get_max_script_byte_size();
for output in body.outputs() {
check_permitted_output_types(constants, output)?;
check_tari_script_byte_size(&output.script, max_script_size)?;
check_not_duplicate_txo(db, output)?;
}
Expand Down Expand Up @@ -741,6 +743,22 @@ pub fn check_blockchain_version(constants: &ConsensusConstants, version: u16) ->
}
}

pub fn check_permitted_output_types(
constants: &ConsensusConstants,
output: &TransactionOutput,
) -> Result<(), ValidationError> {
if !constants
.permitted_output_types()
.contains(&output.features.output_type)
{
return Err(ValidationError::OutputTypeNotPermitted {
output_type: output.features.output_type,
});
}

Ok(())
}

#[cfg(test)]
mod test {
use tari_test_utils::unpack_enum;
Expand Down
12 changes: 7 additions & 5 deletions base_layer/core/src/validation/transaction_validators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use crate::{
consensus::ConsensusConstants,
transactions::{transaction_components::Transaction, CryptoFactories},
validation::{
helpers::{check_inputs_are_utxos, check_outputs, check_total_burned},
helpers::{check_inputs_are_utxos, check_outputs, check_permitted_output_types, check_total_burned},
MempoolTransactionValidation,
ValidationError,
},
Expand Down Expand Up @@ -105,7 +105,7 @@ impl<B: BlockchainBackend> TxConsensusValidator<B> {
) -> Result<(), ValidationError> {
// validate input version
for input in tx.body().inputs() {
if !consensus_constants.input_version_range.contains(&input.version) {
if !consensus_constants.input_version_range().contains(&input.version) {
let msg = format!(
"Transaction input contains a version not allowed by consensus ({:?})",
input.version
Expand All @@ -117,12 +117,12 @@ impl<B: BlockchainBackend> TxConsensusValidator<B> {
// validate output version and output features version
for output in tx.body().outputs() {
let valid_output_version = consensus_constants
.output_version_range
.output_version_range()
.outputs
.contains(&output.version);

let valid_features_version = consensus_constants
.output_version_range
.output_version_range()
.features
.contains(&output.features.version);

Expand All @@ -141,11 +141,13 @@ impl<B: BlockchainBackend> TxConsensusValidator<B> {
);
return Err(ValidationError::ConsensusError(msg));
}

check_permitted_output_types(consensus_constants, output)?;
}

// validate kernel version
for kernel in tx.body().kernels() {
if !consensus_constants.kernel_version_range.contains(&kernel.version) {
if !consensus_constants.kernel_version_range().contains(&kernel.version) {
let msg = format!(
"Transaction kernel version is not allowed by consensus ({:?})",
kernel.version
Expand Down

0 comments on commit a481a06

Please sign in to comment.