Skip to content

Commit

Permalink
Revert "allow transactions with "mixed" token types (mobilecoinfounda…
Browse files Browse the repository at this point in the history
…tion#1827)"

This reverts commit 36ed5152969bba4cfa892445f22696752853886e.

These were the conflicts I resolved:

```
diff --cc Cargo.lock
index f71361a4,fc9f5b6c..00000000
--- a/Cargo.lock
+++ b/Cargo.lock
diff --cc mobilecoind/src/payments.rs
index c3b6f0ed,5e0d6ea4..00000000
--- a/mobilecoind/src/payments.rs
+++ b/mobilecoind/src/payments.rs
@@@ -21,10 -21,10 +21,10 @@@ use mc_transaction_core::
      onetime_keys::recover_onetime_private_key,
      ring_signature::KeyImage,
      tx::{Tx, TxOut, TxOutConfirmationNumber, TxOutMembershipProof},
-     Amount, BlockIndex, BlockVersion, TokenId,
+     BlockIndex, BlockVersion, TokenId,
  };
  use mc_transaction_std::{
 -    ChangeDestination, EmptyMemoBuilder, InputCredentials, TransactionBuilder,
 +    ChangeDestination, EmptyMemoBuilder, InputCredentials, MemoBuilder, TransactionBuilder,
  };
  use mc_util_uri::FogUri;
  use rand::Rng;
@@@ -876,17 -867,19 +876,33 @@@ impl<T: BlockchainConnection + UserTxCo
              fog_resolver_factory(&fog_uris).map_err(Error::Fog)?
          };

++<<<<<<< HEAD
 +        // Create tx_builder.
 +        // TODO (GH #1522): Use RTH memo builder, optionally?
 +        let memo_builder: Box<dyn MemoBuilder + Send + Sync> =
 +            opt_memo_builder.unwrap_or_else(|| Box::new(EmptyMemoBuilder::default()));
 +
 +        let fee_amount = Amount::new(fee, token_id);
 +        let mut tx_builder =
 +            TransactionBuilder::new_with_box(block_version, fee_amount, fog_resolver, memo_builder)
 +                .map_err(|err| {
 +                    Error::TxBuild(format!("Error creating transaction builder: {}", err))
 +                })?;
++=======
+         // TODO: Use RTH memo builder, optionally?
+
+         // Create tx_builder.
+         let mut tx_builder = TransactionBuilder::new(
+             block_version,
+             token_id,
+             fog_resolver,
+             EmptyMemoBuilder::default(),
+         );
+
+         tx_builder
+             .set_fee(fee)
+             .map_err(|err| Error::TxBuild(format!("Error setting fee: {}", err)))?;
++>>>>>>> parent of 36ed5152... allow transactions with "mixed" token types (#1827)

          // Unzip each vec of tuples into a tuple of vecs.
          let mut rings_and_proofs: Vec<(Vec<TxOut>, Vec<TxOutMembershipProof>)> = rings
diff --cc transaction/core/src/ring_signature/rct_bulletproofs.rs
index 9c844bd7,836799f3..00000000
--- a/transaction/core/src/ring_signature/rct_bulletproofs.rs
+++ b/transaction/core/src/ring_signature/rct_bulletproofs.rs
@@@ -830,11 -529,7 +529,15 @@@ mod rct_bulletproofs_tests

                  let value = rng.next_u64();
                  let blinding = Scalar::random(rng);
++<<<<<<< HEAD
 +
 +                let token_id = TokenId::from(token_ids[i % token_ids.len()]);
 +                let generator = generator_cache.get(token_id);
 +
 +                let commitment = CompressedCommitment::new(value, blinding, generator);
++=======
+                 let commitment = CompressedCommitment::new(value, blinding, &generator);
++>>>>>>> parent of 36ed5152... allow transactions with "mixed" token types (#1827)

                  let real_index = rng.next_u64() as usize % (num_mixins + 1);
                  ring.insert(real_index, (onetime_public_key, commitment));
diff --cc transaction/core/src/tx_error.rs
index 29e5c016,e6bf28be..00000000
--- a/transaction/core/src/tx_error.rs
+++ b/transaction/core/src/tx_error.rs
@@@ -65,14 -65,6 +65,17 @@@ pub enum NewMemoError
      OutputsAfterChange,
      /// Changing the fee after the change output is not supported
      FeeAfterChange,
++<<<<<<< HEAD
 +    /// Invalid recipient address
 +    InvalidRecipient,
 +    /// Multiple outputs are not supported
 +    MultipleOutputs,
 +    /// Missing output
 +    MissingOutput,
 +    /// Mixed Token Ids are not supported in these memos
 +    MixedTokenIds,
++=======
++>>>>>>> parent of 36ed5152... allow transactions with "mixed" token types (#1827)
      /// Other: {0}
      Other(String),
  }
diff --cc transaction/core/src/validation/validate.rs
index 27b55868,0e74d16e..00000000
--- a/transaction/core/src/validation/validate.rs
+++ b/transaction/core/src/validation/validate.rs
@@@ -487,13 -484,888 +488,901 @@@ fn check_unique<T: Eq + core::hash::Has
      Ok(())
  }

++<<<<<<< HEAD
 +// NOTE: There are unit tests of every validation function, which appear in
 +// transaction/core/tests/validation.rs.
 +//
 +// The reason that these appear there is,
 +// many of the tests use `mc-transaction-core-test-utils` which itself depends
 +// on `mc-ledger-db` and `mc-transaction-core`, and this creates a circular
 +// dependency which leads to build problems, if the unit tests appear in-line
 +// here.
 +//
 +// Please add tests for any new validation functions there. Thank you!
++=======
+ #[cfg(test)]
+ mod tests {
+     use super::*;
+
+     extern crate alloc;
+
+     use alloc::vec::Vec;
+
+     use crate::{
+         constants::RING_SIZE,
+         tokens::Mob,
+         tx::{Tx, TxOutMembershipHash, TxOutMembershipProof},
+         validation::error::TransactionValidationError,
+         Token,
+     };
+
+     use crate::membership_proofs::Range;
+     use mc_crypto_keys::{CompressedRistrettoPublic, ReprBytes};
+     use mc_ledger_db::{Ledger, LedgerDB};
+     use mc_transaction_core_test_utils::{
+         create_ledger, create_transaction, create_transaction_with_amount_and_comparer,
+         initialize_ledger, AccountKey, InverseTxOutputsOrdering, INITIALIZE_LEDGER_AMOUNT,
+     };
+     use mc_transaction_std::{DefaultTxOutputsOrdering, TxOutputsOrdering};
+     use rand::{rngs::StdRng, SeedableRng};
+     use serde::{de::DeserializeOwned, ser::Serialize};
+
+     // HACK: To test validation we need valid Tx objects. The code to create them is
+     // complicated, and a variant of it resides inside the
+     // `transaction_test_utils` crate. However,when we depend on it in our
+     // [dev-dependencies], it will compile and link against another copy of this
+     // crate, since cargo is weird like that. Relying in the fact that both data
+     // structures are actually the same, this hack lets us convert from the
+     // `transaction` crate being compiled by `transaction_test_utils` to the one
+     // compiled as part of building test tests.
+     // If we want to avoid this hack, we could move transaction validation into its
+     // own crate.
+     fn adapt_hack<Src: Serialize, Dst: DeserializeOwned>(src: &Src) -> Dst {
+         let bytes = mc_util_serial::serialize(src).unwrap();
+         mc_util_serial::deserialize(&bytes).unwrap()
+     }
+
+     fn create_test_tx(block_version: BlockVersion) -> (Tx, LedgerDB) {
+         let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]);
+         let sender = AccountKey::random(&mut rng);
+         let mut ledger = create_ledger();
+         let n_blocks = 1;
+         initialize_ledger(
+             adapt_hack(&block_version),
+             &mut ledger,
+             n_blocks,
+             &sender,
+             &mut rng,
+         );
+
+         // Spend an output from the last block.
+         let block_contents = ledger.get_block_contents(n_blocks - 1).unwrap();
+         let tx_out = block_contents.outputs[0].clone();
+
+         let recipient = AccountKey::random(&mut rng);
+         let tx = create_transaction(
+             adapt_hack(&block_version),
+             &mut ledger,
+             &tx_out,
+             &sender,
+             &recipient.default_subaddress(),
+             n_blocks + 1,
+             &mut rng,
+         );
+
+         (adapt_hack(&tx), ledger)
+     }
+
+     fn create_test_tx_with_amount(
+         block_version: BlockVersion,
+         amount: u64,
+         fee: u64,
+     ) -> (Tx, LedgerDB) {
+         create_test_tx_with_amount_and_comparer::<DefaultTxOutputsOrdering>(
+             block_version,
+             amount,
+             fee,
+         )
+     }
+
+     fn create_test_tx_with_amount_and_comparer<O: TxOutputsOrdering>(
+         block_version: BlockVersion,
+         amount: u64,
+         fee: u64,
+     ) -> (Tx, LedgerDB) {
+         let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]);
+         let sender = AccountKey::random(&mut rng);
+         let mut ledger = create_ledger();
+         let n_blocks = 1;
+         initialize_ledger(
+             adapt_hack(&block_version),
+             &mut ledger,
+             n_blocks,
+             &sender,
+             &mut rng,
+         );
+
+         // Spend an output from the last block.
+         let block_contents = ledger.get_block_contents(n_blocks - 1).unwrap();
+         let tx_out = block_contents.outputs[0].clone();
+
+         let recipient = AccountKey::random(&mut rng);
+         let tx = create_transaction_with_amount_and_comparer::<_, _, O>(
+             adapt_hack(&block_version),
+             &mut ledger,
+             &tx_out,
+             &sender,
+             &recipient.default_subaddress(),
+             amount,
+             fee,
+             n_blocks + 1,
+             &mut rng,
+         );
+
+         (adapt_hack(&tx), ledger)
+     }
+
+     #[test]
+     // Should return MissingMemo when memos are missing in an output
+     fn test_validate_memo_exists() {
+         let (tx, _) = create_test_tx(BlockVersion::ZERO);
+         let tx_out = tx.prefix.outputs.first().unwrap();
+
+         assert!(tx_out.e_memo.is_none());
+         assert_eq!(
+             validate_memo_exists(tx_out),
+             Err(TransactionValidationError::MissingMemo)
+         );
+
+         let (tx, _) = create_test_tx(BlockVersion::ONE);
+         let tx_out = tx.prefix.outputs.first().unwrap();
+
+         assert!(tx_out.e_memo.is_some());
+         assert_eq!(validate_memo_exists(tx_out), Ok(()));
+     }
+
+     #[test]
+     // Should return MemosNotAllowed when memos are present in an output
+     fn test_validate_no_memo_exists() {
+         let (tx, _) = create_test_tx(BlockVersion::ZERO);
+         let tx_out = tx.prefix.outputs.first().unwrap();
+
+         assert!(tx_out.e_memo.is_none());
+         assert_eq!(validate_no_memo_exists(tx_out), Ok(()));
+
+         let (tx, _) = create_test_tx(BlockVersion::ONE);
+         let tx_out = tx.prefix.outputs.first().unwrap();
+
+         assert!(tx_out.e_memo.is_some());
+         assert_eq!(
+             validate_no_memo_exists(tx_out),
+             Err(TransactionValidationError::MemosNotAllowed)
+         );
+     }
+
+     #[test]
+     // Should return MissingMaskedTokenId when masked_token_id are missing in an
+     // output
+     fn test_validate_masked_token_id_exists() {
+         let (tx, _) = create_test_tx(BlockVersion::ONE);
+         let tx_out = tx.prefix.outputs.first().unwrap();
+
+         assert!(tx_out.masked_amount.masked_token_id.is_empty());
+         assert_eq!(
+             validate_masked_token_id_exists(tx_out),
+             Err(TransactionValidationError::MissingMaskedTokenId)
+         );
+
+         let (tx, _) = create_test_tx(BlockVersion::TWO);
+         let tx_out = tx.prefix.outputs.first().unwrap();
+
+         assert!(!tx_out.masked_amount.masked_token_id.is_empty());
+         assert_eq!(validate_memo_exists(tx_out), Ok(()));
+     }
+
+     #[test]
+     // Should return MemosNotAllowed when memos are present in an output
+     fn test_validate_no_masked_token_id_exists() {
+         let (tx, _) = create_test_tx(BlockVersion::ONE);
+         let tx_out = tx.prefix.outputs.first().unwrap();
+
+         assert!(tx_out.masked_amount.masked_token_id.is_empty());
+         assert_eq!(validate_no_masked_token_id_exists(tx_out), Ok(()));
+
+         let (tx, _) = create_test_tx(BlockVersion::TWO);
+         let tx_out = tx.prefix.outputs.first().unwrap();
+
+         assert!(!tx_out.masked_amount.masked_token_id.is_empty());
+         assert_eq!(
+             validate_no_masked_token_id_exists(tx_out),
+             Err(TransactionValidationError::MaskedTokenIdNotAllowed)
+         );
+     }
+
+     #[test]
+     // Should return Ok(()) when the Tx's membership proofs are correct and agree
+     // with ledger.
+     fn test_validate_membership_proofs() {
+         for block_version in BlockVersion::iterator() {
+             let (tx, ledger) = create_test_tx(block_version);
+
+             let highest_indices = tx.get_membership_proof_highest_indices();
+             let root_proofs: Vec<TxOutMembershipProof> = adapt_hack(
+                 &ledger
+                     .get_tx_out_proof_of_memberships(&highest_indices)
+                     .expect("failed getting proofs"),
+             );
+
+             // Validate the transaction prefix without providing the correct ledger context.
+             {
+                 let mut broken_proofs = root_proofs.clone();
+                 broken_proofs[0].elements[0].hash = TxOutMembershipHash::from([1u8; 32]);
+                 assert_eq!(
+                     validate_membership_proofs(&tx.prefix, &broken_proofs),
+                     Err(TransactionValidationError::InvalidTxOutMembershipProof)
+                 );
+             }
+
+             // Validate the transaction prefix with the correct root proofs.
+             {
+                 let highest_indices = tx.get_membership_proof_highest_indices();
+                 let root_proofs: Vec<TxOutMembershipProof> = adapt_hack(
+                     &ledger
+                         .get_tx_out_proof_of_memberships(&highest_indices)
+                         .expect("failed getting proofs"),
+                 );
+                 assert_eq!(validate_membership_proofs(&tx.prefix, &root_proofs), Ok(()));
+             }
+         }
+     }
+
+     #[test]
+     // Should return InvalidRangeProof if a membership proof containing an invalid
+     // Range.
+     fn test_validate_membership_proofs_invalid_range_in_tx() {
+         for block_version in BlockVersion::iterator() {
+             let (mut tx, ledger) = create_test_tx(block_version);
+
+             let highest_indices = tx.get_membership_proof_highest_indices();
+             let root_proofs: Vec<TxOutMembershipProof> = adapt_hack(
+                 &ledger
+                     .get_tx_out_proof_of_memberships(&highest_indices)
+                     .expect("failed getting proofs"),
+             );
+
+             // Modify tx to include an invalid Range.
+             let mut proof = tx.prefix.inputs[0].proofs[0].clone();
+             let mut first_element = proof.elements[0].clone();
+             first_element.range = Range { from: 7, to: 3 };
+             proof.elements[0] = first_element;
+             tx.prefix.inputs[0].proofs[0] = proof;
+
+             assert_eq!(
+                 validate_membership_proofs(&tx.prefix, &root_proofs),
+                 Err(TransactionValidationError::MembershipProofValidationError)
+             );
+         }
+     }
+
+     #[test]
+     // Should return InvalidRangeProof if a root proof containing an invalid Range.
+     fn test_validate_membership_proofs_invalid_range_in_root_proof() {
+         for block_version in BlockVersion::iterator() {
+             let (tx, ledger) = create_test_tx(block_version);
+
+             let highest_indices = tx.get_membership_proof_highest_indices();
+             let mut root_proofs: Vec<TxOutMembershipProof> = adapt_hack(
+                 &ledger
+                     .get_tx_out_proof_of_memberships(&highest_indices)
+                     .expect("failed getting proofs"),
+             );
+
+             // Modify a root proof to include an invalid Range.
+             let mut proof = root_proofs[0].clone();
+             let mut first_element = proof.elements[0].clone();
+             first_element.range = Range { from: 7, to: 3 };
+             proof.elements[0] = first_element;
+             root_proofs[0] = proof;
+
+             assert_eq!(
+                 validate_membership_proofs(&tx.prefix, &root_proofs),
+                 Err(TransactionValidationError::MembershipProofValidationError)
+             );
+         }
+     }
+
+     #[test]
+     fn test_validate_number_of_inputs() {
+         for block_version in BlockVersion::iterator() {
+             let (orig_tx, _ledger) = create_test_tx(block_version);
+             let max_inputs = 25;
+
+             for num_inputs in 0..100 {
+                 let mut tx_prefix = orig_tx.prefix.clone();
+                 tx_prefix.inputs.clear();
+                 for _i in 0..num_inputs {
+                     tx_prefix.inputs.push(orig_tx.prefix.inputs[0].clone());
+                 }
+
+                 let expected_result = if num_inputs == 0 {
+                     Err(TransactionValidationError::NoInputs)
+                 } else if num_inputs > max_inputs {
+                     Err(TransactionValidationError::TooManyInputs)
+                 } else {
+                     Ok(())
+                 };
+
+                 assert_eq!(
+                     validate_number_of_inputs(&tx_prefix, max_inputs),
+                     expected_result,
+                 );
+             }
+         }
+     }
+
+     #[test]
+     fn test_validate_number_of_outputs() {
+         for block_version in BlockVersion::iterator() {
+             let (orig_tx, _ledger) = create_test_tx(block_version);
+             let max_outputs = 25;
+
+             for num_outputs in 0..100 {
+                 let mut tx_prefix = orig_tx.prefix.clone();
+                 tx_prefix.outputs.clear();
+                 for _i in 0..num_outputs {
+                     tx_prefix.outputs.push(orig_tx.prefix.outputs[0].clone());
+                 }
+
+                 let expected_result = if num_outputs == 0 {
+                     Err(TransactionValidationError::NoOutputs)
+                 } else if num_outputs > max_outputs {
+                     Err(TransactionValidationError::TooManyOutputs)
+                 } else {
+                     Ok(())
+                 };
+
+                 assert_eq!(
+                     validate_number_of_outputs(&tx_prefix, max_outputs),
+                     expected_result,
+                 );
+             }
+         }
+     }
+
+     #[test]
+     fn test_validate_ring_sizes() {
+         for block_version in BlockVersion::iterator() {
+             let (tx, _ledger) = create_test_tx(block_version);
+             assert_eq!(tx.prefix.inputs.len(), 1);
+             assert_eq!(tx.prefix.inputs[0].ring.len(), RING_SIZE);
+
+             // A transaction with a single input containing RING_SIZE elements.
+             assert_eq!(validate_ring_sizes(&tx.prefix, RING_SIZE), Ok(()));
+
+             // A single input containing zero elements.
+             {
+                 let mut tx_prefix = tx.prefix.clone();
+                 tx_prefix.inputs[0].ring.clear();
+
+                 assert_eq!(
+                     validate_ring_sizes(&tx_prefix, RING_SIZE),
+                     Err(TransactionValidationError::InsufficientRingSize),
+                 );
+             }
+
+             // A single input containing too few elements.
+             {
+                 let mut tx_prefix = tx.prefix.clone();
+                 tx_prefix.inputs[0].ring.pop();
+
+                 assert_eq!(
+                     validate_ring_sizes(&tx_prefix, RING_SIZE),
+                     Err(TransactionValidationError::InsufficientRingSize),
+                 );
+             }
+
+             // A single input containing too many elements.
+             {
+                 let mut tx_prefix = tx.prefix.clone();
+                 let element = tx_prefix.inputs[0].ring[0].clone();
+                 tx_prefix.inputs[0].ring.push(element);
+
+                 assert_eq!(
+                     validate_ring_sizes(&tx_prefix, RING_SIZE),
+                     Err(TransactionValidationError::ExcessiveRingSize),
+                 );
+             }
+
+             // Two inputs each containing RING_SIZE elements.
+             {
+                 let mut tx_prefix = tx.prefix.clone();
+                 let input = tx_prefix.inputs[0].clone();
+                 tx_prefix.inputs.push(input);
+
+                 assert_eq!(validate_ring_sizes(&tx_prefix, RING_SIZE), Ok(()));
+             }
+
+             // The second input contains too few elements.
+             {
+                 let mut tx_prefix = tx.prefix.clone();
+                 let mut input = tx_prefix.inputs[0].clone();
+                 input.ring.pop();
+                 tx_prefix.inputs.push(input);
+
+                 assert_eq!(
+                     validate_ring_sizes(&tx_prefix, RING_SIZE),
+                     Err(TransactionValidationError::InsufficientRingSize),
+                 );
+             }
+         }
+     }
+
+     #[test]
+     fn test_validate_ring_elements_are_unique() {
+         for block_version in BlockVersion::iterator() {
+             let (tx, _ledger) = create_test_tx(block_version);
+             assert_eq!(tx.prefix.inputs.len(), 1);
+
+             // A transaction with a single input and unique ring elements.
+             assert_eq!(validate_ring_elements_are_unique(&tx.prefix), Ok(()));
+
+             // A transaction with a single input and duplicate ring elements.
+             {
+                 let mut tx_prefix = tx.prefix.clone();
+                 tx_prefix.inputs[0]
+                     .ring
+                     .push(tx.prefix.inputs[0].ring[0].clone());
+
+                 assert_eq!(
+                     validate_ring_elements_are_unique(&tx_prefix),
+                     Err(TransactionValidationError::DuplicateRingElements)
+                 );
+             }
+
+             // A transaction with a multiple inputs and unique ring elements.
+             {
+                 let mut tx_prefix = tx.prefix.clone();
+                 tx_prefix.inputs.push(tx.prefix.inputs[0].clone());
+
+                 for mut tx_out in tx_prefix.inputs[1].ring.iter_mut() {
+                     let mut bytes = tx_out.target_key.to_bytes();
+                     bytes[0] = !bytes[0];
+                     tx_out.target_key = CompressedRistrettoPublic::from_bytes(&bytes).unwrap();
+                 }
+
+                 assert_eq!(validate_ring_elements_are_unique(&tx_prefix), Ok(()));
+             }
+
+             // A transaction with a multiple inputs and duplicate ring elements in different
+             // rings.
+             {
+                 let mut tx_prefix = tx.prefix.clone();
+                 tx_prefix.inputs.push(tx.prefix.inputs[0].clone());
+
+                 assert_eq!(
+                     validate_ring_elements_are_unique(&tx_prefix),
+                     Err(TransactionValidationError::DuplicateRingElements)
+                 );
+             }
+         }
+     }
+
+     #[test]
+     /// validate_ring_elements_are_sorted should reject an unsorted ring.
+     fn test_validate_ring_elements_are_sorted() {
+         for block_version in BlockVersion::iterator() {
+             let (mut tx, _ledger) = create_test_tx(block_version);
+             assert_eq!(validate_ring_elements_are_sorted(&tx.prefix), Ok(()));
+
+             // Change the ordering of a ring.
+             tx.prefix.inputs[0].ring.swap(0, 3);
+             assert_eq!(
+                 validate_ring_elements_are_sorted(&tx.prefix),
+                 Err(TransactionValidationError::UnsortedRingElements)
+             );
+         }
+     }
+
+     #[test]
+     /// validate_inputs_are_sorted should reject unsorted inputs.
+     fn test_validate_inputs_are_sorted() {
+         for block_version in BlockVersion::iterator() {
+             let (tx, _ledger) = create_test_tx(block_version);
+
+             // Add a second input to the transaction.
+             let mut tx_prefix = tx.prefix.clone();
+             tx_prefix.inputs.push(tx.prefix.inputs[0].clone());
+
+             // By removing the first ring element of the second input we ensure the inputs
+             // are different, but remain sorted (since the ring elements are
+             // sorted).
+             tx_prefix.inputs[1].ring.remove(0);
+
+             assert_eq!(validate_inputs_are_sorted(&tx_prefix), Ok(()));
+
+             // Change the ordering of inputs.
+             tx_prefix.inputs.swap(0, 1);
+             assert_eq!(
+                 validate_inputs_are_sorted(&tx_prefix),
+                 Err(TransactionValidationError::UnsortedInputs)
+             );
+         }
+     }
+
+     #[test]
+     /// Should reject a transaction with unsorted outputs.
+     fn test_validate_outputs_are_sorted() {
+         for block_version in BlockVersion::iterator() {
+             let (tx, _ledger) = create_test_tx(block_version);
+
+             let mut output_a = tx.prefix.outputs.get(0).unwrap().clone();
+             output_a.public_key = CompressedRistrettoPublic::from(&[1u8; 32]);
+
+             let mut output_b = output_a.clone();
+             output_b.public_key = CompressedRistrettoPublic::from(&[2u8; 32]);
+
+             assert!(output_a.public_key < output_b.public_key);
+
+             {
+                 let mut tx_prefix = tx.prefix.clone();
+                 // A single output is trivially sorted.
+                 tx_prefix.outputs = vec![output_a.clone()];
+                 assert_eq!(validate_outputs_are_sorted(&tx_prefix), Ok(()));
+             }
+
+             {
+                 let mut tx_prefix = tx.prefix.clone();
+                 // Outputs sorted by public_key, ascending.
+                 tx_prefix.outputs = vec![output_a.clone(), output_b.clone()];
+                 assert_eq!(validate_outputs_are_sorted(&tx_prefix), Ok(()));
+             }
+
+             {
+                 let mut tx_prefix = tx.prefix.clone();
+                 // Outputs are not correctly sorted.
+                 tx_prefix.outputs = vec![output_b.clone(), output_a.clone()];
+                 assert_eq!(
+                     validate_outputs_are_sorted(&tx_prefix),
+                     Err(TransactionValidationError::UnsortedOutputs)
+                 );
+             }
+         }
+     }
+
+     #[test]
+     /// validate_key_images_are_unique rejects duplicate key image.
+     fn test_validate_key_images_are_unique_rejects_duplicate() {
+         for block_version in BlockVersion::iterator() {
+             let (mut tx, _ledger) = create_test_tx(block_version);
+             // Tx only contains a single ring signature, which contains the key image.
+             // Duplicate the ring signature so that tx.key_images() returns a
+             // duplicate key image.
+             let ring_signature = tx.signature.ring_signatures[0].clone();
+             tx.signature.ring_signatures.push(ring_signature);
+
+             assert_eq!(
+                 validate_key_images_are_unique(&tx),
+                 Err(TransactionValidationError::DuplicateKeyImages)
+             );
+         }
+     }
+
+     #[test]
+     /// validate_key_images_are_unique returns Ok if all key images are unique.
+     fn test_validate_key_images_are_unique_ok() {
+         for block_version in BlockVersion::iterator() {
+             let (tx, _ledger) = create_test_tx(block_version);
+             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() {
+         for block_version in BlockVersion::iterator() {
+             let (mut tx, _ledger) = create_test_tx(block_version);
+             // 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() {
+         for block_version in BlockVersion::iterator() {
+             let (tx, _ledger) = create_test_tx(block_version);
+             assert_eq!(validate_outputs_public_keys_are_unique(&tx), Ok(()),);
+         }
+     }
+
+     #[test]
+     // `validate_signature` return OK for a valid transaction.
+     fn test_validate_signature_ok() {
+         let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]);
+
+         for block_version in BlockVersion::iterator() {
+             let (tx, _ledger) = create_test_tx(block_version);
+             assert_eq!(validate_signature(block_version, &tx, &mut rng), Ok(()));
+         }
+     }
+
+     #[test]
+     // Should return InvalidTransactionSignature if an input is modified.
+     fn test_transaction_signature_err_modified_input() {
+         let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]);
+
+         for block_version in BlockVersion::iterator() {
+             let (mut tx, _ledger) = create_test_tx(block_version);
+
+             // Remove an input.
+             tx.prefix.inputs[0].ring.pop();
+
+             match validate_signature(block_version, &tx, &mut rng) {
+                 Err(TransactionValidationError::InvalidTransactionSignature(_e)) => {} // Expected.
+                 Err(e) => {
+                     panic!("Unexpected error {}", e);
+                 }
+                 Ok(()) => panic!("Unexpected success"),
+             }
+         }
+     }
+
+     #[test]
+     // Should return InvalidTransactionSignature if an output is modified.
+     fn test_transaction_signature_err_modified_output() {
+         let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]);
+
+         for block_version in BlockVersion::iterator() {
+             let (mut tx, _ledger) = create_test_tx(block_version);
+
+             // Add an output.
+             let output = tx.prefix.outputs.get(0).unwrap().clone();
+             tx.prefix.outputs.push(output);
+
+             match validate_signature(block_version, &tx, &mut rng) {
+                 Err(TransactionValidationError::InvalidTransactionSignature(_e)) => {} // Expected.
+                 Err(e) => {
+                     panic!("Unexpected error {}", e);
+                 }
+                 Ok(()) => panic!("Unexpected success"),
+             }
+         }
+     }
+
+     #[test]
+     // Should return InvalidTransactionSignature if the fee is modified.
+     fn test_transaction_signature_err_modified_fee() {
+         let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]);
+
+         for block_version in BlockVersion::iterator() {
+             let (mut tx, _ledger) = create_test_tx(block_version);
+
+             tx.prefix.fee += 1;
+
+             match validate_signature(block_version, &tx, &mut rng) {
+                 Err(TransactionValidationError::InvalidTransactionSignature(_e)) => {} // Expected.
+                 Err(e) => {
+                     panic!("Unexpected error {}", e);
+                 }
+                 Ok(()) => panic!("Unexpected success"),
+             }
+         }
+     }
+
+     #[test]
+     // Should return InvalidTransactionSignature if the token_id is modified
+     fn test_transaction_signature_err_modified_token_id() {
+         let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]);
+
+         for _ in 0..3 {
+             let (mut tx, _ledger) = create_test_tx(BlockVersion::TWO);
+
+             tx.prefix.token_id += 1;
+
+             match validate_signature(BlockVersion::TWO, &tx, &mut rng) {
+                 Err(TransactionValidationError::InvalidTransactionSignature(_e)) => {} // Expected.
+                 Err(e) => {
+                     panic!("Unexpected error {}", e);
+                 }
+                 Ok(()) => panic!("Unexpected success"),
+             }
+         }
+     }
+
+     #[test]
+     // Should return InvalidTransactionSignature if block v 1 is validated as 2
+     fn test_transaction_signature_err_version_one_as_two() {
+         let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]);
+
+         for _ in 0..3 {
+             let (tx, _ledger) = create_test_tx(BlockVersion::ONE);
+
+             match validate_signature(BlockVersion::TWO, &tx, &mut rng) {
+                 Err(TransactionValidationError::InvalidTransactionSignature(_e)) => {} // Expected.
+                 Err(e) => {
+                     panic!("Unexpected error {}", e);
+                 }
+                 Ok(()) => panic!("Unexpected success"),
+             }
+         }
+     }
+
+     #[test]
+     // Should return InvalidTransactionSignature if block v 2 is validated as 1
+     fn test_transaction_signature_err_version_two_as_one() {
+         let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]);
+
+         for _ in 0..3 {
+             let (tx, _ledger) = create_test_tx(BlockVersion::TWO);
+
+             match validate_signature(BlockVersion::ONE, &tx, &mut rng) {
+                 Err(TransactionValidationError::InvalidTransactionSignature(_e)) => {} // Expected.
+                 Err(e) => {
+                     panic!("Unexpected error {}", e);
+                 }
+                 Ok(()) => panic!("Unexpected success"),
+             }
+         }
+     }
+
+     #[test]
+     fn test_validate_transaction_fee() {
+         for block_version in BlockVersion::iterator() {
+             {
+                 // Zero fees gets rejected
+                 let (tx, _ledger) =
+                     create_test_tx_with_amount(block_version, INITIALIZE_LEDGER_AMOUNT, 0);
+                 assert_eq!(
+                     validate_transaction_fee(&tx, 1000),
+                     Err(TransactionValidationError::TxFeeError)
+                 );
+             }
+
+             {
+                 // Off by one fee gets rejected
+                 let fee = Mob::MINIMUM_FEE - 1;
+                 let (tx, _ledger) =
+                     create_test_tx_with_amount(block_version, INITIALIZE_LEDGER_AMOUNT - fee, fee);
+                 assert_eq!(
+                     validate_transaction_fee(&tx, Mob::MINIMUM_FEE),
+                     Err(TransactionValidationError::TxFeeError)
+                 );
+             }
+
+             {
+                 // Exact fee amount is okay
+                 let (tx, _ledger) = create_test_tx_with_amount(
+                     block_version,
+                     INITIALIZE_LEDGER_AMOUNT - Mob::MINIMUM_FEE,
+                     Mob::MINIMUM_FEE,
+                 );
+                 assert_eq!(validate_transaction_fee(&tx, Mob::MINIMUM_FEE), Ok(()));
+             }
+
+             {
+                 // Overpaying fees is okay
+                 let fee = Mob::MINIMUM_FEE + 1;
+                 let (tx, _ledger) =
+                     create_test_tx_with_amount(block_version, INITIALIZE_LEDGER_AMOUNT - fee, fee);
+                 assert_eq!(validate_transaction_fee(&tx, Mob::MINIMUM_FEE), Ok(()));
+             }
+         }
+     }
+
+     #[test]
+     /// Should return TombstoneBlockExceeded if the transaction has expired.
+     fn test_validate_tombstone_tombstone_block_exceeded() {
+         {
+             // The tombstone block is in the near future, so Ok.
+             let current_block_index = 888;
+             let tombstone_block_index = 889;
+             assert_eq!(
+                 validate_tombstone(current_block_index, tombstone_block_index),
+                 Ok(())
+             );
+         }
+
+         {
+             // The tombstone block is the current block.
+             let current_block_index = 7;
+             let tombstone_block_index = 7;
+             assert_eq!(
+                 validate_tombstone(current_block_index, tombstone_block_index),
+                 Err(TransactionValidationError::TombstoneBlockExceeded)
+             );
+         }
+
+         {
+             // The tombstone block is in the past.
+             let current_block_index = 888;
+             let tombstone_block_index = 7;
+             assert_eq!(
+                 validate_tombstone(current_block_index, tombstone_block_index),
+                 Err(TransactionValidationError::TombstoneBlockExceeded)
+             );
+         }
+     }
+
+     #[test]
+     /// Should return TombstoneBlockTooFar if the tombstone is too far in the
+     /// future.
+     fn test_validate_tombstone_tombstone_block_too_far() {
+         {
+             // The tombstone block is in the near future, so Ok.
+             let current_block_index = 7;
+             let tombstone_block_index = current_block_index + 1;
+             assert_eq!(
+                 validate_tombstone(current_block_index, tombstone_block_index),
+                 Ok(())
+             );
+         }
+
+         {
+             // Largest tombstone that is still Ok.
+             let current_block_index = 7;
+             let tombstone_block_index = current_block_index + MAX_TOMBSTONE_BLOCKS;
+             assert_eq!(
+                 validate_tombstone(current_block_index, tombstone_block_index),
+                 Ok(())
+             );
+         }
+
+         {
+             // Tombstone is too far in the future.
+             let current_block_index = 7;
+             let tombstone_block_index = current_block_index + MAX_TOMBSTONE_BLOCKS + 1;
+             assert_eq!(
+                 validate_tombstone(current_block_index, tombstone_block_index),
+                 Err(TransactionValidationError::TombstoneBlockTooFar)
+             );
+         }
+     }
+
+     #[test]
+     fn test_global_validate_for_blocks_with_sorted_outputs() {
+         let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]);
+         let fee = Mob::MINIMUM_FEE + 1;
+         for block_version in BlockVersion::iterator() {
+             // for block version < 3 it doesn't matter
+             // for >= 3 it shall return an error about unsorted outputs
+             let (tx, _ledger) = create_test_tx_with_amount_and_comparer::<InverseTxOutputsOrdering>(
+                 block_version,
+                 INITIALIZE_LEDGER_AMOUNT - fee,
+                 fee,
+             );
+
+             let highest_indices = tx.get_membership_proof_highest_indices();
+             let root_proofs: Vec<TxOutMembershipProof> = adapt_hack(
+                 &_ledger
+                     .get_tx_out_proof_of_memberships(&highest_indices)
+                     .expect("failed getting proofs"),
+             );
+
+             let result = validate(
+                 &tx,
+                 tx.prefix.tombstone_block - 1,
+                 block_version,
+                 &root_proofs,
+                 0,
+                 &mut rng,
+             );
+
+             assert_eq!(
+                 result,
+                 match block_version.validate_transaction_outputs_are_sorted() {
+                     true => Err(TransactionValidationError::UnsortedOutputs),
+                     false => Ok(()),
+                 }
+             )
+         }
+     }
+ }
++>>>>>>> parent of 36ed5152... allow transactions with "mixed" token types (#1827)
diff --cc transaction/std/src/transaction_builder.rs
index 68cb805e,1ef00663..00000000
--- a/transaction/std/src/transaction_builder.rs
+++ b/transaction/std/src/transaction_builder.rs
@@@ -2697,548 -2590,4 +2593,551 @@@ pub mod transaction_builder_tests
                  .is_err());
          }
      }
++<<<<<<< HEAD
 +
 +    #[test]
 +    // Transaction builder with Burn Redemption memo builder
 +    fn test_transaction_builder_burn_redemption_memos() {
 +        let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]);
 +        let block_version = BlockVersion::MAX;
 +        let token_id = TokenId::from(5);
 +        let fog_resolver = MockFogResolver::default();
 +        let sender = AccountKey::random(&mut rng);
 +        let change_destination = ChangeDestination::from(&sender);
 +
 +        // Adding an output that is not to the burn address is not allowed.
 +        {
 +            let memo_builder = BurnRedemptionMemoBuilder::new([2u8; 64]);
 +
 +            let mut transaction_builder = TransactionBuilder::new(
 +                block_version,
 +                Amount::new(10, token_id),
 +                fog_resolver.clone(),
 +                memo_builder,
 +            )
 +            .unwrap();
 +
 +            let recipient = AccountKey::random(&mut rng);
 +            let result = transaction_builder.add_output(
 +                Amount::new(100, token_id),
 +                &recipient.default_subaddress(),
 +                &mut rng,
 +            );
 +            assert_matches!(
 +                result,
 +                Err(TxBuilderError::NewTx(NewTxError::Memo(
 +                    NewMemoError::InvalidRecipient
 +                )))
 +            );
 +        }
 +
 +        // Adding two burn outputs is not allowed.
 +        {
 +            let memo_builder = BurnRedemptionMemoBuilder::new([2u8; 64]);
 +
 +            let mut transaction_builder = TransactionBuilder::new(
 +                block_version,
 +                Amount::new(10, token_id),
 +                fog_resolver.clone(),
 +                memo_builder,
 +            )
 +            .unwrap();
 +
 +            transaction_builder
 +                .add_output(Amount::new(100, token_id), &burn_address(), &mut rng)
 +                .unwrap();
 +
 +            let result = transaction_builder.add_output(
 +                Amount::new(100, token_id),
 +                &burn_address(),
 +                &mut rng,
 +            );
 +            assert_matches!(
 +                result,
 +                Err(TxBuilderError::NewTx(NewTxError::Memo(
 +                    NewMemoError::MultipleOutputs
 +                )))
 +            );
 +        }
 +
 +        // Adding a change output before a burn output is not allowed.
 +        {
 +            let mut memo_builder = BurnRedemptionMemoBuilder::new([2u8; 64]);
 +            memo_builder.enable_destination_memo();
 +
 +            let mut transaction_builder = TransactionBuilder::new(
 +                block_version,
 +                Amount::new(10, token_id),
 +                fog_resolver.clone(),
 +                memo_builder,
 +            )
 +            .unwrap();
 +
 +            let result = transaction_builder.add_change_output(
 +                Amount::new(10, token_id),
 +                &change_destination,
 +                &mut rng,
 +            );
 +
 +            assert_matches!(
 +                result,
 +                Err(TxBuilderError::NewTx(NewTxError::Memo(
 +                    NewMemoError::MissingOutput
 +                )))
 +            );
 +        }
 +
 +        // Setting fee after change output has been written is not allowed.
 +        {
 +            let mut memo_builder = BurnRedemptionMemoBuilder::new([3u8; 64]);
 +            memo_builder.enable_destination_memo();
 +
 +            let mut transaction_builder = TransactionBuilder::new(
 +                block_version,
 +                Amount::new(10, token_id),
 +                fog_resolver.clone(),
 +                memo_builder,
 +            )
 +            .unwrap();
 +
 +            transaction_builder.set_fee(3).unwrap();
 +
 +            let input_credentials = get_input_credentials(
 +                block_version,
 +                Amount::new(113, token_id),
 +                &AccountKey::random(&mut rng),
 +                &fog_resolver,
 +                &mut rng,
 +            );
 +            transaction_builder.add_input(input_credentials);
 +
 +            let (_burn_tx_out, _confirmation) = transaction_builder
 +                .add_output(Amount::new(100, token_id), &burn_address(), &mut rng)
 +                .unwrap();
 +
 +            transaction_builder
 +                .add_change_output(Amount::new(10, token_id), &change_destination, &mut rng)
 +                .unwrap();
 +
 +            let result = transaction_builder.set_fee(1235);
 +            assert_matches!(
 +                result,
 +                Err(TxBuilderError::Memo(NewMemoError::FeeAfterChange))
 +            );
 +        }
 +
 +        // Change in a different token is not allowed.
 +        {
 +            let mut memo_builder = BurnRedemptionMemoBuilder::new([3u8; 64]);
 +            memo_builder.enable_destination_memo();
 +
 +            let mut transaction_builder = TransactionBuilder::new(
 +                block_version,
 +                Amount::new(10, Mob::ID),
 +                fog_resolver.clone(),
 +                memo_builder,
 +            )
 +            .unwrap();
 +
 +            let (_burn_tx_out, _confirmation) = transaction_builder
 +                .add_output(Amount::new(100, token_id), &burn_address(), &mut rng)
 +                .unwrap();
 +
 +            let result = transaction_builder.add_change_output(
 +                Amount::new(10, token_id),
 +                &change_destination,
 +                &mut rng,
 +            );
 +
 +            assert_matches!(
 +                result,
 +                Err(TxBuilderError::NewTx(NewTxError::Memo(
 +                    NewMemoError::MixedTokenIds
 +                )))
 +            );
 +        }
 +
 +        // Happy flow without change
 +        {
 +            let mut memo_builder = BurnRedemptionMemoBuilder::new([2u8; 64]);
 +            memo_builder.enable_destination_memo();
 +
 +            let mut transaction_builder = TransactionBuilder::new(
 +                block_version,
 +                Amount::new(10, token_id),
 +                fog_resolver.clone(),
 +                memo_builder,
 +            )
 +            .unwrap();
 +
 +            transaction_builder.set_fee(3).unwrap();
 +
 +            let input_credentials = get_input_credentials(
 +                block_version,
 +                Amount::new(113, token_id),
 +                &AccountKey::random(&mut rng),
 +                &fog_resolver,
 +                &mut rng,
 +            );
 +            transaction_builder.add_input(input_credentials);
 +
 +            let (burn_output, _confirmation) = transaction_builder
 +                .add_output(Amount::new(110, token_id), &burn_address(), &mut rng)
 +                .unwrap();
 +
 +            let tx = transaction_builder.build(&mut rng).expect("build tx");
 +
 +            assert_eq!(tx.prefix.outputs.len(), 1);
 +            assert_eq!(burn_output, tx.prefix.outputs[0]);
 +
 +            // Test that view key matching works with the burn tx out with burn address view
 +            // key
 +            let (amount, _) = burn_output
 +                .view_key_match(&burn_address_view_private())
 +                .unwrap();
 +            assert_eq!(amount, Amount::new(110, token_id));
 +
 +            // Burn output should have a burn redemption memo
 +            let ss = get_tx_out_shared_secret(
 +                &burn_address_view_private(),
 +                &RistrettoPublic::try_from(&burn_output.public_key).unwrap(),
 +            );
 +            let memo = burn_output.e_memo.unwrap().decrypt(&ss);
 +            match MemoType::try_from(&memo).expect("Couldn't decrypt memo") {
 +                MemoType::BurnRedemption(memo) => {
 +                    assert_eq!(memo.memo_data(), &[2u8; 64],);
 +                }
 +                _ => {
 +                    panic!("unexpected memo type")
 +                }
 +            }
 +        }
 +
 +        // Happy flow with change
 +        {
 +            let mut memo_builder = BurnRedemptionMemoBuilder::new([3u8; 64]);
 +            memo_builder.enable_destination_memo();
 +
 +            let mut transaction_builder = TransactionBuilder::new(
 +                block_version,
 +                Amount::new(10, token_id),
 +                fog_resolver.clone(),
 +                memo_builder,
 +            )
 +            .unwrap();
 +
 +            transaction_builder.set_fee(3).unwrap();
 +
 +            let input_credentials = get_input_credentials(
 +                block_version,
 +                Amount::new(113, token_id),
 +                &AccountKey::random(&mut rng),
 +                &fog_resolver,
 +                &mut rng,
 +            );
 +            transaction_builder.add_input(input_credentials);
 +
 +            let (burn_tx_out, _confirmation) = transaction_builder
 +                .add_output(Amount::new(100, token_id), &burn_address(), &mut rng)
 +                .unwrap();
 +
 +            transaction_builder
 +                .add_change_output(Amount::new(10, token_id), &change_destination, &mut rng)
 +                .unwrap();
 +
 +            let tx = transaction_builder.build(&mut rng).expect("build tx");
 +
 +            assert_eq!(tx.prefix.outputs.len(), 2);
 +
 +            let burn_output = tx
 +                .prefix
 +                .outputs
 +                .iter()
 +                .find(|tx_out| tx_out.public_key == burn_tx_out.public_key)
 +                .expect("Didn't find recipient's output");
 +            let change_output = tx
 +                .prefix
 +                .outputs
 +                .iter()
 +                .find(|tx_out| {
 +                    subaddress_matches_tx_out(&sender, CHANGE_SUBADDRESS_INDEX, tx_out).unwrap()
 +                })
 +                .expect("Didn't find sender's output");
 +
 +            // Test that view key matching works with the burn tx out with burn address view
 +            // key
 +            let (amount, _) = burn_output
 +                .view_key_match(&burn_address_view_private())
 +                .unwrap();
 +            assert_eq!(amount, Amount::new(100, token_id));
 +
 +            assert!(change_output
 +                .view_key_match(&burn_address_view_private())
 +                .is_err());
 +
 +            // Test that view key matching works with the change tx out with sender's view
 +            // key
 +            let (amount, _) = change_output
 +                .view_key_match(sender.view_private_key())
 +                .unwrap();
 +            assert_eq!(amount, Amount::new(10, token_id));
 +
 +            assert!(burn_output
 +                .view_key_match(sender.view_private_key())
 +                .is_err());
 +
 +            // Burn output should have a burn redemption memo
 +            let ss = get_tx_out_shared_secret(
 +                &burn_address_view_private(),
 +                &RistrettoPublic::try_from(&burn_output.public_key).unwrap(),
 +            );
 +            let memo = burn_output.e_memo.unwrap().decrypt(&ss);
 +            match MemoType::try_from(&memo).expect("Couldn't decrypt memo") {
 +                MemoType::BurnRedemption(memo) => {
 +                    assert_eq!(memo.memo_data(), &[3u8; 64],);
 +                }
 +                _ => {
 +                    panic!("unexpected memo type")
 +                }
 +            }
 +
 +            // Change output should have a destination memo
 +            let ss = get_tx_out_shared_secret(
 +                sender.view_private_key(),
 +                &RistrettoPublic::try_from(&change_output.public_key).unwrap(),
 +            );
 +            let memo = change_output.e_memo.unwrap().decrypt(&ss);
 +            match MemoType::try_from(&memo).expect("Couldn't decrypt memo") {
 +                MemoType::Destination(memo) => {
 +                    assert_eq!(
 +                        memo.get_address_hash(),
 +                        &ShortAddressHash::from(&burn_address()),
 +                        "lookup based on address hash failed"
 +                    );
 +                    assert_eq!(memo.get_num_recipients(), 1);
 +                    assert_eq!(memo.get_fee(), 3);
 +                    assert_eq!(
 +                        memo.get_total_outlay(),
 +                        103,
 +                        "outlay should be amount sent to recipient + fee"
 +                    );
 +                }
 +                _ => {
 +                    panic!("unexpected memo type")
 +                }
 +            }
 +        }
 +    }
 +
 +    #[test]
 +    // Test that sending mixed transactions works
 +    //
 +    // This test uses inputs of two different token IDs, paying the fee and creating
 +    // change with TokenID1, and "passing through" the second token ID with its
 +    // full amount as output.
 +    fn test_mixed_transactions() {
 +        let mut rng: StdRng = SeedableRng::from_seed([18u8; 32]);
 +
 +        let fog_resolver = MockFogResolver::default();
 +        let sender = AccountKey::random(&mut rng);
 +        let sender_change_dest = ChangeDestination::from(&sender);
 +        let recipient = AccountKey::random(&mut rng);
 +        let recipient_addr = recipient.default_subaddress();
 +
 +        let amount1 = Amount::new(1475 * MILLIMOB_TO_PICOMOB, Mob::ID);
 +        let change_amount = Amount::new(128 * MILLIMOB_TO_PICOMOB, Mob::ID);
 +        let amount2 = Amount::new(999999, 2.into());
 +
 +        let tx_out1_right_amount = Amount::new(
 +            amount1.value - change_amount.value - Mob::MINIMUM_FEE,
 +            Mob::ID,
 +        );
 +
 +        for block_version in 3..=*BlockVersion::MAX {
 +            let block_version = BlockVersion::try_from(block_version).unwrap();
 +            let memo_builder = EmptyMemoBuilder::default();
 +
 +            let mut transaction_builder = TransactionBuilder::new(
 +                block_version,
 +                Amount::new(Mob::MINIMUM_FEE, Mob::ID),
 +                fog_resolver.clone(),
 +                memo_builder,
 +            )
 +            .unwrap();
 +
 +            let input_credentials =
 +                get_input_credentials(block_version, amount1, &sender, &fog_resolver, &mut rng);
 +            transaction_builder.add_input(input_credentials);
 +
 +            let input_credentials =
 +                get_input_credentials(block_version, amount2, &sender, &fog_resolver, &mut rng);
 +            transaction_builder.add_input(input_credentials);
 +
 +            let (tx_out1, _confirmation) = transaction_builder
 +                .add_output(tx_out1_right_amount, &recipient_addr, &mut rng)
 +                .unwrap();
 +
 +            let (tx_out2, _confirmation) = transaction_builder
 +                .add_output(amount2, &recipient_addr, &mut rng)
 +                .unwrap();
 +
 +            transaction_builder
 +                .add_change_output(change_amount, &sender_change_dest, &mut rng)
 +                .unwrap();
 +
 +            let tx = transaction_builder.build(&mut rng).unwrap();
 +
 +            assert_eq!(tx.prefix.outputs.len(), 3);
 +            let idx1 = tx
 +                .prefix
 +                .outputs
 +                .iter()
 +                .position(|tx_out| tx_out.public_key == tx_out1.public_key)
 +                .unwrap();
 +            let idx2 = tx
 +                .prefix
 +                .outputs
 +                .iter()
 +                .position(|tx_out| tx_out.public_key == tx_out2.public_key)
 +                .unwrap();
 +            let change_idx = (0..3).find(|x| *x != idx1 && *x != idx2).unwrap();
 +
 +            let change_tx_out = &tx.prefix.outputs[change_idx];
 +
 +            // Test that sender's change subaddress owns the change, and not the other tx
 +            // outs
 +            assert!(
 +                !subaddress_matches_tx_out(&sender, CHANGE_SUBADDRESS_INDEX, &tx_out1).unwrap()
 +            );
 +            assert!(
 +                !subaddress_matches_tx_out(&sender, CHANGE_SUBADDRESS_INDEX, &tx_out2).unwrap()
 +            );
 +            assert!(
 +                subaddress_matches_tx_out(&sender, CHANGE_SUBADDRESS_INDEX, change_tx_out).unwrap()
 +            );
 +
 +            // Test that recipients's default subaddress owns the correct output, and not
 +            // the other tx outs
 +            assert!(
 +                subaddress_matches_tx_out(&recipient, DEFAULT_SUBADDRESS_INDEX, &tx_out1).unwrap()
 +            );
 +            assert!(
 +                subaddress_matches_tx_out(&recipient, DEFAULT_SUBADDRESS_INDEX, &tx_out2).unwrap()
 +            );
 +            assert!(!subaddress_matches_tx_out(
 +                &recipient,
 +                DEFAULT_SUBADDRESS_INDEX,
 +                change_tx_out
 +            )
 +            .unwrap());
 +
 +            // Test that view key matching works with the two tx outs
 +            let (amount, _) = tx_out1
 +                .view_key_match(recipient.view_private_key())
 +                .unwrap();
 +            assert_eq!(
 +                amount.value,
 +                amount1.value - change_amount.value - Mob::MINIMUM_FEE
 +            );
 +            assert_eq!(amount.token_id, Mob::ID);
 +
 +            let (amount, _) = tx_out2
 +                .view_key_match(recipient.view_private_key())
 +                .unwrap();
 +            assert_eq!(amount, amount2);
 +
 +            assert!(change_tx_out
 +                .view_key_match(recipient.view_private_key())
 +                .is_err());
 +
 +            // Test that view key matching works with the change tx out with sender's view
 +            // key
 +            let (amount, _) = change_tx_out
 +                .view_key_match(sender.view_private_key())
 +                .unwrap();
 +            assert_eq!(amount.value, change_amount.value);
 +
 +            assert!(tx_out1.view_key_match(sender.view_private_key()).is_err());
 +
 +            assert!(tx_out2.view_key_match(sender.view_private_key()).is_err());
 +        }
 +    }
 +
 +    #[test]
 +    // Test mixed transactions expected failures (imbalanced transactions)
 +    fn test_mixed_transactions_expected_failure_imbalanced_transactions() {
 +        let mut rng: StdRng = SeedableRng::from_seed([18u8; 32]);
 +
 +        let fog_resolver = MockFogResolver::default();
 +        let sender = AccountKey::random(&mut rng);
 +        let sender_change_dest = ChangeDestination::from(&sender);
 +        let recipient = AccountKey::random(&mut rng);
 +        let recipient_addr = recipient.default_subaddress();
 +
 +        let amount1 = Amount::new(1475 * MILLIMOB_TO_PICOMOB, Mob::ID);
 +        let change_amount = Amount::new(128 * MILLIMOB_TO_PICOMOB, Mob::ID);
 +        let amount2 = Amount::new(999999, 2.into());
 +
 +        let tx_out1_right_amount = Amount::new(
 +            amount1.value - change_amount.value - Mob::MINIMUM_FEE,
 +            Mob::ID,
 +        );
 +
 +        // Builds a transaction using a particular amount in place of tx_out1, returning
 +        // result of `.build()`
 +        let mut test_fn = |block_version, tx_out1_amount| -> Result<_, _> {
 +            let memo_builder = EmptyMemoBuilder::default();
 +
 +            let mut transaction_builder = TransactionBuilder::new(
 +                block_version,
 +                Amount::new(Mob::MINIMUM_FEE, Mob::ID),
 +                fog_resolver.clone(),
 +                memo_builder,
 +            )
 +            .unwrap();
 +
 +            let input_credentials =
 +                get_input_credentials(block_version, amount1, &sender, &fog_resolver, &mut rng);
 +            transaction_builder.add_input(input_credentials);
 +
 +            let input_credentials =
 +                get_input_credentials(block_version, amount2, &sender, &fog_resolver, &mut rng);
 +            transaction_builder.add_input(input_credentials);
 +
 +            let (_tx_out1, _confirmation) = transaction_builder
 +                .add_output(tx_out1_amount, &recipient_addr, &mut rng)
 +                .unwrap();
 +
 +            let (_tx_out2, _confirmation) = transaction_builder
 +                .add_output(amount2, &recipient_addr, &mut rng)
 +                .unwrap();
 +
 +            transaction_builder
 +                .add_change_output(change_amount, &sender_change_dest, &mut rng)
 +                .unwrap();
 +
 +            transaction_builder.build(&mut rng)
 +        };
 +
 +        for block_version in 3..=*BlockVersion::MAX {
 +            let block_version = BlockVersion::try_from(block_version).unwrap();
 +
 +            assert!(test_fn(block_version, tx_out1_right_amount).is_ok());
 +
 +            let mut tx_out1_wrong_amount = tx_out1_right_amount;
 +            tx_out1_wrong_amount.value -= 1;
 +            assert!(test_fn(block_version, tx_out1_wrong_amount).is_err());
 +
 +            tx_out1_wrong_amount.value += 2;
 +            assert!(test_fn(block_version, tx_out1_wrong_amount).is_err());
 +
 +            tx_out1_wrong_amount.token_id = 99.into();
 +            assert!(test_fn(block_version, tx_out1_wrong_amount).is_err());
 +
 +            tx_out1_wrong_amount.value -= 1;
 +            assert!(test_fn(block_version, tx_out1_wrong_amount).is_err());
 +        }
 +    }
++=======
++>>>>>>> parent of 36ed5152... allow transactions with "mixed" token types (#1827)
  }
```
  • Loading branch information
