Merge branch 'grarco/tx-expiration-update' (#2315)

* origin/grarco/tx-expiration-update:
  Updates masp fixtures
  Changelog #2315
  Updates masp tx generation to include expiration
  MASP vp expiration always required
  MASP vp checks transaction's expiration
  Updates TODO for disposable signer
  Validates tx expiration again for decrypted tx

.changelog/unreleased/SDK/2315-tx-expiration-update.md

@@ -0,0 +1,2 @@
+- Updated `gen_shielded_transfer` to attach a sensible expiration to a MASP
+  `Transaction`. ([\#2315](https://github.com/anoma/namada/pull/2315))

.changelog/unreleased/improvements/2315-tx-expiration-update.md

@@ -0,0 +1,2 @@
+- Improved validation on transaction's expiration. Added an expiration for MASP
+  transfers. ([\#2315](https://github.com/anoma/namada/pull/2315))

apps/src/lib/node/ledger/shell/process_proposal.rs

@@ -546,11 +546,26 @@ where
                                     .into(),
                             }
                         } else {
-                            TxResult {
-                                code: ResultCode::Ok.into(),
-                                info: "Process Proposal accepted this \
-                                       transaction"
-                                    .into(),
+                            match tx.header().expiration {
+                                Some(tx_expiration)
+                                    if block_time > tx_expiration =>
+                                {
+                                    TxResult {
+                                        code: ResultCode::ExpiredDecryptedTx
+                                            .into(),
+                                        info: format!(
+                                            "Tx expired at {:#?}, block time: \
+                                             {:#?}",
+                                            tx_expiration, block_time
+                                        ),
+                                    }
+                                }
+                                _ => TxResult {
+                                    code: ResultCode::Ok.into(),
+                                    info: "Process Proposal accepted this \
+                                           transaction"
+                                        .into(),
+                                },
                             }
                         }
                     }
@@ -1751,6 +1766,55 @@ mod test_process_proposal {
         }
     }
 
+    /// Test that an expired decrypted transaction is marked as rejected but
+    /// still allows the block to be accepted
+    #[test]
+    fn test_expired_decrypted() {
+        let (mut shell, _recv, _, _) = test_utils::setup();
+        let keypair = crate::wallet::defaults::daewon_keypair();
+
+        let mut wrapper =
+            Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new(
+                Fee {
+                    amount_per_gas_unit: DenominatedAmount::native(1.into()),
+                    token: shell.wl_storage.storage.native_token.clone(),
+                },
+                keypair.ref_to(),
+                Epoch(0),
+                GAS_LIMIT_MULTIPLIER.into(),
+                None,
+            ))));
+        wrapper.header.chain_id = shell.chain_id.clone();
+        wrapper.header.expiration = Some(DateTimeUtc::default());
+        wrapper.set_code(Code::new("wasm_code".as_bytes().to_owned(), None));
+        wrapper.set_data(Data::new("transaction data".as_bytes().to_owned()));
+        wrapper.add_section(Section::Signature(Signature::new(
+            wrapper.sechashes(),
+            [(0, keypair)].into_iter().collect(),
+            None,
+        )));
+
+        shell.enqueue_tx(wrapper.clone(), GAS_LIMIT_MULTIPLIER.into());
+
+        let decrypted =
+            wrapper.update_header(TxType::Decrypted(DecryptedTx::Decrypted));
+
+        // Run validation
+        let request = ProcessProposal {
+            txs: vec![decrypted.to_bytes()],
+        };
+        match shell.process_proposal(request) {
+            Ok(txs) => {
+                assert_eq!(txs.len(), 1);
+                assert_eq!(
+                    txs[0].result.code,
+                    u32::from(ResultCode::ExpiredDecryptedTx)
+                );
+            }
+            Err(_) => panic!("Test failed"),
+        }
+    }
+
     /// Check that a tx requiring more gas than the block limit causes a block
     /// rejection
     #[test]

core/src/types/transaction/mod.rs

