port a select subset of the changes from mixed transactions for candi…

…date 1.2 (#1909)

* port a select subset of the changes from mixed transactions for candidate 1.2

The changes ported here are:
* Rename `token_id` to `fee_token_id` in `TxPrefix`. This avoids a breaking
  change between this and 1.3 in the hashes.
* Make `TransactionBuilder::new` take a fee amount instead of a fee token id
  This also changes te memo builder trait, so that it will be compatible with
  the burn redemption memo builder which we hope to port next.
  This also makes `TransactionBuilder` possibly return an error.
* Adapt all clients and sdks for `TransactionBuilder::new` changes.
* Adapt all tests for this change
* Bring the  `Amount::new` function which makes the test code nicer.

* small change to `TransactionBuilder::get_fee` per Remoun

android-bindings/src/bindings.rs

@@ -48,7 +48,7 @@ use mc_transaction_core::{
     ring_signature::KeyImage,
     tokens::Mob,
     tx::{Tx, TxOut, TxOutConfirmationNumber, TxOutMembershipProof},
-    BlockVersion, CompressedCommitment, MaskedAmount, Token,
+    Amount, BlockVersion, CompressedCommitment, MaskedAmount, Token,
 };
 
 use mc_transaction_std::{
@@ -1602,13 +1602,13 @@ pub unsafe extern "C" fn Java_com_mobilecoin_lib_TransactionBuilder_init_1jni(
             env.take_rust_field(memo_builder_box, RUST_OBJ_FIELD)?;
         // 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, Mob::ID);
         let tx_builder = TransactionBuilder::new_with_box(
             block_version,
-            token_id,
+            fee_amount,
             fog_resolver.clone(),
             memo_builder_box,
-        );
+        )?;
 
         Ok(env.set_rust_field(obj, RUST_OBJ_FIELD, tx_builder)?)
     })

api/proto/external.proto

@@ -242,8 +242,8 @@ message TxPrefix {
     // The block index at which this transaction is no longer valid.
     uint64 tombstone_block = 4;
 
-    // Token id for this transaction
-    fixed64 token_id = 5;
+    // Token id for the fee for this transaction
+    fixed64 fee_token_id = 5;
 }
 
 message RingMLSAG {

api/src/convert/tx.rs

@@ -34,7 +34,7 @@ mod tests {
         onetime_keys::recover_onetime_private_key,
         tokens::Mob,
         tx::{Tx, TxOut, TxOutMembershipProof},
-        BlockVersion, Token,
+        Amount, BlockVersion, Token,
     };
     use mc_transaction_core_test_utils::MockFogResolver;
     use mc_transaction_std::{EmptyMemoBuilder, InputCredentials, TransactionBuilder};
@@ -70,12 +70,14 @@ mod tests {
                 )
             };
 
+            let fee_amount = Amount::new(Mob::MINIMUM_FEE, Mob::ID);
             let mut transaction_builder = TransactionBuilder::new(
                 block_version,
-                Mob::ID,
+                fee_amount,
                 MockFogResolver::default(),
                 EmptyMemoBuilder::default(),
-            );
+            )
+            .unwrap();
 
             let ring: Vec<TxOut> = minted_outputs.clone();
             let public_key = RistrettoPublic::try_from(&minted_outputs[0].public_key).unwrap();

api/src/convert/tx_prefix.rs

