diff --git a/consensus/api/proto/consensus_common.proto b/consensus/api/proto/consensus_common.proto index 8f985c355b..04ce7883bf 100644 --- a/consensus/api/proto/consensus_common.proto +++ b/consensus/api/proto/consensus_common.proto @@ -56,14 +56,16 @@ enum ProposeTxResult { UnsortedKeyImages = 26; ContainsSpentKeyImage = 27; DuplicateKeyImages = 28; - MissingTxOutMembershipProof = 29; - InvalidTxOutMembershipProof = 30; - InvalidRistrettoPublicKey = 31; - InvalidLedgerContext = 32; - Ledger = 33; - MembershipProofValidationError = 34; - TxFeeError = 35; - KeyError = 36; + DuplicateOutputPublicKey = 29; + ContainsExistingOutputPublicKey = 30; + MissingTxOutMembershipProof = 31; + InvalidTxOutMembershipProof = 32; + InvalidRistrettoPublicKey = 33; + InvalidLedgerContext = 34; + Ledger = 35; + MembershipProofValidationError = 36; + TxFeeError = 37; + KeyError = 38; } /// Response from TxPropose RPC call. diff --git a/consensus/api/src/conversions.rs b/consensus/api/src/conversions.rs index 5d12aa117f..7931c7aba1 100644 --- a/consensus/api/src/conversions.rs +++ b/consensus/api/src/conversions.rs @@ -40,6 +40,10 @@ impl From for ProposeTxResult { TransactionValidationError::UnsortedKeyImages => Self::UnsortedKeyImages, TransactionValidationError::ContainsSpentKeyImage => Self::ContainsSpentKeyImage, TransactionValidationError::DuplicateKeyImages => Self::DuplicateKeyImages, + TransactionValidationError::DuplicateOutputPublicKey => Self::DuplicateOutputPublicKey, + TransactionValidationError::ContainsExistingOutputPublicKey => { + Self::ContainsExistingOutputPublicKey + } TransactionValidationError::MissingTxOutMembershipProof => { Self::MissingTxOutMembershipProof } @@ -94,6 +98,12 @@ impl TryInto for ProposeTxResult { Self::UnsortedKeyImages => Ok(TransactionValidationError::UnsortedKeyImages), Self::ContainsSpentKeyImage => Ok(TransactionValidationError::ContainsSpentKeyImage), Self::DuplicateKeyImages => Ok(TransactionValidationError::DuplicateKeyImages), + Self::DuplicateOutputPublicKey => { + Ok(TransactionValidationError::DuplicateOutputPublicKey) + } + Self::ContainsExistingOutputPublicKey => { + Ok(TransactionValidationError::ContainsExistingOutputPublicKey) + } Self::MissingTxOutMembershipProof => { Ok(TransactionValidationError::MissingTxOutMembershipProof) } diff --git a/consensus/enclave/api/src/lib.rs b/consensus/enclave/api/src/lib.rs index a8e6628469..f37098d3d9 100644 --- a/consensus/enclave/api/src/lib.rs +++ b/consensus/enclave/api/src/lib.rs @@ -19,7 +19,7 @@ use mc_attest_enclave_api::{ PeerAuthResponse, PeerSession, }; use mc_common::ResponderId; -use mc_crypto_keys::{Ed25519Public, X25519Public}; +use mc_crypto_keys::{CompressedRistrettoPublic, Ed25519Public, X25519Public}; use mc_transaction_core::{ ring_signature::KeyImage, tx::{Tx, TxHash, TxOutMembershipProof}, @@ -56,6 +56,9 @@ pub struct WellFormedTxContext { /// Highest membership proofs indices. highest_indices: Vec, + + /// Output public keys. + output_public_keys: Vec, } impl WellFormedTxContext { @@ -78,6 +81,10 @@ impl WellFormedTxContext { pub fn highest_indices(&self) -> &Vec { &self.highest_indices } + + pub fn output_public_keys(&self) -> &Vec { + &self.output_public_keys + } } impl From<&Tx> for WellFormedTxContext { @@ -88,6 +95,7 @@ impl From<&Tx> for WellFormedTxContext { tombstone_block: tx.prefix.tombstone_block, key_images: tx.key_images(), highest_indices: tx.get_membership_proof_highest_indices(), + output_public_keys: tx.output_public_keys(), } } } @@ -101,6 +109,7 @@ pub struct TxContext { pub tx_hash: TxHash, pub highest_indices: Vec, pub key_images: Vec, + pub output_public_keys: Vec, } pub type SealedBlockSigningKey = Vec; diff --git a/consensus/enclave/impl/src/lib.rs b/consensus/enclave/impl/src/lib.rs index a904619cfa..3e505b0390 100644 --- a/consensus/enclave/impl/src/lib.rs +++ b/consensus/enclave/impl/src/lib.rs @@ -49,6 +49,9 @@ use mc_transaction_core::{ use prost::Message; use rand_core::{CryptoRng, RngCore}; +/// Domain seperator for unified fees transaction private key. +pub const FEES_OUTPUT_PRIVATE_KEY_DOMAIN_TAG: &str = "mc_fees_output_private_key"; + /// A well-formed transaction. #[derive(Clone, Eq, PartialEq, Message)] pub struct WellFormedTx { @@ -231,12 +234,14 @@ impl ConsensusEnclave for SgxConsensusEnclave { let tx_hash = tx.tx_hash(); let highest_indices = tx.get_membership_proof_highest_indices(); let key_images: Vec = tx.key_images(); + let output_public_keys = tx.output_public_keys(); Ok(TxContext { locally_encrypted_tx, tx_hash, highest_indices, key_images, + output_public_keys, }) } @@ -261,12 +266,14 @@ impl ConsensusEnclave for SgxConsensusEnclave { let tx_hash = tx.tx_hash(); let highest_indices = tx.get_membership_proof_highest_indices(); let key_images: Vec = tx.key_images(); + let output_public_keys = tx.output_public_keys(); Ok(TxContext { locally_encrypted_tx, tx_hash, highest_indices, key_images, + output_public_keys, }) }) .collect() @@ -422,11 +429,26 @@ impl ConsensusEnclave for SgxConsensusEnclave { } } + // Duplicate output public keys are not allowed. + let mut seen_output_public_keys = BTreeSet::default(); + for tx in &transactions { + for public_key in tx.output_public_keys() { + if seen_output_public_keys.contains(&public_key) { + return Err(Error::FormBlock(format!( + "Duplicate output public key: {:?}", + public_key + ))); + } + seen_output_public_keys.insert(public_key); + } + } + // Create an aggregate fee output. let fee_tx_private_key = { let hash_value: [u8; 32] = { let mut hasher = Blake2b256::new(); - // TODO: domain separator. + FEES_OUTPUT_PRIVATE_KEY_DOMAIN_TAG.digest(&mut hasher); + parent_block.id.digest(&mut hasher); transactions.digest(&mut hasher); hasher .result() @@ -889,6 +911,98 @@ mod tests { assert_eq!(form_block_result, expected); } + #[test] + /// form_block should return an error if the input transactions contain a duplicate output + /// public key. + fn test_form_block_prevents_duplicate_output_public_key() { + let enclave = SgxConsensusEnclave::default(); + let mut rng = Hc128Rng::from_seed([77u8; 32]); + + // Initialize a ledger. `sender` is the owner of all outputs in the initial ledger. + let sender = AccountKey::random(&mut rng); + let mut ledger = create_ledger(); + let n_blocks = 3; + initialize_ledger(&mut ledger, n_blocks, &sender, &mut rng); + + // Create a few transactions from `sender` to `recipient`. + let num_transactions = 5; + let recipient = AccountKey::random(&mut rng); + + // The first block contains RING_SIZE outputs. + let block_zero_contents = ledger.get_block_contents(0).unwrap(); + + // Re-create the rng so that we could more easily generate a duplicate output public key. + let mut rng = Hc128Rng::from_seed([77u8; 32]); + + let mut new_transactions = Vec::new(); + for i in 0..num_transactions - 1 { + let tx_out = &block_zero_contents.outputs[i]; + + let tx = create_transaction( + &mut ledger, + tx_out, + &sender, + &recipient.default_subaddress(), + n_blocks + 1, + &mut rng, + ); + new_transactions.push(tx); + } + + // Re-creating the rng here would result in a duplicate output public key. + { + let mut rng = Hc128Rng::from_seed([77u8; 32]); + let tx_out = &block_zero_contents.outputs[num_transactions - 1]; + + let tx = create_transaction( + &mut ledger, + tx_out, + &sender, + &recipient.default_subaddress(), + n_blocks + 1, + &mut rng, + ); + new_transactions.push(tx); + + assert_eq!( + new_transactions[0].prefix.outputs[0].public_key, + new_transactions[num_transactions - 1].prefix.outputs[0].public_key, + ); + } + + // Create WellFormedEncryptedTxs + proofs + let well_formed_encrypted_txs_with_proofs: Vec<_> = new_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(); + + // Form block + let parent_block = ledger.get_block(ledger.num_blocks().unwrap() - 1).unwrap(); + + let form_block_result = + enclave.form_block(&parent_block, &well_formed_encrypted_txs_with_proofs); + let expected_duplicate_output_public_key = new_transactions[0].output_public_keys()[0]; + + // Check + let expected = Err(Error::FormBlock(format!( + "Duplicate output public key: {:?}", + expected_duplicate_output_public_key + ))); + + assert_eq!(form_block_result, expected); + } + #[test] fn form_block_refuses_duplicate_root_elements() { let enclave = SgxConsensusEnclave::default(); diff --git a/consensus/enclave/mock/src/lib.rs b/consensus/enclave/mock/src/lib.rs index 870776d1ad..ce33a59344 100644 --- a/consensus/enclave/mock/src/lib.rs +++ b/consensus/enclave/mock/src/lib.rs @@ -45,12 +45,14 @@ impl ConsensusServiceMockEnclave { let tx_hash = tx.tx_hash(); let highest_indices = tx.get_membership_proof_highest_indices(); let key_images: Vec = tx.key_images(); + let output_public_keys = tx.output_public_keys(); TxContext { locally_encrypted_tx, tx_hash, highest_indices, key_images, + output_public_keys, } } } diff --git a/consensus/service/src/client_api_service.rs b/consensus/service/src/client_api_service.rs index ef766629ab..f85a0a4cbd 100644 --- a/consensus/service/src/client_api_service.rs +++ b/consensus/service/src/client_api_service.rs @@ -99,6 +99,7 @@ impl ClientApiService { // These errors are common, so only trace them if err == TransactionValidationError::TombstoneBlockExceeded || err == TransactionValidationError::ContainsSpentKeyImage + || err == TransactionValidationError::ContainsExistingOutputPublicKey { log::trace!( logger, diff --git a/consensus/service/src/test_utils/mod.rs b/consensus/service/src/test_utils/mod.rs index 81408f4300..e6e373ed95 100644 --- a/consensus/service/src/test_utils/mod.rs +++ b/consensus/service/src/test_utils/mod.rs @@ -2,6 +2,7 @@ use crate::tx_manager::UntrustedInterfaces; use mc_consensus_enclave::WellFormedTxContext; +use mc_crypto_keys::CompressedRistrettoPublic; use mc_transaction_core::{ ring_signature::KeyImage, tx::{TxHash, TxOutMembershipProof}, @@ -17,6 +18,7 @@ impl UntrustedInterfaces for TrivialTxManagerUntrustedInterfaces { &self, _highest_indices: &[u64], _key_images: &[KeyImage], + _output_public_keys: &[CompressedRistrettoPublic], ) -> TransactionValidationResult<(u64, Vec)> { Ok((1, Vec::new())) } diff --git a/consensus/service/src/tx_manager.rs b/consensus/service/src/tx_manager.rs index 11c2a26ec0..b202aeb217 100644 --- a/consensus/service/src/tx_manager.rs +++ b/consensus/service/src/tx_manager.rs @@ -13,6 +13,7 @@ use mc_consensus_enclave::{ ConsensusEnclaveProxy, Error as ConsensusEnclaveError, TxContext, WellFormedEncryptedTx, WellFormedTxContext, }; +use mc_crypto_keys::CompressedRistrettoPublic; use mc_ledger_db::{Error as LedgerDbError, Ledger}; use mc_transaction_core::{ constants::MAX_TRANSACTIONS_PER_BLOCK, @@ -95,6 +96,7 @@ pub trait UntrustedInterfaces: Clone { &self, highest_indices: &[u64], key_images: &[KeyImage], + output_public_keys: &[CompressedRistrettoPublic], ) -> TransactionValidationResult<(u64, Vec)>; /// Checks if a transaction is valid (see definition in validators.rs). @@ -172,9 +174,11 @@ impl TxManager TxManager TxManagerUntrustedInterfaces for DefaultTxManagerUntrustedInterf &self, highest_indices: &[u64], key_images: &[KeyImage], + output_public_keys: &[CompressedRistrettoPublic], ) -> TransactionValidationResult<(u64, Vec)> { // The `key_images` must not have already been spent. // TODO: this should use proofs of non-membership. @@ -58,6 +60,17 @@ impl TxManagerUntrustedInterfaces for DefaultTxManagerUntrustedInterf return Err(TransactionValidationError::ContainsSpentKeyImage); } + // Output public keys should not exist in the ledger. + let contains_existing_public_key = output_public_keys.iter().any(|public_key| { + self.ledger + .contains_tx_out_public_key(public_key) + .unwrap_or(true) + }); + if contains_existing_public_key { + // At least one public key is already in the ledger, or the ledger returned an error. + return Err(TransactionValidationError::ContainsExistingOutputPublicKey); + } + let membership_proofs = self .ledger .get_tx_out_proof_of_memberships(highest_indices) @@ -97,6 +110,17 @@ impl TxManagerUntrustedInterfaces for DefaultTxManagerUntrustedInterf return Err(TransactionValidationError::ContainsSpentKeyImage); } + // The `output_public_keys` must not appear in the ledger. + let contains_existing_public_key = context.output_public_keys().iter().any(|public_key| { + self.ledger + .contains_tx_out_public_key(public_key) + .unwrap_or(true) + }); + if contains_existing_public_key { + // At least one public key is already in the ledger, or the ledger returned an error. + return Err(TransactionValidationError::ContainsExistingOutputPublicKey); + } + // `tx` is safe to append. Ok(()) } @@ -118,6 +142,7 @@ impl TxManagerUntrustedInterfaces for DefaultTxManagerUntrustedInterf // Allow transactions that do not introduce key image double-spends. let mut allowed_hashes = BTreeSet::new(); let mut used_key_images = HashSet::default(); + let mut seen_output_public_keys = HashSet::default(); for tx_context in tx_contexts.iter() { if allowed_hashes.len() >= max_elements { @@ -130,15 +155,31 @@ impl TxManagerUntrustedInterfaces for DefaultTxManagerUntrustedInterf } let key_images: HashSet = tx_context.key_images().iter().cloned().collect(); - let no_duplicate_key_images = - used_key_images.intersection(&key_images).next().is_none(); + let duplicate_key_images = used_key_images.intersection(&key_images).next().is_some(); - if no_duplicate_key_images { - used_key_images = used_key_images.union(&key_images).cloned().collect(); - allowed_hashes.insert(tx_context.tx_hash().clone()); - } else { + if duplicate_key_images { // Omitting tx_context to avoid key image double-spend. + continue; + } + + let output_public_keys: HashSet = + tx_context.output_public_keys().iter().cloned().collect(); + let duplicate_output_public_keys = seen_output_public_keys + .intersection(&output_public_keys) + .next() + .is_some(); + + if duplicate_output_public_keys { + // Omitting tx_context to avoid repeated output public keys. + continue; } + + used_key_images = used_key_images.union(&key_images).cloned().collect(); + seen_output_public_keys = seen_output_public_keys + .union(&output_public_keys) + .cloned() + .collect(); + allowed_hashes.insert(tx_context.tx_hash().clone()); } allowed_hashes @@ -166,9 +207,13 @@ pub mod well_formed_tests { let key_images: Vec = tx.key_images(); let membership_proof_highest_indices = tx.get_membership_proof_highest_indices(); + let output_public_keys = tx.output_public_keys(); - let (cur_block_index, membership_proofs) = - untrusted.well_formed_check(&membership_proof_highest_indices[..], &key_images[..])?; + let (cur_block_index, membership_proofs) = untrusted.well_formed_check( + &membership_proof_highest_indices[..], + &key_images[..], + &output_public_keys[..], + )?; mc_transaction_core::validation::validate( &tx, @@ -276,6 +321,40 @@ pub mod well_formed_tests { ); } + #[test_with_logger] + /// `is_well_formed` should reject a transaction that contains an output public key that is in the ledger. + fn is_well_formed_rejects_duplicate_output_public_key(_logger: Logger) { + let mut rng = Hc128Rng::from_seed([79u8; 32]); + + let sender = AccountKey::random(&mut rng); + let recipient = AccountKey::random(&mut rng); + + let mut ledger = create_ledger(); + let n_blocks = 3; + initialize_ledger(&mut ledger, n_blocks, &sender, &mut rng); + + // Choose a TxOut to spend. Only the TxOut in the last block is unspent. + let block_contents = ledger.get_block_contents(n_blocks - 1).unwrap(); + let tx_out = block_contents.outputs[0].clone(); + + let mut tx = create_transaction( + &mut ledger, + &tx_out, + &sender, + &recipient.default_subaddress(), + n_blocks + 1, + &mut rng, + ); + + // Change the public key so that it is no longer unique. + tx.prefix.outputs[0].public_key = tx_out.public_key.clone(); + + assert_eq!( + Err(TransactionValidationError::ContainsExistingOutputPublicKey), + is_well_formed(&tx, &ledger) + ); + } + #[test_with_logger] /// `is_well_formed` should reject a transaction that contains an invalid proof-of-membership. fn is_well_formed_rejects_missing_input(_logger: Logger) { @@ -556,7 +635,7 @@ mod is_valid_tests { } #[test] - /// `is_valid` should reject a transaction with an already spent key image . + /// `is_valid` should reject a transaction with an already spent key image. fn is_valid_rejects_spent_keyimage() { let mut rng = Hc128Rng::from_seed([79u8; 32]); @@ -586,6 +665,38 @@ mod is_valid_tests { is_valid(&tx, &ledger) ); } + + #[test] + /// `is_valid` should reject a transaction with an already used output public key. + fn is_valid_rejects_non_unique_output_public_key() { + let mut rng = Hc128Rng::from_seed([79u8; 32]); + + let sender = AccountKey::random(&mut rng); + let recipient = AccountKey::random(&mut rng); + + let mut ledger = create_ledger(); + let n_blocks = 3; + initialize_ledger(&mut ledger, n_blocks, &sender, &mut rng); + + // Choose a TxOut to spend. Only the output of the last block is unspent. + let block_contents = ledger.get_block_contents(n_blocks - 1).unwrap(); + let tx_out = block_contents.outputs[0].clone(); + + let mut tx = create_transaction( + &mut ledger, + &tx_out, + &sender, + &recipient.default_subaddress(), + n_blocks + 5, + &mut rng, + ); + + tx.prefix.outputs[0].public_key = block_contents.outputs[0].public_key.clone(); + assert_eq!( + Err(TransactionValidationError::ContainsExistingOutputPublicKey), + is_valid(&tx, &ledger) + ); + } } #[cfg(test)] @@ -913,4 +1024,173 @@ mod combine_tests { assert_eq!(combined_transactions.len(), 2); assert!(combined_transactions.contains(third_client_tx.tx_hash())); } + + #[test] + // `combine` should omit transactions that would cause an output public key to appear twice. + fn combine_reject_duplicate_output_public_key() { + let mut rng = Hc128Rng::from_seed([1u8; 32]); + + let alice = AccountKey::random(&mut rng); + let bob = AccountKey::random(&mut rng); + + // Create two TxOuts that were sent to Alice. + let tx_out1 = TxOut::new( + 123, + &alice.default_subaddress(), + &RistrettoPrivate::from_random(&mut rng), + Default::default(), + &mut rng, + ) + .unwrap(); + + let tx_out2 = TxOut::new( + 123, + &alice.default_subaddress(), + &RistrettoPrivate::from_random(&mut rng), + Default::default(), + &mut rng, + ) + .unwrap(); + + // Alice creates InputCredentials to spend her tx_outs. + let onetime_private_key1 = recover_onetime_private_key( + &RistrettoPublic::try_from(&tx_out1.public_key).unwrap(), + alice.view_private_key(), + &alice.default_subaddress_spend_private(), + ); + + let onetime_private_key2 = recover_onetime_private_key( + &RistrettoPublic::try_from(&tx_out2.public_key).unwrap(), + alice.view_private_key(), + &alice.default_subaddress_spend_private(), + ); + + // Create a transaction that sends the full value of `tx_out1` to bob. + let first_client_tx: WellFormedTxContext = { + let ring = vec![tx_out1.clone()]; + let membership_proofs: Vec = ring + .iter() + .map(|_tx_out| { + // TODO: provide valid proofs for each tx_out. + TxOutMembershipProof::new(0, 0, HashMap::default()) + }) + .collect(); + + let input_credentials = InputCredentials::new( + ring, + membership_proofs, + 0, + onetime_private_key1, + *alice.view_private_key(), + ) + .unwrap(); + + let mut transaction_builder = TransactionBuilder::new(); + transaction_builder.add_input(input_credentials); + transaction_builder.set_fee(0); + transaction_builder + .add_output(123, &bob.default_subaddress(), None, &mut rng) + .unwrap(); + + let tx = transaction_builder.build(&mut rng).unwrap(); + WellFormedTxContext::from(&tx) + }; + + // Create another transaction that attempts to spend `tx_out2` but has the same output + // public key. + let second_client_tx: WellFormedTxContext = { + let recipient_account = AccountKey::random(&mut rng); + let ring: Vec = vec![tx_out2]; + let membership_proofs: Vec = ring + .iter() + .map(|_tx_out| { + // TODO: provide valid proofs for each tx_out. + TxOutMembershipProof::new(0, 0, HashMap::default()) + }) + .collect(); + + let input_credentials = InputCredentials::new( + ring, + membership_proofs, + 0, + onetime_private_key2, + *alice.view_private_key(), + ) + .unwrap(); + + let mut transaction_builder = TransactionBuilder::new(); + transaction_builder.add_input(input_credentials); + transaction_builder.set_fee(0); + transaction_builder + .add_output(123, &recipient_account.default_subaddress(), None, &mut rng) + .unwrap(); + + let mut tx = transaction_builder.build(&mut rng).unwrap(); + tx.prefix.outputs[0].public_key = first_client_tx.output_public_keys()[0].clone(); + WellFormedTxContext::from(&tx) + }; + + // This transaction spends a different TxOut, unrelated to `first_client_tx` and `second_client_tx`. + let third_client_tx: WellFormedTxContext = { + let recipient_account = AccountKey::random(&mut rng); + + // The transaction keys. + let tx_secret_key_for_txo = RistrettoPrivate::from_random(&mut rng); + let tx_out = TxOut::new( + 123, + &alice.default_subaddress(), + &tx_secret_key_for_txo, + Default::default(), + &mut rng, + ) + .unwrap(); + let tx_public_key_for_txo = RistrettoPublic::try_from(&tx_out.public_key).unwrap(); + + // Step 2: Create a transaction that sends the full value of `tx_out` to `recipient_account`. + + // Create InputCredentials to spend the TxOut. + let onetime_private_key = recover_onetime_private_key( + &tx_public_key_for_txo, + alice.view_private_key(), + &alice.default_subaddress_spend_private(), + ); + + let ring: Vec = vec![tx_out]; + let membership_proofs: Vec = ring + .iter() + .map(|_tx_out| { + // TODO: provide valid proofs for each tx_out. + TxOutMembershipProof::new(0, 0, HashMap::default()) + }) + .collect(); + + let input_credentials = InputCredentials::new( + ring, + membership_proofs, + 0, + onetime_private_key, + *alice.view_private_key(), + ) + .unwrap(); + + let mut transaction_builder = TransactionBuilder::new(); + transaction_builder.add_input(input_credentials); + transaction_builder.set_fee(0); + transaction_builder + .add_output(123, &recipient_account.default_subaddress(), None, &mut rng) + .unwrap(); + + let tx = transaction_builder.build(&mut rng).unwrap(); + WellFormedTxContext::from(&tx) + }; + + // `combine` the set of transactions. + let transaction_set = vec![first_client_tx, second_client_tx, third_client_tx.clone()]; + + let combined_transactions = combine(transaction_set, 10); + // `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())); + } } diff --git a/ledger/db/Cargo.toml b/ledger/db/Cargo.toml index 5ceec1e320..51b1e59809 100644 --- a/ledger/db/Cargo.toml +++ b/ledger/db/Cargo.toml @@ -5,11 +5,11 @@ authors = ["MobileCoin"] edition = "2018" [features] -test_utils = ["rand", "mc-crypto-keys"] +test_utils = ["rand"] [dependencies] mc-common = { path = "../../common", features = ["log"] } -mc-crypto-keys = { path = "../../crypto/keys", optional = true } +mc-crypto-keys = { path = "../../crypto/keys" } mc-transaction-core = { path = "../../transaction/core" } mc-util-from-random = { path = "../../util/from-random" } mc-util-serial = { path = "../../util/serial", features = ["std"] } diff --git a/ledger/db/src/error.rs b/ledger/db/src/error.rs index 2c1d2866da..0bdc2e9a65 100644 --- a/ledger/db/src/error.rs +++ b/ledger/db/src/error.rs @@ -24,6 +24,9 @@ pub enum Error { #[fail(display = "KeyImageAlreadySpent")] KeyImageAlreadySpent, + #[fail(display = "DuplicateOutputPublicKey")] + DuplicateOutputPublicKey, + #[fail(display = "InvalidBlockContents")] InvalidBlockContents, diff --git a/ledger/db/src/ledger_trait.rs b/ledger/db/src/ledger_trait.rs index cc6728719f..12bfe4c913 100644 --- a/ledger/db/src/ledger_trait.rs +++ b/ledger/db/src/ledger_trait.rs @@ -2,6 +2,7 @@ use crate::Error; use mc_common::Hash; +use mc_crypto_keys::CompressedRistrettoPublic; use mc_transaction_core::{ ring_signature::KeyImage, tx::{TxOut, TxOutMembershipProof}, @@ -35,6 +36,12 @@ pub trait Ledger: Clone + Send { /// Returns the index of the TxOut with the given hash. fn get_tx_out_index_by_hash(&self, tx_out_hash: &Hash) -> Result; + /// Returns the index of the TxOut with the given public key. + fn get_tx_out_index_by_public_key( + &self, + tx_out_public_key: &CompressedRistrettoPublic, + ) -> Result; + /// Gets a TxOut by its index in the ledger. fn get_tx_out_by_index(&self, index: u64) -> Result; @@ -44,6 +51,12 @@ pub trait Ledger: Clone + Send { indexes: &[u64], ) -> Result, Error>; + /// Returns true if the Ledger contains the given TxOut public key. + fn contains_tx_out_public_key( + &self, + public_key: &CompressedRistrettoPublic, + ) -> Result; + /// Returns true if the Ledger contains the given key image. fn contains_key_image(&self, key_image: &KeyImage) -> Result { self.check_key_image(key_image).map(|x| x.is_some()) diff --git a/ledger/db/src/lib.rs b/ledger/db/src/lib.rs index 5f109b7cb3..579b890e94 100644 --- a/ledger/db/src/lib.rs +++ b/ledger/db/src/lib.rs @@ -12,9 +12,16 @@ use lmdb::{ Database, DatabaseFlags, Environment, EnvironmentFlags, RoTransaction, RwTransaction, Transaction, WriteFlags, }; -use mc_transaction_core::{Block, BlockContents, BlockID, BlockSignature, BLOCK_VERSION}; +use mc_common::logger::global_log; +use mc_crypto_keys::CompressedRistrettoPublic; +use mc_transaction_core::{ + ring_signature::KeyImage, + tx::{TxOut, TxOutMembershipProof}, + Block, BlockContents, BlockID, BlockSignature, BLOCK_VERSION, +}; use mc_util_serial::{decode, encode, Message}; use std::{path::PathBuf, sync::Arc}; +use tx_out_store::TxOutStore; mod error; mod ledger_trait; @@ -26,12 +33,7 @@ pub mod test_utils; pub use error::Error; pub use ledger_trait::Ledger; -use mc_transaction_core::{ - ring_signature::KeyImage, - tx::{TxOut, TxOutMembershipProof}, -}; pub use metadata::MetadataStore; -use tx_out_store::TxOutStore; const MAX_LMDB_FILE_SIZE: usize = 1_099_511_627_776; // 1 TB @@ -200,6 +202,16 @@ impl Ledger for LedgerDB { .get_tx_out_index_by_hash(tx_out_hash, &db_transaction) } + /// Returns the index of the TxOut with the given public key. + fn get_tx_out_index_by_public_key( + &self, + tx_out_public_key: &CompressedRistrettoPublic, + ) -> Result { + let db_transaction: RoTransaction = self.env.begin_ro_txn()?; + self.tx_out_store + .get_tx_out_index_by_public_key(tx_out_public_key, &db_transaction) + } + /// Gets a TxOut by its index in the ledger. fn get_tx_out_by_index(&self, index: u64) -> Result { let db_transaction = self.env.begin_ro_txn()?; @@ -207,6 +219,22 @@ impl Ledger for LedgerDB { .get_tx_out_by_index(index, &db_transaction) } + /// Returns true if the Ledger contains the given TxOut public key. + fn contains_tx_out_public_key( + &self, + public_key: &CompressedRistrettoPublic, + ) -> Result { + let db_transaction = self.env.begin_ro_txn()?; + match self + .tx_out_store + .get_tx_out_index_by_public_key(public_key, &db_transaction) + { + Ok(_) => Ok(true), + Err(Error::NotFound) => Ok(false), + Err(e) => Err(e), + } + } + /// Returns true if the Ledger contains the given KeyImage. fn check_key_image(&self, key_image: &KeyImage) -> Result, Error> { let db_transaction = self.env.begin_ro_txn()?; @@ -248,14 +276,43 @@ impl Ledger for LedgerDB { impl LedgerDB { /// Opens an existing Ledger Database in the given path. + #[allow(clippy::unreadable_literal)] pub fn open(path: PathBuf) -> Result { let env = Environment::new() - .set_max_dbs(20) + .set_max_dbs(21) .set_map_size(MAX_LMDB_FILE_SIZE) // TODO - needed because currently our test cloud machines have slow disks. .set_flags(EnvironmentFlags::NO_SYNC) .open(&path)?; + // Check if the database we opened is compatible with the current implementation. + let metadata_store = MetadataStore::new(&env)?; + let db_txn = env.begin_ro_txn()?; + let version = metadata_store.get_version(&db_txn)?; + global_log::info!("Ledger db is currently at version: {:?}", version); + db_txn.commit()?; + + match version.is_compatible_with_latest() { + Ok(_) => {} + // Version 20200610 introduced the TxOut public key -> index store. + Err(Error::VersionIncompatible(20200427, 20200610)) => { + global_log::info!("Ledger db migrating from version 20200427 to 20200610, this might take awhile..."); + + TxOutStore::construct_tx_out_index_by_public_key_from_existing_data(&env)?; + + let mut db_txn = env.begin_rw_txn()?; + metadata_store.set_version_to_latest(&mut db_txn)?; + global_log::info!( + "Ledger db migration complete, now at version: {:?}", + metadata_store.get_version(&db_txn), + ); + db_txn.commit()?; + } + Err(err) => { + return Err(err); + } + }; + let counts = env.open_db(Some(COUNTS_DB_NAME))?; let blocks = env.open_db(Some(BLOCKS_DB_NAME))?; let block_signatures = env.open_db(Some(BLOCK_SIGNATURES_DB_NAME))?; @@ -263,15 +320,8 @@ impl LedgerDB { let key_images_by_block = env.open_db(Some(KEY_IMAGES_BY_BLOCK_DB_NAME))?; let tx_outs_by_block = env.open_db(Some(TX_OUTS_BY_BLOCK_DB_NAME))?; - let metadata_store = MetadataStore::new(&env)?; let tx_out_store = TxOutStore::new(&env)?; - // Check if the database we opened is compatible with the current implementation. - let db_txn = env.begin_ro_txn()?; - let version = metadata_store.get_version(&db_txn)?; - version.is_compatible_with_latest()?; - db_txn.commit()?; - Ok(LedgerDB { env: Arc::new(env), path, @@ -414,6 +464,10 @@ impl LedgerDB { // Write the actual TxOuts. for tx_out in tx_outs { + if self.contains_tx_out_public_key(&tx_out.public_key)? { + return Err(Error::DuplicateOutputPublicKey); + } + self.tx_out_store.push(tx_out, db_transaction)?; } @@ -470,6 +524,13 @@ impl LedgerDB { } } + // Check that none of the output public keys appear in the ledger. + for output in block_contents.outputs.iter() { + if self.contains_tx_out_public_key(&output.public_key)? { + return Err(Error::DuplicateOutputPublicKey); + } + } + // Validate block id. if !block.is_block_id_valid() { return Err(Error::InvalidBlockID); @@ -1036,6 +1097,51 @@ mod ledger_db_test { ); } + #[test] + /// Appending a block with a pre-existing output public key should return Error::DuplicateOutputPublicKey. + fn test_append_block_with_duplicate_output_public_key() { + let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]); + let mut ledger_db = create_db(); + + // Write a block to the ledger. + let origin_account_key = AccountKey::random(&mut rng); + let (origin_block, origin_block_contents) = + get_origin_block_and_contents(&origin_account_key); + ledger_db + .append_block(&origin_block, &origin_block_contents, None) + .unwrap(); + + // The next block reuses a public key. + let existing_tx_out = ledger_db.get_tx_out_by_index(0).unwrap(); + let account_key = AccountKey::random(&mut rng); + + let block_one_contents = { + let mut tx_out = TxOut::new( + 33, + &account_key.default_subaddress(), + &RistrettoPrivate::from_random(&mut rng), + Default::default(), + &mut rng, + ) + .unwrap(); + tx_out.public_key = existing_tx_out.public_key.clone(); + let outputs = vec![tx_out]; + BlockContents::new(vec![KeyImage::from(rng.next_u64())], outputs) + }; + + let block_one = Block::new_with_parent( + BLOCK_VERSION, + &origin_block, + &Default::default(), + &block_one_contents, + ); + + assert_eq!( + ledger_db.append_block(&block_one, &block_one_contents, None), + Err(Error::DuplicateOutputPublicKey) + ); + } + #[test] // append_block rejects invalid blocks. fn test_append_invalid_blocks() { diff --git a/ledger/db/src/metadata.rs b/ledger/db/src/metadata.rs index e424fc2e45..a9addc2171 100644 --- a/ledger/db/src/metadata.rs +++ b/ledger/db/src/metadata.rs @@ -1,5 +1,5 @@ use crate::Error; -use lmdb::{Database, DatabaseFlags, Environment, Transaction, WriteFlags}; +use lmdb::{Database, DatabaseFlags, Environment, RwTransaction, Transaction, WriteFlags}; use mc_util_serial::{decode, encode}; use prost::Message; @@ -7,7 +7,7 @@ use prost::Message; // If this is properly maintained, we could check during ledger db opening for any // incompatibilities, and either refuse to open or perform a migration. #[allow(clippy::unreadable_literal)] -pub const LATEST_VERSION: u64 = 20200427; +pub const LATEST_VERSION: u64 = 20200610; // Metadata information about the ledger databse. #[derive(Clone, Message)] @@ -85,4 +85,14 @@ impl MetadataStore { pub fn get_version(&self, db_txn: &impl Transaction) -> Result { Ok(decode(db_txn.get(self.metadata, &METADATA_VERSION_KEY)?)?) } + + // Set version to latest. + pub fn set_version_to_latest(&self, db_txn: &mut RwTransaction) -> Result<(), Error> { + Ok(db_txn.put( + self.metadata, + &METADATA_VERSION_KEY, + &encode(&MetadataVersion::latest()), + WriteFlags::empty(), + )?) + } } diff --git a/ledger/db/src/test_utils/mock_ledger.rs b/ledger/db/src/test_utils/mock_ledger.rs index df043c6729..88f13a2e79 100644 --- a/ledger/db/src/test_utils/mock_ledger.rs +++ b/ledger/db/src/test_utils/mock_ledger.rs @@ -2,7 +2,7 @@ use crate::{Error, Ledger}; use mc_common::{HashMap, HashSet}; -use mc_crypto_keys::RistrettoPrivate; +use mc_crypto_keys::{CompressedRistrettoPublic, RistrettoPrivate}; use mc_transaction_core::{ account_keys::AccountKey, ring_signature::KeyImage, @@ -135,6 +135,20 @@ impl Ledger for MockLedger { unimplemented!() } + fn get_tx_out_index_by_public_key( + &self, + _tx_out_public_key: &CompressedRistrettoPublic, + ) -> Result { + unimplemented!(); + } + + fn contains_tx_out_public_key( + &self, + _public_key: &CompressedRistrettoPublic, + ) -> Result { + unimplemented!(); + } + fn check_key_image(&self, key_image: &KeyImage) -> Result, Error> { // Unused for these tests. Ok(self.lock().key_images.get(key_image).cloned()) diff --git a/ledger/db/src/tx_out_store.rs b/ledger/db/src/tx_out_store.rs index c13aca56d8..85b4c55af9 100644 --- a/ledger/db/src/tx_out_store.rs +++ b/ledger/db/src/tx_out_store.rs @@ -25,7 +25,8 @@ use crate::{key_bytes_to_u64, u64_to_key_bytes, Error}; use lmdb::{Database, DatabaseFlags, Environment, RwTransaction, Transaction, WriteFlags}; -use mc_common::{Hash, HashMap}; +use mc_common::{logger::global_log, Hash, HashMap}; +use mc_crypto_keys::CompressedRistrettoPublic; use mc_transaction_core::{ membership_proofs::*, range::Range, @@ -36,6 +37,7 @@ use mc_util_serial::{decode, encode}; // LMDB Database names. const COUNTS_DB_NAME: &str = "tx_out_store:counts"; const TX_OUT_INDEX_BY_HASH_DB_NAME: &str = "tx_out_store:tx_out_index_by_hash"; +const TX_OUT_INDEX_BY_PUBLIC_KEY_DB_NAME: &str = "tx_out_store:tx_out_index_by_public_key"; const TX_OUT_BY_INDEX_DB_NAME: &str = "tx_out_store:tx_out_by_index"; const MERKLE_HASH_BY_RANGE_DB_NAME: &str = "tx_out_store:merkle_hash_by_range"; @@ -54,6 +56,9 @@ pub struct TxOutStore { /// `tx_out.hash() -> u64_to_key_bytes(index)` tx_out_index_by_hash: Database, + /// `tx_out.public_key -> u64_to_key_bytes(index)` + tx_out_index_by_public_key: Database, + /// Merkle hashes of subtrees. Range -> Merkle Hash of subtree containing TxOuts with indices in `[range.from, range.to]`. /// range.to_key_bytes --> [u8; 32] merkle_hashes: Database, @@ -65,6 +70,7 @@ impl TxOutStore { Ok(TxOutStore { counts: env.open_db(Some(COUNTS_DB_NAME))?, tx_out_index_by_hash: env.open_db(Some(TX_OUT_INDEX_BY_HASH_DB_NAME))?, + tx_out_index_by_public_key: env.open_db(Some(TX_OUT_INDEX_BY_PUBLIC_KEY_DB_NAME))?, tx_out_by_index: env.open_db(Some(TX_OUT_BY_INDEX_DB_NAME))?, merkle_hashes: env.open_db(Some(MERKLE_HASH_BY_RANGE_DB_NAME))?, }) @@ -74,6 +80,10 @@ impl TxOutStore { pub fn create(env: &Environment) -> Result<(), Error> { let counts = env.create_db(Some(COUNTS_DB_NAME), DatabaseFlags::empty())?; env.create_db(Some(TX_OUT_INDEX_BY_HASH_DB_NAME), DatabaseFlags::empty())?; + env.create_db( + Some(TX_OUT_INDEX_BY_PUBLIC_KEY_DB_NAME), + DatabaseFlags::empty(), + )?; env.create_db(Some(TX_OUT_BY_INDEX_DB_NAME), DatabaseFlags::empty())?; env.create_db(Some(MERKLE_HASH_BY_RANGE_DB_NAME), DatabaseFlags::empty())?; @@ -108,7 +118,14 @@ impl TxOutStore { self.tx_out_index_by_hash, &tx_out.hash(), &u64_to_key_bytes(index), - WriteFlags::empty(), + WriteFlags::NO_OVERWRITE, + )?; + + db_transaction.put( + self.tx_out_index_by_public_key, + &tx_out.public_key, + &u64_to_key_bytes(index), + WriteFlags::NO_OVERWRITE, )?; let tx_out_bytes: Vec = encode(tx_out); @@ -117,7 +134,7 @@ impl TxOutStore { self.tx_out_by_index, &u64_to_key_bytes(index), &tx_out_bytes, - WriteFlags::empty(), + WriteFlags::NO_OVERWRITE, )?; self.update_merkle_hashes(index, db_transaction)?; @@ -142,6 +159,16 @@ impl TxOutStore { Ok(key_bytes_to_u64(index_bytes)) } + /// Returns the index of the TxOut with the public key. + pub fn get_tx_out_index_by_public_key( + &self, + tx_out_public_key: &CompressedRistrettoPublic, + db_transaction: &T, + ) -> Result { + let index_bytes = db_transaction.get(self.tx_out_index_by_public_key, tx_out_public_key)?; + Ok(key_bytes_to_u64(index_bytes)) + } + /// Gets a TxOut by its index in the ledger. pub fn get_tx_out_by_index( &self, @@ -173,6 +200,7 @@ impl TxOutStore { Err(Error::CapacityExceeded) } } + /// Writes the Merkle hash value for a node spanning the given range. fn write_merkle_hash( &self, @@ -311,6 +339,46 @@ impl TxOutStore { range_to_hash, )) } + + /// A utility function for constructing the tx_out_index_by_public_key store using existing + /// data. + pub(crate) fn construct_tx_out_index_by_public_key_from_existing_data( + env: &Environment, + ) -> Result<(), Error> { + // When constructing the tx out index by public key database, we first need to create it. + env.create_db( + Some(TX_OUT_INDEX_BY_PUBLIC_KEY_DB_NAME), + DatabaseFlags::empty(), + )?; + + // After the database has been created, we can use TxOutStore as normal. + let instance = Self::new(env)?; + + let mut db_txn = env.begin_rw_txn()?; + + let num_tx_outs = instance.num_tx_outs(&db_txn)?; + let mut percents: u64 = 0; + for tx_out_index in 0..num_tx_outs { + let tx_out = instance.get_tx_out_by_index(tx_out_index, &db_txn)?; + db_txn.put( + instance.tx_out_index_by_public_key, + &tx_out.public_key, + &u64_to_key_bytes(tx_out_index), + WriteFlags::NO_OVERWRITE, + )?; + + // Throttled logging. + let new_percents = tx_out_index * 100 / num_tx_outs; + if new_percents != percents { + percents = new_percents; + global_log::info!( + "Constructing tx_out_index_by_public_key: {}% complete", + percents + ); + } + } + Ok(db_txn.commit()?) + } } /// Converts this Range to bytes for use as an LMDB key. @@ -715,7 +783,7 @@ pub mod tx_out_store_tests { use crate::Error; use lmdb::{Environment, RoTransaction, RwTransaction, Transaction}; use mc_common::Hash; - use mc_crypto_keys::{RistrettoPrivate, RistrettoPublic}; + use mc_crypto_keys::{CompressedRistrettoPublic, RistrettoPrivate, RistrettoPublic}; use mc_transaction_core::{ account_keys::AccountKey, amount::Amount, @@ -828,6 +896,52 @@ pub mod tx_out_store_tests { } } + #[test] + // `get_tx_out_index_by_public_key` should return the correct index, or Error::NotFound. + fn test_get_tx_out_index_by_public_key() { + let (tx_out_store, env) = init_tx_out_store(); + let tx_outs = get_tx_outs(111); + + { + // Push a number of TxOuts to the store. + let mut rw_transaction: RwTransaction = env.begin_rw_txn().unwrap(); + for tx_out in &tx_outs { + tx_out_store.push(tx_out, &mut rw_transaction).unwrap(); + } + rw_transaction.commit().unwrap(); + } + + let ro_transaction: RoTransaction = env.begin_ro_txn().unwrap(); + assert_eq!( + tx_outs.len() as u64, + tx_out_store.num_tx_outs(&ro_transaction).unwrap() + ); + + // `get_tx_out_by_index_by_hash` should return the correct index when given a recognized hash. + for (index, tx_out) in tx_outs.iter().enumerate() { + assert_eq!( + index as u64, + tx_out_store + .get_tx_out_index_by_public_key(&tx_out.public_key, &ro_transaction) + .unwrap() + ); + } + + // `get_tx_out_index_by_public_key` should return `Error::NotFound` for an unrecognized hash. + let unrecognized_public_key = CompressedRistrettoPublic::from(&[0; 32]); + match tx_out_store.get_tx_out_index_by_public_key(&unrecognized_public_key, &ro_transaction) + { + Ok(index) => panic!(format!( + "Returned index {:?} for unrecognized public key.", + index + )), + Err(Error::NotFound) => { + // This is expected. + } + Err(e) => panic!("Unexpected Error {:?}", e), + } + } + #[test] // `get_tx_out_by_index` should return the correct TxOut, or Error::NotFound. fn test_get_tx_out_by_index() { @@ -872,6 +986,54 @@ pub mod tx_out_store_tests { ro_transaction.commit().unwrap(); } + #[test] + // Pushing a duplicate TxOut should fail. + fn test_push_duplicate_txout_fails() { + let (tx_out_store, env) = init_tx_out_store(); + let tx_outs = get_tx_outs(10); + + { + // Push a number of TxOuts to the store. + let mut rw_transaction: RwTransaction = env.begin_rw_txn().unwrap(); + for tx_out in &tx_outs { + tx_out_store.push(tx_out, &mut rw_transaction).unwrap(); + } + rw_transaction.commit().unwrap(); + } + + let mut rw_transaction: RwTransaction = env.begin_rw_txn().unwrap(); + match tx_out_store.push(&tx_outs[0], &mut rw_transaction) { + Err(Error::LmdbError(lmdb::Error::KeyExist)) => {} + Ok(_) => panic!("unexpected success"), + Err(_) => panic!("unexpected error"), + }; + } + + #[test] + // Pushing a TxOut with a duplicate public key should fail. + fn test_push_duplicate_public_key_fails() { + let (tx_out_store, env) = init_tx_out_store(); + let mut tx_outs = get_tx_outs(10); + + { + // Push a number of TxOuts to the store. + let mut rw_transaction: RwTransaction = env.begin_rw_txn().unwrap(); + for tx_out in &tx_outs[1..] { + tx_out_store.push(tx_out, &mut rw_transaction).unwrap(); + } + rw_transaction.commit().unwrap(); + } + + tx_outs[0].public_key = tx_outs[1].public_key; + + let mut rw_transaction: RwTransaction = env.begin_rw_txn().unwrap(); + match tx_out_store.push(&tx_outs[0], &mut rw_transaction) { + Err(Error::LmdbError(lmdb::Error::KeyExist)) => {} + Ok(_) => panic!("unexpected success"), + Err(_) => panic!("unexpected error"), + }; + } + #[test] // `push` should add a TxOut to the correct index. fn test_push() { diff --git a/transaction/core/src/tx.rs b/transaction/core/src/tx.rs index e093270de6..ecbd205982 100644 --- a/transaction/core/src/tx.rs +++ b/transaction/core/src/tx.rs @@ -134,6 +134,15 @@ impl Tx { pub fn get_membership_proof_highest_indices(&self) -> Vec { self.prefix.get_membership_proof_highest_indices() } + + /// Output public keys contained in this transaction. + pub fn output_public_keys(&self) -> Vec { + self.prefix + .outputs + .iter() + .map(|tx_out| tx_out.public_key) + .collect() + } } /// TxPrefix is the Tx struct without the signature. It is used to diff --git a/transaction/core/src/validation/error.rs b/transaction/core/src/validation/error.rs index aa00223a7f..2149cf19b1 100644 --- a/transaction/core/src/validation/error.rs +++ b/transaction/core/src/validation/error.rs @@ -87,6 +87,14 @@ pub enum TransactionValidationError { #[fail(display = "DuplicateKeyImages")] DuplicateKeyImages, + /// Output public keys in the transaction must be unique. + #[fail(display = "DuplicateOutputPublicKey")] + DuplicateOutputPublicKey, + + /// Contains an output public key that has previously appeared in the ledger. + #[fail(display = "ContainsExistingOutputPublicKey")] + ContainsExistingOutputPublicKey, + /// Each ring element must have a corresponding proof of membership. #[fail(display = "MissingTxOutMembershipProof")] MissingTxOutMembershipProof, diff --git a/transaction/core/src/validation/validate.rs b/transaction/core/src/validation/validate.rs index f5eb1d2841..99f66872eb 100644 --- a/transaction/core/src/validation/validate.rs +++ b/transaction/core/src/validation/validate.rs @@ -48,6 +48,8 @@ pub fn validate( validate_key_images_are_unique(&tx)?; + validate_outputs_public_keys_are_unique(&tx)?; + validate_tombstone(current_block_index, tx.prefix.tombstone_block)?; // Note: The transaction must not contain a Key Image that has previously been spent. @@ -156,6 +158,16 @@ fn validate_key_images_are_unique(tx: &Tx) -> TransactionValidationResult<()> { Ok(()) } +/// All output public keys within the transaction must be unique. +fn validate_outputs_public_keys_are_unique(tx: &Tx) -> TransactionValidationResult<()> { + let mut uniques = HashSet::default(); + for public_key in tx.output_public_keys() { + if !uniques.insert(public_key) { + return Err(TransactionValidationError::DuplicateOutputPublicKey); + } + } + Ok(()) +} /// Verifies the transaction signature. /// /// A valid RctBulletproofs signature implies that: @@ -333,8 +345,9 @@ mod tests { validate::{ validate_key_images_are_unique, validate_membership_proofs, validate_number_of_inputs, validate_number_of_outputs, - validate_ring_elements_are_unique, validate_ring_sizes, validate_signature, - validate_tombstone, validate_transaction_fee, MAX_TOMBSTONE_BLOCKS, + validate_outputs_public_keys_are_unique, validate_ring_elements_are_unique, + validate_ring_sizes, validate_signature, validate_tombstone, + validate_transaction_fee, MAX_TOMBSTONE_BLOCKS, }, }, }; @@ -649,6 +662,28 @@ mod tests { assert_eq!(validate_key_images_are_unique(&tx), Ok(()),); } + #[test] + /// validate_outputs_public_keys_are_unique rejects duplicate public key. + fn test_validate_output_public_keys_are_unique_rejects_duplicate() { + let (mut tx, _ledger) = create_test_tx(); + // Tx only contains a single output. Duplicate the + // output so that tx.output_public_keys() returns a duplicate public key. + let tx_out = tx.prefix.outputs[0].clone(); + tx.prefix.outputs.push(tx_out); + + assert_eq!( + validate_outputs_public_keys_are_unique(&tx), + Err(TransactionValidationError::DuplicateOutputPublicKey) + ); + } + + #[test] + /// validate_outputs_public_keys_are_unique returns Ok if all public keys are unique. + fn test_validate_output_public_keys_are_unique_ok() { + let (tx, _ledger) = create_test_tx(); + assert_eq!(validate_outputs_public_keys_are_unique(&tx), Ok(()),); + } + #[test] // `validate_signature` return OK for a valid transaction. fn test_validate_signature_ok() {