Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
29 changes: 25 additions & 4 deletions ledger/helpers/unsafe/src/ledger_8/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,26 @@ impl<D: DB + Clone> LedgerContext<D> {
let mut all_events: Vec<Event<D>> = 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"
Comment thread
gilescope marked this conversation as resolved.
);
},
Comment thread
LGLO marked this conversation as resolved.
Err(e) => return Err(e),
}
}

let mut latest_ledger_state = self
Expand Down Expand Up @@ -383,6 +399,11 @@ impl<D: DB + Clone> LedgerContext<D> {
)
}

/// 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<S: SignatureKind<D>, P: ProofKind<D> + std::fmt::Debug>(
&self,
tx: &SerdeTransaction<S, P, D>,
Expand Down
29 changes: 25 additions & 4 deletions ledger/helpers/unsafe/src/ledger_9/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,26 @@ impl<D: DB + Clone> LedgerContext<D> {
let mut all_events: Vec<Event<D>> = 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"
);
},
Comment thread
LGLO marked this conversation as resolved.
Err(e) => return Err(e),
}
}

let mut latest_ledger_state = self
Expand Down Expand Up @@ -383,6 +399,11 @@ impl<D: DB + Clone> LedgerContext<D> {
)
}

/// 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<S: SignatureKind<D>, P: ProofKind<D> + std::fmt::Debug>(
&self,
tx: &SerdeTransaction<S, P, D>,
Expand Down
Loading