Tx expiration updates

.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/mod.rs

@@ -156,6 +156,7 @@ pub enum ErrorCodes {
     FeeError = 12,
     InvalidVoteExtension = 13,
     TooLarge = 14,
+    ExpiredDecryptedTx = 15,
 }
 
 impl ErrorCodes {
@@ -166,7 +167,7 @@ impl ErrorCodes {
         // NOTE: pattern match on all `ErrorCodes` 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

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

@@ -546,11 +546,26 @@ where
                                     .into(),
                             }
                         } else {
-                            TxResult {
-                                code: ErrorCodes::Ok.into(),
-                                info: "Process Proposal accepted this \
-                                       tranasction"
-                                    .into(),
+                            match tx.header().expiration {
+                                Some(tx_expiration)
+                                    if block_time > tx_expiration =>
+                                {
+                                    TxResult {
+                                        code: ErrorCodes::ExpiredDecryptedTx
+                                            .into(),
+                                        info: format!(
+                                            "Tx expired at {:#?}, block time: \
+                                             {:#?}",
+                                            tx_expiration, block_time
+                                        ),
+                                    }
+                                }
+                                _ => TxResult {
+                                    code: ErrorCodes::Ok.into(),
+                                    info: "Process Proposal accepted this \
+                                           tranasction"
+                                        .into(),
+                                },
                             }
                         }
                     }
@@ -1735,6 +1750,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: 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(ErrorCodes::ExpiredDecryptedTx)
+                );
+            }
+            Err(_) => panic!("Test failed"),
+        }
+    }
+
     /// Check that a tx requiring more gas than the block limit causes a block
     /// rejection
     #[test]

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

@@ -656,8 +656,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,

shared/src/ledger/native_vp/masp.rs

@@ -124,6 +124,14 @@ where
     ) -> Result<bool> {
         let epoch = self.ctx.get_block_epoch()?;
         let (transfer, shielded_tx) = self.ctx.get_shielded_action(tx_data)?;
+
+        if u64::from(self.ctx.get_block_height()?)
+            > u64::from(shielded_tx.expiry_height())
+        {
+            tracing::debug!("MASP transaction is expired");
+            return Ok(false);
+        }
+
         let mut transparent_tx_pool = I128Sum::zero();
         // The Sapling value balance adds to the transparent tx pool
         transparent_tx_pool += shielded_tx.sapling_value_balance();