diff --git a/Cargo.lock b/Cargo.lock index 2dfd4ceb0c..4276b9f57d 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/api/proto/blockchain.proto b/api/proto/blockchain.proto index 205ceb4505..60e185bd86 100644 --- a/api/proto/blockchain.proto +++ b/api/proto/blockchain.proto @@ -43,6 +43,9 @@ message Block { // Hash of the block's contents. BlockContentsHash contents_hash = 7; + + // The block's timestamp. ms since Unix epoch + uint64 timestamp = 8; } message BlockContents { diff --git a/api/src/convert/block.rs b/api/src/convert/block.rs index 1194ee5f98..38ad008c50 100644 --- a/api/src/convert/block.rs +++ b/api/src/convert/block.rs @@ -17,6 +17,7 @@ impl From<&Block> for blockchain::Block { block.set_cumulative_txo_count(other.cumulative_txo_count); block.set_root_element((&other.root_element).into()); block.set_contents_hash(blockchain::BlockContentsHash::from(&other.contents_hash)); + block.set_timestamp(other.timestamp); block } } @@ -39,6 +40,7 @@ impl TryFrom<&blockchain::Block> for Block { cumulative_txo_count: value.cumulative_txo_count, root_element, contents_hash, + timestamp: value.timestamp, }; Ok(block) } @@ -65,6 +67,7 @@ mod tests { hash: TxOutMembershipHash::from([12u8; 32]), }, contents_hash: BlockContentsHash::try_from(&[66u8; 32][..]).unwrap(), + timestamp: 357, }; let block = blockchain::Block::from(&source_block); @@ -77,6 +80,7 @@ mod tests { assert_eq!(block.get_root_element().get_range().get_to(), 20); assert_eq!(block.get_root_element().get_hash().get_data(), &[12u8; 32]); assert_eq!(block.get_contents_hash().get_data(), [66u8; 32]); + assert_eq!(block.get_timestamp(), 357); } #[test] @@ -103,6 +107,7 @@ mod tests { source_block.set_index(2); source_block.set_root_element(root_element); source_block.set_contents_hash(contents_hash); + source_block.set_timestamp(411); let block = Block::try_from(&source_block).unwrap(); assert_eq!(block.id.as_ref(), [10u8; 32]); @@ -113,6 +118,7 @@ mod tests { assert_eq!(block.root_element.range.to, 20); assert_eq!(block.root_element.hash.as_ref(), &[13u8; 32]); assert_eq!(block.contents_hash.as_ref(), [66u8; 32]); + assert_eq!(block.timestamp, 411); } #[test] @@ -131,6 +137,7 @@ mod tests { hash: TxOutMembershipHash::from([12u8; 32]), }, contents_hash: BlockContentsHash::try_from(&[66u8; 32][..]).unwrap(), + timestamp: 911, }; // Encode using `protobuf`, decode using `prost`. diff --git a/blockchain/test-utils/src/lib.rs b/blockchain/test-utils/src/lib.rs index f877a0547a..3c3fb4f2b3 100644 --- a/blockchain/test-utils/src/lib.rs +++ b/blockchain/test-utils/src/lib.rs @@ -111,7 +111,18 @@ pub fn get_blocks_with_recipients( let block = match &prev_block { Some(parent) => { - Block::new_with_parent(block_version, parent, &Default::default(), &block_contents) + let timestamp = if block_version.timestamps_are_supported() { + parent.timestamp + 1 + } else { + 0 + }; + Block::new_with_parent( + block_version, + parent, + &Default::default(), + &block_contents, + timestamp, + ) } None => Block::new_origin_block(&block_contents.outputs), }; @@ -242,6 +253,7 @@ mod tests { block.cumulative_txo_count, &block.root_element, &block.contents_hash, + block.timestamp, ); assert_eq!(derived_block_id, block.id); diff --git a/blockchain/types/src/block.rs b/blockchain/types/src/block.rs index aa63d59bd9..5e6c7c96be 100644 --- a/blockchain/types/src/block.rs +++ b/blockchain/types/src/block.rs @@ -45,6 +45,11 @@ pub struct Block { /// Hash of the block's contents. #[prost(message, required, tag = "7")] pub contents_hash: BlockContentsHash, + + /// Timestamp of the block. ms since Unix epoch + #[prost(uint64, tag = "8")] + #[digestible(omit_when = 0)] + pub timestamp: u64, } impl Block { @@ -58,6 +63,7 @@ impl Block { let index: BlockIndex = 0; let cumulative_txo_count = outputs.len() as u64; let root_element = TxOutMembershipElement::default(); + let timestamp = 0; // The origin block does not contain anything but TxOuts. let block_contents = BlockContents { @@ -73,6 +79,7 @@ impl Block { cumulative_txo_count, &root_element, &contents_hash, + timestamp, ); Self { id, @@ -82,6 +89,7 @@ impl Block { cumulative_txo_count, root_element, contents_hash, + timestamp, } } @@ -100,6 +108,7 @@ impl Block { parent: &Block, root_element: &TxOutMembershipElement, block_contents: &BlockContents, + timestamp: u64, ) -> Self { Block::new( version, @@ -108,6 +117,7 @@ impl Block { parent.cumulative_txo_count + block_contents.outputs.len() as u64, root_element, block_contents, + timestamp, ) } @@ -123,6 +133,7 @@ impl Block { /// block* /// * `root_element` - The root element for membership proofs /// * `block_contents` - Contents of the block. + /// * `timestamp` - The timestamp of the block in ms. pub fn new( version: BlockVersion, parent_id: &BlockID, @@ -130,6 +141,7 @@ impl Block { cumulative_txo_count: u64, root_element: &TxOutMembershipElement, block_contents: &BlockContents, + timestamp: u64, ) -> Self { let contents_hash = block_contents.hash(); let id = compute_block_id( @@ -139,6 +151,7 @@ impl Block { cumulative_txo_count, root_element, &contents_hash, + timestamp, ); Self { @@ -149,6 +162,7 @@ impl Block { cumulative_txo_count, root_element: root_element.clone(), contents_hash, + timestamp, } } @@ -165,6 +179,7 @@ impl Block { self.cumulative_txo_count, &self.root_element, &self.contents_hash, + self.timestamp, ); self.id == expected_id @@ -182,6 +197,7 @@ pub fn compute_block_id( cumulative_txo_count: u64, root_element: &TxOutMembershipElement, contents_hash: &BlockContentsHash, + timestamp: u64, ) -> BlockID { let mut transcript = MerlinTranscript::new(b"mobilecoin-block-id"); @@ -192,6 +208,12 @@ pub fn compute_block_id( root_element.append_to_transcript(b"root_element", &mut transcript); contents_hash.append_to_transcript(b"contents_hash", &mut transcript); + let timestamps_supported = + BlockVersion::try_from(version).map(|v| v.timestamps_are_supported()); + if timestamps_supported.unwrap_or_default() { + timestamp.append_to_transcript(b"timestamp", &mut transcript); + } + let mut result = [0u8; 32]; transcript.extract_digest(&mut result); @@ -252,7 +274,7 @@ mod block_tests { (key_images, outputs) } - fn get_block(rng: &mut RNG) -> Block { + fn get_block_version_1(rng: &mut RNG) -> Block { let bytes = [14u8; 32]; let parent_id = BlockID::try_from(&bytes[..]).unwrap(); @@ -270,6 +292,29 @@ mod block_tests { 400, &root_element, &block_contents, + 0, // timestamp of 0 for earlier block versions + ) + } + + fn get_block_version_4(rng: &mut RNG) -> Block { + let bytes = [14u8; 32]; + let parent_id = BlockID::try_from(&bytes[..]).unwrap(); + + let root_element = TxOutMembershipElement { + range: Range::new(0, 15).unwrap(), + hash: TxOutMembershipHash::from([0u8; 32]), + }; + + let block_contents = get_block_contents(rng); + + Block::new( + BlockVersion::FOUR, + &parent_id, + 3, + 400, + &root_element, + &block_contents, + 10, ) } @@ -298,6 +343,7 @@ mod block_tests { 400, &root_element, &block_contents, + 0, ) } @@ -305,7 +351,7 @@ mod block_tests { /// The block returned by `get_block` should have a valid BlockID. fn test_get_block_has_valid_id() { let mut rng = get_seeded_rng(); - let block = get_block(&mut rng); + let block = get_block_version_1(&mut rng); assert!(block.is_block_id_valid()); } @@ -313,7 +359,7 @@ mod block_tests { /// The block ID should depend on the block version. fn test_block_id_includes_version() { let mut rng = get_seeded_rng(); - let mut block = get_block(&mut rng); + let mut block = get_block_version_1(&mut rng); block.version += 1; assert!(!block.is_block_id_valid()); } @@ -322,7 +368,7 @@ mod block_tests { /// The block ID should depend on the parent_id. fn test_block_id_includes_parent_id() { let mut rng = get_seeded_rng(); - let mut block = get_block(&mut rng); + let mut block = get_block_version_1(&mut rng); let mut bytes = [0u8; 32]; rng.fill_bytes(&mut bytes); @@ -336,7 +382,7 @@ mod block_tests { /// The block ID should depend on the block's index. fn test_block_id_includes_block_index() { let mut rng = get_seeded_rng(); - let mut block = get_block(&mut rng); + let mut block = get_block_version_1(&mut rng); block.index += 1; assert!(!block.is_block_id_valid()); } @@ -345,7 +391,7 @@ mod block_tests { /// The block ID should depend on the root element. fn test_block_id_includes_root_element() { let mut rng = get_seeded_rng(); - let mut block = get_block(&mut rng); + let mut block = get_block_version_1(&mut rng); let wrong_root_element = TxOutMembershipElement { range: Range::new(13, 17).unwrap(), @@ -359,7 +405,7 @@ mod block_tests { /// The block ID should depend on the content_hash. fn test_block_id_includes_content_hash() { let mut rng = get_seeded_rng(); - let mut block = get_block(&mut rng); + let mut block = get_block_version_1(&mut rng); let mut bytes = [0u8; 32]; rng.fill_bytes(&mut bytes); @@ -386,7 +432,7 @@ mod block_tests { let mut rng = get_seeded_rng(); //Check hash with memo - let block = get_block(&mut rng); + let block = get_block_version_1(&mut rng); assert_eq!( block.id.as_ref(), &[ @@ -422,4 +468,82 @@ mod block_tests { ] ); } + #[test] + /// The block ID hash do not change as the code evolves. + /// This test was written by writing a failed assert and then copying the + /// actual block id into the test. This should hopefully catches cases where + /// we add/change Block/BlockContents and accidentally break id + /// calculation of old blocks. + fn test_hashing_is_consistent_block_version_four() { + let mut rng = get_seeded_rng(); + + let block = get_block_version_4(&mut rng); + assert_eq!( + block.id.as_ref(), + &[ + 156, 155, 244, 98, 84, 234, 204, 146, 224, 142, 236, 197, 11, 69, 5, 74, 109, 160, + 123, 173, 206, 100, 224, 171, 72, 35, 208, 137, 150, 168, 43, 93 + ] + ); + } + + #[test] + fn test_block_version_3_ignores_timestamp_in_id() { + 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 timestamp_1 = 1; + let id_1 = compute_block_id( + 3, + &parent_id, + index, + cumulative_txo_count, + &root_element, + &contents_hash, + timestamp_1, + ); + let timestamp_2 = 2; + let id_2 = compute_block_id( + 3, + &parent_id, + index, + cumulative_txo_count, + &root_element, + &contents_hash, + timestamp_2, + ); + assert_eq!(id_1, id_2); + } + + #[test] + fn test_block_version_4_takes_timestamp_into_account() { + 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 timestamp_1 = 1; + let id_1 = compute_block_id( + 4, + &parent_id, + index, + cumulative_txo_count, + &root_element, + &contents_hash, + timestamp_1, + ); + let timestamp_2 = 2; + let id_2 = compute_block_id( + 4, + &parent_id, + index, + cumulative_txo_count, + &root_element, + &contents_hash, + timestamp_2, + ); + assert_ne!(id_1, id_2); + } } 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..ebb3bf8232 100644 --- a/consensus/enclave/api/src/lib.rs +++ b/consensus/enclave/api/src/lib.rs @@ -222,6 +222,9 @@ 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. + 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 fc1b9695f8..cbd3807806 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 @@ -937,13 +952,16 @@ impl ConsensusEnclave for SgxConsensusEnclave { validated_mint_config_txs, mint_txs, }; - // + + let timestamp = Self::validate_timestamp(inputs.timestamp, parent_block)?; + // Form the block. let block = Block::new_with_parent( config.block_version, parent_block, root_element, &block_contents, + timestamp, ); // Sign the block. @@ -1034,6 +1052,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}; @@ -1056,6 +1076,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 @@ -1432,6 +1453,7 @@ mod tests { &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -1609,6 +1631,7 @@ mod tests { &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -1784,6 +1807,7 @@ mod tests { &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -1901,6 +1925,7 @@ mod tests { &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2012,6 +2037,7 @@ mod tests { &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2102,6 +2128,7 @@ mod tests { &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2191,6 +2218,7 @@ mod tests { &parent_block, FormBlockInputs { well_formed_encrypted_txs_with_proofs, + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2296,6 +2324,7 @@ mod tests { mint_config_tx2.prefix.configs[0].clone(), ), ], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2460,6 +2489,7 @@ mod tests { mint_config_tx.clone(), mint_config_tx.prefix.configs[0].clone(), )], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2631,6 +2661,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, @@ -2651,6 +2682,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, @@ -2732,6 +2764,7 @@ mod tests { mint_config_tx1.prefix.configs[0].clone(), ), ], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2817,6 +2850,7 @@ mod tests { mint_config_tx1.clone(), mint_config_tx1.prefix.configs[0].clone(), )], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -2886,6 +2920,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, @@ -2978,6 +3013,7 @@ mod tests { &parent_block, FormBlockInputs { mint_config_txs: vec![mint_config_tx1.clone()], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -3044,6 +3080,7 @@ mod tests { &parent_block, FormBlockInputs { mint_config_txs: vec![mint_config_tx1.clone()], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -3105,6 +3142,7 @@ mod tests { &parent_block, FormBlockInputs { mint_config_txs: vec![mint_config_tx1.clone(), mint_config_tx1.clone()], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -3240,6 +3278,7 @@ mod tests { mint_config_tx2.prefix.configs[0].clone(), ), ], + timestamp: parent_block.timestamp + 1, ..Default::default() }, &root_element, @@ -3372,6 +3411,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, @@ -3563,4 +3603,158 @@ 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(); + + assert_matches!( + enclave.form_block( + &parent_block, + FormBlockInputs { + well_formed_encrypted_txs_with_proofs, + ..Default::default() + }, + &root_element, + ), + Err(Error::FormBlock(_)) + ); + } + } } diff --git a/consensus/enclave/mock/src/lib.rs b/consensus/enclave/mock/src/lib.rs index 63417a5f35..c7ff651a23 100644 --- a/consensus/enclave/mock/src/lib.rs +++ b/consensus/enclave/mock/src/lib.rs @@ -350,8 +350,13 @@ impl ConsensusEnclave for ConsensusServiceMockEnclave { validated_mint_config_txs, }; - let block = - Block::new_with_parent(block_version, parent_block, root_element, &block_contents); + let block = Block::new_with_parent( + block_version, + parent_block, + root_element, + &block_contents, + inputs.timestamp, + ); let signature = BlockSignature::from_block_and_keypair(&block, &self.signing_keypair)?; 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 a518ad0039..f1a6573445 100644 --- a/consensus/service/src/byzantine_ledger/pending_values.rs +++ b/consensus/service/src/byzantine_ledger/pending_values.rs @@ -22,15 +22,16 @@ pub struct PendingValues { /// We need to store pending values vec so we can process values /// on a first-come first-served basis. However, we want to be able to: - /// 1) Efficiently see if we already have a given transaction and ignore - /// duplicates 2) Track how long each transaction took to externalize. + /// 1. Efficiently see if we already have a given transaction and ignore + /// duplicates + /// 2. Track how long each transaction took to externalize. /// /// To accomplish these goals we store, in addition to the queue of pending /// values, a map that maps a value to when we first encountered it. /// This essentially gives us an ordered HashMap. /// - /// Note that we only store a timestamp for values that were handed to us - /// directly from a client. That behavior is enforced by + /// Note that we only store a received time for values that were handed to + /// us directly from a client. That behavior is enforced by /// ByzantineLedger. We skip tracking processing times for relayed /// values since we want to track the time from when the network first /// saw a value, and not when a specific node saw it. @@ -65,17 +66,17 @@ impl PendingValues { self.pending_values.len() } - /// Try and add a pending value, associated with a given timestamp, to the - /// list. Returns `true` if the value is valid and not already on the - /// list, false otherwise. - pub fn push(&mut self, value: ConsensusValue, timestamp: Option) -> bool { + /// Try and add a pending value, associated with the time the value was + /// received at, to the list. Returns `true` if the value is valid and + /// not already on the list, false otherwise. + pub fn push(&mut self, value: ConsensusValue, received_time: Option) -> bool { if let Vacant(entry) = self.pending_values_map.entry(value.clone()) { 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(timestamp); + entry.insert(received_time); self.pending_values.push(value); true } else { @@ -86,11 +87,11 @@ 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. - entry.insert(timestamp); + entry.insert(received_time); self.pending_values.push(value); true } else { @@ -99,9 +100,9 @@ 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(timestamp); + entry.insert(received_time); self.pending_values.push(value); true } else { @@ -119,8 +120,8 @@ impl PendingValues { self.pending_values.iter() } - /// Try and get the timestamp associated with a given value. - pub fn get_timestamp_for_value(&self, tx_hash: &ConsensusValue) -> Option { + /// Try and get the received time associated with a given value. + pub fn get_received_time_for_value(&self, tx_hash: &ConsensusValue) -> Option { self.pending_values_map.get(tx_hash).cloned().flatten() } @@ -146,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() } }); } @@ -180,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 = @@ -265,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 e525ffee2a..f05471077e 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,8 +560,11 @@ impl< // Update pending value processing time metrics. for value in externalized.iter() { - if let Some(timestamp) = self.pending_values.get_timestamp_for_value(value) { - let duration = Instant::now().saturating_duration_since(timestamp); + 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,8 @@ impl< .get_root_tx_out_membership_element() .expect("Failed getting root tx out membership element"); + 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 +891,7 @@ impl< well_formed_encrypted_txs_with_proofs, mint_config_txs, mint_txs_with_config, + timestamp, }, &root_element, ) @@ -962,7 +987,7 @@ mod tests { num_blocks: u64, ) -> ( MockConsensusEnclave, - MockScpNode, + MockScpNode, MockLedger, MockLedgerSync, MockTxManager, @@ -1325,7 +1350,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 +1358,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 +1368,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 +1464,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 +1518,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 +1700,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 +1757,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..dff038b949 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(timestamp, &latest_block)?; + } + // 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,25 @@ 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 - }); - - Ok(allowed_txs) + ) -> MintTxManagerResult> { + let mut candidates = timestamp_validator::sort_and_dedup(txs.iter()); + 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(timestamp, &latest_block)?; + } + // Ensure that we have not seen this transaction before. if self .ledger_db @@ -160,13 +154,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 +167,39 @@ 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 candidates = timestamp_validator::sort_and_dedup(txs.iter()); - 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 +233,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 +246,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 +271,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 +318,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 +363,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 +377,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 +407,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 +420,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 +451,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 +483,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 +544,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 +607,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 +654,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 +667,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 +688,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 +702,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 +744,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 +803,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 +815,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 +870,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 +886,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 +901,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 +947,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 +962,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 +980,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 +994,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 +1037,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 +1052,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 +1070,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 +1084,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 +1124,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 +1179,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 +1191,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 +1236,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 +1248,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 +1357,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 +1450,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 +1553,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..86259d76f2 --- /dev/null +++ b/consensus/service/src/timestamp_validator.rs @@ -0,0 +1,236 @@ +// Copyright (c) 2023 The MobileCoin Foundation + +use displaydoc::Display; +use mc_blockchain_types::Block; + +/// Provides logic for validating a timestamp used in consensus + +const MAX_TIMESTAMP_AGE: u64 = 30 * 1000; // 30 seconds + +/// Errors related to validating timestamps +#[derive(Clone, Debug, Display, Eq, PartialEq)] +pub enum Error { + /// The timestamp is 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), + /// The timestamp is too far in the past now: {0}, timestamp: {1} + TooOld(u64, u64), +} + +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 { + return Err(Error::InFuture(now, timestamp)); + } + + if timestamp + MAX_TIMESTAMP_AGE < now { + return Err(Error::TooOld(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(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. +fn sort<'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(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_older_than_30_seconds_fails() { + let now = std::time::SystemTime::now(); + // Need to add 1 since at MAX_TIMESTAMP_AGE is still good + let too_old = now + .checked_sub(std::time::Duration::from_millis(MAX_TIMESTAMP_AGE + 1)) + .unwrap() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as u64; + + let latest_block = Block::default(); + + assert_matches!( + validate(too_old, &latest_block), + Err(Error::TooOld(_, timestamp)) if timestamp == too_old + ); + } + + #[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(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(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..ce900bd962 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,7 @@ 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)?; Ok(()) } else { log::warn!( @@ -237,15 +245,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 +273,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, _)| { // 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 +576,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 +591,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 +628,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 +639,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 +652,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 +685,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 +697,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..84241ef155 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,6 +270,8 @@ 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, @@ -271,8 +280,10 @@ mod is_valid_tests { #[test] /// `is_valid` should accept a valid transaction. fn is_valid_ok() { - // Number of blocks in the local ledger. - let num_blocks = 53; + let latest_block = Block { + index: 52, + ..Default::default() + }; let well_formed_tx_context = { let key_images = vec![ @@ -289,7 +300,10 @@ mod is_valid_tests { WellFormedTxContext::new( Default::default(), Default::default(), - num_blocks + 17, + // "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, @@ -301,9 +315,9 @@ 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)); // Key images must not be in the ledger. ledger @@ -319,19 +333,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 +361,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 +377,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 +396,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,8 +411,10 @@ 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 latest_block = Block { + index: 52, + ..Default::default() + }; let well_formed_tx_context = { let key_images = vec![ @@ -403,7 +426,7 @@ mod is_valid_tests { WellFormedTxContext::new( Default::default(), Default::default(), - num_blocks + 17, + latest_block.index + 2, key_images, Default::default(), Default::default(), @@ -415,9 +438,9 @@ 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)); // A key image has been spent. ledger @@ -428,7 +451,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,8 +460,10 @@ 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 latest_block = Block { + index: 52, + ..Default::default() + }; let well_formed_tx_context = { let key_images = vec![ @@ -455,7 +480,7 @@ mod is_valid_tests { WellFormedTxContext::new( Default::default(), Default::default(), - num_blocks + 17, + latest_block.index + 2, key_images, vec![9, 10, 8], output_public_keys, @@ -467,9 +492,9 @@ 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)); // Key images must not be in the ledger. ledger @@ -486,10 +511,129 @@ 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 = { + 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(), + // "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, + ) + }; + + // 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 = { + 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(), + // "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, + ) + }; + + // 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 +654,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 +1041,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 +1251,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 +1266,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/fog/test_infra/src/db_tests.rs b/fog/test_infra/src/db_tests.rs index d750533a0e..3cc29eb075 100644 --- a/fog/test_infra/src/db_tests.rs +++ b/fog/test_infra/src/db_tests.rs @@ -675,6 +675,7 @@ pub fn random_block( 0, &Default::default(), &Default::default(), + 0, ); let test_rows: Vec = (0..num_txs).map(|_| random_tx_row(rng)).collect(); (block, test_rows) diff --git a/fog/view/server/test-utils/src/lib.rs b/fog/view/server/test-utils/src/lib.rs index cb88374685..2911e179a5 100644 --- a/fog/view/server/test-utils/src/lib.rs +++ b/fog/view/server/test-utils/src/lib.rs @@ -357,6 +357,7 @@ pub fn add_block_data( cumulative_tx_out_count, &Default::default(), &Default::default(), + 0, ), 0, txs, diff --git a/fog/view/server/tests/unary_smoke_tests.rs b/fog/view/server/tests/unary_smoke_tests.rs index 3c87ff0488..9a12111d0f 100644 --- a/fog/view/server/tests/unary_smoke_tests.rs +++ b/fog/view/server/tests/unary_smoke_tests.rs @@ -75,6 +75,7 @@ fn test_view_integration(view_omap_capacity: u64, store_count: usize, blocks_per 2, &Default::default(), &Default::default(), + 0, ), 0, &txs[..2], @@ -90,6 +91,7 @@ fn test_view_integration(view_omap_capacity: u64, store_count: usize, blocks_per 6, &Default::default(), &Default::default(), + 0, ), 0, &txs[2..6], @@ -113,6 +115,7 @@ fn test_view_integration(view_omap_capacity: u64, store_count: usize, blocks_per 12, &Default::default(), &Default::default(), + 0, ), 0, &txs[6..12], @@ -148,6 +151,7 @@ fn test_view_integration(view_omap_capacity: u64, store_count: usize, blocks_per 12, &Default::default(), &Default::default(), + 0, ), 0, &[], @@ -163,6 +167,7 @@ fn test_view_integration(view_omap_capacity: u64, store_count: usize, blocks_per 16, &Default::default(), &Default::default(), + 0, ), 0, &txs[12..16], @@ -180,6 +185,7 @@ fn test_view_integration(view_omap_capacity: u64, store_count: usize, blocks_per 20, &Default::default(), &Default::default(), + 0, ), 0, &txs[16..20], diff --git a/ledger/db/src/ledger_db.rs b/ledger/db/src/ledger_db.rs index b6172a64dd..c8986dc5d4 100644 --- a/ledger/db/src/ledger_db.rs +++ b/ledger/db/src/ledger_db.rs @@ -2100,6 +2100,7 @@ mod ledger_db_test { origin.block(), &Default::default(), &block_contents, + 1, ); assert_eq!( @@ -2400,6 +2401,7 @@ mod ledger_db_test { &last_block, &Default::default(), &block_contents, + last_block.timestamp + 1, ); assert_eq!( ledger_db.append_block(&invalid_block, &block_contents, None, None), @@ -2412,6 +2414,7 @@ mod ledger_db_test { &last_block, &Default::default(), &block_contents, + last_block.timestamp + 1, ); assert_eq!( ledger_db.append_block(&invalid_block, &block_contents, None, None), @@ -2450,6 +2453,7 @@ mod ledger_db_test { &last_block, &Default::default(), &block_contents, + last_block.timestamp + 1, ); let metadata = make_block_metadata(last_block.id.clone(), &mut rng); @@ -2504,6 +2508,7 @@ mod ledger_db_test { origin.cumulative_txo_count, &Default::default(), &block_contents, + 1, ); assert_eq!( ledger_db.append_block(&new_block, &block_contents, None, None), @@ -2584,6 +2589,7 @@ mod ledger_db_test { origin.block(), &Default::default(), &block_one_contents, + origin.block().timestamp + 1, ); assert_eq!( @@ -2645,6 +2651,7 @@ mod ledger_db_test { origin.block().cumulative_txo_count, &Default::default(), &block_contents, + origin.block().timestamp + 1, ); assert_eq!( @@ -2660,6 +2667,7 @@ mod ledger_db_test { origin.block().cumulative_txo_count, &Default::default(), &block_contents, + origin.block().timestamp + 1, ); assert_eq!( diff --git a/ledger/db/src/test_utils/mod.rs b/ledger/db/src/test_utils/mod.rs index c11846f920..c365a412d8 100644 --- a/ledger/db/src/test_utils/mod.rs +++ b/ledger/db/src/test_utils/mod.rs @@ -429,7 +429,13 @@ pub fn add_block_contents_to_ledger( let new_block = if num_blocks > 0 { let parent = ledger_db.get_block(num_blocks - 1)?; - Block::new_with_parent(block_version, &parent, &Default::default(), &block_contents) + Block::new_with_parent( + block_version, + &parent, + &Default::default(), + &block_contents, + parent.timestamp + 1, + ) } else { Block::new_origin_block(&block_contents.outputs) }; diff --git a/ledger/sync/src/ledger_sync/ledger_sync_service.rs b/ledger/sync/src/ledger_sync/ledger_sync_service.rs index 6f2b12f9fe..8a772fb450 100644 --- a/ledger/sync/src/ledger_sync/ledger_sync_service.rs +++ b/ledger/sync/src/ledger_sync/ledger_sync_service.rs @@ -831,6 +831,7 @@ pub fn identify_safe_blocks( block.cumulative_txo_count, &block.root_element, &block_contents.hash(), + block.timestamp, ); if block.id != derived_block_id { log::error!( diff --git a/light-client/relayer/tests/block_processing.rs b/light-client/relayer/tests/block_processing.rs index 95c8abcc41..7be4924269 100644 --- a/light-client/relayer/tests/block_processing.rs +++ b/light-client/relayer/tests/block_processing.rs @@ -45,6 +45,7 @@ fn append_tx_outs_as_block( &last_block, &Default::default(), &block_contents, + last_block.timestamp + 1, ); // Sign this block with all the signer identities diff --git a/light-client/verifier/src/verifier.rs b/light-client/verifier/src/verifier.rs index 5aa01ea5e9..ec31c33f8f 100644 --- a/light-client/verifier/src/verifier.rs +++ b/light-client/verifier/src/verifier.rs @@ -200,6 +200,7 @@ mod tests { 88, &Default::default(), &Default::default(), + 88, ); let lcv = get_light_client_verifier(BTreeSet::from([block88.id.clone()])); @@ -211,6 +212,7 @@ mod tests { 99, &Default::default(), &Default::default(), + 99, ); let block9999 = Block::new( Default::default(), @@ -219,6 +221,7 @@ mod tests { 9999, &Default::default(), &Default::default(), + 9999, ); let block99999 = Block::new( Default::default(), @@ -227,6 +230,7 @@ mod tests { 99999, &Default::default(), &Default::default(), + 99999, ); // Block 88 verifies even without any signatures, because we put it in the @@ -365,6 +369,7 @@ mod tests { 9999, &Default::default(), &bc, + 9999, ); let metadata = sign_block_id_for_test_node_ids(&block9999.id, &[1, 2, 3]); @@ -459,6 +464,7 @@ mod tests { 9999, &Default::default(), &bc, + 9999, ); let metadata = sign_block_id_for_test_node_ids(&block9999.id, &[1, 2, 3]); 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/tools/local-network/local_network.py b/tools/local-network/local_network.py index f42097e08b..e9e311729e 100755 --- a/tools/local-network/local_network.py +++ b/tools/local-network/local_network.py @@ -162,7 +162,7 @@ def __init__(self, name, node_num, client_port, peer_port, admin_port, admin_htt self.peers = peers self.quorum_set = quorum_set self.minimum_fee = 400_000_000 - self.block_version = block_version or 3 + self.block_version = block_version or 4 self.consensus_process = None self.ledger_distribution_process = None 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 { diff --git a/transaction/types/src/block_version.rs b/transaction/types/src/block_version.rs index 0277aa8b1b..d3ba49de56 100644 --- a/transaction/types/src/block_version.rs +++ b/transaction/types/src/block_version.rs @@ -154,6 +154,11 @@ impl BlockVersion { pub fn nested_multisigs_are_supported(&self) -> bool { self >= &Self::THREE } + + /// Timestamps are supported starting from v4. + pub fn timestamps_are_supported(&self) -> bool { + self >= &Self::FOUR + } } impl Deref for BlockVersion { diff --git a/util/generate-sample-ledger/src/lib.rs b/util/generate-sample-ledger/src/lib.rs index 868b67f96c..bafd2982da 100644 --- a/util/generate-sample-ledger/src/lib.rs +++ b/util/generate-sample-ledger/src/lib.rs @@ -65,7 +65,7 @@ pub fn bootstrap_ledger( ); let block_version = if max_token_id > 0 { - BlockVersion::THREE + BlockVersion::MAX } else { // This is historically the version created by bootstrap BlockVersion::ZERO