diff --git a/Cargo.lock b/Cargo.lock index 495da77e38..06bfd5757e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3011,6 +3011,7 @@ dependencies = [ name = "mc-consensus-enclave-impl" version = "5.1.1" dependencies = [ + "assert_matches", "cargo-emit", "hex", "mbedtls", @@ -3042,6 +3043,7 @@ dependencies = [ "rand_core", "rand_hc", "subtle", + "yare 2.0.0", ] [[package]] @@ -3186,6 +3188,7 @@ dependencies = [ name = "mc-consensus-service" version = "5.1.1" dependencies = [ + "assert_matches", "base64 0.21.5", "chrono", "clap 4.4.8", @@ -3245,6 +3248,7 @@ dependencies = [ "serde_json", "serial_test", "tempfile", + "yare 2.0.0", ] [[package]] diff --git a/blockchain/types/src/block.rs b/blockchain/types/src/block.rs index 02e276ccf4..59eb391847 100644 --- a/blockchain/types/src/block.rs +++ b/blockchain/types/src/block.rs @@ -147,6 +147,8 @@ impl Block { block_contents: &BlockContents, timestamp: u64, ) -> Self { + assert!((timestamp == 0) ^ version.timestamps_are_supported()); + let contents_hash = block_contents.hash(); let id = compute_block_id( *version, diff --git a/consensus/api/proto/consensus_common.proto b/consensus/api/proto/consensus_common.proto index 20f2b6c887..17b7af9edc 100644 --- a/consensus/api/proto/consensus_common.proto +++ b/consensus/api/proto/consensus_common.proto @@ -99,6 +99,7 @@ enum ProposeTxResult { InputRuleAmount = 53; LedgerTxOutIndexOutOfBounds = 54; FeeMapDigestMismatch = 55; + Timestamp = 56; } /// Response from TxPropose RPC call. diff --git a/consensus/api/src/conversions.rs b/consensus/api/src/conversions.rs index a11392adab..4a06854166 100644 --- a/consensus/api/src/conversions.rs +++ b/consensus/api/src/conversions.rs @@ -63,6 +63,7 @@ impl From for ProposeTxResult { Error::InputRulesNotAllowed => Self::InputRulesNotAllowed, Error::InputRule(ir) => ir.into(), Error::UnknownMaskedAmountVersion => Self::UnknownMaskedAmountVersion, + Error::Timestamp(_) => Self::Timestamp, } } } diff --git a/consensus/enclave/api/src/lib.rs b/consensus/enclave/api/src/lib.rs index f6945bfbce..13b5810c2b 100644 --- a/consensus/enclave/api/src/lib.rs +++ b/consensus/enclave/api/src/lib.rs @@ -222,6 +222,11 @@ pub struct FormBlockInputs { /// Minting transactions coupled with configuration information. pub mint_txs_with_config: Vec<(MintTx, MintConfigTx, MintConfig)>, + + /// The timestamp to use for the block. ms since Unix epoch. + /// If timestamps are not supported by the block version this + /// value will be ignored and 0 will be used instead. + pub timestamp: u64, } /// The API for interacting with a consensus node's enclave. diff --git a/consensus/enclave/impl/Cargo.toml b/consensus/enclave/impl/Cargo.toml index 7f845ccb7d..0730124bc8 100644 --- a/consensus/enclave/impl/Cargo.toml +++ b/consensus/enclave/impl/Cargo.toml @@ -53,6 +53,7 @@ rand_core = { version = "0.6", default-features = false } subtle = { version = "2.4", default-features = false, features = ["i128"] } [dev-dependencies] +assert_matches = "1" mc-common = { path = "../../../common", features = ["loggers"] } mc-crypto-multisig = { path = "../../../crypto/multisig" } mc-ledger-db = { path = "../../../ledger/db", features = ["test_utils"] } @@ -60,6 +61,7 @@ mc-transaction-core-test-utils = { path = "../../../transaction/core/test-utils" rand = "0.8" rand_hc = "0.3" +yare = "2" [build-dependencies] mc-crypto-keys = { path = "../../../crypto/keys" } diff --git a/consensus/enclave/impl/src/lib.rs b/consensus/enclave/impl/src/lib.rs index 8b99377065..f9d5ca0da9 100644 --- a/consensus/enclave/impl/src/lib.rs +++ b/consensus/enclave/impl/src/lib.rs @@ -322,6 +322,21 @@ impl SgxConsensusEnclave { Ok(transactions) } + /// Validate the provided timestamp is valid for use in building a block. + /// + /// # Arguments + /// * `timestamp` - The timestamp to validate + /// * `parent_block` - The parent block of the block being built. + fn validate_timestamp(timestamp: u64, parent_block: &Block) -> Result { + if timestamp <= parent_block.timestamp { + return Err(Error::FormBlock(format!( + "Timestamp ({timestamp}) must be greater than parent block timestamp ({})", + parent_block.timestamp + ))); + } + Ok(timestamp) + } + /// Validate a list of MintConfigTxs. /// /// # Arguments @@ -938,13 +953,23 @@ impl ConsensusEnclave for SgxConsensusEnclave { mint_txs, }; + let timestamp = if config.block_version.timestamps_are_supported() { + Self::validate_timestamp(inputs.timestamp, parent_block)? + } else { + // We are ignoring `inputs.timestamp` here as a block version that + // doesn't support timestamps should have 0. However the callers + // into this enclave may not know what block version this enclave + // is using and so may pass in a non-zero timestamp. + 0 + }; + // Form the block. let block = Block::new_with_parent( config.block_version, parent_block, root_element, &block_contents, - 0, + timestamp, ); // Sign the block. @@ -1035,6 +1060,8 @@ fn mint_output( mod tests { use super::*; use alloc::vec; + use assert_matches::assert_matches; + use mc_blockchain_types::{compute_block_id, BlockContentsHash, BlockID}; use mc_common::{logger::test_with_logger, HashMap, HashSet}; use mc_consensus_enclave_api::{GovernorsMap, GovernorsSigner}; use mc_crypto_keys::{Ed25519Private, Ed25519Signature, Signer}; @@ -1057,6 +1084,7 @@ mod tests { use mc_util_from_random::FromRandom; use rand_core::SeedableRng; use rand_hc::Hc128Rng; + use yare::parameterized; // The private keys here are only used by tests. They do not need to be // specified for main net. The public keys associated with this private keys @@ -1428,11 +1456,18 @@ mod tests { let parent_block = ledger.get_block(ledger.num_blocks().unwrap() - 1).unwrap(); let root_element = ledger.get_root_tx_out_membership_element().unwrap(); + let timestamp = if block_version.timestamps_are_supported() { + parent_block.timestamp + 1 + } else { + 0 + }; + let (block, block_contents, signature) = enclave .form_block( &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp, ..Default::default() }, &root_element, @@ -1605,11 +1640,18 @@ mod tests { let parent_block = ledger.get_block(ledger.num_blocks().unwrap() - 1).unwrap(); let root_element = ledger.get_root_tx_out_membership_element().unwrap(); + let timestamp = if block_version.timestamps_are_supported() { + parent_block.timestamp + 1 + } else { + 0 + }; + let (block, block_contents, signature) = enclave .form_block( &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp, ..Default::default() }, &root_element, @@ -1781,10 +1823,17 @@ mod tests { let parent_block = ledger.get_block(ledger.num_blocks().unwrap() - 1).unwrap(); let root_element = ledger.get_root_tx_out_membership_element().unwrap(); + let timestamp = if block_version.timestamps_are_supported() { + parent_block.timestamp + 1 + } else { + 0 + }; + let form_block_result = enclave.form_block( &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp, ..Default::default() }, &root_element, @@ -1902,6 +1951,7 @@ mod tests { &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2013,6 +2063,7 @@ mod tests { &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2103,6 +2154,7 @@ mod tests { &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2192,6 +2244,7 @@ mod tests { &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2281,6 +2334,12 @@ mod tests { let root_element = ledger.get_root_tx_out_membership_element().unwrap(); + let timestamp = if block_version.timestamps_are_supported() { + parent_block.timestamp + 1 + } else { + 0 + }; + let (block, block_contents, signature) = enclave .form_block( &parent_block, @@ -2297,6 +2356,7 @@ mod tests { mint_config_tx2.prefix.configs[0].clone(), ), ], + timestamp, ..Default::default() }, &root_element, @@ -2461,6 +2521,7 @@ mod tests { mint_config_tx.clone(), mint_config_tx.prefix.configs[0].clone(), )], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2632,6 +2693,7 @@ mod tests { valid_mint_config_tx.clone(), valid_mint_config_tx.prefix.configs[0].clone(), )], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2652,6 +2714,7 @@ mod tests { invalid_mint_config_tx.clone(), invalid_mint_config_tx.prefix.configs[0].clone(), )], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2733,6 +2796,7 @@ mod tests { mint_config_tx1.prefix.configs[0].clone(), ), ], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2818,6 +2882,7 @@ mod tests { mint_config_tx1.clone(), mint_config_tx1.prefix.configs[0].clone(), )], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2882,11 +2947,18 @@ mod tests { let root_element = ledger.get_root_tx_out_membership_element().unwrap(); + let timestamp = if block_version.timestamps_are_supported() { + parent_block.timestamp + 1 + } else { + 0 + }; + let (block, block_contents, signature) = enclave .form_block( &parent_block, FormBlockInputs { mint_config_txs: vec![mint_config_tx1.clone(), mint_config_tx2.clone()], + timestamp, ..Default::default() }, &root_element, @@ -2975,10 +3047,16 @@ mod tests { let root_element = ledger.get_root_tx_out_membership_element().unwrap(); + let timestamp = if block_version.timestamps_are_supported() { + parent_block.timestamp + 1 + } else { + 0 + }; let form_block_result = enclave.form_block( &parent_block, FormBlockInputs { mint_config_txs: vec![mint_config_tx1.clone()], + timestamp, ..Default::default() }, &root_element, @@ -3041,10 +3119,17 @@ mod tests { let root_element = ledger.get_root_tx_out_membership_element().unwrap(); + let timestamp = if block_version.timestamps_are_supported() { + parent_block.timestamp + 1 + } else { + 0 + }; + let form_block_result = enclave.form_block( &parent_block, FormBlockInputs { mint_config_txs: vec![mint_config_tx1.clone()], + timestamp, ..Default::default() }, &root_element, @@ -3102,10 +3187,17 @@ mod tests { let root_element = ledger.get_root_tx_out_membership_element().unwrap(); + let timestamp = if block_version.timestamps_are_supported() { + parent_block.timestamp + 1 + } else { + 0 + }; + let form_block_result = enclave.form_block( &parent_block, FormBlockInputs { mint_config_txs: vec![mint_config_tx1.clone(), mint_config_tx1.clone()], + timestamp, ..Default::default() }, &root_element, @@ -3224,6 +3316,12 @@ mod tests { let parent_block = ledger.get_block(ledger.num_blocks().unwrap() - 1).unwrap(); let root_element = ledger.get_root_tx_out_membership_element().unwrap(); + let timestamp = if block_version.timestamps_are_supported() { + parent_block.timestamp + 1 + } else { + 0 + }; + let (block, block_contents, signature) = enclave .form_block( &parent_block, @@ -3241,6 +3339,7 @@ mod tests { mint_config_tx2.prefix.configs[0].clone(), ), ], + timestamp, ..Default::default() }, &root_element, @@ -3373,6 +3472,7 @@ mod tests { &parent_block, FormBlockInputs { mint_config_txs: vec![mint_config_tx1.clone(), mint_config_tx2.clone()], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -3564,4 +3664,172 @@ mod tests { ) .expect("Mint txs should be valid"); } + + #[parameterized( + same_as_parent = {23456, 23456}, + earlier_than_parent = {1233, 1234}, + zero = {0, 0}, + )] + fn timestamp_is_not_valid(timestamp: u64, parent_timestamp: u64) { + let version = 4; + let parent_id = BlockID::try_from(&[1u8; 32][..]).unwrap(); + let index = 1; + let cumulative_txo_count = 1; + let root_element = TxOutMembershipElement::default(); + let contents_hash = BlockContentsHash::default(); + let id = compute_block_id( + version, + &parent_id, + index, + cumulative_txo_count, + &root_element, + &contents_hash, + parent_timestamp, + ); + let parent_block = Block { + id, + version, + parent_id, + index, + cumulative_txo_count, + root_element, + contents_hash, + timestamp: parent_timestamp, + }; + assert_matches!( + SgxConsensusEnclave::validate_timestamp(timestamp, &parent_block), + Err(Error::FormBlock(_)) + ); + } + + #[parameterized( + one_ms_later_than_parent = {9877, 9876}, + much_later_than_parent = {8765, 4567}, + one_more_than_zero = {1, 0}, + )] + fn timestamp_is_valid(timestamp: u64, parent_timestamp: u64) { + let version = 4; + let parent_id = BlockID::try_from(&[1u8; 32][..]).unwrap(); + let index = 1; + let cumulative_txo_count = 1; + let root_element = TxOutMembershipElement::default(); + let contents_hash = BlockContentsHash::default(); + let id = compute_block_id( + version, + &parent_id, + index, + cumulative_txo_count, + &root_element, + &contents_hash, + parent_timestamp, + ); + let parent_block = Block { + id, + version, + parent_id, + index, + cumulative_txo_count, + root_element, + contents_hash, + timestamp: parent_timestamp, + }; + assert_eq!( + SgxConsensusEnclave::validate_timestamp(timestamp, &parent_block), + Ok(timestamp) + ); + } + + #[test_with_logger] + fn test_form_block_fails_for_invalid_timestamp(logger: Logger) { + let mut rng = Hc128Rng::from_seed([77u8; 32]); + + for block_version in BlockVersion::iterator() { + let enclave = SgxConsensusEnclave::new(logger.clone()); + let blockchain_config = BlockchainConfig { + block_version, + ..Default::default() + }; + enclave + .enclave_init( + &Default::default(), + &Default::default(), + &None, + blockchain_config, + ) + .unwrap(); + + // Create a valid test transaction. + let sender = AccountKey::random(&mut rng); + let recipient = AccountKey::random(&mut rng); + + let mut ledger = create_ledger(); + let n_blocks = 1; + initialize_ledger(block_version, &mut ledger, n_blocks, &sender, &mut rng); + + // Spend outputs from the origin block. + let origin_block_contents = ledger.get_block_contents(0).unwrap(); + + let input_transactions: Vec = (0..3) + .map(|i| { + let tx_out = origin_block_contents.outputs[i].clone(); + + create_transaction( + block_version, + &ledger, + &tx_out, + &sender, + &recipient.default_subaddress(), + n_blocks + 1, + &mut rng, + ) + }) + .collect(); + + // Create WellFormedEncryptedTxs + proofs + let well_formed_encrypted_txs_with_proofs: Vec<_> = input_transactions + .iter() + .map(|tx| { + let well_formed_tx = WellFormedTx::from(tx.clone()); + let encrypted_tx = enclave + .encrypt_well_formed_tx(&well_formed_tx, &mut rng) + .unwrap(); + + let highest_indices = well_formed_tx.tx.get_membership_proof_highest_indices(); + let membership_proofs = ledger + .get_tx_out_proof_of_memberships(&highest_indices) + .expect("failed getting proof"); + (encrypted_tx, membership_proofs) + }) + .collect(); + + let parent_block = ledger.get_block(ledger.num_blocks().unwrap() - 1).unwrap(); + let root_element = ledger.get_root_tx_out_membership_element().unwrap(); + + // For block versions that support timestamps the timestamp should + // be newer than the parent block. For ones that don't support + // timestamps, it should be 0, and the parent timestamp should be 0. + // Thus using the parent timestamp will fail for timestamp supported + // versions and succeed for those not supporting timestamps + let timestamp = parent_block.timestamp; + let result = enclave.form_block( + &parent_block, + FormBlockInputs { + well_formed_encrypted_txs_with_proofs, + timestamp, + ..Default::default() + }, + &root_element, + ); + + match block_version { + BlockVersion::ZERO + | BlockVersion::ONE + | BlockVersion::TWO + | BlockVersion::THREE => { + assert!(result.is_ok()); + } + _ => assert_matches!(result, Err(Error::FormBlock(e)) if e.contains("timestamp")), + } + } + } } diff --git a/consensus/enclave/mock/src/lib.rs b/consensus/enclave/mock/src/lib.rs index 44c59da9d9..10b8adb2e7 100644 --- a/consensus/enclave/mock/src/lib.rs +++ b/consensus/enclave/mock/src/lib.rs @@ -351,7 +351,7 @@ impl ConsensusEnclave for ConsensusServiceMockEnclave { }; let timestamp = if block_version.timestamps_are_supported() { - parent_block.timestamp + 1 + inputs.timestamp } else { 0 }; diff --git a/consensus/service/Cargo.toml b/consensus/service/Cargo.toml index e888eeff83..bded054c29 100644 --- a/consensus/service/Cargo.toml +++ b/consensus/service/Cargo.toml @@ -77,10 +77,12 @@ mc-transaction-core-test-utils = { path = "../../transaction/core/test-utils" } mc-util-from-random = { path = "../../util/from-random" } mc-util-logger-macros = { path = "../../util/logger-macros" } +assert_matches = "1" mockall = "0.11.4" rand_core = { version = "0.6", default-features = false } rand_hc = "0.3" serial_test = "2.0" tempfile = "3.8" +yare = "2" curve25519-dalek = { version = "4.1.1", default-features = false } diff --git a/consensus/service/src/api/client_api_service.rs b/consensus/service/src/api/client_api_service.rs index caff391783..787603e04d 100644 --- a/consensus/service/src/api/client_api_service.rs +++ b/consensus/service/src/api/client_api_service.rs @@ -161,7 +161,7 @@ impl ClientApiService { // Validate the transaction. // This is done here as a courtesy to give clients immediate feedback about the // transaction. - self.tx_manager.validate(&tx_hash)?; + self.tx_manager.validate(&tx_hash, None)?; // The transaction can be considered by the network. (*self.propose_tx_callback)(ConsensusValue::TxHash(tx_hash), None, None); @@ -189,7 +189,7 @@ impl ClientApiService { // This is done here as a courtesy to give clients immediate feedback about the // transaction. self.mint_tx_manager - .validate_mint_config_tx(&mint_config_tx)?; + .validate_mint_config_tx(&mint_config_tx, None)?; // The transaction can be considered by the network. (*self.propose_tx_callback)(ConsensusValue::MintConfigTx(mint_config_tx), None, None); @@ -214,7 +214,7 @@ impl ClientApiService { // Validate the transaction. // This is done here as a courtesy to give clients immediate feedback about the // transaction. - self.mint_tx_manager.validate_mint_tx(&mint_tx)?; + self.mint_tx_manager.validate_mint_tx(&mint_tx, None)?; // The transaction can be considered by the network. (*self.propose_tx_callback)(ConsensusValue::MintTx(mint_tx), None, None); diff --git a/consensus/service/src/api/grpc_error.rs b/consensus/service/src/api/grpc_error.rs index b99c9d34f3..7a8d929a16 100644 --- a/consensus/service/src/api/grpc_error.rs +++ b/consensus/service/src/api/grpc_error.rs @@ -1,6 +1,9 @@ // Copyright (c) 2018-2022 The MobileCoin Foundation -use crate::{mint_tx_manager::MintTxManagerError, tx_manager::TxManagerError}; +use crate::{ + mint_tx_manager::MintTxManagerError, timestamp_validator::Error as TimestampError, + tx_manager::TxManagerError, +}; use displaydoc::Display; use grpcio::{RpcStatus, RpcStatusCode}; use mc_common::logger::global_log; @@ -36,6 +39,9 @@ pub enum ConsensusGrpcError { /// Mint transaction validation error `{0}` MintValidation(MintValidationError), + /// Timestamp validation error `{0}` + Timestamp(TimestampError), + /// Invalid argument `{0}` InvalidArgument(String), @@ -95,10 +101,17 @@ impl From for ConsensusGrpcError { match src { MintTxManagerError::MintValidation(err) => Self::from(err), MintTxManagerError::LedgerDb(err) => Self::from(err), + MintTxManagerError::Timestamp(err) => Self::from(err), } } } +impl From for ConsensusGrpcError { + fn from(src: TimestampError) -> Self { + Self::Timestamp(src) + } +} + impl From for ConsensusGrpcError { fn from(src: ConfigError) -> Self { Self::Config(src) diff --git a/consensus/service/src/byzantine_ledger/mod.rs b/consensus/service/src/byzantine_ledger/mod.rs index cf1aeaf5a8..28e06f8442 100644 --- a/consensus/service/src/byzantine_ledger/mod.rs +++ b/consensus/service/src/byzantine_ledger/mod.rs @@ -27,7 +27,8 @@ use mc_crypto_keys::Ed25519Pair; use mc_ledger_db::Ledger; use mc_ledger_sync::{LedgerSyncService, ReqwestTransactionsFetcher}; use mc_peers::{ - Broadcast, ConsensusConnection, ConsensusMsg, ConsensusValue, VerifiedConsensusMsg, + Broadcast, ConsensusConnection, ConsensusMsg, ConsensusValue, ConsensusValueWithTimestamp, + VerifiedConsensusMsg, }; use mc_transaction_core::mint::constants::{MAX_MINT_CONFIG_TXS_PER_BLOCK, MAX_MINT_TXS_PER_BLOCK}; use mc_util_metered_channel::Sender; @@ -129,7 +130,7 @@ impl ByzantineLedger { logger: Logger, ) -> Self { // TODO: this should be passed in as an argument. - let scp_node: Box> = { + let scp_node: Box> = { let tx_manager_validate = tx_manager.clone(); let tx_manager_combine = tx_manager.clone(); let mint_tx_manager_validate = mint_tx_manager.clone(); @@ -139,19 +140,21 @@ impl ByzantineLedger { node_id.clone(), quorum_set.clone(), // Validation callback - Arc::new(move |scp_value| match scp_value { - ConsensusValue::TxHash(tx_hash) => tx_manager_validate - .validate(tx_hash) - .map_err(UnifiedNodeError::from), - - ConsensusValue::MintConfigTx(mint_config_tx) => mint_tx_manager_validate - .validate_mint_config_tx(mint_config_tx) - .map_err(UnifiedNodeError::from), - - ConsensusValue::MintTx(mint_tx) => mint_tx_manager_validate - .validate_mint_tx(mint_tx) - .map_err(UnifiedNodeError::from), - }), + Arc::new( + move |scp_value: &ConsensusValueWithTimestamp| match &scp_value.value { + ConsensusValue::TxHash(tx_hash) => tx_manager_validate + .validate(tx_hash, Some(scp_value.timestamp)) + .map_err(UnifiedNodeError::from), + + ConsensusValue::MintConfigTx(mint_config_tx) => mint_tx_manager_validate + .validate_mint_config_tx(mint_config_tx, Some(scp_value.timestamp)) + .map_err(UnifiedNodeError::from), + + ConsensusValue::MintTx(mint_tx) => mint_tx_manager_validate + .validate_mint_tx(mint_tx, Some(scp_value.timestamp)) + .map_err(UnifiedNodeError::from), + }, + ), // Combine callback Arc::new(move |scp_values| { let mut tx_hashes = Vec::new(); @@ -159,30 +162,37 @@ impl ByzantineLedger { let mut mint_txs = Vec::new(); for value in scp_values { - match value { - ConsensusValue::TxHash(tx_hash) => tx_hashes.push(*tx_hash), + match &value.value { + ConsensusValue::TxHash(tx_hash) => { + tx_hashes.push((*tx_hash, value.timestamp)) + } ConsensusValue::MintConfigTx(mint_config_tx) => { - mint_config_txs.push(mint_config_tx.clone()); + mint_config_txs.push((mint_config_tx.clone(), value.timestamp)); } ConsensusValue::MintTx(mint_tx) => { - mint_txs.push(mint_tx.clone()); + mint_txs.push((mint_tx.clone(), value.timestamp)); } } } let tx_hashes = tx_manager_combine.combine(&tx_hashes[..])?; - let tx_hashes_iter = tx_hashes.into_iter().map(ConsensusValue::TxHash); + let tx_hashes_iter = tx_hashes.into_iter().map(|(tx, timestamp)| { + ConsensusValueWithTimestamp::new(tx.into(), timestamp) + }); let mint_config_txs = mint_tx_manager_combine.combine_mint_config_txs( &mint_config_txs[..], MAX_MINT_CONFIG_TXS_PER_BLOCK, )?; - let mint_config_txs_iter = mint_config_txs - .into_iter() - .map(ConsensusValue::MintConfigTx); + let mint_config_txs_iter = + mint_config_txs.into_iter().map(|(tx, timestamp)| { + ConsensusValueWithTimestamp::new(tx.into(), timestamp) + }); let mint_txs = mint_tx_manager_combine .combine_mint_txs(&mint_txs[..], MAX_MINT_TXS_PER_BLOCK)?; - let mint_txs_iter = mint_txs.into_iter().map(ConsensusValue::MintTx); + let mint_txs_iter = mint_txs.into_iter().map(|(tx, timestamp)| { + ConsensusValueWithTimestamp::new(tx.into(), timestamp) + }); Ok(tx_hashes_iter .chain(mint_config_txs_iter) @@ -354,7 +364,7 @@ mod tests { use std::{ collections::BTreeSet, sync::{Arc, Mutex}, - time::Instant, + time::{Duration, Instant, SystemTime}, }; // Run these tests with a particular block version @@ -655,6 +665,11 @@ mod tests { .unwrap() .into(); + let timestamp_before = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .expect("Failed getting the system time") + .as_millis() as u64; + byzantine_ledger.push_values( vec![ hash_tx_zero.clone(), @@ -666,6 +681,45 @@ mod tests { let slot_index = num_blocks as SlotIndex; + let deadline = Instant::now() + Duration::from_secs(60); + while Instant::now() < deadline { + { + if !mock_peer_state + .lock() + .expect("Could not lock mock peer state") + .msgs + .is_empty() + { + break; + } + } + + thread::sleep(Duration::from_millis(100_u64)); + } + + let timestamp_after = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .expect("Failed getting the system time") + .as_millis() as u64; + + let timestamp = { + let msgs = &mock_peer_state + .lock() + .expect("Could not lock mock peer state") + .msgs; + let front = msgs.front().unwrap(); + if let Topic::Nominate(ref payload) = front.scp_msg.topic { + payload.X.iter().next().unwrap().timestamp + } else { + panic!("Expected Nominate msg") + } + }; + + assert!( + timestamp_before <= timestamp && timestamp <= timestamp_after, + "Timestamp in message doesn't reflect the time when the message was received" + ); + // After some time, this node should nominate its client values. let expected_msg = ConsensusMsg::from_scp_msg( &ledger, @@ -675,9 +729,18 @@ mod tests { slot_index, Topic::Nominate(NominatePayload { X: BTreeSet::from_iter(vec![ - hash_tx_zero.clone(), - hash_tx_one.clone(), - hash_tx_two.clone(), + ConsensusValueWithTimestamp { + value: hash_tx_zero.clone(), + timestamp, + }, + ConsensusValueWithTimestamp { + value: hash_tx_one.clone(), + timestamp, + }, + ConsensusValueWithTimestamp { + value: hash_tx_two.clone(), + timestamp, + }, ]), Y: BTreeSet::default(), }), @@ -686,22 +749,6 @@ mod tests { ) .unwrap(); - let deadline = Instant::now() + Duration::from_secs(60); - while Instant::now() < deadline { - { - if mock_peer_state - .lock() - .expect("Could not lock mock peer state") - .msgs - .contains(&expected_msg) - { - break; - } - } - - thread::sleep(Duration::from_millis(100_u64)); - } - { let msgs = &mock_peer_state .lock() @@ -725,9 +772,18 @@ mod tests { B: Ballot::new( 100, &[ - hash_tx_zero.clone(), - hash_tx_one.clone(), - hash_tx_two.clone(), + ConsensusValueWithTimestamp { + value: hash_tx_zero.clone(), + timestamp, + }, + ConsensusValueWithTimestamp { + value: hash_tx_one.clone(), + timestamp, + }, + ConsensusValueWithTimestamp { + value: hash_tx_two.clone(), + timestamp, + }, ], ), PN: 77, @@ -754,9 +810,18 @@ mod tests { B: Ballot::new( 100, &[ - hash_tx_zero.clone(), - hash_tx_one.clone(), - hash_tx_two.clone(), + ConsensusValueWithTimestamp { + value: hash_tx_zero.clone(), + timestamp, + }, + ConsensusValueWithTimestamp { + value: hash_tx_one.clone(), + timestamp, + }, + ConsensusValueWithTimestamp { + value: hash_tx_two.clone(), + timestamp, + }, ], ), PN: 77, @@ -820,7 +885,23 @@ mod tests { local_quorum_set.clone(), slot_index, Topic::Externalize(ExternalizePayload { - C: Ballot::new(55, &[hash_tx_zero, hash_tx_one, hash_tx_two,]), + C: Ballot::new( + 55, + &[ + ConsensusValueWithTimestamp { + value: hash_tx_zero, + timestamp, + }, + ConsensusValueWithTimestamp { + value: hash_tx_one, + timestamp, + }, + ConsensusValueWithTimestamp { + value: hash_tx_two, + timestamp, + }, + ] + ), HN: 66, }), ), @@ -993,6 +1074,11 @@ mod tests { &mut rng, ); + let timestamp_before = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .expect("Failed getting the system time") + .as_millis() as u64; + byzantine_ledger.push_values( vec![ ConsensusValue::MintTx(tx1.clone()), @@ -1005,6 +1091,45 @@ mod tests { let num_blocks = ledger.num_blocks().unwrap(); let slot_index = num_blocks as SlotIndex; + let deadline = Instant::now() + Duration::from_secs(60); + while Instant::now() < deadline { + { + if !mock_peer_state + .lock() + .expect("Could not lock mock peer state") + .msgs + .is_empty() + { + break; + } + } + + thread::sleep(Duration::from_millis(100_u64)); + } + + let timestamp_after = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .expect("Failed getting the system time") + .as_millis() as u64; + + let timestamp = { + let msgs = &mock_peer_state + .lock() + .expect("Could not lock mock peer state") + .msgs; + let front = msgs.front().unwrap(); + if let Topic::Nominate(ref payload) = front.scp_msg.topic { + payload.X.iter().next().unwrap().timestamp + } else { + panic!("Expected Nominate msg") + } + }; + + assert!( + timestamp_before <= timestamp && timestamp <= timestamp_after, + "Timestamp in message doesn't reflect the time when the message was received" + ); + // After some time, this node should nominate its client values. let expected_msg = ConsensusMsg::from_scp_msg( &ledger, @@ -1014,9 +1139,18 @@ mod tests { slot_index, Topic::Nominate(NominatePayload { X: BTreeSet::from_iter(vec![ - ConsensusValue::MintTx(tx1.clone()), - ConsensusValue::MintTx(tx2.clone()), - ConsensusValue::MintTx(tx3.clone()), + ConsensusValueWithTimestamp { + value: ConsensusValue::MintTx(tx1.clone()), + timestamp, + }, + ConsensusValueWithTimestamp { + value: ConsensusValue::MintTx(tx2.clone()), + timestamp, + }, + ConsensusValueWithTimestamp { + value: ConsensusValue::MintTx(tx3.clone()), + timestamp, + }, ]), Y: BTreeSet::default(), }), @@ -1025,22 +1159,6 @@ mod tests { ) .unwrap(); - let deadline = Instant::now() + Duration::from_secs(60); - while Instant::now() < deadline { - { - if mock_peer_state - .lock() - .expect("Could not lock mock peer state") - .msgs - .contains(&expected_msg) - { - break; - } - } - - thread::sleep(Duration::from_millis(100_u64)); - } - { let msgs = &mock_peer_state .lock() @@ -1064,9 +1182,18 @@ mod tests { B: Ballot::new( 100, &[ - ConsensusValue::MintTx(tx1.clone()), - ConsensusValue::MintTx(tx2.clone()), - ConsensusValue::MintTx(tx3.clone()), + ConsensusValueWithTimestamp { + value: ConsensusValue::MintTx(tx1.clone()), + timestamp, + }, + ConsensusValueWithTimestamp { + value: ConsensusValue::MintTx(tx2.clone()), + timestamp, + }, + ConsensusValueWithTimestamp { + value: ConsensusValue::MintTx(tx3.clone()), + timestamp, + }, ], ), PN: 77, @@ -1093,9 +1220,18 @@ mod tests { B: Ballot::new( 100, &[ - ConsensusValue::MintTx(tx1.clone()), - ConsensusValue::MintTx(tx2.clone()), - ConsensusValue::MintTx(tx3.clone()), + ConsensusValueWithTimestamp { + value: ConsensusValue::MintTx(tx1.clone()), + timestamp, + }, + ConsensusValueWithTimestamp { + value: ConsensusValue::MintTx(tx2.clone()), + timestamp, + }, + ConsensusValueWithTimestamp { + value: ConsensusValue::MintTx(tx3.clone()), + timestamp, + }, ], ), PN: 77, @@ -1141,9 +1277,18 @@ mod tests { C: Ballot::new( 55, &[ - ConsensusValue::MintTx(tx1), - ConsensusValue::MintTx(tx2), - ConsensusValue::MintTx(tx3), + ConsensusValueWithTimestamp { + value: ConsensusValue::MintTx(tx1), + timestamp, + }, + ConsensusValueWithTimestamp { + value: ConsensusValue::MintTx(tx2), + timestamp, + }, + ConsensusValueWithTimestamp { + value: ConsensusValue::MintTx(tx3), + timestamp, + }, ] ), HN: 66, diff --git a/consensus/service/src/byzantine_ledger/pending_values.rs b/consensus/service/src/byzantine_ledger/pending_values.rs index 45e0aee5eb..f1a6573445 100644 --- a/consensus/service/src/byzantine_ledger/pending_values.rs +++ b/consensus/service/src/byzantine_ledger/pending_values.rs @@ -74,7 +74,7 @@ impl PendingValues { match value { ConsensusValue::TxHash(tx_hash) => { // A new transaction. - if self.tx_manager.validate(&tx_hash).is_ok() { + if self.tx_manager.validate(&tx_hash, None).is_ok() { // The transaction is well-formed and valid. entry.insert(received_time); self.pending_values.push(value); @@ -87,7 +87,7 @@ impl PendingValues { ConsensusValue::MintConfigTx(ref mint_config_tx) => { if self .mint_tx_manager - .validate_mint_config_tx(mint_config_tx) + .validate_mint_config_tx(mint_config_tx, None) .is_ok() { // The transaction is well-formed and valid. @@ -100,7 +100,7 @@ impl PendingValues { } ConsensusValue::MintTx(ref mint_tx) => { - if self.mint_tx_manager.validate_mint_tx(mint_tx).is_ok() { + if self.mint_tx_manager.validate_mint_tx(mint_tx, None).is_ok() { // The transaction is well-formed and valid. entry.insert(received_time); self.pending_values.push(value); @@ -147,12 +147,12 @@ impl PendingValues { let tx_manager = self.tx_manager.clone(); let mint_tx_manager = self.mint_tx_manager.clone(); self.retain(|value| match value { - ConsensusValue::TxHash(tx_hash) => tx_manager.validate(tx_hash).is_ok(), + ConsensusValue::TxHash(tx_hash) => tx_manager.validate(tx_hash, None).is_ok(), ConsensusValue::MintConfigTx(ref mint_config_tx) => mint_tx_manager - .validate_mint_config_tx(mint_config_tx) + .validate_mint_config_tx(mint_config_tx, None) .is_ok(), ConsensusValue::MintTx(ref mint_tx) => { - mint_tx_manager.validate_mint_tx(mint_tx).is_ok() + mint_tx_manager.validate_mint_tx(mint_tx, None).is_ok() } }); } @@ -181,18 +181,18 @@ mod tests { // `validate` should be called one for each pending value. tx_manager .expect_validate() - .with(eq(values[0])) + .with(eq(values[0]), eq(None)) .return_const(Ok(())); // This transaction has expired. tx_manager .expect_validate() - .with(eq(values[1])) + .with(eq(values[1]), eq(None)) .return_const(Err(TxManagerError::TransactionValidation( TransactionValidationError::TombstoneBlockExceeded, ))); tx_manager .expect_validate() - .with(eq(values[2])) + .with(eq(values[2]), eq(None)) .return_const(Ok(())); let mut pending_values = @@ -266,18 +266,18 @@ mod tests { // `validate` should be called one for each pending value. tx_manager .expect_validate() - .with(eq(tx_hashes[0])) + .with(eq(tx_hashes[0]), eq(None)) .return_const(Ok(())); // This transaction has expired. tx_manager .expect_validate() - .with(eq(tx_hashes[1])) + .with(eq(tx_hashes[1]), eq(None)) .return_const(Err(TxManagerError::TransactionValidation( TransactionValidationError::TombstoneBlockExceeded, ))); tx_manager .expect_validate() - .with(eq(tx_hashes[2])) + .with(eq(tx_hashes[2]), eq(None)) .return_const(Ok(())); // Create new PendingValues and forcefully shove the pending tx_hashes into it diff --git a/consensus/service/src/byzantine_ledger/worker.rs b/consensus/service/src/byzantine_ledger/worker.rs index 06de21323f..d44049eeb1 100644 --- a/consensus/service/src/byzantine_ledger/worker.rs +++ b/consensus/service/src/byzantine_ledger/worker.rs @@ -25,8 +25,8 @@ use mc_crypto_keys::Ed25519Pair; use mc_ledger_db::Ledger; use mc_ledger_sync::{LedgerSync, NetworkState, SCPNetworkState}; use mc_peers::{ - Broadcast, ConsensusConnection, ConsensusMsg, ConsensusValue, Error as PeerError, - RetryableConsensusConnection, VerifiedConsensusMsg, + Broadcast, ConsensusConnection, ConsensusMsg, ConsensusValue, ConsensusValueWithTimestamp, + Error as PeerError, RetryableConsensusConnection, VerifiedConsensusMsg, }; use mc_transaction_core::tx::TxHash; use mc_util_metered_channel::Receiver; @@ -40,7 +40,7 @@ use std::{ Arc, Mutex, }, thread, - time::{Duration, Instant}, + time::{Duration, Instant, SystemTime}, }; /// Default number of consensus messages to process per batch. @@ -58,7 +58,7 @@ pub struct ByzantineLedgerWorker< enclave: E, // SCP implementation. - scp_node: Box>, + scp_node: Box>, // SCP message signing key. msg_signer_key: Arc, @@ -147,7 +147,7 @@ impl< #[allow(clippy::too_many_arguments)] pub fn new( enclave: E, - scp_node: Box>, + scp_node: Box>, msg_signer_key: Arc, ledger: L, ledger_sync_service: LS, @@ -433,6 +433,19 @@ impl< fn propose_pending_values(&mut self) { assert!(!self.pending_values.is_empty()); + // The timestamp here will potentially be the timestamp that lands on + // the block. + // The timestamp is intentionally not part of the `pending_values` so + // that when a value doesn't land and is tried next time, it uses a new + // timestamp. If a new timestamp wasn't used each time the value was + // proposed then it would be older than the latest block and be + // rejected. + let now = SystemTime::now(); + let timestamp = now + .duration_since(SystemTime::UNIX_EPOCH) + .expect("Failed getting the system time") + .as_millis() as u64; + // Fairness heuristics: // * Values are proposed in the order that they were received. // * Each node limits the total number of values it proposes per slot. @@ -441,6 +454,7 @@ impl< .iter() .take(MAX_PENDING_VALUES_TO_NOMINATE) .cloned() + .map(|v| ConsensusValueWithTimestamp::new(v, timestamp)) .collect(); let msg_opt = self @@ -538,7 +552,7 @@ impl< } } - fn complete_current_slot(&mut self, externalized: Vec) { + fn complete_current_slot(&mut self, externalized: Vec) { let tracer = tracer!(); let span = start_block_span(&tracer, "complete_current_slot", self.current_slot_index); @@ -546,7 +560,10 @@ impl< // Update pending value processing time metrics. for value in externalized.iter() { - if let Some(received_time) = self.pending_values.get_received_time_for_value(value) { + if let Some(received_time) = self + .pending_values + .get_received_time_for_value(&value.value) + { let duration = Instant::now().saturating_duration_since(received_time); counters::PENDING_VALUE_PROCESSING_TIME.observe(duration.as_secs_f64()); } @@ -555,7 +572,7 @@ impl< // Invariant: pending_values only contains valid values that were not // externalized. self.pending_values - .retain(|value| !externalized.contains(value)); + .retain(|value| !externalized.iter().any(|e| &e.value == value)); log::info!( self.logger, @@ -651,7 +668,7 @@ impl< fn fetch_missing_txs( &mut self, - scp_msg: &Msg, + scp_msg: &Msg, from_responder_id: &ResponderId, ) -> bool { // Hashes of transactions that are not currently cached. @@ -659,7 +676,7 @@ impl< .values() .into_iter() .filter_map(|value| { - if let ConsensusValue::TxHash(tx_hash) = value { + if let ConsensusValue::TxHash(tx_hash) = value.value { if self.tx_manager.contains(&tx_hash) { None } else { @@ -760,7 +777,10 @@ impl< } /// Broadcast a consensus message issued by this node. - fn issue_consensus_message(&mut self, msg: Msg) -> Result<(), &'static str> { + fn issue_consensus_message( + &mut self, + msg: Msg, + ) -> Result<(), &'static str> { let consensus_msg = ConsensusMsg::from_scp_msg(&self.ledger, msg, self.msg_signer_key.as_ref()) .map_err(|_| "Failed creating ConsensusMsg")?; @@ -814,7 +834,7 @@ impl< fn form_block_from_externalized_values( &self, - externalized_values: Vec, + externalized_values: Vec, ) -> BlockData { let parent_block = self .ledger @@ -825,9 +845,11 @@ impl< let mut tx_hashes = Vec::new(); let mut mint_config_txs = Vec::new(); let mut mint_txs = Vec::new(); + let mut timestamps = Vec::new(); for value in externalized_values { - match value { + timestamps.push(value.timestamp); + match value.value { ConsensusValue::TxHash(tx_hash) => tx_hashes.push(tx_hash), ConsensusValue::MintConfigTx(mint_config_tx) => { mint_config_txs.push(mint_config_tx); @@ -858,6 +880,14 @@ impl< .get_root_tx_out_membership_element() .expect("Failed getting root tx out membership element"); + // We use the max timestamp as that should be the closest time to "now". + // i.e. it may have taken 100ms to process the block. The last + // timestamp, in an ideal system, may be 100ms later than the first + // timestamp in the values, but should still be slightly before now. + // Using the max timestamp is also called out in the + // [Stellar Whitepaper](https://www.stellar.org/papers/stellar-consensus-protocol). + let timestamp = timestamps.into_iter().max().unwrap_or(0); + // Request the enclave to form the next block. let (block, block_contents, mut signature) = self .enclave @@ -867,6 +897,7 @@ impl< well_formed_encrypted_txs_with_proofs, mint_config_txs, mint_txs_with_config, + timestamp, }, &root_element, ) @@ -962,7 +993,7 @@ mod tests { num_blocks: u64, ) -> ( MockConsensusEnclave, - MockScpNode, + MockScpNode, MockLedger, MockLedgerSync, MockTxManager, @@ -1325,7 +1356,7 @@ mod tests { for tx_hash in &tx_hashes[0..100] { tx_manager .expect_validate() - .with(eq(*tx_hash)) + .with(eq(*tx_hash), eq(None)) .return_const(Ok(())); } @@ -1333,7 +1364,7 @@ mod tests { for tx_hash in &tx_hashes[100..103] { tx_manager .expect_validate() - .with(eq(*tx_hash)) + .with(eq(*tx_hash), eq(None)) .return_const(Err(TxManagerError::TransactionValidation( TransactionValidationError::TombstoneBlockExceeded, ))); @@ -1343,7 +1374,7 @@ mod tests { for tx_hash in &tx_hashes[103..] { tx_manager .expect_validate() - .with(eq(*tx_hash)) + .with(eq(*tx_hash), eq(None)) .return_const(Ok(())); } @@ -1439,7 +1470,7 @@ mod tests { for tx_hash in &tx_hashes { tx_manager .expect_validate() - .with(eq(*tx_hash)) + .with(eq(*tx_hash), eq(None)) .return_const(Err(TxManagerError::TransactionValidation( TransactionValidationError::TombstoneBlockExceeded, ))); @@ -1493,7 +1524,7 @@ mod tests { signer_key: &Ed25519Pair, ledger: &L, ) -> VerifiedConsensusMsg { - let msg: Msg = Msg { + let msg: Msg = Msg { sender_id: sender_id.clone(), slot_index: 1, quorum_set: QuorumSet { @@ -1675,10 +1706,22 @@ mod tests { .return_const(5_usize); scp_node.expect_process_timeouts().return_const(Vec::new()); scp_node.expect_get_externalized_values().return_const(vec![ - ConsensusValue::TxHash(hash_tx1), - ConsensusValue::TxHash(hash_tx2), - ConsensusValue::TxHash(hash_tx3), - ConsensusValue::MintTx(mint_tx1.clone()), + ConsensusValueWithTimestamp { + value: ConsensusValue::TxHash(hash_tx1), + timestamp: 10, + }, + ConsensusValueWithTimestamp { + value: ConsensusValue::TxHash(hash_tx2), + timestamp: 20, + }, + ConsensusValueWithTimestamp { + value: ConsensusValue::TxHash(hash_tx3), + timestamp: 30, + }, + ConsensusValueWithTimestamp { + value: ConsensusValue::MintTx(mint_tx1.clone()), + timestamp: 40, + }, ]); scp_node .expect_get_current_slot_metrics() @@ -1720,6 +1763,7 @@ mod tests { assert_eq!(block.index, parent_block.index + 1); assert_eq!(block.parent_id, parent_block.id); + assert_eq!(block.timestamp, 40); // The mock enclave does not mint a fee output, so the number of outputs matches // the number of transactions that we fed into it. diff --git a/consensus/service/src/lib.rs b/consensus/service/src/lib.rs index a7ea23505c..97569eba54 100644 --- a/consensus/service/src/lib.rs +++ b/consensus/service/src/lib.rs @@ -19,6 +19,7 @@ mod background_work_queue; mod byzantine_ledger; mod counters; mod peer_keepalive; +mod timestamp_validator; lazy_static::lazy_static! { pub static ref SVC_COUNTERS: ServiceMetrics = ServiceMetrics::new_and_registered("consensus_service"); diff --git a/consensus/service/src/mint_tx_manager/error.rs b/consensus/service/src/mint_tx_manager/error.rs index 0c24e31724..3e07ee9b4d 100644 --- a/consensus/service/src/mint_tx_manager/error.rs +++ b/consensus/service/src/mint_tx_manager/error.rs @@ -1,5 +1,6 @@ // Copyright (c) 2018-2022 The MobileCoin Foundation +use crate::timestamp_validator::Error as TimestampError; use displaydoc::Display; use mc_ledger_db::Error as LedgerDbError; use mc_transaction_core::mint::MintValidationError; @@ -11,6 +12,9 @@ pub enum MintTxManagerError { /// Ledger error: {0} LedgerDb(LedgerDbError), + + /// Timestamp error: {0} + Timestamp(TimestampError), } impl From for MintTxManagerError { @@ -25,4 +29,10 @@ impl From for MintTxManagerError { } } +impl From for MintTxManagerError { + fn from(err: TimestampError) -> Self { + Self::Timestamp(err) + } +} + pub type MintTxManagerResult = Result; diff --git a/consensus/service/src/mint_tx_manager/mod.rs b/consensus/service/src/mint_tx_manager/mod.rs index a3351659aa..e3b4c4d533 100644 --- a/consensus/service/src/mint_tx_manager/mod.rs +++ b/consensus/service/src/mint_tx_manager/mod.rs @@ -15,6 +15,7 @@ pub use traits::MintTxManager; #[cfg(test)] pub use traits::MockMintTxManager; +use crate::timestamp_validator; use mc_common::{ logger::{log, Logger}, HashSet, @@ -60,15 +61,18 @@ impl MintTxManagerImpl { } } -#[derive(Debug, Eq, Hash, PartialEq)] -struct NonceByTokenId { - nonce: Vec, - token_id: u64, -} - impl MintTxManager for MintTxManagerImpl { - /// Validate a MintConfigTx transaction against the current ledger. - fn validate_mint_config_tx(&self, mint_config_tx: &MintConfigTx) -> MintTxManagerResult<()> { + fn validate_mint_config_tx( + &self, + mint_config_tx: &MintConfigTx, + timestamp: Option, + ) -> MintTxManagerResult<()> { + let latest_block = self.ledger_db.get_latest_block()?; + + if let Some(timestamp) = timestamp { + timestamp_validator::validate_with_logger(timestamp, &latest_block, &self.logger)?; + } + // Ensure that we have not seen this transaction before. if self .ledger_db @@ -92,13 +96,10 @@ impl MintTxManager for MintTxManagerImpl { MintValidationError::NoGovernors(token_id), ))?; - // Get the index of the block currently being built. - let current_block_index = self.ledger_db.num_blocks()?; - // Perform the actual validation. validate_mint_config_tx( mint_config_tx, - Some(current_block_index), + Some(latest_block.index + 1), self.block_version, &governors, )?; @@ -108,32 +109,31 @@ impl MintTxManager for MintTxManagerImpl { fn combine_mint_config_txs( &self, - txs: &[MintConfigTx], + txs: &[(MintConfigTx, u64)], max_elements: usize, - ) -> MintTxManagerResult> { - let mut candidates = txs.to_vec(); - candidates.sort(); - - let mut seen_nonces = HashSet::default(); - let (allowed_txs, _rejected_txs) = candidates.into_iter().partition(|tx| { - let nonce_with_token_id = NonceByTokenId { - nonce: tx.prefix.nonce.clone(), - token_id: tx.prefix.token_id, - }; - if seen_nonces.len() >= max_elements { - return false; - } - if seen_nonces.contains(&nonce_with_token_id) { - return false; - } - seen_nonces.insert(nonce_with_token_id); - true + ) -> MintTxManagerResult> { + let mut candidates = timestamp_validator::sort_by_value_then_timestamp(txs.iter()); + // A nonce should only be used once per token id. We only check these + // fields to avoid having different configs with the same + // (nonce, token_id). + candidates.dedup_by(|(tx1, _timestamp1), (tx2, _timestamp2)| { + tx1.prefix.token_id == tx2.prefix.token_id && tx1.prefix.nonce == tx2.prefix.nonce }); - - Ok(allowed_txs) + candidates.truncate(max_elements); + Ok(candidates) } - fn validate_mint_tx(&self, mint_tx: &MintTx) -> MintTxManagerResult<()> { + fn validate_mint_tx( + &self, + mint_tx: &MintTx, + timestamp: Option, + ) -> MintTxManagerResult<()> { + let latest_block = self.ledger_db.get_latest_block()?; + + if let Some(timestamp) = timestamp { + timestamp_validator::validate_with_logger(timestamp, &latest_block, &self.logger)?; + } + // Ensure that we have not seen this transaction before. if self .ledger_db @@ -160,13 +160,10 @@ impl MintTxManager for MintTxManagerImpl { err => err.into(), })?; - // Get the index of the block currently being built. - let current_block_index = self.ledger_db.num_blocks()?; - // Perform the actual validation. validate_mint_tx( mint_tx, - current_block_index, + latest_block.index + 1, self.block_version, &active_mint_config.mint_config, )?; @@ -176,48 +173,45 @@ impl MintTxManager for MintTxManagerImpl { fn combine_mint_txs( &self, - txs: &[MintTx], + txs: &[(MintTx, u64)], max_elements: usize, - ) -> MintTxManagerResult> { - let mut candidates = txs.to_vec(); - candidates.sort(); + ) -> MintTxManagerResult> { + let mut candidates = timestamp_validator::sort_by_value_then_timestamp(txs.iter()); + candidates.dedup_by(|(tx1, _timestamp1), (tx2, _timestamp2)| { + // A nonce should only be used once per token id. We only check these + // fields to avoid having different transactions with the same + // (nonce, token_id). + tx1.prefix.token_id == tx2.prefix.token_id && tx1.prefix.nonce == tx2.prefix.nonce + }); - let mut seen_nonces = HashSet::default(); let mut seen_mint_configs = HashSet::default(); - let (allowed_txs, _rejected_txs) = candidates.into_iter().partition(|tx| { - let nonce_with_token_id = NonceByTokenId { - nonce: tx.prefix.nonce.clone(), - token_id: tx.prefix.token_id, - }; - if seen_nonces.len() >= max_elements { - return false; - } - if seen_nonces.contains(&nonce_with_token_id) { - return false; - } - - // We allow a specific MintConfig to be used only once per block - this - // simplifies enforcing the total mint limit. - let active_mint_config = match self.ledger_db.get_active_mint_config_for_mint_tx(tx) { - Ok(active_mint_config) => active_mint_config, - Err(err) => { - log::warn!( - self.logger, - "failed finding an active mint config for mint tx {}: {}", - tx, - err - ); + let allowed_txs = candidates + .into_iter() + .filter(|(tx, _)| { + // We allow a specific MintConfig to be used only once per block - this + // simplifies enforcing the total mint limit. + let active_mint_config = match self.ledger_db.get_active_mint_config_for_mint_tx(tx) + { + Ok(active_mint_config) => active_mint_config, + Err(err) => { + log::warn!( + self.logger, + "failed finding an active mint config for mint tx {}: {}", + tx, + err + ); + return false; + } + }; + if seen_mint_configs.contains(&active_mint_config.mint_config) { return false; } - }; - if seen_mint_configs.contains(&active_mint_config.mint_config) { - return false; - } - - seen_nonces.insert(nonce_with_token_id); - seen_mint_configs.insert(active_mint_config.mint_config); - true - }); + + seen_mint_configs.insert(active_mint_config.mint_config); + true + }) + .take(max_elements) + .collect(); Ok(allowed_txs) } @@ -251,6 +245,7 @@ impl MintTxManager for MintTxManagerImpl { #[cfg(test)] mod mint_config_tx_tests { use super::*; + use assert_matches::assert_matches; use mc_blockchain_types::BlockContents; use mc_common::logger::test_with_logger; use mc_crypto_multisig::SignerSet; @@ -263,6 +258,7 @@ mod mint_config_tx_tests { mint_config_tx_to_validated as to_validated, AccountKey, }; use rand::{rngs::StdRng, SeedableRng}; + use std::time::SystemTime; const BLOCK_VERSION: BlockVersion = BlockVersion::MAX; @@ -287,8 +283,13 @@ mod mint_config_tx_tests { let mint_tx_manager = MintTxManagerImpl::new(ledger, BLOCK_VERSION, token_id_to_governors, logger); + let now = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .expect("Failed getting the system time") + .as_millis() as u64; + assert_eq!( - mint_tx_manager.validate_mint_config_tx(&mint_config_tx), + mint_tx_manager.validate_mint_config_tx(&mint_config_tx, Some(now)), Ok(()) ); } @@ -329,17 +330,17 @@ mod mint_config_tx_tests { MintTxManagerImpl::new(ledger, BLOCK_VERSION, token_id_to_governors, logger); assert_eq!( - mint_tx_manager.validate_mint_config_tx(&mint_config_tx1), + mint_tx_manager.validate_mint_config_tx(&mint_config_tx1, None), Ok(()) ); assert_eq!( - mint_tx_manager.validate_mint_config_tx(&mint_config_tx2), + mint_tx_manager.validate_mint_config_tx(&mint_config_tx2, None), Ok(()) ); assert_eq!( - mint_tx_manager.validate_mint_config_tx(&mint_config_tx3), + mint_tx_manager.validate_mint_config_tx(&mint_config_tx3, None), Ok(()) ); } @@ -374,7 +375,7 @@ mod mint_config_tx_tests { // At first we should succeed since we have not yet exceeded the tombstone // block. assert_eq!( - mint_tx_manager.validate_mint_config_tx(&mint_config_tx), + mint_tx_manager.validate_mint_config_tx(&mint_config_tx, None), Ok(()) ); @@ -388,7 +389,7 @@ mod mint_config_tx_tests { // Try again, we should fail. assert_eq!( - mint_tx_manager.validate_mint_config_tx(&mint_config_tx), + mint_tx_manager.validate_mint_config_tx(&mint_config_tx, None), Err(MintTxManagerError::MintValidation( MintValidationError::TombstoneBlockExceeded )) @@ -418,7 +419,7 @@ mod mint_config_tx_tests { // At first we should succeed since the nonce is not yet in the ledger. assert_eq!( - mint_tx_manager.validate_mint_config_tx(&mint_config_tx), + mint_tx_manager.validate_mint_config_tx(&mint_config_tx, None), Ok(()) ); @@ -431,7 +432,7 @@ mod mint_config_tx_tests { // Try again, we should fail. assert_eq!( - mint_tx_manager.validate_mint_config_tx(&mint_config_tx), + mint_tx_manager.validate_mint_config_tx(&mint_config_tx, None), Err(MintTxManagerError::MintValidation( MintValidationError::NonceAlreadyUsed )) @@ -462,7 +463,7 @@ mod mint_config_tx_tests { let (mint_config_tx2, _signers) = create_mint_config_tx_and_signers(token_id_2, &mut rng); assert_eq!( - mint_tx_manager.validate_mint_config_tx(&mint_config_tx2), + mint_tx_manager.validate_mint_config_tx(&mint_config_tx2, None), Err(MintTxManagerError::MintValidation( MintValidationError::NoGovernors(token_id_2) )) @@ -494,13 +495,42 @@ mod mint_config_tx_tests { mint_config_tx.prefix.tombstone_block += 1; assert_eq!( - mint_tx_manager.validate_mint_config_tx(&mint_config_tx), + mint_tx_manager.validate_mint_config_tx(&mint_config_tx, None), Err(MintTxManagerError::MintValidation( MintValidationError::InvalidSignature )) ); } + #[test_with_logger] + fn validate_mint_config_tx_rejects_old_timestamp(logger: Logger) { + let mut rng: StdRng = SeedableRng::from_seed([77u8; 32]); + let token_id_1 = TokenId::from(1); + + let mut ledger = create_ledger(); + let n_blocks = 1; + let sender = AccountKey::random(&mut rng); + initialize_ledger(BLOCK_VERSION, &mut ledger, n_blocks, &sender, &mut rng); + + let (mint_config_tx, signers) = create_mint_config_tx_and_signers(token_id_1, &mut rng); + let token_id_to_governors = GovernorsMap::try_from_iter(vec![( + token_id_1, + SignerSet::new(signers.iter().map(|s| s.public_key()).collect(), 1), + )]) + .unwrap(); + let mint_tx_manager = + MintTxManagerImpl::new(ledger, BLOCK_VERSION, token_id_to_governors, logger); + + // 1970 might be a bit extreme, but we only want to show we hooked up + // the timestamp validator, not the edge cases of it. + let timestamp_too_old = 0; + + assert_matches!( + mint_tx_manager.validate_mint_config_tx(&mint_config_tx, Some(timestamp_too_old)), + Err(MintTxManagerError::Timestamp(_)) + ); + } + /// combine_mint_config_txs adequately sorts inputs and disposes of /// duplicates. #[test_with_logger] @@ -526,25 +556,25 @@ mod mint_config_tx_tests { MintTxManagerImpl::new(ledger, BLOCK_VERSION, token_id_to_governors, logger); let mut expected_result = vec![ - mint_config_tx1.clone(), - mint_config_tx2.clone(), - mint_config_tx3.clone(), - mint_config_tx4.clone(), + (mint_config_tx1.clone(), 100), + (mint_config_tx2.clone(), 200), + (mint_config_tx3.clone(), 300), + (mint_config_tx4.clone(), 400), ]; expected_result.sort(); assert_eq!( mint_tx_manager.combine_mint_config_txs( &[ - mint_config_tx3.clone(), - mint_config_tx4, - mint_config_tx1.clone(), - mint_config_tx3.clone(), - mint_config_tx3, - mint_config_tx2.clone(), - mint_config_tx1.clone(), - mint_config_tx1, - mint_config_tx2, + (mint_config_tx3.clone(), 3), + (mint_config_tx4, 400), + (mint_config_tx1.clone(), 1), + (mint_config_tx3.clone(), 300), + (mint_config_tx3, 3), + (mint_config_tx2.clone(), 2), + (mint_config_tx1.clone(), 100), + (mint_config_tx1, 1), + (mint_config_tx2, 200), ], 100 ), @@ -589,17 +619,20 @@ mod mint_config_tx_tests { let mint_tx_manager = MintTxManagerImpl::new(ledger, BlockVersion::MAX, token_id_to_governors, logger); - let mut expected_result = vec![mint_config_tx1.clone(), mint_config_tx2.clone()]; + let mut expected_result = vec![ + (mint_config_tx1.clone(), 1000), + (mint_config_tx2.clone(), 2000), + ]; expected_result.sort(); assert_eq!( mint_tx_manager.combine_mint_config_txs( &[ - mint_config_tx1.clone(), - mint_config_tx2.clone(), - mint_config_tx1.clone(), - mint_config_tx1, - mint_config_tx2, + (mint_config_tx1.clone(), 10), + (mint_config_tx2.clone(), 20), + (mint_config_tx1.clone(), 1000), + (mint_config_tx1, 100), + (mint_config_tx2, 2000), ], 100 ), @@ -633,12 +666,12 @@ mod mint_config_tx_tests { MintTxManagerImpl::new(ledger, BLOCK_VERSION, token_id_to_governors, logger); let mut expected_result = vec![ - mint_config_tx1.clone(), - mint_config_tx2.clone(), - mint_config_tx3.clone(), - mint_config_tx4.clone(), - mint_config_tx5.clone(), - mint_config_tx6.clone(), + (mint_config_tx1.clone(), 100), + (mint_config_tx2.clone(), 200), + (mint_config_tx3.clone(), 300), + (mint_config_tx4.clone(), 400), + (mint_config_tx5.clone(), 500), + (mint_config_tx6.clone(), 600), ]; expected_result.sort(); expected_result.truncate(3); @@ -646,16 +679,16 @@ mod mint_config_tx_tests { assert_eq!( mint_tx_manager.combine_mint_config_txs( &[ - mint_config_tx3.clone(), - mint_config_tx4, - mint_config_tx1.clone(), - mint_config_tx5, - mint_config_tx3, - mint_config_tx2.clone(), - mint_config_tx1.clone(), - mint_config_tx1, - mint_config_tx2, - mint_config_tx6, + (mint_config_tx3.clone(), 3), + (mint_config_tx4, 400), + (mint_config_tx1.clone(), 1), + (mint_config_tx5, 500), + (mint_config_tx3, 300), + (mint_config_tx2.clone(), 2), + (mint_config_tx1.clone(), 100), + (mint_config_tx1, 1), + (mint_config_tx2, 200), + (mint_config_tx6, 600), ], 3 ), @@ -667,6 +700,7 @@ mod mint_config_tx_tests { #[cfg(test)] mod mint_tx_tests { use super::*; + use assert_matches::assert_matches; use mc_blockchain_types::BlockContents; use mc_common::logger::test_with_logger; use mc_crypto_keys::Ed25519Pair; @@ -680,6 +714,7 @@ mod mint_tx_tests { mint_config_tx_to_validated as to_validated, AccountKey, }; use rand::{rngs::StdRng, SeedableRng}; + use std::time::SystemTime; const BLOCK_VERSION: BlockVersion = BlockVersion::MAX; @@ -721,7 +756,15 @@ mod mint_tx_tests { &mut rng, ); - assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx), Ok(())); + let now = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .expect("Failed getting the system time") + .as_millis() as u64; + + assert_eq!( + mint_tx_manager.validate_mint_tx(&mint_tx, Some(now)), + Ok(()) + ); } /// validate_mint_tx accepts a valid mint tx when two tokens are configured. @@ -772,7 +815,7 @@ mod mint_tx_tests { &mut rng, ); - assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx1), Ok(())); + assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx1, None), Ok(())); let mint_tx2 = create_mint_tx( token_id_2, @@ -784,7 +827,7 @@ mod mint_tx_tests { &mut rng, ); - assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx2), Ok(())); + assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx2, None), Ok(())); } /// validate_mint_tx rejects a mint tx when it cannot be matched with an @@ -839,7 +882,7 @@ mod mint_tx_tests { ); assert_eq!( - mint_tx_manager.validate_mint_tx(&mint_tx), + mint_tx_manager.validate_mint_tx(&mint_tx, None), Err(MintTxManagerError::MintValidation( MintValidationError::NoMatchingMintConfig )) @@ -855,7 +898,7 @@ mod mint_tx_tests { ); assert_eq!( - mint_tx_manager.validate_mint_tx(&mint_tx), + mint_tx_manager.validate_mint_tx(&mint_tx, None), Err(MintTxManagerError::MintValidation( MintValidationError::NoMatchingMintConfig )) @@ -870,7 +913,7 @@ mod mint_tx_tests { ); assert_eq!( - mint_tx_manager.validate_mint_tx(&mint_tx), + mint_tx_manager.validate_mint_tx(&mint_tx, None), Err(MintTxManagerError::MintValidation( MintValidationError::NoMatchingMintConfig )) @@ -916,7 +959,7 @@ mod mint_tx_tests { ); assert_eq!( - mint_tx_manager.validate_mint_tx(&mint_tx), + mint_tx_manager.validate_mint_tx(&mint_tx, None), Err(MintTxManagerError::MintValidation( MintValidationError::AmountExceedsMintLimit )) @@ -931,7 +974,7 @@ mod mint_tx_tests { &mut rng, ); - assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx), Ok(())); + assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx, None), Ok(())); let block_contents = BlockContents { mint_txs: vec![mint_tx], @@ -949,7 +992,7 @@ mod mint_tx_tests { ); assert_eq!( - mint_tx_manager.validate_mint_tx(&mint_tx), + mint_tx_manager.validate_mint_tx(&mint_tx, None), Err(MintTxManagerError::MintValidation( MintValidationError::AmountExceedsMintLimit )) @@ -963,7 +1006,7 @@ mod mint_tx_tests { &mut rng, ); - assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx), Ok(())); + assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx, None), Ok(())); } /// validate_mint_tx rejects a mint tx that exceeds the overall mint limit. @@ -1006,7 +1049,7 @@ mod mint_tx_tests { ); assert_eq!( - mint_tx_manager.validate_mint_tx(&mint_tx), + mint_tx_manager.validate_mint_tx(&mint_tx, None), Err(MintTxManagerError::MintValidation( MintValidationError::AmountExceedsMintLimit )) @@ -1021,7 +1064,7 @@ mod mint_tx_tests { &mut rng, ); - assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx), Ok(())); + assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx, None), Ok(())); let block_contents = BlockContents { mint_txs: vec![mint_tx], @@ -1039,7 +1082,7 @@ mod mint_tx_tests { ); assert_eq!( - mint_tx_manager.validate_mint_tx(&mint_tx), + mint_tx_manager.validate_mint_tx(&mint_tx, None), Err(MintTxManagerError::MintValidation( MintValidationError::AmountExceedsMintLimit )) @@ -1053,7 +1096,7 @@ mod mint_tx_tests { &mut rng, ); - assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx), Ok(())); + assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx, None), Ok(())); } /// validate_mint_tx rejects invalid signature. @@ -1093,12 +1136,12 @@ mod mint_tx_tests { &mut rng, ); - assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx), Ok(())); + assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx, None), Ok(())); // Now mess with the data so the signature is no longer valid. mint_tx.prefix.amount += 1; assert_eq!( - mint_tx_manager.validate_mint_tx(&mint_tx), + mint_tx_manager.validate_mint_tx(&mint_tx, None), Err(MintTxManagerError::MintValidation( MintValidationError::NoMatchingMintConfig )) @@ -1148,7 +1191,7 @@ mod mint_tx_tests { // At first we should succeed since we have not yet exceeded the tombstone // block. - assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx), Ok(())); + assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx, None), Ok(())); // Append a block to the ledger. let block_contents = BlockContents { @@ -1160,7 +1203,7 @@ mod mint_tx_tests { // Try again, we should fail. assert_eq!( - mint_tx_manager.validate_mint_tx(&mint_tx), + mint_tx_manager.validate_mint_tx(&mint_tx, None), Err(MintTxManagerError::MintValidation( MintValidationError::TombstoneBlockExceeded )) @@ -1205,7 +1248,7 @@ mod mint_tx_tests { ); // At first we should succeed since the nonce is not yet in the ledger. - assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx), Ok(())); + assert_eq!(mint_tx_manager.validate_mint_tx(&mint_tx, None), Ok(())); // Append to the ledger. let block_contents = BlockContents { @@ -1217,13 +1260,59 @@ mod mint_tx_tests { // Try again, we should fail. assert_eq!( - mint_tx_manager.validate_mint_tx(&mint_tx), + mint_tx_manager.validate_mint_tx(&mint_tx, None), Err(MintTxManagerError::MintValidation( MintValidationError::NonceAlreadyUsed )) ); } + #[test_with_logger] + fn mint_tx_rejects_timestamp_too_old(logger: Logger) { + let mut rng: StdRng = SeedableRng::from_seed([77u8; 32]); + let token_id_1 = TokenId::from(1); + + let mut ledger = create_ledger(); + let n_blocks = 3; + let sender = AccountKey::random(&mut rng); + initialize_ledger(BLOCK_VERSION, &mut ledger, n_blocks, &sender, &mut rng); + + // Create a mint configuration and append it to the ledger. + let (mint_config_tx, signers) = create_mint_config_tx_and_signers(token_id_1, &mut rng); + + let block_contents = BlockContents { + validated_mint_config_txs: vec![to_validated(&mint_config_tx)], + ..Default::default() + }; + add_block_contents_to_ledger(&mut ledger, BLOCK_VERSION, block_contents, &mut rng).unwrap(); + + // Create MintTxManagerImpl + let token_id_to_governors = GovernorsMap::try_from_iter(vec![( + token_id_1, + SignerSet::new(signers.iter().map(|s| s.public_key()).collect(), 1), + )]) + .unwrap(); + let mint_tx_manager = + MintTxManagerImpl::new(ledger, BLOCK_VERSION, token_id_to_governors, logger); + + // Create a valid MintTx signed by the governor. + let mint_tx = create_mint_tx( + token_id_1, + &[Ed25519Pair::from(signers[0].private_key())], + 1, + &mut rng, + ); + + // 1970 might be a bit extreme, but we only want to show we hooked up + // the timestamp validator, not the edge cases of it. + let timestamp_too_old = 0; + + assert_matches!( + mint_tx_manager.validate_mint_tx(&mint_tx, Some(timestamp_too_old)), + Err(MintTxManagerError::Timestamp(_)) + ); + } + /// combine_mint_txs adequately sorts inputs and disposes of /// duplicates. #[test_with_logger] @@ -1280,24 +1369,28 @@ mod mint_tx_tests { let mint_tx_manager = MintTxManagerImpl::new(ledger, BLOCK_VERSION, token_id_to_governors, logger); - let mut expected_result = mint_txs.clone(); + let mut expected_result = vec![ + (mint_txs[0].clone(), 100), + (mint_txs[1].clone(), 200), + (mint_txs[2].clone(), 300), + ]; expected_result.sort(); assert_eq!( mint_tx_manager.combine_mint_txs( &[ - mint_txs[0].clone(), - mint_txs[0].clone(), - mint_txs[1].clone(), - mint_txs[2].clone(), - mint_txs[1].clone(), - mint_txs[0].clone(), - mint_txs[0].clone(), - mint_txs[2].clone(), - mint_txs[1].clone(), - mint_txs[0].clone(), - mint_txs[1].clone(), - mint_txs[2].clone(), + (mint_txs[0].clone(), 10), + (mint_txs[0].clone(), 11), + (mint_txs[1].clone(), 200), + (mint_txs[2].clone(), 31), + (mint_txs[1].clone(), 20), + (mint_txs[0].clone(), 100), + (mint_txs[0].clone(), 12), + (mint_txs[2].clone(), 32), + (mint_txs[1].clone(), 21), + (mint_txs[0].clone(), 13), + (mint_txs[1].clone(), 22), + (mint_txs[2].clone(), 300), ], 100 ), @@ -1369,21 +1462,21 @@ mod mint_tx_tests { let mint_tx_manager = MintTxManagerImpl::new(ledger, BlockVersion::MAX, token_id_to_governors, logger); - let mut expected_result = mint_txs.clone(); + let mut expected_result = vec![(mint_txs[0].clone(), 1000), (mint_txs[1].clone(), 2000)]; expected_result.sort(); assert_eq!( mint_tx_manager.combine_mint_txs( &[ - mint_txs[0].clone(), - mint_txs[0].clone(), - mint_txs[1].clone(), - mint_txs[1].clone(), - mint_txs[0].clone(), - mint_txs[0].clone(), - mint_txs[1].clone(), - mint_txs[0].clone(), - mint_txs[1].clone(), + (mint_txs[0].clone(), 0), + (mint_txs[0].clone(), 3), + (mint_txs[1].clone(), 1997), + (mint_txs[1].clone(), 1998), + (mint_txs[0].clone(), 1000), + (mint_txs[0].clone(), 1), + (mint_txs[1].clone(), 2000), + (mint_txs[0].clone(), 2), + (mint_txs[1].clone(), 1999), ], 100 ), @@ -1472,32 +1565,36 @@ mod mint_tx_tests { let combined = mint_tx_manager .combine_mint_txs( &[ - mint_txs[0].clone(), - mint_txs[1].clone(), - mint_txs[2].clone(), - mint_txs[3].clone(), - mint_txs[4].clone(), - mint_txs[3].clone(), - mint_txs[4].clone(), - mint_txs[5].clone(), - mint_txs[5].clone(), - mint_txs[5].clone(), - mint_txs[0].clone(), - mint_txs[1].clone(), - mint_txs[2].clone(), - mint_txs[2].clone(), - mint_txs[3].clone(), - mint_txs[3].clone(), - mint_txs[4].clone(), - mint_txs[5].clone(), + (mint_txs[0].clone(), 10), + (mint_txs[1].clone(), 200), + (mint_txs[2].clone(), 30), + (mint_txs[3].clone(), 400), + (mint_txs[4].clone(), 50), + (mint_txs[3].clone(), 40), + (mint_txs[4].clone(), 500), + (mint_txs[5].clone(), 60), + (mint_txs[5].clone(), 61), + (mint_txs[5].clone(), 600), + (mint_txs[0].clone(), 100), + (mint_txs[1].clone(), 20), + (mint_txs[2].clone(), 300), + (mint_txs[2].clone(), 31), + (mint_txs[3].clone(), 41), + (mint_txs[3].clone(), 42), + (mint_txs[4].clone(), 51), + (mint_txs[5].clone(), 62), ], 100, ) .unwrap(); assert_eq!( - HashSet::from_iter([10, 20, 30]), - HashSet::from_iter(combined.iter().map(|tx| tx.prefix.amount)) + HashSet::from_iter([(10, 100), (20, 200), (30, 300)]), + HashSet::from_iter( + combined + .iter() + .map(|(tx, timestamp)| (tx.prefix.amount, *timestamp)) + ) ); } } diff --git a/consensus/service/src/mint_tx_manager/traits.rs b/consensus/service/src/mint_tx_manager/traits.rs index ba5b8f831a..98d22e3a88 100644 --- a/consensus/service/src/mint_tx_manager/traits.rs +++ b/consensus/service/src/mint_tx_manager/traits.rs @@ -12,44 +12,63 @@ use mockall::*; #[cfg_attr(test, automock)] pub trait MintTxManager: Send { /// Validate a MintConfigTx transaction against the current ledger. - fn validate_mint_config_tx(&self, mint_config_tx: &MintConfigTx) -> MintTxManagerResult<()>; + /// + /// # Arguments + /// * `mint_config_tx` - The tx config to validate. + /// * `timestamp` - The timestamp to validate. ms since Unix epoch. If None, + /// then only the validity of the `mint_config_tx` will be checked. + fn validate_mint_config_tx( + &self, + mint_config_tx: &MintConfigTx, + timestamp: Option, + ) -> MintTxManagerResult<()>; /// Combines a set of "candidate values" into a "composite value". /// This assumes all values are well-formed and safe to append to the ledger /// individually. /// /// # Arguments - /// * `txs` - "Candidate" transactions. Each is assumed to be individually - /// valid. + /// * `txs` - "Candidate" transactions, and their proposed timestamp in ms + /// since Unix epoch. Each transaction and timestamp pair is assumed to be + /// individually valid. /// * `max_elements` - Maximal number of elements to output. /// /// Returns a bounded, deterministically-ordered list of transactions that - /// are safe to append to the ledger. + /// are safe to append to the ledger. If there are any duplicate + /// transactions the ones with the largest timestamp will be returned. fn combine_mint_config_txs( &self, - txs: &[MintConfigTx], + txs: &[(MintConfigTx, u64)], max_elements: usize, - ) -> MintTxManagerResult>; + ) -> MintTxManagerResult>; /// Validate a MintTx transaction against the current ledger. - fn validate_mint_tx(&self, mint_tx: &MintTx) -> MintTxManagerResult<()>; + /// + /// # Arguments + /// * `mint_tx` - The tx to validate. + /// * `timestamp` - The timestamp to validate. ms since Unix epoch. If None, + /// then only the validity of the `mint_tx` will be checked. + fn validate_mint_tx(&self, mint_tx: &MintTx, timestamp: Option) + -> MintTxManagerResult<()>; /// Combines a set of "candidate values" into a "composite value". /// This assumes all values are well-formed and safe to append to the ledger /// individually. /// /// # Arguments - /// * `txs` - "Candidate" transactions. Each is assumed to be individually - /// valid. + /// * `txs` - "Candidate" transactions, and their proposed timestamp in ms + /// since Unix epoch. Each transaction and timestamp pair is assumed to be + /// individually valid. /// * `max_elements` - Maximal number of elements to output. /// /// Returns a bounded, deterministically-ordered list of transactions that - /// are safe to append to the ledger. + /// are safe to append to the ledger. If there are any duplicate + /// transactions the ones with the largest timestamp will be returned. fn combine_mint_txs( &self, - txs: &[MintTx], + txs: &[(MintTx, u64)], max_elements: usize, - ) -> MintTxManagerResult>; + ) -> MintTxManagerResult>; /// Lookup active mint configuration for a list of mint transactions. /// This is used by the consensus enclave to determine whether MintTxs are diff --git a/consensus/service/src/timestamp_validator.rs b/consensus/service/src/timestamp_validator.rs new file mode 100644 index 0000000000..2af9276559 --- /dev/null +++ b/consensus/service/src/timestamp_validator.rs @@ -0,0 +1,249 @@ +// Copyright (c) 2023 The MobileCoin Foundation + +use displaydoc::Display; +use mc_blockchain_types::Block; +use mc_common::logger::{log, Logger}; + +/// Provides logic for validating a timestamp used in consensus + +/// The maximum allowed skew between the system time and the timestamp. This +/// allows the system time to be a bit behind the timestamp. +/// The reason for this is that during consensus multiple nodes will be looking +/// at the time and those nodes may have skew between their clocks. If the node +/// with the latest time proposes a value, it's possible that the other nodes +/// will reject it because the timestamp is in the future. +/// +/// The 3 seconds is taken from the `signed_at` values from the first 2 million +/// blocks of the blockchain. Throwing away significant outlier nodes most nodes +/// were almost always within 3 seconds of each other. +const ALLOWED_FUTURE_SKEW: u64 = 3 * 1000; + +/// Errors related to validating timestamps +#[derive(Clone, Debug, Display, Eq, PartialEq)] +pub enum Error { + /// The timestamp is too far in the future now: {0}, timestamp: {1} + InFuture(u64, u64), + /** The timestamp is not newer than the last bock. last_bock timestamp: {0}, + timestamp: {1} */ + NotNewerThanLastBlock(u64, u64), +} + +pub fn validate_with_logger( + timestamp: u64, + latest_block: &Block, + logger: &Logger, +) -> Result<(), Error> { + validate(timestamp, latest_block).map_err(|e| { + log::warn!(logger, "Consensus Value timestamp invalid: {e}"); + e + }) +} + +pub fn validate(timestamp: u64, latest_block: &Block) -> Result<(), Error> { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("Failed to get system time") + .as_millis() as u64; + + if timestamp > (now + ALLOWED_FUTURE_SKEW) { + return Err(Error::InFuture(now, timestamp)); + } + + if timestamp <= latest_block.timestamp { + return Err(Error::NotNewerThanLastBlock( + latest_block.timestamp, + timestamp, + )); + } + + Ok(()) +} + +/// Will sort and deduplicate the values, `V` such that only one instance of +/// `V` exists with the largest u64. +/// +/// ```ignore +/// use crate::timestamp_validator; +/// let values = vec![("a", 1), ("a", 2), ("b", 3), ("b", 4)]; +/// let deduped = timestamp_validator::sort_and_dedup(values.iter()); +/// assert_eq!(deduped, vec![("a", 2), ("b", 4)]); +/// ``` +pub fn sort_and_dedup<'a, V: Clone + Ord + 'a>( + values: impl Iterator, +) -> Vec<(V, u64)> { + let sorted = sort_by_value_then_timestamp(values); + dedup(sorted) +} + +/// Deduplicates based on the `V` of the tuple, ignoring the second element of +/// the tuple. +/// +/// This deduplication only works on adjacent elements so the input should be +/// sorted. +fn dedup(mut values: Vec<(V, u64)>) -> Vec<(V, u64)> { + values.dedup_by(|a, b| a.0 == b.0); + values +} + +/// Will sort the values by the first element of the tuple, and then descending +/// by the second element of the tuple, the timestamp. +/// +/// The reason for this sorting is to place the latest timestamp for a value +/// first in the sequence of the same value. +pub fn sort_by_value_then_timestamp<'a, V: Clone + Ord + 'a>( + values: impl IntoIterator, +) -> Vec<(V, u64)> { + let mut values: Vec<_> = values.into_iter().cloned().collect(); + values.sort_by(|a, b| { + if a.0 == b.0 { + b.1.cmp(&a.1) + } else { + a.0.cmp(&b.0) + } + }); + values +} + +#[cfg(test)] +mod test { + use super::*; + use assert_matches::assert_matches; + use yare::parameterized; + + #[test] + fn timestamp_in_the_future_fails_to_validate() { + // Because of the use of system time we can't test right at the + // boundary, but 100ms should be sufficient to prevent someone + // putting newer values into the blockchain and slowing down the + // network. + let now = std::time::SystemTime::now(); + let future = now + .checked_add(std::time::Duration::from_millis(ALLOWED_FUTURE_SKEW + 100)) + .unwrap() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as u64; + + let latest_block = Block::default(); + + assert_matches!( + validate(future, &latest_block), + Err(Error::InFuture(_, timestamp)) if timestamp == future + ); + } + + #[test] + fn current_timestamp_succeeds() { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as u64; + + let latest_block = Block::default(); + + assert_eq!(validate(now, &latest_block), Ok(())); + } + + #[test] + fn timestamp_up_to_allowed_future_skew_succeeds() { + let skewed_now = std::time::SystemTime::now() + .checked_add(std::time::Duration::from_millis(ALLOWED_FUTURE_SKEW)) + .unwrap() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as u64; + + let latest_block = Block::default(); + + assert_eq!(validate(skewed_now, &latest_block), Ok(())); + } + + #[test] + fn timestamp_same_as_last_block_timestamp() { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as u64; + + let latest_block = Block { + timestamp: now, + ..Default::default() + }; + + assert_eq!( + validate(now, &latest_block), + Err(Error::NotNewerThanLastBlock(now, now)) + ); + } + + #[test] + fn timestamp_earlier_than_last_block_timestamp() { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as u64; + + let latest_block = Block { + timestamp: now, + ..Default::default() + }; + + assert_eq!( + validate(now - 1, &latest_block), + Err(Error::NotNewerThanLastBlock(now, now - 1)) + ); + } + + #[test] + fn sorting_empty() { + let values: Vec<(&str, u64)> = vec![]; + let sorted = sort_by_value_then_timestamp(values.iter()); + assert_eq!(sorted, vec![]) + } + + #[parameterized( + one = {&[("a", 1)], &[("a", 1)]}, + all_the_same = {&[("a", 1), ("a", 1), ("a", 1)], &[("a", 1), ("a", 1), ("a", 1)]}, + already_sorted_by_value = {&[("a", 1), ("b", 2), ("c", 3)], &[("a", 1), ("b", 2), ("c", 3)]}, + reversed_value = {&[("z", 1), ("y", 2), ("x", 3)], &[("x", 3), ("y", 2), ("z", 1)]}, + already_sorted_by_timestamp = {&[("a", 3), ("a", 2), ("a", 1)], &[("a", 3), ("a", 2), ("a", 1)]}, + reversed_timestamp = {&[("a", 1), ("a", 2), ("a", 3)], &[("a", 3), ("a", 2), ("a", 1)]}, + unsorted_duplicates_with_different_timestamps = {&[("b", 10), ("a", 2), ("a", 3), ("b", 11)], &[("a", 3), ("a", 2), ("b", 11), ("b", 10)]}, + )] + fn sorting(unsorted: &[(&str, u64)], expected: &[(&str, u64)]) { + let sorted = sort_by_value_then_timestamp(unsorted); + assert_eq!(sorted, expected) + } + + #[test] + fn dedup_with_no_elements() { + let deduped: Vec<(&str, u64)> = dedup(vec![]); + assert_eq!(deduped, vec![]) + } + + #[parameterized( + one = {&[("a", 1)], &[("a", 1)]}, + two_the_same = {&[("a", 1), ("a", 1)], &[("a", 1)]}, + two_different = {&[("a", 1), ("b", 2)], &[("a", 1), ("b", 2)]}, + last_two_duplicate_with_disparate_timestamp = {&[("a", 1), ("b", 2), ("b", 1)], &[("a", 1), ("b", 2)]}, + all_duplicate_with_disparate_timestamps = {&[("a", 100), ("a", 10), ("a", 1), ("b", 200), ("b", 20), ("c", 300)], &[("a", 100), ("b", 200), ("c", 300)]}, + )] + fn deduping(unsorted: &[(&str, u64)], expected: &[(&str, u64)]) { + let deduped = dedup(unsorted.to_vec()); + assert_eq!(deduped, expected) + } + + #[test] + fn sort_and_dedup_with_no_elements() { + let deduped: Vec<(&str, u64)> = sort_and_dedup([].iter()); + assert_eq!(deduped, vec![]) + } + + #[test] + fn sort_and_dedup_doc_example() { + // The example cant be run as a doctest without making this module public + let values = [("a", 1), ("a", 2), ("b", 3), ("b", 4)]; + let deduped = sort_and_dedup(values.iter()); + assert_eq!(deduped, vec![("a", 2), ("b", 4)]); + } +} diff --git a/consensus/service/src/tx_manager/mod.rs b/consensus/service/src/tx_manager/mod.rs index 2f1b5f5831..4ccb86f8ae 100644 --- a/consensus/service/src/tx_manager/mod.rs +++ b/consensus/service/src/tx_manager/mod.rs @@ -8,7 +8,8 @@ //! "working set" of transactions that the consensus service may operate on. #![allow(clippy::result_large_err)] -use crate::counters; + +use crate::{counters, timestamp_validator}; use mc_attest_enclave_api::{EnclaveMessage, PeerSession}; use mc_common::{ logger::{log, Logger}, @@ -21,7 +22,10 @@ use mc_transaction_core::{ constants::MAX_TRANSACTIONS_PER_BLOCK, tx::{TxHash, TxOutMembershipProof}, }; -use std::sync::{Arc, Mutex, MutexGuard}; +use std::{ + iter::repeat, + sync::{Arc, Mutex, MutexGuard}, +}; mod error; mod tx_manager_trait; @@ -111,30 +115,34 @@ impl TxManagerImpl( cache: &'a MutexGuard>, tx_hashes: I, - ) -> Result, TxManagerError> + ) -> Result, TxManagerError> where - I: Iterator, + I: Iterator, { // Split `tx_hashes` into a list of found hashes and missing ones. This allows // us to return an error with the entire list of missing hashes. let (entries, not_found) = tx_hashes - .map(|tx_hash| { - cache - .get(tx_hash) - .map_or_else(|| (*tx_hash, None), |entry| (*tx_hash, Some(entry))) + .map(|(tx_hash, timestamp)| { + cache.get(tx_hash).map_or_else( + || (*tx_hash, None, timestamp), + |entry| (*tx_hash, Some(entry), timestamp), + ) }) - .partition::, _>(|(_tx_hash, result)| result.is_some()); + .partition::, _>(|(_tx_hash, result, _timestamp)| result.is_some()); // If we are missing any hashes, return error. if !not_found.is_empty() { - let not_found_tx_hashes = not_found.into_iter().map(|(tx_hash, _)| tx_hash).collect(); + let not_found_tx_hashes = not_found + .into_iter() + .map(|(tx_hash, _, _)| tx_hash) + .collect(); return Err(TxManagerError::NotInCache(not_found_tx_hashes)); } // Otherwise, return the found entries. Ok(entries .into_iter() - .map(|(_tx_hash, entry)| entry.unwrap()) + .map(|(_tx_hash, entry, timestamp)| (entry.unwrap(), *timestamp)) .collect()) } } @@ -216,7 +224,7 @@ impl TxManager /// Validate the transaction corresponding to the given hash against the /// current ledger. - fn validate(&self, tx_hash: &TxHash) -> TxManagerResult<()> { + fn validate(&self, tx_hash: &TxHash, timestamp: Option) -> TxManagerResult<()> { let context_opt = { let cache = self.lock_cache(); cache.get(tx_hash).map(|entry| entry.context.clone()) @@ -224,7 +232,10 @@ impl TxManager if let Some(context) = context_opt { let _timer = counters::VALIDATE_TX_TIME.start_timer(); - self.untrusted.is_valid(context)?; + self.untrusted.is_valid(context, timestamp).map_err(|e| { + log::warn!(self.logger, "Failed validating tx_hash {tx_hash}: {e}"); + e + })?; Ok(()) } else { log::warn!( @@ -237,15 +248,15 @@ impl TxManager } /// Combines the transactions that correspond to the given hashes. - fn combine(&self, tx_hashes: &[TxHash]) -> TxManagerResult> { - let tx_hashes: HashSet<&TxHash> = tx_hashes.iter().collect(); // Dedup + fn combine(&self, tx_hashes: &[(TxHash, u64)]) -> TxManagerResult> { + let tx_hashes = timestamp_validator::sort_and_dedup(tx_hashes.iter()); let cache = self.lock_cache(); - let cache_entries = Self::get_cache_entries(&cache, tx_hashes.iter().copied())?; + let cache_entries = Self::get_cache_entries(&cache, tx_hashes.iter())?; let tx_contexts = cache_entries .iter() - .map(|entry| entry.context.clone()) + .map(|(entry, timestamp)| (entry.context.clone(), *timestamp)) .collect::>(); // Perform the combine operation. @@ -265,11 +276,14 @@ impl TxManager tx_hashes: &[TxHash], ) -> TxManagerResult)>> { let cache = self.lock_cache(); - let cache_entries = Self::get_cache_entries(&cache, tx_hashes.iter())?; + // We don't have a timestamp for each transaction and we aren't going + // to return one, so we use 0 for the timestamp used in `get_cache_entries` + let hashes = tx_hashes.iter().cloned().zip(repeat(0)).collect::>(); + let cache_entries = Self::get_cache_entries(&cache, hashes.iter())?; cache_entries .into_iter() - .map(|entry| { + .map(|(entry, _timestamp)| { // Highest indices proofs must be w.r.t. the current ledger. // Recreating them here is a crude way to ensure that. let highest_index_proofs: Vec<_> = self @@ -565,7 +579,7 @@ mod tests { .unwrap() .insert(tx_context.tx_hash, cache_entry); - assert!(tx_manager.validate(&tx_context.tx_hash).is_ok()); + assert!(tx_manager.validate(&tx_context.tx_hash, Some(555)).is_ok()); } #[test_with_logger] @@ -580,7 +594,7 @@ mod tests { let mock_enclave = MockConsensusEnclave::new(); let tx_manager = TxManagerImpl::new(mock_enclave, mock_untrusted, logger); - match tx_manager.validate(&tx_context.tx_hash) { + match tx_manager.validate(&tx_context.tx_hash, Some(555)) { Err(TxManagerError::NotInCache(_)) => {} // This is expected. _ => panic!(), } @@ -617,7 +631,7 @@ mod tests { .unwrap() .insert(tx_context.tx_hash, cache_entry); - match tx_manager.validate(&tx_context.tx_hash) { + match tx_manager.validate(&tx_context.tx_hash, Some(555)) { Err(TxManagerError::TransactionValidation( TransactionValidationError::ContainsSpentKeyImage, )) => {} // This is expected. @@ -628,7 +642,7 @@ mod tests { #[test_with_logger] // Should return Ok if the transactions are in the cache. fn test_combine_ok(logger: Logger) { - let tx_hashes: Vec<_> = (0..10).map(|i| TxHash([i as u8; 32])).collect(); + let tx_hashes: Vec<_> = (0..10).map(|i| (TxHash([i as u8; 32]), i)).collect(); let mut mock_untrusted = MockUntrustedInterfaces::new(); let expected: Vec<_> = tx_hashes.iter().take(5).cloned().collect(); @@ -641,7 +655,7 @@ mod tests { let tx_manager = TxManagerImpl::new(mock_enclave, mock_untrusted, logger); // Add transactions to the cache. - for tx_hash in &tx_hashes { + for (tx_hash, _) in &tx_hashes { let context = WellFormedTxContext::new( Default::default(), *tx_hash, @@ -674,7 +688,9 @@ mod tests { // Should return Err if any transaction is not in the cache. fn test_combine_err_not_in_cache(logger: Logger) { let n_transactions = 10; - let tx_hashes: Vec<_> = (0..n_transactions).map(|i| TxHash([i as u8; 32])).collect(); + let tx_hashes: Vec<_> = (0..n_transactions) + .map(|i| (TxHash([i as u8; 32]), i)) + .collect(); // UntrustedInterfaces should not be called. let mock_untrusted = MockUntrustedInterfaces::new(); @@ -684,7 +700,7 @@ mod tests { let tx_manager = TxManagerImpl::new(mock_enclave, mock_untrusted, logger); // Add some transactions, but not all, to the cache. - for tx_hash in &tx_hashes[2..] { + for (tx_hash, _) in &tx_hashes[2..] { let context = WellFormedTxContext::new( Default::default(), *tx_hash, diff --git a/consensus/service/src/tx_manager/tx_manager_trait.rs b/consensus/service/src/tx_manager/tx_manager_trait.rs index 9df3b1f500..03338d30e9 100644 --- a/consensus/service/src/tx_manager/tx_manager_trait.rs +++ b/consensus/service/src/tx_manager/tx_manager_trait.rs @@ -29,10 +29,23 @@ pub trait TxManager: Send { /// Validate the transaction corresponding to the given hash against the /// current ledger. - fn validate(&self, tx_hash: &TxHash) -> TxManagerResult<()>; + /// + /// # Arguments + /// * `tx_hash` - The tx to validate. + /// * `timestamp` - The timestamp to validate. ms since Unix epoch. If None, + /// then only the validity of the `tx_hash` will be checked. + fn validate(&self, tx_hash: &TxHash, timestamp: Option) -> TxManagerResult<()>; /// Combines the transactions that correspond to the given hashes. - fn combine(&self, tx_hashes: &[TxHash]) -> TxManagerResult>; + /// + /// # Arguments + /// * `tx_hashes` - "Candidate" transactions, and their proposed timestamp + /// in ms since Unix epoch. + /// + /// Returns a bounded, deterministically-ordered list of transactions that + /// are safe to append to the ledger. If there are any duplicate + /// hashes the ones with the largest timestamp will be returned. + fn combine(&self, tx_hashes: &[(TxHash, u64)]) -> TxManagerResult>; /// Get an array of well-formed encrypted transactions and membership proofs /// that correspond to the provided tx hashes. diff --git a/consensus/service/src/tx_manager/untrusted_interfaces.rs b/consensus/service/src/tx_manager/untrusted_interfaces.rs index 51d7e5ac77..02d0f852f0 100644 --- a/consensus/service/src/tx_manager/untrusted_interfaces.rs +++ b/consensus/service/src/tx_manager/untrusted_interfaces.rs @@ -24,21 +24,34 @@ pub trait UntrustedInterfaces: Send + Sync { ) -> TransactionValidationResult<(u64, Vec)>; /// Checks if a transaction is valid (see definition in validators.rs). - fn is_valid(&self, context: Arc) -> TransactionValidationResult<()>; + /// + /// # Arguments + /// * `context` - The tx context to validate. + /// * `timestamp` - The timestamp to validate. ms since Unix epoch. If None, + /// then only the validity of the `context` will be checked. + fn is_valid( + &self, + context: Arc, + timestamp: Option, + ) -> TransactionValidationResult<()>; /// Combines a set of "candidate values" into a "composite value". /// This assumes all values are well-formed and safe to append to the ledger /// individually. /// /// # Arguments - /// * `tx_contexts` - "Candidate" transactions. Each is assumed to be - /// individually valid. + /// * `tx_contexts` - "Candidate" transactions and their proposed timestamp + /// in ms since Unix epoch. Each is assumed to be individually valid. /// * `max_elements` - Maximal number of elements to output. /// /// Returns a bounded, deterministically-ordered list of transactions that - /// are safe to append to the ledger. - fn combine(&self, tx_contexts: &[Arc], max_elements: usize) - -> Vec; + /// are safe to append to the ledger. If there are any duplicate + /// transactions the ones with the largest timestamp will be returned. + fn combine( + &self, + tx_contexts: &[(Arc, u64)], + max_elements: usize, + ) -> Vec<(TxHash, u64)>; fn get_tx_out_proof_of_memberships( &self, diff --git a/consensus/service/src/validators.rs b/consensus/service/src/validators.rs index c81015905e..df1815612a 100644 --- a/consensus/service/src/validators.rs +++ b/consensus/service/src/validators.rs @@ -4,25 +4,24 @@ //! the ledger. //! //! Validation is broken into two parts: -//! 1) "Well formed"-ness - A transaction is considered "well formed" if all the -//! data in it that is not affected by future changes to the ledger is -//! correct. This includes checks like inputs/outputs counts, range proofs, -//! signature validation, membership proofs, etc. A transaction that is -//! well-formed remains well-formed if additional transactions are appended -//! to the ledger. However, a transaction could transition from not well-formed -//! to well-formed: for example, the transaction may include inputs that are -//! not yet in the local ledger because the local ledger is out of sync with -//! the consensus ledger. -//! -//! 2) "Is valid [to add to the ledger]" - This checks whether a **single** -//! transaction can be safely appended to a ledger in it's current state. A -//! valid transaction must also be well-formed. +//! 1. "Well formed"-ness - A transaction is considered "well formed" if all +//! the data in it that is not affected by future changes to the ledger +//! is correct. This includes checks like inputs/outputs counts, range +//! proofs, signature validation, membership proofs, etc. A transaction +//! that is well-formed remains well-formed if additional transactions +//! are appended to the ledger. However, a transaction could transition +//! from not well-formed to well-formed: for example, the transaction may +//! include inputs that are not yet in the local ledger because the local +//! ledger is out of sync with the consensus ledger. +//! 2. "Is valid [to add to the ledger]" - This checks whether a **single** +//! transaction can be safely appended to a ledger in it's current +//! state. A valid transaction must also be well-formed. //! //! This definition differs from what the `mc_transaction_core::validation` //! module - the check provided by it is actually the "Is well formed" check, //! and might be renamed in the future to match this. -use crate::tx_manager::UntrustedInterfaces as TxManagerUntrustedInterfaces; +use crate::{timestamp_validator, tx_manager::UntrustedInterfaces as TxManagerUntrustedInterfaces}; use mc_consensus_enclave::{TxContext, WellFormedTxContext}; use mc_crypto_keys::CompressedRistrettoPublic; use mc_ledger_db::{Error as LedgerError, Ledger}; @@ -71,16 +70,24 @@ impl TxManagerUntrustedInterfaces for DefaultTxManagerUntruste } /// Checks if a transaction is valid (see definition at top of this file). - fn is_valid(&self, context: Arc) -> TransactionValidationResult<()> { - // Get the index of the current block we will be building. - let current_block_index = self + fn is_valid( + &self, + context: Arc, + timestamp: Option, + ) -> TransactionValidationResult<()> { + let latest_block = self .ledger - .num_blocks() + .get_latest_block() .map_err(|e| TransactionValidationError::Ledger(e.to_string()))?; + if let Some(timestamp) = timestamp { + timestamp_validator::validate(timestamp, &latest_block) + .map_err(|e| TransactionValidationError::Timestamp(e.to_string()))?; + } + // The transaction must not have expired, and the tombstone block must not be // too far in the future. - validate_tombstone(current_block_index, context.tombstone_block())?; + validate_tombstone(latest_block.index + 1, context.tombstone_block())?; // The `key_images` must not have already been spent. let contains_spent_key_image = context @@ -122,9 +129,9 @@ impl TxManagerUntrustedInterfaces for DefaultTxManagerUntruste /// are safe to append to the ledger. fn combine( &self, - tx_contexts: &[Arc], + tx_contexts: &[(Arc, u64)], max_elements: usize, - ) -> Vec { + ) -> Vec<(TxHash, u64)> { // WellFormedTxContext defines the sort order of transactions within a block. let mut candidates: Vec<_> = tx_contexts.to_vec(); candidates.sort(); @@ -135,7 +142,7 @@ impl TxManagerUntrustedInterfaces for DefaultTxManagerUntruste let mut used_key_images: HashSet<&KeyImage> = HashSet::default(); let mut used_output_public_keys: HashSet<&CompressedRistrettoPublic> = HashSet::default(); - for candidate in &candidates { + for (candidate, timestamp) in &candidates { // Enforce maximum size. if allowed_hashes.len() >= max_elements { break; @@ -154,7 +161,7 @@ impl TxManagerUntrustedInterfaces for DefaultTxManagerUntruste } // The transaction is allowed. - allowed_hashes.push(*candidate.tx_hash()); + allowed_hashes.push((*candidate.tx_hash(), *timestamp)); used_key_images.extend(&key_images); used_output_public_keys.extend(&output_public_keys); } @@ -263,47 +270,56 @@ pub mod well_formed_tests { #[cfg(test)] mod is_valid_tests { use super::*; + use assert_matches::assert_matches; + use mc_blockchain_types::Block; use mc_ledger_db::{Error as LedgerError, MockLedger}; use mc_transaction_core::{ constants::MAX_TOMBSTONE_BLOCKS, validation::TransactionValidationError, }; - #[test] - /// `is_valid` should accept a valid transaction. - fn is_valid_ok() { - // Number of blocks in the local ledger. - let num_blocks = 53; + fn well_formed_tx_context(latest_block: &Block) -> WellFormedTxContext { + let key_images = vec![ + KeyImage::default(), + KeyImage::default(), + KeyImage::default(), + ]; - let well_formed_tx_context = { - let key_images = vec![ - KeyImage::default(), - KeyImage::default(), - KeyImage::default(), - ]; + let output_public_keys = vec![ + CompressedRistrettoPublic::default(), + CompressedRistrettoPublic::default(), + ]; - let output_public_keys = vec![ - CompressedRistrettoPublic::default(), - CompressedRistrettoPublic::default(), - ]; + WellFormedTxContext::new( + Default::default(), + Default::default(), + // "2", because the block this transaction would go on will have + // an index 1 greater than the latest block. The tombstone needs to be greater than + // the block being built for the transaction to be valid. + latest_block.index + 2, + key_images, + vec![9, 10, 8], + output_public_keys, + ) + } - WellFormedTxContext::new( - Default::default(), - Default::default(), - num_blocks + 17, - key_images, - vec![9, 10, 8], - output_public_keys, - ) + #[test] + /// `is_valid` should accept a valid transaction. + fn is_valid_ok() { + let latest_block = Block { + index: 52, + ..Default::default() }; + let well_formed_tx_context = well_formed_tx_context(&latest_block); + // Mock the local ledger. let mut ledger = MockLedger::new(); // Untrusted should request num_blocks. ledger - .expect_num_blocks() + .expect_get_latest_block() .times(1) - .return_const(Ok(num_blocks)); + .return_const(Ok(latest_block)); // Key images must not be in the ledger. ledger @@ -319,19 +335,24 @@ mod is_valid_tests { let untrusted = DefaultTxManagerUntrustedInterfaces::new(ledger); - assert_eq!(untrusted.is_valid(Arc::new(well_formed_tx_context)), Ok(())); + assert_eq!( + untrusted.is_valid(Arc::new(well_formed_tx_context), None), + Ok(()) + ); } #[test] /// `is_valid` should reject a transaction if num_blocks > tombstone_block. fn is_valid_rejects_expired_transaction() { - // Number of blocks in the local ledger. - let num_blocks = 53; + let latest_block = Block { + index: 52, + ..Default::default() + }; let well_formed_tx_context = WellFormedTxContext::new( Default::default(), Default::default(), - 17, // The local ledger has advanced beyond the tombstone block. + latest_block.index + 1, Default::default(), Default::default(), Default::default(), @@ -342,14 +363,14 @@ mod is_valid_tests { // Untrusted should request num_blocks. ledger - .expect_num_blocks() + .expect_get_latest_block() .times(1) - .return_const(Ok(num_blocks)); + .return_const(Ok(latest_block)); let untrusted = DefaultTxManagerUntrustedInterfaces::new(ledger); assert_eq!( - untrusted.is_valid(Arc::new(well_formed_tx_context)), + untrusted.is_valid(Arc::new(well_formed_tx_context), None), Err(TransactionValidationError::TombstoneBlockExceeded), ); } @@ -358,13 +379,15 @@ mod is_valid_tests { /// `is_valid` should reject a transaction if tombstone_block is too far in /// the future. fn is_valid_rejects_tombstone_too_far() { - // Number of blocks in the local ledger. - let num_blocks = 53; + let latest_block = Block { + index: 52, + ..Default::default() + }; let well_formed_tx_context = WellFormedTxContext::new( Default::default(), Default::default(), - num_blocks + MAX_TOMBSTONE_BLOCKS + 1, + latest_block.index + MAX_TOMBSTONE_BLOCKS + 2, Default::default(), Default::default(), Default::default(), @@ -375,14 +398,14 @@ mod is_valid_tests { // Untrusted should request num_blocks. ledger - .expect_num_blocks() + .expect_get_latest_block() .times(1) - .return_const(Ok(num_blocks)); + .return_const(Ok(latest_block)); let untrusted = DefaultTxManagerUntrustedInterfaces::new(ledger); assert_eq!( - untrusted.is_valid(Arc::new(well_formed_tx_context)), + untrusted.is_valid(Arc::new(well_formed_tx_context), None), Err(TransactionValidationError::TombstoneBlockTooFar), ); } @@ -390,34 +413,21 @@ mod is_valid_tests { #[test] /// `is_valid` should reject a transaction with an already spent key image. fn is_valid_rejects_spent_key_image() { - // Number of blocks in the local ledger. - let num_blocks = 53; - - let well_formed_tx_context = { - let key_images = vec![ - KeyImage::default(), - KeyImage::default(), - KeyImage::default(), - ]; - - WellFormedTxContext::new( - Default::default(), - Default::default(), - num_blocks + 17, - key_images, - Default::default(), - Default::default(), - ) + let latest_block = Block { + index: 52, + ..Default::default() }; + let well_formed_tx_context = well_formed_tx_context(&latest_block); + // Mock the local ledger. let mut ledger = MockLedger::new(); // Untrusted should request num_blocks. ledger - .expect_num_blocks() + .expect_get_latest_block() .times(1) - .return_const(Ok(num_blocks)); + .return_const(Ok(latest_block)); // A key image has been spent. ledger @@ -428,7 +438,7 @@ mod is_valid_tests { let untrusted = DefaultTxManagerUntrustedInterfaces::new(ledger); assert_eq!( - untrusted.is_valid(Arc::new(well_formed_tx_context)), + untrusted.is_valid(Arc::new(well_formed_tx_context), None), Err(TransactionValidationError::ContainsSpentKeyImage), ); } @@ -437,39 +447,21 @@ mod is_valid_tests { /// `is_valid` should reject a transaction with an already used output /// public key. fn is_valid_rejects_non_unique_output_public_key() { - // Number of blocks in the local ledger. - let num_blocks = 53; - - let well_formed_tx_context = { - let key_images = vec![ - KeyImage::default(), - KeyImage::default(), - KeyImage::default(), - ]; - - let output_public_keys = vec![ - CompressedRistrettoPublic::default(), - CompressedRistrettoPublic::default(), - ]; - - WellFormedTxContext::new( - Default::default(), - Default::default(), - num_blocks + 17, - key_images, - vec![9, 10, 8], - output_public_keys, - ) + let latest_block = Block { + index: 52, + ..Default::default() }; + let well_formed_tx_context = well_formed_tx_context(&latest_block); + // Mock the local ledger. let mut ledger = MockLedger::new(); // Untrusted should request num_blocks. ledger - .expect_num_blocks() + .expect_get_latest_block() .times(1) - .return_const(Ok(num_blocks)); + .return_const(Ok(latest_block)); // Key images must not be in the ledger. ledger @@ -486,10 +478,83 @@ mod is_valid_tests { let untrusted = DefaultTxManagerUntrustedInterfaces::new(ledger); assert_eq!( - untrusted.is_valid(Arc::new(well_formed_tx_context)), + untrusted.is_valid(Arc::new(well_formed_tx_context), None), Err(TransactionValidationError::ContainsExistingOutputPublicKey), ); } + + #[test] + fn valid_with_timestamp() { + let latest_block = Block { + index: 52, + ..Default::default() + }; + + let well_formed_tx_context = well_formed_tx_context(&latest_block); + + // Mock the local ledger. + let mut ledger = MockLedger::new(); + + // Untrusted should request num_blocks. + ledger + .expect_get_latest_block() + .times(1) + .return_const(Ok(latest_block)); + + // Key images must not be in the ledger. + ledger + .expect_contains_key_image() + .times(well_formed_tx_context.key_images().len()) + .return_const(Ok(false)); + + // Output public keys must not be in the ledger. + ledger + .expect_contains_tx_out_public_key() + .times(well_formed_tx_context.output_public_keys().len()) + .return_const(Ok(false)); + + let untrusted = DefaultTxManagerUntrustedInterfaces::new(ledger); + + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as u64; + + assert_eq!( + untrusted.is_valid(Arc::new(well_formed_tx_context), Some(now)), + Ok(()) + ); + } + + #[test] + fn invalid_with_a_timestamp_that_is_very_old() { + let latest_block = Block { + index: 52, + ..Default::default() + }; + + let well_formed_tx_context = well_formed_tx_context(&latest_block); + + // Mock the local ledger. + let mut ledger = MockLedger::new(); + + // Untrusted should request num_blocks. + ledger + .expect_get_latest_block() + .times(1) + .return_const(Ok(latest_block)); + + let untrusted = DefaultTxManagerUntrustedInterfaces::new(ledger); + + // 1970 might be a bit extreme, but we only want to show we hooked up + // the timestamp validator, not the edge cases of it. + let timestamp_too_old = 0; + + assert_matches!( + untrusted.is_valid(Arc::new(well_formed_tx_context), Some(timestamp_too_old)), + Err(TransactionValidationError::Timestamp(_)) + ); + } } #[cfg(test)] @@ -510,10 +575,13 @@ mod combine_tests { use rand::SeedableRng; use rand_hc::Hc128Rng; - fn combine(tx_contexts: Vec, max_elements: usize) -> Vec { + fn combine(tx_contexts: Vec, max_elements: usize) -> Vec<(TxHash, u64)> { let ledger = get_mock_ledger(10); let untrusted = DefaultTxManagerUntrustedInterfaces::new(ledger); - let tx_contexts: Vec<_> = tx_contexts.into_iter().map(Arc::new).collect(); + let tx_contexts: Vec<_> = tx_contexts + .into_iter() + .map(|tx| (Arc::new(tx), 10)) + .collect(); untrusted.combine(&tx_contexts, max_elements) } @@ -894,7 +962,7 @@ mod combine_tests { // `combine` should only allow one of the transactions that attempts to use the // same key image. assert_eq!(combined_transactions.len(), 2); - assert!(combined_transactions.contains(third_client_tx.tx_hash())); + assert!(combined_transactions.contains(&(*third_client_tx.tx_hash(), 10))); } } @@ -1104,7 +1172,7 @@ mod combine_tests { // `combine` should only allow one of the transactions that attempts to use the // same output public key. assert_eq!(combined_transactions.len(), 2); - assert!(combined_transactions.contains(third_client_tx.tx_hash())); + assert!(combined_transactions.contains(&(*third_client_tx.tx_hash(), 10))); } } @@ -1119,7 +1187,11 @@ mod combine_tests { let hashes = combine(tx_contexts, 10); // Transactions should be ordered from highest fee to lowest fee. - let expected_hashes = vec![TxHash([2u8; 32]), TxHash([1u8; 32]), TxHash([3u8; 32])]; + let expected_hashes = vec![ + (TxHash([2u8; 32]), 10), + (TxHash([1u8; 32]), 10), + (TxHash([3u8; 32]), 10), + ]; assert_eq!(hashes, expected_hashes); } } diff --git a/peers/src/consensus_msg.rs b/peers/src/consensus_msg.rs index 504049c53c..a80ca25850 100644 --- a/peers/src/consensus_msg.rs +++ b/peers/src/consensus_msg.rs @@ -37,6 +37,35 @@ impl From for ConsensusValue { } } +impl From for ConsensusValue { + fn from(config: MintConfigTx) -> Self { + Self::MintConfigTx(config) + } +} + +impl From for ConsensusValue { + fn from(tx: MintTx) -> Self { + Self::MintTx(tx) + } +} + +#[derive( + Clone, Debug, Deserialize, Digestible, Display, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize, +)] +/// A [`ConsensusValue`] with a timestamp of when it was proposed. +pub struct ConsensusValueWithTimestamp { + /// The consensus value {0}. + pub value: ConsensusValue, + /// Timestamp {0} ms since UNIX epoch. + pub timestamp: u64, +} + +impl ConsensusValueWithTimestamp { + pub fn new(value: ConsensusValue, timestamp: u64) -> Self { + Self { value, timestamp } + } +} + /// A consensus message holds the data that is exchanged by consensus service /// nodes as part of the process of reaching agreement on the contents of the /// next block. @@ -44,7 +73,7 @@ impl From for ConsensusValue { pub struct ConsensusMsg { /// An SCP message, used to reach agreement on the set of values the next /// block will contain. - pub scp_msg: Msg, + pub scp_msg: Msg, /// The block ID of the block the message is trying to append values to. pub prev_block_id: BlockID, @@ -60,7 +89,7 @@ pub struct VerifiedConsensusMsg { } impl VerifiedConsensusMsg { - pub fn scp_msg(&self) -> &Msg { + pub fn scp_msg(&self) -> &Msg { &self.inner.scp_msg } @@ -143,7 +172,7 @@ impl From for ConsensusMsgError { impl ConsensusMsg { pub fn from_scp_msg( ledger: &impl Ledger, - scp_msg: Msg, + scp_msg: Msg, signer_key: &Ed25519Pair, ) -> StdResult { if scp_msg.slot_index == 0 { @@ -222,7 +251,13 @@ mod tests { local_quorum_set, num_blocks as u64, Topic::Commit(CommitPayload { - B: Ballot::new(100, &[ConsensusValue::TxHash(hash_tx)]), + B: Ballot::new( + 100, + &[ConsensusValueWithTimestamp { + value: ConsensusValue::TxHash(hash_tx), + timestamp: 0, + }], + ), PN: 77, CN: 55, HN: 66, @@ -270,11 +305,11 @@ mod tests { assert_eq!(msg.scp_msg.quorum_set, m); let ser = mc_util_serial::serialize(&msg.scp_msg.topic).unwrap(); - let m: Topic = mc_util_serial::deserialize(&ser).unwrap(); + let m: Topic = mc_util_serial::deserialize(&ser).unwrap(); assert_eq!(msg.scp_msg.topic, m); let ser = mc_util_serial::serialize(&msg.scp_msg).unwrap(); - let m: Msg = mc_util_serial::deserialize(&ser).unwrap(); + let m: Msg = mc_util_serial::deserialize(&ser).unwrap(); assert_eq!(msg.scp_msg, m); let ser = mc_util_serial::serialize(&msg.prev_block_id).unwrap(); diff --git a/peers/src/lib.rs b/peers/src/lib.rs index fd09881daf..120c30bc7e 100644 --- a/peers/src/lib.rs +++ b/peers/src/lib.rs @@ -18,7 +18,8 @@ pub use crate::{ broadcast::{Broadcast, MockBroadcast}, connection::PeerConnection, consensus_msg::{ - ConsensusMsg, ConsensusMsgError, ConsensusValue, TxProposeAAD, VerifiedConsensusMsg, + ConsensusMsg, ConsensusMsgError, ConsensusValue, ConsensusValueWithTimestamp, TxProposeAAD, + VerifiedConsensusMsg, }, error::{Error, Result}, threaded_broadcaster::ThreadedBroadcaster, diff --git a/peers/test-utils/src/lib.rs b/peers/test-utils/src/lib.rs index c44446b5d9..35858c7b86 100644 --- a/peers/test-utils/src/lib.rs +++ b/peers/test-utils/src/lib.rs @@ -19,7 +19,8 @@ use mc_consensus_scp::{ use mc_crypto_keys::{Ed25519Pair, Ed25519Public}; use mc_ledger_db::{test_utils::MockLedger, Ledger}; use mc_peers::{ - ConsensusConnection, ConsensusMsg, ConsensusValue, Error as PeerError, Result as PeerResult, + ConsensusConnection, ConsensusMsg, ConsensusValue, ConsensusValueWithTimestamp, + Error as PeerError, Result as PeerResult, }; use mc_transaction_core::tx::TxHash; use mc_util_uri::{ConnectionUri, ConsensusPeerUri as PeerUri}; @@ -230,7 +231,10 @@ pub fn create_consensus_msg( ) -> ConsensusMsg { let msg_hash = TxHash::try_from(Sha512_256::digest(msg.as_bytes()).as_slice()) .expect("Could not hash message into TxHash"); - let value = ConsensusValue::TxHash(msg_hash); + let value = ConsensusValueWithTimestamp { + value: ConsensusValue::TxHash(msg_hash), + timestamp: 1, + }; let mut payload = NominatePayload { X: BTreeSet::default(), Y: BTreeSet::default(), diff --git a/transaction/core/src/validation/error.rs b/transaction/core/src/validation/error.rs index be6bff1d5a..32be105ae5 100644 --- a/transaction/core/src/validation/error.rs +++ b/transaction/core/src/validation/error.rs @@ -153,6 +153,9 @@ pub enum TransactionValidationError { /// Unknown Masked Amount version UnknownMaskedAmountVersion, + + /// Tx Timestamp: {0} + Timestamp(String), } impl From for TransactionValidationError {