diff --git a/changes/toolkit/changed/fix-replay-abort-on-well-formed-failure.md b/changes/toolkit/changed/fix-replay-abort-on-well-formed-failure.md new file mode 100644 index 000000000..d257f2ee6 --- /dev/null +++ b/changes/toolkit/changed/fix-replay-abort-on-well-formed-failure.md @@ -0,0 +1,28 @@ +#toolkit #bugfix +# Don't abort context replay on a `well_formed` failure the chain itself tolerated + +`LedgerContext::apply_txs_collect_events` (in both the ledger-8 and ledger-9 +implementations, behind `update_from_block`) aborted the whole block replay +whenever a transaction's `well_formed` check returned `Err`, e.g. +`OutOfDustValidityWindow` for a dust action whose `ctime` lands a couple of +seconds past the including block's `tblock`. This made every toolkit workflow +that replays a context (transaction generation, wallet inspection, faucets, +test-data generators) unusable against a chain — such as Preview — carrying a +transaction the chain itself had accepted: on-chain, +`pallet_midnight::send_mn_transaction` hits this same check via +`LedgerApi::apply_transaction`, but only fails that one extrinsic's dispatch +(storage rolled back, `ExtrinsicFailed` emitted) without affecting block +validity, so a `well_formed` failure alone is not evidence of an invalid block. + +`apply_txs_collect_events` now matches on `LedgerContextError::InvalidTransaction` +specifically and, on that variant, logs a warning and moves on to the next +transaction instead of propagating the error and aborting the replay. Every +other error variant still aborts. `update_from_tx` (used by the `batches` +generator to validate each transaction it builds before chaining more on top +of it) is untouched and keeps hard-failing: a generator needs to know +immediately if its own just-built transaction is doomed, since tolerating it +there would let a doomed transaction ship in the output batch and further +batches get built against funds it never actually created. + +PR: https://github.com/midnightntwrk/midnight-node/pull/2098 +Issue: https://github.com/midnightntwrk/midnight-node/issues/2070 diff --git a/ledger/helpers/src/ledger_8/context.rs b/ledger/helpers/src/ledger_8/context.rs index 1f3f9a88e..8ffa15cfe 100644 --- a/ledger/helpers/src/ledger_8/context.rs +++ b/ledger/helpers/src/ledger_8/context.rs @@ -173,10 +173,26 @@ impl LedgerContext { let mut all_events: Vec> = Vec::new(); let strictness = Self::strictness_for(block_context, root_verified); for tx in txs { - let (events, cost) = - self.update_from_tx_with_strictness(tx, block_context, strictness)?; - all_events.extend(events); - total_cost = total_cost + cost; + match self.update_from_tx_with_strictness(tx, block_context, strictness) { + Ok((events, cost)) => { + all_events.extend(events); + total_cost = total_cost + cost; + }, + // A `well_formed` rejection (e.g. `OutOfDustValidityWindow` from a dust action + // whose `ctime` lands a couple of seconds past the including block's `tblock`) + // is not evidence of an invalid block: on-chain, `pallet_midnight::send_mn_transaction` + // hits this same check via `LedgerApi::apply_transaction` and simply fails that + // one extrinsic's dispatch (storage rolled back, `ExtrinsicFailed` emitted) + // without affecting block validity. Already-committed, external chain history is + // replayed here, so mirror that instead of aborting the whole replay. + Err(LedgerContextError::InvalidTransaction(reason)) => { + let hash = hex::encode(tx.transaction_hash().0.0); + log::warn!( + "Tolerating well_formed rejection {reason} of tx 0x{hash} while replaying block" + ); + }, + Err(e) => return Err(e), + } } let mut latest_ledger_state = self @@ -383,6 +399,11 @@ impl LedgerContext { ) } + /// A `well_formed` rejection surfaces as `Err(LedgerContextError::InvalidTransaction(_))`, + /// distinct from every other (fatal) error variant, so callers can decide for themselves + /// whether to tolerate it: `apply_txs_collect_events` (block replay) does, `update_from_tx` + /// (this function's only other caller, used to validate a transaction the caller is about + /// to build more transactions on top of or submit itself) does not. fn update_from_tx_with_strictness, P: ProofKind + std::fmt::Debug>( &self, tx: &SerdeTransaction, diff --git a/ledger/helpers/src/ledger_9/context.rs b/ledger/helpers/src/ledger_9/context.rs index b9f07178f..c0627610d 100644 --- a/ledger/helpers/src/ledger_9/context.rs +++ b/ledger/helpers/src/ledger_9/context.rs @@ -173,10 +173,26 @@ impl LedgerContext { let mut all_events: Vec> = Vec::new(); let strictness = Self::strictness_for(block_context, root_verified); for tx in txs { - let (events, cost) = - self.update_from_tx_with_strictness(tx, block_context, strictness)?; - all_events.extend(events); - total_cost = total_cost + cost; + match self.update_from_tx_with_strictness(tx, block_context, strictness) { + Ok((events, cost)) => { + all_events.extend(events); + total_cost = total_cost + cost; + }, + // A `well_formed` rejection (e.g. `OutOfDustValidityWindow` from a dust action + // whose `ctime` lands a couple of seconds past the including block's `tblock`) + // is not evidence of an invalid block: on-chain, `pallet_midnight::send_mn_transaction` + // hits this same check via `LedgerApi::apply_transaction` and simply fails that + // one extrinsic's dispatch (storage rolled back, `ExtrinsicFailed` emitted) + // without affecting block validity. Already-committed, external chain history is + // replayed here, so mirror that instead of aborting the whole replay. + Err(LedgerContextError::InvalidTransaction(reason)) => { + let hash = hex::encode(tx.transaction_hash().0.0); + log::warn!( + "Tolerating well_formed rejection {reason} of tx 0x{hash} while replaying block" + ); + }, + Err(e) => return Err(e), + } } let mut latest_ledger_state = self @@ -383,6 +399,11 @@ impl LedgerContext { ) } + /// A `well_formed` rejection surfaces as `Err(LedgerContextError::InvalidTransaction(_))`, + /// distinct from every other (fatal) error variant, so callers can decide for themselves + /// whether to tolerate it: `apply_txs_collect_events` (block replay) does, `update_from_tx` + /// (this function's only other caller, used to validate a transaction the caller is about + /// to build more transactions on top of or submit itself) does not. fn update_from_tx_with_strictness, P: ProofKind + std::fmt::Debug>( &self, tx: &SerdeTransaction,