@@ -84,6 +84,8 @@ pub enum ResultCode {
     InvalidVoteExtension = 13,
     /// Tx is too large
     TooLarge = 14,
+    /// Decrypted tx is expired
+    ExpiredDecryptedTx = 15,
     // =========================================================================
     // WARN: These codes shouldn't be changed between version!
 }
@@ -96,7 +98,7 @@ impl ResultCode {
         // NOTE: pattern match on all `ResultCode` variants, in order
         // to catch potential bugs when adding new codes
         match self {
-            Ok | WasmRuntimeError => true,
+            Ok | WasmRuntimeError | ExpiredDecryptedTx => true,
             InvalidTx | InvalidSig | InvalidOrder | ExtraTxs
             | Undecryptable | AllocationError | ReplayTx | InvalidChainId
             | ExpiredTx | TxGasLimit | FeeError | InvalidVoteExtension

sdk/src/masp.rs

@@ -55,6 +55,7 @@ use namada_core::types::masp::{
     TransferTarget,
 };
 use namada_core::types::storage::{BlockHeight, Epoch, Key, KeySeg, TxIndex};
+use namada_core::types::time::{DateTimeUtc, DurationSecs};
 use namada_core::types::token;
 use namada_core::types::token::{
     Change, MaspDenom, Transfer, HEAD_TX_KEY, PIN_KEY_PREFIX, TX_KEY_PREFIX,
@@ -1590,8 +1591,50 @@ impl<U: ShieldedUtils + MaybeSend + MaybeSync> ShieldedContext<U> {
         };
 
         // Now we build up the transaction within this object
-        let mut builder =
-            Builder::<TestNetwork, _>::new_with_rng(NETWORK, 1.into(), rng);
+        let expiration_height: u32 = match context.tx_builder().expiration {
+            Some(expiration) => {
+                // Try to match a DateTime expiration with a plausible
+                // corresponding block height
+                let last_block_height: u64 =
+                    crate::rpc::query_block(context.client())
+                        .await?
+                        .map_or_else(|| 1, |block| u64::from(block.height));
+                let current_time = DateTimeUtc::now();
+                let delta_time =
+                    expiration.0.signed_duration_since(current_time.0);
+
+                let max_expected_time_per_block_key =
+                    namada_core::ledger::parameters::storage::get_max_expected_time_per_block_key();
+                let max_block_time =
+                    crate::rpc::query_storage_value::<_, DurationSecs>(
+                        context.client(),
+                        &max_expected_time_per_block_key,
+                    )
+                    .await?;
+
+                let delta_blocks = u32::try_from(
+                    delta_time.num_seconds() / max_block_time.0 as i64,
+                )
+                .map_err(|e| Error::Other(e.to_string()))?;
+                u32::try_from(last_block_height)
+                    .map_err(|e| Error::Other(e.to_string()))?
+                    + delta_blocks
+            }
+            None => {
+                // NOTE: The masp library doesn't support optional expiration so
+                // we set the max to mimic a never-expiring tx. We also need to
+                // remove 20 which is going to be added back by the builder
+                u32::MAX - 20
+            }
+        };
+        let mut builder = Builder::<TestNetwork, _>::new_with_rng(
+            NETWORK,
+            // NOTE: this is going to add 20 more blocks to the actual
+            // expiration but there's no other exposed function that we could
+            // use from the masp crate to specify the expiration better
+            expiration_height.into(),
+            rng,
+        );
 
         // Convert transaction amount into MASP types
         let (asset_types, masp_amount) =

sdk/src/wallet/mod.rs

@@ -683,8 +683,9 @@ impl<U: WalletIo> Wallet<U> {
             alias = format!("disposable_{ctr}");
         }
         // Generate a disposable keypair to sign the wrapper if requested
-        // TODO: once the wrapper transaction has been accepted, this key can be
-        // deleted from wallet
+        // TODO: once the wrapper transaction has been applied, this key can be
+        // deleted from wallet (the transaction being accepted is not enough
+        // cause we could end up doing a rollback)
         let (alias, disposable_keypair) = self
             .gen_store_secret_key(
                 SchemeType::Ed25519,