@@ -18,7 +18,7 @@ impl From<&tx::TxPrefix> for external::TxPrefix {
 
         tx_prefix.set_fee(source.fee);
 
-        tx_prefix.set_token_id(source.token_id);
+        tx_prefix.set_fee_token_id(source.fee_token_id);
 
         tx_prefix.set_tombstone_block(source.tombstone_block);
 
@@ -47,7 +47,7 @@ impl TryFrom<&external::TxPrefix> for tx::TxPrefix {
             inputs,
             outputs,
             fee: source.get_fee(),
-            token_id: source.get_token_id(),
+            fee_token_id: source.get_fee_token_id(),
             tombstone_block: source.get_tombstone_block(),
         };
         Ok(tx_prefix)

consensus/enclave/impl/src/lib.rs

@@ -236,7 +236,7 @@ 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 token_id = TokenId::from(tx.prefix.token_id);
+            let token_id = TokenId::from(tx.prefix.fee_token_id);
 
             let minimum_fee = ct_min_fees
                 .get(&token_id)
@@ -685,7 +685,7 @@ impl ConsensusEnclave for SgxConsensusEnclave {
             .decrypt_bytes(locally_encrypted_tx.0)?;
         let tx: Tx = mc_util_serial::decode(&decrypted_bytes)?;
 
-        let token_id = TokenId::from(tx.prefix.token_id);
+        let token_id = TokenId::from(tx.prefix.fee_token_id);
 
         // Validate.
         let mut csprng = McRng::default();
@@ -782,7 +782,7 @@ 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 token_id = TokenId::from(tx.prefix.token_id);
+            let token_id = TokenId::from(tx.prefix.fee_token_id);
             total_fees.add(&token_id, tx.prefix.fee as u128);
         }
 

consensus/service/src/validators.rs

@@ -576,12 +576,14 @@ mod combine_tests {
             )
             .unwrap();
 
+            let fee_amount = Amount::new(Mob::MINIMUM_FEE, Mob::ID);
             let mut transaction_builder = TransactionBuilder::new(
                 block_version,
-                Mob::ID,
+                fee_amount,
                 MockFogResolver::default(),
                 EmptyMemoBuilder::default(),
-            );
+            )
+            .unwrap();
             transaction_builder.add_input(input_credentials);
             transaction_builder.set_fee(0).unwrap();
             transaction_builder
@@ -634,12 +636,14 @@ mod combine_tests {
                     // Step 2: Create a transaction that sends the full value of `tx_out` to
                     // `recipient_account`.
 
+                    let fee_amount = Amount::new(Mob::MINIMUM_FEE, Mob::ID);
                     let mut transaction_builder = TransactionBuilder::new(
                         block_version,
-                        Mob::ID,
+                        fee_amount,
                         MockFogResolver::default(),
                         EmptyMemoBuilder::default(),
-                    );
+                    )
+                    .unwrap();
 
                     // Create InputCredentials to spend the TxOut.
                     let onetime_private_key = recover_onetime_private_key(
@@ -735,12 +739,14 @@ mod combine_tests {
                 )
                 .unwrap();
 
+                let fee_amount = Amount::new(Mob::MINIMUM_FEE, Mob::ID);
                 let mut transaction_builder = TransactionBuilder::new(
                     block_version,
-                    Mob::ID,
+                    fee_amount,
                     MockFogResolver::default(),
                     EmptyMemoBuilder::default(),
-                );
+                )
+                .unwrap();
                 transaction_builder.add_input(input_credentials);
                 transaction_builder.set_fee(0).unwrap();
                 transaction_builder
@@ -772,12 +778,14 @@ mod combine_tests {
                 )
                 .unwrap();
 
+                let fee_amount = Amount::new(Mob::MINIMUM_FEE, Mob::ID);
                 let mut transaction_builder = TransactionBuilder::new(
                     block_version,
-                    Mob::ID,
+                    fee_amount,
                     MockFogResolver::default(),
                     EmptyMemoBuilder::default(),
-                );
+                )
+                .unwrap();
                 transaction_builder.add_input(input_credentials);
                 transaction_builder.set_fee(0).unwrap();
                 transaction_builder
@@ -835,12 +843,14 @@ mod combine_tests {
                 )
                 .unwrap();
 
+                let fee_amount = Amount::new(Mob::MINIMUM_FEE, Mob::ID);
                 let mut transaction_builder = TransactionBuilder::new(
                     block_version,
-                    Mob::ID,
+                    fee_amount,
                     MockFogResolver::default(),
                     EmptyMemoBuilder::default(),
-                );
+                )
+                .unwrap();
                 transaction_builder.add_input(input_credentials);
                 transaction_builder.set_fee(0).unwrap();
                 transaction_builder