cbeck88 committed Apr 29, 2022
1 parent a0d083b commit f94b184
Show file tree
Hide file tree
Showing 33 changed files with 558 additions and 1,641 deletions.
66 changes: 0 additions & 66 deletions Cargo.lock

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

25 changes: 5 additions & 20 deletions android-bindings/src/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ use mc_transaction_core::{
ring_signature::KeyImage,
tokens::Mob,
tx::{Tx, TxOut, TxOutConfirmationNumber, TxOutMembershipProof},
Amount, BlockVersion, CompressedCommitment, MaskedAmount, Token,
BlockVersion, CompressedCommitment, MaskedAmount, Token,
};

use mc_transaction_std::{
Expand Down Expand Up @@ -1603,13 +1603,12 @@ pub unsafe extern "C" fn Java_com_mobilecoin_lib_TransactionBuilder_init_1jni(
// FIXME #1595: The token id should be a parameter and not hard coded to Mob
// here
let token_id = Mob::ID;
let fee_amount = Amount::new(Mob::MINIMUM_FEE, token_id);
let tx_builder = TransactionBuilder::new_with_box(
block_version,
fee_amount,
token_id,
fog_resolver.clone(),
memo_builder_box,
)?;
);

Ok(env.set_rust_field(obj, RUST_OBJ_FIELD, tx_builder)?)
})
Expand Down Expand Up @@ -1693,19 +1692,12 @@ pub unsafe extern "C" fn Java_com_mobilecoin_lib_TransactionBuilder_add_1output(

let value = jni_big_int_to_u64(env, value)?;

// TODO (GH #1867): If you want to do mixed transactions, use something other
// than fee_token_id here.
let amount = Amount {
value: value as u64,
token_id: tx_builder.get_fee_token_id(),
};

let recipient: MutexGuard<PublicAddress> =
env.get_rust_field(recipient, RUST_OBJ_FIELD)?;

let mut rng = McRng::default();
let (tx_out, confirmation_number) =
tx_builder.add_output(amount, &recipient, &mut rng)?;
tx_builder.add_output(value as u64, &recipient, &mut rng)?;
if !confirmation_number_out.is_null() {
let len = env.get_array_length(confirmation_number_out)?;
if len as usize >= confirmation_number.to_vec().len() {
Expand Down Expand Up @@ -1751,15 +1743,8 @@ pub unsafe extern "C" fn Java_com_mobilecoin_lib_TransactionBuilder_add_1change_
ChangeDestination::from(&*source_account_key);
let mut rng = McRng::default();

// TODO (GH #1867): If you want to do mixed transactions, use something other
// than fee_token_id here.
let amount = Amount {
value: value as u64,
token_id: tx_builder.get_fee_token_id(),
};

let (tx_out, confirmation_number) =
tx_builder.add_change_output(amount, &change_destination, &mut rng)?;
tx_builder.add_change_output(value, &change_destination, &mut rng)?;
if !confirmation_number_out.is_null() {
let len = env.get_array_length(confirmation_number_out)?;
if len as usize >= confirmation_number.to_vec().len() {
Expand Down
35 changes: 3 additions & 32 deletions api/proto/external.proto
Original file line number Diff line number Diff line change
Expand Up @@ -242,49 +242,20 @@ message TxPrefix {
// The block index at which this transaction is no longer valid.
uint64 tombstone_block = 4;

// Token id for the fee of this transaction
fixed64 fee_token_id = 5;
// Token id for this transaction
fixed64 token_id = 5;
}

// A ring mlsag is a group-ring signature conferring spending authority of one TxOut
// which is part of a TxIn.
message RingMLSAG {
// The initial challenge value for the ring signature
CurveScalar c_zero = 1;
// The "responses", one for each input which is signed
repeated CurveScalar responses = 2;
// The key image is a hash unique to the "true" spent input. This cannot
// be linked back to determine the true spent input, but the input cannot be
// spent again without producing the same key image value, so this is used to
// prevent double-spends.
KeyImage key_image = 3;
}

message SignatureRctBulletproofs {
// A ring-signature, one for each TxIn, producing one pseudo-output and key image.
repeated RingMLSAG ring_signatures = 1;
// The amount commitments for each pseudo-output.
// There must be one of these for each TxIn.
repeated CompressedRistretto pseudo_output_commitments = 2;
// Before mixed transactions feature, there is one range proof for all pseudo-output
// and output commitments, whose serialized bytes appear here.
// After mixed transactions feature, this field is empty.
bytes range_proof_bytes = 3;
// Before mixed transactions feature, this field is empty.
// After mixed transactions feature, this field contains one range proof for each
// token id which appears in the transaction, in sorted order of token ids.
// It range-proofs the pseudo-outputs and outputs with that token id, in the order
// that they appear in the transaction.
repeated bytes range_proofs = 4;
// The token ids of each pseudo ouptut. There must be one of these for each TxIn.
// Before mixed transactions feature, this field is empty, and the token ids of
// all pseudo-outputs are inferred from the tx.prefix.fee_token_id.
repeated fixed64 pseudo_output_token_ids = 5;
// The token ids of each output. There must be one of these for each output of the Tx.
// (tx.prefix.outputs).
// Before mixed transactions feature, this field is empty, and the token ids of
// all outputs are inferred from the tx.prefix.fee_token_id.
repeated fixed64 output_token_ids = 6;
bytes range_proofs = 3;
}

message Tx {
Expand Down
14 changes: 2 additions & 12 deletions api/src/convert/signature_rct_bulletproofs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use mc_transaction_core::{
ring_signature::{RingMLSAG, SignatureRctBulletproofs},
CompressedCommitment,
};
use protobuf::RepeatedField;
use std::convert::TryFrom;

impl From<&SignatureRctBulletproofs> for external::SignatureRctBulletproofs {
Expand All @@ -26,10 +25,7 @@ impl From<&SignatureRctBulletproofs> for external::SignatureRctBulletproofs {
.collect();
signature.set_pseudo_output_commitments(pseudo_output_commitments.into());

signature.set_range_proof_bytes(source.range_proof_bytes.clone());
signature.set_range_proofs(RepeatedField::from_vec(source.range_proofs.clone()));
signature.set_pseudo_output_token_ids(source.pseudo_output_token_ids.clone());
signature.set_output_token_ids(source.output_token_ids.clone());
signature.set_range_proofs(source.range_proof_bytes.clone());

signature
}
Expand All @@ -50,18 +46,12 @@ impl TryFrom<&external::SignatureRctBulletproofs> for SignatureRctBulletproofs {
.push(CompressedCommitment::try_from(pseudo_output_commitment)?);
}

let range_proof_bytes = source.get_range_proof_bytes().to_vec();
let range_proofs = source.get_range_proofs().to_vec();
let pseudo_output_token_ids = source.get_pseudo_output_token_ids().to_vec();
let output_token_ids = source.get_output_token_ids().to_vec();
let range_proof_bytes = source.get_range_proofs().to_vec();

Ok(SignatureRctBulletproofs {
ring_signatures,
pseudo_output_commitments,
range_proof_bytes,
range_proofs,
pseudo_output_token_ids,
output_token_ids,
})
}
}
13 changes: 4 additions & 9 deletions api/src/convert/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ mod tests {
onetime_keys::recover_onetime_private_key,
tokens::Mob,
tx::{Tx, TxOut, TxOutMembershipProof},
Amount, BlockVersion, Token,
BlockVersion, Token,
};
use mc_transaction_core_test_utils::MockFogResolver;
use mc_transaction_std::{EmptyMemoBuilder, InputCredentials, TransactionBuilder};
Expand Down Expand Up @@ -72,11 +72,10 @@ mod tests {

let mut transaction_builder = TransactionBuilder::new(
block_version,
Amount::new(Mob::MINIMUM_FEE, Mob::ID),
Mob::ID,
MockFogResolver::default(),
EmptyMemoBuilder::default(),
)
.unwrap();
);

let ring: Vec<TxOut> = minted_outputs.clone();
let public_key = RistrettoPublic::try_from(&minted_outputs[0].public_key).unwrap();
Expand Down Expand Up @@ -107,11 +106,7 @@ mod tests {
transaction_builder.add_input(input_credentials);
transaction_builder.set_fee(0).unwrap();
transaction_builder
.add_output(
Amount::new(65536, Mob::ID),
&bob.default_subaddress(),
&mut rng,
)
.add_output(65536, &bob.default_subaddress(), &mut rng)
.unwrap();

let tx = transaction_builder.build(&mut rng).unwrap();
Expand Down
4 changes: 2 additions & 2 deletions api/src/convert/tx_prefix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ impl From<&tx::TxPrefix> for external::TxPrefix {

tx_prefix.set_fee(source.fee);

tx_prefix.set_fee_token_id(source.fee_token_id);
tx_prefix.set_token_id(source.token_id);

tx_prefix.set_tombstone_block(source.tombstone_block);

Expand Down Expand Up @@ -47,7 +47,7 @@ impl TryFrom<&external::TxPrefix> for tx::TxPrefix {
inputs,
outputs,
fee: source.get_fee(),
fee_token_id: source.get_fee_token_id(),
token_id: source.get_token_id(),
tombstone_block: source.get_tombstone_block(),
};
Ok(tx_prefix)
Expand Down
12 changes: 6 additions & 6 deletions consensus/enclave/impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,10 @@ impl SgxConsensusEnclave {
// We need to make sure all transactions are valid. We also ensure they all
// point at the same root membership element.
for (tx, proofs) in transactions_with_proofs.iter() {
let fee_token_id = TokenId::from(tx.prefix.fee_token_id);
let token_id = TokenId::from(tx.prefix.token_id);

let minimum_fee = ct_min_fees
.get(&fee_token_id)
.get(&token_id)
.ok_or(TransactionValidationError::TokenNotYetConfigured)?;

mc_transaction_core::validation::validate(
Expand Down Expand Up @@ -685,12 +685,12 @@ impl ConsensusEnclave for SgxConsensusEnclave {
.decrypt_bytes(locally_encrypted_tx.0)?;
let tx: Tx = mc_util_serial::decode(&decrypted_bytes)?;

let fee_token_id = TokenId::from(tx.prefix.fee_token_id);
let token_id = TokenId::from(tx.prefix.token_id);

// Validate.
let mut csprng = McRng::default();
let minimum_fee = ct_min_fee_map
.get(&fee_token_id)
.get(&token_id)
.ok_or(TransactionValidationError::TokenNotYetConfigured)?;
mc_transaction_core::validation::validate(
&tx,
Expand Down Expand Up @@ -782,8 +782,8 @@ impl ConsensusEnclave for SgxConsensusEnclave {
// Compute the total fees for each known token id, for tx's in this block.
let mut total_fees: CtTokenMap<u128> = ct_min_fee_map.keys().cloned().collect();
for tx in transactions.iter() {
let fee_token_id = TokenId::from(tx.prefix.fee_token_id);
total_fees.add(&fee_token_id, tx.prefix.fee as u128);
let token_id = TokenId::from(tx.prefix.token_id);
total_fees.add(&token_id, tx.prefix.fee as u128);
}

// Sort the token ids which did not appear to the end.
Expand Down
Loading

0 comments on commit f94b184

Please sign in to comment.