Skip to content

Commit

Permalink
refactor(core)!: replace OutputFlags with OutputType (#4174)
Browse files Browse the repository at this point in the history
Description
---
- Replaces `OutputFlags` With`OutputType` enum
- Updates transactionBuilder for integration tests
- Updates test faucet json files (The reason this PR is so massive!)

Motivation and Context
---
Many new types of UTXOs are being added to the protocol. The OutputFlags type uses 2 bytes to represent up to 16 different types. These flags could not meaningfully be combined which is the primary use case of using bitflags.

This PR replaces the 2 byte bitflags with a 1-byte enum that represents the various output types available.

**New dependency**
`serde_repr` - allows repr enums to be de/serialized with serde as an integer (https://serde.rs/enum-number.html)

How Has This Been Tested?
---
Existing tests updated as necessary
  • Loading branch information
sdbondi authored Jun 13, 2022
1 parent 5e55bf2 commit d779f43
Show file tree
Hide file tree
Showing 48 changed files with 20,321 additions and 20,276 deletions.
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions applications/tari_app_grpc/proto/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,8 @@ message TransactionOutput {
message OutputFeatures {
// Version
uint32 version = 1;
// Flags are the feature flags that differentiate between outputs, eg Coinbase all of which has different rules
uint32 flags = 2;
// The type of output, eg Coinbase, all of which have different consensus rules
uint32 output_type = 2;
// The maturity of the specific UTXO. This is the min lock height at which an UTXO can be spend. Coinbase UTXO
// require a min maturity of the Coinbase_lock_height, this should be checked on receiving new blocks.
uint64 maturity = 3;
Expand Down
11 changes: 7 additions & 4 deletions applications/tari_app_grpc/src/conversions/output_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use tari_core::transactions::transaction_components::{
MintNonFungibleFeatures,
OutputFeatures,
OutputFeaturesVersion,
OutputFlags,
OutputType,
SideChainCheckpointFeatures,
SideChainFeatures,
TemplateParameter,
Expand Down Expand Up @@ -60,13 +60,16 @@ impl TryFrom<grpc::OutputFeatures> for OutputFeatures {
.map(SideChainFeatures::try_from)
.transpose()?;

let flags = u16::try_from(features.flags).map_err(|_| "Invalid output flags: overflowed u16")?;
let output_type = features
.output_type
.try_into()
.map_err(|_| "Invalid output type: overflow")?;

Ok(OutputFeatures::new(
OutputFeaturesVersion::try_from(
u8::try_from(features.version).map_err(|_| "Invalid version: overflowed u8")?,
)?,
OutputFlags::from_bits(flags).ok_or_else(|| "Invalid or unrecognised output flags".to_string())?,
OutputType::from_byte(output_type).ok_or_else(|| "Invalid or unrecognised output type".to_string())?,
features.maturity,
u8::try_from(features.recovery_byte).map_err(|_| "Invalid recovery byte: overflowed u8")?,
features.metadata,
Expand All @@ -85,7 +88,7 @@ impl From<OutputFeatures> for grpc::OutputFeatures {
fn from(features: OutputFeatures) -> Self {
Self {
version: features.version as u32,
flags: u32::from(features.flags.bits()),
output_type: u32::from(features.output_type.as_byte()),
maturity: features.maturity,
metadata: features.metadata,
unique_id: features.unique_id.unwrap_or_default(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,7 @@ fn write_utxos_to_csv_file(utxos: Vec<UnblindedOutput>, file_path: PathBuf) -> R
.commitment()
.map_err(|e| CommandError::WalletError(WalletError::TransactionError(e)))?
.to_hex(),
utxo.features.flags,
utxo.features.output_type,
utxo.features.maturity,
utxo.script.to_hex(),
utxo.input_data.to_hex(),
Expand Down
1 change: 1 addition & 0 deletions base_layer/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ rand = "0.8"
randomx-rs = { git = "https://github.com/tari-project/randomx-rs", tag = "v1.1.12", optional = true }
serde = { version = "1.0.106", features = ["derive"] }
serde_json = "1.0"
serde_repr = "0.1.8"
sha3 = "0.9"
strum_macros = "0.22"
thiserror = "1.0.26"
Expand Down
4 changes: 2 additions & 2 deletions base_layer/core/src/blocks/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use crate::{
tari_amount::MicroTari,
transaction_components::{
KernelFeatures,
OutputFlags,
OutputType,
Transaction,
TransactionError,
TransactionInput,
Expand Down Expand Up @@ -309,7 +309,7 @@ impl From<&Block> for NewBlock {
.body
.outputs()
.iter()
.find(|o| o.features.flags.contains(OutputFlags::COINBASE_OUTPUT))
.find(|o| o.features.output_type == OutputType::Coinbase)
.cloned()
.expect("Invalid block given to NewBlock::from, no coinbase output");

Expand Down
8,000 changes: 4,000 additions & 4,000 deletions base_layer/core/src/blocks/faucets/alphanet_faucet.json

Large diffs are not rendered by default.

8,000 changes: 4,000 additions & 4,000 deletions base_layer/core/src/blocks/faucets/dibbler_faucet.json

Large diffs are not rendered by default.

8,000 changes: 4,000 additions & 4,000 deletions base_layer/core/src/blocks/faucets/ridcully_faucet.json

Large diffs are not rendered by default.

8,000 changes: 4,000 additions & 4,000 deletions base_layer/core/src/blocks/faucets/stibbons_faucet.json

Large diffs are not rendered by default.

8,000 changes: 4,000 additions & 4,000 deletions base_layer/core/src/blocks/faucets/weatherwax_faucet.json

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions base_layer/core/src/blocks/genesis_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ use crate::{
KernelFeatures,
OutputFeatures,
OutputFeaturesVersion,
OutputFlags,
OutputType,
TransactionKernel,
TransactionKernelVersion,
TransactionOutput,
Expand Down Expand Up @@ -99,7 +99,7 @@ fn get_igor_genesis_block_raw() -> Block {
vec![],
vec![TransactionOutput::new_current_version(
OutputFeatures {
flags: OutputFlags::COINBASE_OUTPUT,
output_type: OutputType::Coinbase,
maturity: 60,
.. Default::default()
},
Expand Down Expand Up @@ -214,7 +214,7 @@ pub fn get_dibbler_genesis_block() -> ChainBlock {
// hardcode the Merkle roots once they've been computed above
block.header.kernel_mr = from_hex("5b91bebd33e18798e03e9c5d831d161ee9c3d12560f50b987e1a8c3ec53146df").unwrap();
block.header.witness_mr = from_hex("11227f6ce9ff34349d7dcab606b633f55234d5c8a73696a68c6e9ddc7cd3bc40").unwrap();
block.header.output_mr = from_hex("12c50da7232c05dc969b388b86eb6f093c8eb358322fee2486ff5ea84975edde").unwrap();
block.header.output_mr = from_hex("5e69274e72f8590e1cf91c189e24368527414aed966de62135d9273a6c14c3ef").unwrap();

let accumulated_data = BlockHeaderAccumulatedData {
hash: block.hash(),
Expand All @@ -238,7 +238,7 @@ fn get_dibbler_genesis_block_raw() -> Block {
TransactionOutputVersion::V0,
OutputFeatures {
version:OutputFeaturesVersion::V0,
flags:OutputFlags::COINBASE_OUTPUT,
output_type: OutputType::Coinbase,
maturity:60,
recovery_byte: 0,
metadata: Vec::new(),
Expand Down
11 changes: 5 additions & 6 deletions base_layer/core/src/chain_storage/tests/blockchain_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use crate::{
transactions::{
tari_amount::T,
test_helpers::{schema_to_transaction, TransactionSchema},
transaction_components::{OutputFeatures, OutputFlags, Transaction, UnblindedOutput},
transaction_components::{OutputFeatures, OutputType, Transaction, UnblindedOutput},
},
txn_schema,
};
Expand Down Expand Up @@ -471,7 +471,7 @@ mod add_block {
let (_, asset_pk) = PublicKey::random_keypair(&mut OsRng);
let unique_id = vec![1; 3];
let features = OutputFeatures {
flags: OutputFlags::MINT_NON_FUNGIBLE,
output_type: OutputType::MintNonFungible,
parent_public_key: Some(asset_pk.clone()),
unique_id: Some(unique_id),
..Default::default()
Expand All @@ -484,7 +484,7 @@ mod add_block {

let unique_id = vec![2; 3];
let features = OutputFeatures {
flags: OutputFlags::MINT_NON_FUNGIBLE,
output_type: OutputType::MintNonFungible,
parent_public_key: Some(asset_pk),
unique_id: Some(unique_id),
..Default::default()
Expand Down Expand Up @@ -713,7 +713,7 @@ mod fetch_utxo_by_unique_id {
use tari_crypto::{commitment::HomomorphicCommitmentFactory, ristretto::RistrettoPublicKey};

use super::*;
use crate::transactions::transaction_components::OutputFlags;
use crate::transactions::transaction_components::OutputType;

#[test]
fn it_returns_none_if_empty() {
Expand All @@ -733,7 +733,7 @@ mod fetch_utxo_by_unique_id {
let (blocks, outputs) = add_many_chained_blocks(1, &db);

let mut features = OutputFeatures {
flags: OutputFlags::MINT_NON_FUNGIBLE,
output_type: OutputType::MintNonFungible,
parent_public_key: Some(asset_pk.clone()),
unique_id: Some(unique_id.clone()),
..Default::default()
Expand Down Expand Up @@ -769,7 +769,6 @@ mod fetch_utxo_by_unique_id {
);

let mut features = OutputFeatures {
flags: OutputFlags::empty(),
parent_public_key: Some(asset_pk.clone()),
unique_id: Some(unique_id.clone()),
..Default::default()
Expand Down
14 changes: 7 additions & 7 deletions base_layer/core/src/covenants/fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl OutputField {
SenderOffsetPublicKey => &output.sender_offset_public_key as &dyn Any,
Covenant => &output.covenant as &dyn Any,
Features => &output.features as &dyn Any,
FeaturesFlags => &output.features.flags as &dyn Any,
FeaturesFlags => &output.features.output_type as &dyn Any,
FeaturesMaturity => &output.features.maturity as &dyn Any,
FeaturesUniqueId => &output.features.unique_id as &dyn Any,
FeaturesParentPublicKey => &output.features.parent_public_key as &dyn Any,
Expand Down Expand Up @@ -123,7 +123,7 @@ impl OutputField {
SenderOffsetPublicKey => output.sender_offset_public_key.to_consensus_bytes(),
Covenant => output.covenant.to_consensus_bytes(),
Features => output.features.to_consensus_bytes(),
FeaturesFlags => output.features.flags.to_consensus_bytes(),
FeaturesFlags => output.features.output_type.to_consensus_bytes(),
FeaturesMaturity => output.features.maturity.to_consensus_bytes(),
FeaturesUniqueId => output.features.unique_id.to_consensus_bytes(),
FeaturesParentPublicKey => output.features.parent_public_key.to_consensus_bytes(),
Expand Down Expand Up @@ -156,7 +156,7 @@ impl OutputField {
.unwrap_or(false),
FeaturesFlags => input
.features()
.map(|features| features.flags == output.features.flags)
.map(|features| features.output_type == output.features.output_type)
.unwrap_or(false),
FeaturesMaturity => input
.features()
Expand Down Expand Up @@ -387,7 +387,7 @@ mod test {
covenants::test::{create_input, create_outputs},
transactions::{
test_helpers::UtxoTestParams,
transaction_components::{OutputFeatures, OutputFlags, SpentOutput},
transaction_components::{OutputFeatures, OutputType, SpentOutput},
},
};

Expand Down Expand Up @@ -419,7 +419,7 @@ mod test {
.is_eq(&output, &output.features.maturity)
.unwrap());
assert!(OutputField::FeaturesFlags
.is_eq(&output, &output.features.flags)
.is_eq(&output, &output.features.output_type)
.unwrap());
assert!(OutputField::FeaturesSideChainFeatures
.is_eq(&output, output.features.sidechain_features.as_ref().unwrap())
Expand Down Expand Up @@ -459,7 +459,7 @@ mod test {
.unwrap());
assert!(!OutputField::FeaturesMaturity.is_eq(&output, &123u64).unwrap());
assert!(!OutputField::FeaturesFlags
.is_eq(&output, &OutputFlags::COINBASE_OUTPUT)
.is_eq(&output, &OutputType::Coinbase)
.unwrap());
assert!(!OutputField::FeaturesSideChainFeatures
.is_eq(&output, &SideChainFeatures::new(FixedHash::hash_bytes(b"B")))
Expand Down Expand Up @@ -553,7 +553,7 @@ mod test {
fn it_constructs_challenge_using_consensus_encoding() {
let features = OutputFeatures {
maturity: 42,
flags: OutputFlags::COINBASE_OUTPUT,
output_type: OutputType::Coinbase,
..Default::default()
};
let output = create_outputs(1, UtxoTestParams {
Expand Down
10 changes: 5 additions & 5 deletions base_layer/core/src/covenants/filters/fields_preserved.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ mod test {
use crate::{
covenant,
covenants::{filters::test::setup_filter_test, test::create_input},
transactions::transaction_components::{OutputFlags, SideChainFeatures},
transactions::transaction_components::{OutputType, SideChainFeatures},
};

#[test]
Expand All @@ -52,17 +52,17 @@ mod test {
let mut input = create_input();
input.set_maturity(42).unwrap();
input.features_mut().unwrap().sidechain_features = Some(SideChainFeatures::new(hash));
input.features_mut().unwrap().flags = OutputFlags::SIDECHAIN_CHECKPOINT;
input.features_mut().unwrap().output_type = OutputType::ContractDefinition;
let (mut context, outputs) = setup_filter_test(&covenant, &input, 0, |outputs| {
outputs[5].features.maturity = 42;
outputs[5].features.sidechain_features = Some(SideChainFeatures::new(hash));
outputs[5].features.flags = OutputFlags::SIDECHAIN_CHECKPOINT;
outputs[5].features.output_type = OutputType::ContractDefinition;
outputs[7].features.maturity = 42;
outputs[7].features.flags = OutputFlags::SIDECHAIN_CHECKPOINT;
outputs[7].features.output_type = OutputType::ContractDefinition;
outputs[7].features.sidechain_features = Some(SideChainFeatures::new(FixedHash::hash_bytes("B")));
outputs[8].features.maturity = 42;
outputs[8].features.sidechain_features = Some(SideChainFeatures::new(hash));
outputs[8].features.flags = OutputFlags::SIDECHAIN_CHECKPOINT | OutputFlags::COINBASE_OUTPUT;
outputs[8].features.output_type = OutputType::Coinbase;
});
let mut output_set = OutputSet::new(&outputs);

Expand Down
11 changes: 7 additions & 4 deletions base_layer/core/src/proto/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ use crate::{
MintNonFungibleFeatures,
OutputFeatures,
OutputFeaturesVersion,
OutputFlags,
OutputType,
PublicFunction,
RequirementsForConstitutionChange,
SideChainCheckpointFeatures,
Expand Down Expand Up @@ -305,13 +305,16 @@ impl TryFrom<proto::types::OutputFeatures> for OutputFeatures {
.map(SideChainFeatures::try_from)
.transpose()?;

let flags = u16::try_from(features.flags).map_err(|_| "Invalid output flags: overflowed u16")?;
let flags = features
.flags
.try_into()
.map_err(|_| "Invalid output type: overflowed")?;

Ok(OutputFeatures::new(
OutputFeaturesVersion::try_from(
u8::try_from(features.version).map_err(|_| "Invalid version: overflowed u8")?,
)?,
OutputFlags::from_bits(flags).ok_or_else(|| "Invalid or unrecognised output flags".to_string())?,
OutputType::from_byte(flags).ok_or_else(|| "Invalid or unrecognised output type".to_string())?,
features.maturity,
u8::try_from(features.recovery_byte).map_err(|_| "Invalid recovery byte: overflowed u8")?,
features.metadata,
Expand All @@ -329,7 +332,7 @@ impl TryFrom<proto::types::OutputFeatures> for OutputFeatures {
impl From<OutputFeatures> for proto::types::OutputFeatures {
fn from(features: OutputFeatures) -> Self {
Self {
flags: u32::from(features.flags.bits()),
flags: u32::from(features.output_type.as_byte()),
maturity: features.maturity,
metadata: features.metadata,
unique_id: features.unique_id.unwrap_or_default(),
Expand Down
4 changes: 2 additions & 2 deletions base_layer/core/src/transactions/aggregated_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ use crate::{
transaction_components::{
KernelFeatures,
KernelSum,
OutputFlags,
OutputType,
Transaction,
TransactionError,
TransactionInput,
Expand Down Expand Up @@ -284,7 +284,7 @@ impl AggregateBody {
let mut coinbase_kernel = None;
let mut coinbase_counter = 0; // there should be exactly 1 coinbase
for utxo in self.outputs() {
if utxo.features.flags.contains(OutputFlags::COINBASE_OUTPUT) {
if utxo.features.output_type == OutputType::Coinbase {
coinbase_counter += 1;
if utxo.features.maturity < (height + coinbase_lock_height) {
warn!(target: LOG_TARGET, "Coinbase {} found with maturity set too low", utxo);
Expand Down
4 changes: 2 additions & 2 deletions base_layer/core/src/transactions/coinbase_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ mod test {
crypto_factories::CryptoFactories,
tari_amount::uT,
test_helpers::TestParams,
transaction_components::{KernelFeatures, OutputFeatures, OutputFlags, TransactionError},
transaction_components::{KernelFeatures, OutputFeatures, OutputType, TransactionError},
transaction_protocol::RewindData,
CoinbaseBuilder,
},
Expand Down Expand Up @@ -348,7 +348,7 @@ mod test {
.commitment
.open_value(&p.spend_key, block_reward.into(), utxo.commitment()));
utxo.verify_range_proof(&factories.range_proof).unwrap();
assert!(utxo.features.flags.contains(OutputFlags::COINBASE_OUTPUT));
assert_eq!(utxo.features.output_type, OutputType::Coinbase);
tx.body
.check_coinbase_output(
block_reward,
Expand Down
4 changes: 2 additions & 2 deletions base_layer/core/src/transactions/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use crate::{
KernelBuilder,
KernelFeatures,
OutputFeatures,
OutputFlags,
OutputType,
Transaction,
TransactionInput,
TransactionKernel,
Expand Down Expand Up @@ -545,7 +545,7 @@ pub fn create_unblinded_txos(
amount_for_last_output
};
let test_params = TestParams::new();
let script_offset_pvt_key = if output_features.flags.contains(OutputFlags::COINBASE_OUTPUT) {
let script_offset_pvt_key = if output_features.output_type == OutputType::Coinbase {
PrivateKey::default()
} else {
test_params.sender_offset_private_key.clone()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub use kernel_sum::KernelSum;
pub use mint_non_fungible_features::MintNonFungibleFeatures;
pub use output_features::OutputFeatures;
pub use output_features_version::OutputFeaturesVersion;
pub use output_flags::OutputFlags;
pub use output_type::OutputType;
pub use rewind_result::RewindResult;
pub use side_chain::{ContractConstitution, *};
pub use side_chain_checkpoint_features::SideChainCheckpointFeatures;
Expand Down Expand Up @@ -63,7 +63,7 @@ mod kernel_sum;
mod mint_non_fungible_features;
mod output_features;
mod output_features_version;
mod output_flags;
mod output_type;
mod rewind_result;
mod side_chain;
mod side_chain_checkpoint_features;
Expand Down
Loading

0 comments on commit d779f43

Please sign in to comment.