@@ -928,12 +938,14 @@ mod combine_tests {
                 )
                 .unwrap();
 
+                let fee_amount = Amount::new(Mob::MINIMUM_FEE, Mob::ID);
                 let mut transaction_builder = TransactionBuilder::new(
                     block_version,
-                    Mob::ID,
+                    fee_amount,
                     MockFogResolver::default(),
                     EmptyMemoBuilder::default(),
-                );
+                )
+                .unwrap();
                 transaction_builder.add_input(input_credentials);
                 transaction_builder.set_fee(0).unwrap();
                 transaction_builder
@@ -966,12 +978,14 @@ mod combine_tests {
                 )
                 .unwrap();
 
+                let fee_amount = Amount::new(Mob::MINIMUM_FEE, Mob::ID);
                 let mut transaction_builder = TransactionBuilder::new(
                     block_version,
-                    Mob::ID,
+                    fee_amount,
                     MockFogResolver::default(),
                     EmptyMemoBuilder::default(),
-                );
+                )
+                .unwrap();
                 transaction_builder.add_input(input_credentials);
                 transaction_builder.set_fee(0).unwrap();
                 transaction_builder
@@ -1030,12 +1044,14 @@ mod combine_tests {
                 )
                 .unwrap();
 
+                let fee_amount = Amount::new(Mob::MINIMUM_FEE, Mob::ID);
                 let mut transaction_builder = TransactionBuilder::new(
                     block_version,
-                    Mob::ID,
+                    fee_amount,
                     MockFogResolver::default(),
                     EmptyMemoBuilder::default(),
-                );
+                )
+                .unwrap();
                 transaction_builder.add_input(input_credentials);
                 transaction_builder.set_fee(0).unwrap();
                 transaction_builder

fog/distribution/src/main.rs

@@ -712,20 +712,18 @@ fn build_tx(
     let block_version = BlockVersion::try_from(BLOCK_VERSION.load(Ordering::SeqCst))
         .expect("Unsupported block version");
 
-    // Use token id for first spendable tx out
-    let token_id = spendable_txouts.first().unwrap().amount.token_id;
+    // FIXME: This needs to be the fee for the current token, not MOB.
+    // However, bootstrapping non MOB tokens is not supported right now.
+    let fee_amount = Amount::new(MOB_FEE.load(Ordering::SeqCst), Mob::ID);
 
     // Create tx_builder.
     let mut tx_builder = TransactionBuilder::new(
         block_version,
-        token_id,
+        fee_amount,
         fog_resolver,
         EmptyMemoBuilder::default(),
-    );
-
-    // FIXME: This needs to be the fee for the current token, not MOB.
-    // However, bootstrapping non MOB tokens is not supported right now.
-    tx_builder.set_fee(MOB_FEE.load(Ordering::SeqCst)).unwrap();
+    )
+    .unwrap();
 
     // Unzip each vec of tuples into a tuple of vecs.
     let mut rings_and_proofs: Vec<(Vec<TxOut>, Vec<TxOutMembershipProof>)> = rings
