Skip to content

Commit

Permalink
fix(core): disable covenants for all networks except igor and localnet (
Browse files Browse the repository at this point in the history
#5505)

Description
---
- Adds check for covenant token length in outputs
- Disables (max len == 0) covenants on all networks except igor and
localnet
- Rearranged some of the static and cheap checks before more expansive
checks e.g script and covenant execution

Motivation and Context
---
Allows limiting covenant length in consensus constants. We disable them
for now in all non-test networks.

We check covenant length in outputs but not inputs, as this may render
previous UTXOs unspendable if the max length was set lower at later
heights (unlikely). This feels like a debatable choice, as always, open
to comments.

How Has This Been Tested?
---
Manually, synced base node

What process can a PR reviewer use to test or verify this change?
---

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->

Co-authored-by: SW van Heerden <swvheerden@gmail.com>
  • Loading branch information
sdbondi and SWvheerden committed Jun 26, 2023
1 parent af305f0 commit 308f529
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 51 deletions.
13 changes: 13 additions & 0 deletions base_layer/core/src/consensus/consensus_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ pub struct ConsensusConstants {
permitted_range_proof_types: &'static [RangeProofType],
/// Coinbase outputs are allowed to have metadata, but it has the following length limit
coinbase_output_features_extra_max_length: u32,
/// Maximum number of token elements permitted in covenants
max_covenant_length: u32,
/// Epoch duration in blocks
vn_epoch_length: u64,
/// The number of Epochs that a validator node registration is valid
Expand Down Expand Up @@ -307,6 +309,11 @@ impl ConsensusConstants {
self.permitted_range_proof_types
}

/// The maximum permitted token length of all covenants. A value of 0 is equivalent to disabling covenants.
pub fn max_covenant_length(&self) -> u32 {
self.max_covenant_length
}

pub fn validator_node_validity_period_epochs(&self) -> VnEpoch {
self.vn_validity_period_epochs
}
Expand Down Expand Up @@ -373,6 +380,7 @@ impl ConsensusConstants {
kernel_version_range,
permitted_output_types: OutputType::all(),
permitted_range_proof_types: RangeProofType::all(),
max_covenant_length: 100,
vn_epoch_length: 10,
vn_validity_period_epochs: VnEpoch(100),
vn_registration_min_deposit_amount: MicroTari(0),
Expand Down Expand Up @@ -435,6 +443,7 @@ impl ConsensusConstants {
// igor is the first network to support the new output types
permitted_output_types: OutputType::all(),
permitted_range_proof_types: RangeProofType::all(),
max_covenant_length: 100,
vn_epoch_length: 10,
vn_validity_period_epochs: VnEpoch(3),
vn_registration_min_deposit_amount: MicroTari(0),
Expand Down Expand Up @@ -494,6 +503,7 @@ impl ConsensusConstants {
kernel_version_range,
permitted_output_types: Self::current_permitted_output_types(),
permitted_range_proof_types: Self::current_permitted_range_proof_types(),
max_covenant_length: 0,
vn_epoch_length: 60,
vn_validity_period_epochs: VnEpoch(100),
vn_registration_min_deposit_amount: MicroTari(0),
Expand Down Expand Up @@ -547,6 +557,7 @@ impl ConsensusConstants {
kernel_version_range,
permitted_output_types: Self::current_permitted_output_types(),
permitted_range_proof_types: Self::current_permitted_range_proof_types(),
max_covenant_length: 0,
vn_epoch_length: 60,
vn_validity_period_epochs: VnEpoch(100),
vn_registration_min_deposit_amount: MicroTari(0),
Expand Down Expand Up @@ -594,6 +605,7 @@ impl ConsensusConstants {
kernel_version_range,
permitted_output_types: Self::current_permitted_output_types(),
permitted_range_proof_types: Self::current_permitted_range_proof_types(),
max_covenant_length: 0,
vn_epoch_length: 60,
vn_validity_period_epochs: VnEpoch(100),
vn_registration_min_deposit_amount: MicroTari(0),
Expand Down Expand Up @@ -642,6 +654,7 @@ impl ConsensusConstants {
kernel_version_range,
permitted_output_types: Self::current_permitted_output_types(),
permitted_range_proof_types: Self::current_permitted_range_proof_types(),
max_covenant_length: 0,
vn_epoch_length: 60,
vn_validity_period_epochs: VnEpoch(100),
vn_registration_min_deposit_amount: MicroTari(0),
Expand Down
8 changes: 8 additions & 0 deletions base_layer/core/src/covenants/covenant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,14 @@ impl Covenant {
pub(super) fn tokens(&self) -> &[CovenantToken] {
&self.tokens
}

pub fn num_tokens(&self) -> usize {
self.tokens.len()
}

pub fn is_empty(&self) -> bool {
self.tokens.is_empty()
}
}

impl FromIterator<CovenantToken> for Covenant {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ use crate::{
},
validation::{
helpers::{
check_covenant_length,
check_permitted_output_types,
check_permitted_range_proof_types,
check_tari_script_byte_size,
Expand Down Expand Up @@ -105,15 +106,25 @@ impl AggregateBodyInternalConsistencyValidator {
// old internal validator
verify_kernel_signatures(body)?;

check_script_size(
body,
self.consensus_manager
.consensus_constants(height)
.get_max_script_byte_size(),
)?;
let constants = self.consensus_manager.consensus_constants(height);

validate_versions(body, constants)?;

for output in body.outputs() {
check_permitted_output_types(constants, output)?;
check_script_size(output, constants.get_max_script_byte_size())?;
check_covenant_length(&output.covenant, constants.max_covenant_length())?;
check_permitted_range_proof_types(constants, output)?;
check_validator_node_registration_utxo(constants, output)?;
}

check_weight(body, height, constants)?;
check_sorting_and_duplicates(body)?;

// Check that the inputs are are allowed to be spent
check_maturity(height, body.inputs())?;
check_kernel_lock_height(height, body.kernels())?;

let total_offset = self.factories.commitment.commit_value(tx_offset, total_reward.0);
validate_kernel_sum(body, total_offset, &self.factories.commitment)?;

Expand All @@ -126,24 +137,8 @@ impl AggregateBodyInternalConsistencyValidator {
validate_script_and_script_offset(body, script_offset_g, &self.factories.commitment, prev_header, height)?;
validate_covenants(body, height)?;

// orphan candidate block validator
let constants = self.consensus_manager.consensus_constants(height);

validate_versions(body, constants)?;
check_weight(body, height, constants)?;
// check_sorting_and_duplicates(body)?;
for output in body.outputs() {
check_permitted_output_types(constants, output)?;
check_permitted_range_proof_types(constants, output)?;
check_validator_node_registration_utxo(constants, output)?;
}
check_total_burned(body)?;

// Check that the inputs are are allowed to be spent
check_maturity(height, body.inputs())?;
check_kernel_lock_height(height, body.kernels())?;
// check_output_features(body, constants)?;

Ok(())
}
}
Expand All @@ -162,17 +157,14 @@ fn verify_kernel_signatures(body: &AggregateBody) -> Result<(), ValidationError>
}

/// Verify that the TariScript is not larger than the max size
fn check_script_size(body: &AggregateBody, max_script_size: usize) -> Result<(), ValidationError> {
for output in body.outputs() {
check_tari_script_byte_size(output.script(), max_script_size).map_err(|e| {
warn!(
target: LOG_TARGET,
"output ({}) script size exceeded max size {:?}.", output, e
);
e
})?;
}
Ok(())
fn check_script_size(output: &TransactionOutput, max_script_size: usize) -> Result<(), ValidationError> {
check_tari_script_byte_size(output.script(), max_script_size).map_err(|e| {
warn!(
target: LOG_TARGET,
"output ({}) script size exceeded max size {:?}.", output, e
);
e
})
}

/// This function checks for duplicate inputs and outputs. There should be no duplicate inputs or outputs in a
Expand Down Expand Up @@ -342,24 +334,15 @@ fn check_kernel_lock_height(height: u64, kernels: &[TransactionKernel]) -> Resul

/// Checks that all inputs have matured at the given height
fn check_maturity(height: u64, inputs: &[TransactionInput]) -> Result<(), TransactionError> {
inputs
.iter()
.map(|input| match input.is_mature_at(height) {
Ok(mature) => {
if mature {
Ok(0)
} else {
warn!(
target: LOG_TARGET,
"Input found that has not yet matured to spending height: {}", input
);
Err(TransactionError::InputMaturity)
}
},
Err(e) => Err(e),
})
.sum::<Result<usize, TransactionError>>()?;

for input in inputs {
if !input.is_mature_at(height)? {
warn!(
target: LOG_TARGET,
"Input found that has not yet matured to spending height: {}", input
);
return Err(TransactionError::InputMaturity);
}
}
Ok(())
}

Expand Down
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 @@ -153,6 +153,8 @@ pub enum ValidationError {
NotEnoughTimestamps { expected: usize, actual: usize },
#[error("Invalid difficulty: {0}")]
DifficultyError(#[from] DifficultyError),
#[error("Covenant too large. Max size: {max_size}, Actual size: {actual_size}")]
CovenantTooLarge { max_size: usize, actual_size: usize },
}

// ChainStorageError has a ValidationError variant, so to prevent a cyclic dependency we use a string representation in
Expand Down
12 changes: 12 additions & 0 deletions base_layer/core/src/validation/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use crate::{
borsh::SerializedSize,
chain_storage::{BlockchainBackend, MmrRoots, MmrTree},
consensus::ConsensusConstants,
covenants::Covenant,
proof_of_work::{
monero_difficulty,
randomx_factory::RandomXFactory,
Expand Down Expand Up @@ -332,6 +333,17 @@ pub fn check_permitted_output_types(
Ok(())
}

pub fn check_covenant_length(covenant: &Covenant, max_token_len: u32) -> Result<(), ValidationError> {
if covenant.num_tokens() > max_token_len as usize {
return Err(ValidationError::CovenantTooLarge {
max_size: max_token_len as usize,
actual_size: covenant.num_tokens(),
});
}

Ok(())
}

pub fn check_permitted_range_proof_types(
constants: &ConsensusConstants,
output: &TransactionOutput,
Expand Down

0 comments on commit 308f529

Please sign in to comment.