@@ -804,7 +802,7 @@ fn build_tx(
 
     // Add ouputs
     for (i, (utxo, _proof)) in utxos_with_proofs.iter().enumerate() {
-        if utxo.amount.token_id == token_id {
+        if utxo.amount.token_id == Mob::ID {
             let mut value = utxo.amount.value;
             // Use the first input to pay for the fee.
             if i == 0 {

fog/sample-paykit/src/client.rs

@@ -606,14 +606,14 @@ fn build_transaction_helper<T: RngCore + CryptoRng, FPR: FogPubkeyResolver>(
         memo_builder.set_sender_credential(SenderMemoCredential::from(source_account_key));
         memo_builder.enable_destination_memo();
 
-        TransactionBuilder::new(block_version, amount.token_id, fog_resolver, memo_builder)
+        let fee_amount = Amount::new(fee, amount.token_id);
+        TransactionBuilder::new(block_version, fee_amount, fog_resolver, memo_builder)?
     };
-    tx_builder.set_fee(fee)?;
 
     let input_amount = inputs
         .iter()
         .fold(0, |acc, (txo, _)| acc + txo.amount.value);
-    let fee = tx_builder.get_fee();
+    let fee = tx_builder.get_fee().value;
     if (amount.value + fee) > input_amount {
         return Err(Error::InsufficientFunds);
     }

libmobilecoin/src/transaction.rs

@@ -19,7 +19,7 @@ use mc_transaction_core::{
     ring_signature::KeyImage,
     tokens::Mob,
     tx::{TxOut, TxOutConfirmationNumber, TxOutMembershipProof},
-    BlockVersion, CompressedCommitment, EncryptedMemo, MaskedAmount, Token,
+    Amount, BlockVersion, CompressedCommitment, EncryptedMemo, MaskedAmount, Token,
 };
 
 use mc_transaction_std::{
@@ -373,19 +373,17 @@ pub extern "C" fn mc_transaction_builder_create(
         // version that fog ledger told us about, or that we got from ledger-db
         //let block_version = BlockVersion::ZERO;
 
-        // TODO #1596: Support token id other than Mob
-        let token_id = Mob::ID;
+        // TODO #1596: Support token id other than Mob (but not in this release)
+        let fee_amount = Amount::new(fee, Mob::ID);
 
         let mut transaction_builder = TransactionBuilder::new_with_box(
             block_version,
-            token_id,
+            fee_amount,
             fog_resolver,
             memo_builder_box,
-        );
+        )
+        .expect("failure not expected");
 
-        transaction_builder
-            .set_fee(fee)
-            .expect("failure not expected");
         transaction_builder.set_tombstone_block(tombstone_block);
         Some(transaction_builder)
     })

mobilecoind/src/payments.rs

@@ -21,7 +21,7 @@ use mc_transaction_core::{
     onetime_keys::recover_onetime_private_key,
     ring_signature::KeyImage,
     tx::{Tx, TxOut, TxOutConfirmationNumber, TxOutMembershipProof},
-    BlockIndex, BlockVersion, TokenId,
+    Amount, BlockIndex, BlockVersion, TokenId,
 };
 use mc_transaction_std::{
     ChangeDestination, EmptyMemoBuilder, InputCredentials, TransactionBuilder,
@@ -869,17 +869,16 @@ impl<T: BlockchainConnection + UserTxConnection + 'static, FPR: FogPubkeyResolve
 
         // TODO: Use RTH memo builder, optionally?
 
+        let fee_amount = Amount::new(fee, token_id);
+
         // Create tx_builder.
         let mut tx_builder = TransactionBuilder::new(
             block_version,
-            token_id,
+            fee_amount,
             fog_resolver,
             EmptyMemoBuilder::default(),
-        );
-
-        tx_builder
-            .set_fee(fee)
-            .map_err(|err| Error::TxBuild(format!("Error setting fee: {}", err)))?;
+        )
+        .map_err(|err| Error::TxBuild(format!("Error cretaing TransactionBuilder: {}", err)))?;
 
         // Unzip each vec of tuples into a tuple of vecs.
         let mut rings_and_proofs: Vec<(Vec<TxOut>, Vec<TxOutMembershipProof>)> = rings
@@ -980,7 +979,7 @@ impl<T: BlockchainConnection + UserTxConnection + 'static, FPR: FogPubkeyResolve
         if total_value > input_value {
             return Err(Error::InsufficientFunds);
         }
-        let change = input_value - total_value - tx_builder.get_fee();
+        let change = input_value - total_value - tx_builder.get_fee().value;
 
         // If we do, add an output for that as well.
         // TODO: Should the exchange write destination memos?