-
Notifications
You must be signed in to change notification settings - Fork 44
Fix issue #2070. Don't fail context update when there is a #2098
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| #toolkit #bugfix | ||
|
|
||
| # Don't abort context replay on a `well_formed` failure the chain itself tolerated | ||
|
|
||
| `LedgerContext::update_from_tx_with_strictness` hard-failed the whole replay | ||
| (`LedgerContextError::InvalidTransaction`) 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. | ||
|
|
||
| `update_from_tx_with_strictness` now mirrors that on-chain behaviour: a | ||
| `well_formed` error is logged and the transaction is treated like a failed | ||
| apply (ledger state unchanged, no events, zero cost) instead of aborting the | ||
| replay. | ||
|
|
||
| PR: https://github.com/midnightntwrk/midnight-node/pull/2098 | ||
| Issue: https://github.com/midnightntwrk/midnight-node/issues/2070 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,13 +17,14 @@ use crate::ledger_8::{ | |
| Offer, OutputMode, PUBLIC_PARAMS, PedersenDowngradeable, ProofKind, PureGeneratorPedersen, | ||
| Resolver, SerdeTransaction, Serializable, SignatureKind, Sp, Storable, SyntheticCost, Tagged, | ||
| Timestamp, Transaction, TransactionContext, TransactionResult, UnshieldedSignatureScheme, Utxo, | ||
| VerifiedTransaction, Wallet, WalletAddress, WalletSeed, WellFormedStrictness, ZswapChainState, | ||
| clamp_and_normalize, compute_overall_fullness, default_storage, deserialize, | ||
| mn_ledger_serialize as serialize, mn_ledger_storage as storage, types::StorableSyntheticCost, | ||
| Wallet, WalletAddress, WalletSeed, WellFormedStrictness, ZswapChainState, clamp_and_normalize, | ||
| compute_overall_fullness, default_storage, deserialize, mn_ledger_serialize as serialize, | ||
| mn_ledger_storage as storage, types::StorableSyntheticCost, | ||
| }; | ||
| use derive_where::derive_where; | ||
| use hex::encode as hex_encode; | ||
| use lazy_static::lazy_static; | ||
| use mn_ledger_8::{error::MalformedTransaction, structure::VerifiedTransaction}; | ||
| use std::{ | ||
| collections::{HashMap, HashSet}, | ||
| sync::Mutex, | ||
|
|
@@ -409,37 +410,60 @@ impl<D: DB + Clone> LedgerContext<D> { | |
| // the ledger has no strictness knob for zswap offer proofs. | ||
| let ref_state = &tx_context.ref_state; | ||
| let tblock = tx_context.block_context.tblock; | ||
| let valid_tx: VerifiedTransaction<_> = if strictness.verify_native_proofs { | ||
| tx.well_formed(ref_state, strictness, tblock) | ||
| } else { | ||
| tx.erase_proofs().well_formed(ref_state, strictness, tblock) | ||
| } | ||
| .map_err(|e| LedgerContextError::InvalidTransaction(format!("{e:?}")))?; | ||
| let cost = tx | ||
| .cost(&tx_context.ref_state.parameters, false) | ||
| .map_err(|e| LedgerContextError::CostCalculation(format!("{e:?}")))?; | ||
|
|
||
| let (new_ledger_state, result) = tx_context.ref_state.apply(&valid_tx, &tx_context); | ||
| let offers = Self::successful_shielded_offers(tx, &result); | ||
| match result { | ||
| TransactionResult::Success(events) => (new_ledger_state, offers, events, cost), | ||
| TransactionResult::PartialSuccess(failure, events) => { | ||
| crate::replay_stats::PARTIALLY_FAILED_TXS | ||
| .fetch_add(1, std::sync::atomic::Ordering::Relaxed); | ||
| let hash = hex::encode(tx.transaction_hash().0.0); | ||
| log::debug!( | ||
| "Partially failing result {failure:?} of applying tx 0x{hash} to update Local Ledger State" | ||
| ); | ||
| (new_ledger_state, offers, events, cost) | ||
| let valid_tx: Result<VerifiedTransaction<_>, MalformedTransaction<_>> = | ||
| if strictness.verify_native_proofs { | ||
| tx.well_formed(ref_state, strictness, tblock) | ||
| } else { | ||
| tx.erase_proofs().well_formed(ref_state, strictness, tblock) | ||
| }; | ||
|
|
||
| match valid_tx { | ||
| Ok(valid_tx) => { | ||
| let cost = tx | ||
| .cost(&tx_context.ref_state.parameters, false) | ||
| .map_err(|e| LedgerContextError::CostCalculation(format!("{e:?}")))?; | ||
|
|
||
| let (new_ledger_state, result) = | ||
| tx_context.ref_state.apply(&valid_tx, &tx_context); | ||
| let offers = Self::successful_shielded_offers(tx, &result); | ||
| match result { | ||
| TransactionResult::Success(events) => { | ||
| (new_ledger_state, offers, events, cost) | ||
| }, | ||
| TransactionResult::PartialSuccess(failure, events) => { | ||
| crate::replay_stats::PARTIALLY_FAILED_TXS | ||
| .fetch_add(1, std::sync::atomic::Ordering::Relaxed); | ||
| let hash = hex::encode(tx.transaction_hash().0.0); | ||
| log::debug!( | ||
| "Partially failing result {failure:?} of applying tx 0x{hash} to update Local Ledger State" | ||
| ); | ||
| (new_ledger_state, offers, events, cost) | ||
| }, | ||
| TransactionResult::Failure(failure) => { | ||
| crate::replay_stats::FAILED_TXS | ||
| .fetch_add(1, std::sync::atomic::Ordering::Relaxed); | ||
| let hash = hex::encode(tx.transaction_hash().0.0); | ||
| log::warn!( | ||
| "Failing result {failure:?} of applying tx 0x{hash} to update Local Ledger State" | ||
| ); | ||
| (new_ledger_state, offers, vec![], SyntheticCost::ZERO) | ||
| }, | ||
| } | ||
| }, | ||
| TransactionResult::Failure(failure) => { | ||
| crate::replay_stats::FAILED_TXS | ||
| .fetch_add(1, std::sync::atomic::Ordering::Relaxed); | ||
| Err(err) => { | ||
| // A `well_formed` failure (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. Mirror that here instead of hard-failing | ||
| // the whole replay. | ||
| let hash = hex::encode(tx.transaction_hash().0.0); | ||
| log::warn!( | ||
| "Failing result {failure:?} of applying tx 0x{hash} to update Local Ledger State" | ||
| "Failing result {err:?} of validating tx 0x{hash} \nto update Local Ledger State" | ||
| ); | ||
| (new_ledger_state, offers, vec![], SyntheticCost::ZERO) | ||
| (tx_context.ref_state.clone(), vec![], vec![], SyntheticCost::ZERO) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This fallback also applies to the public Useful? React with 👍 / 👎. |
||
| }, | ||
| } | ||
| }, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new handling exists only in the ledger-8 context, while
LedgerVersion::from_spec_versionroutes every 2.0.0+ block to ledger 9 andapply_block_9invokes the unchangedledger_9::LedgerContext::update_from_block. Consequently, any failedsend_mn_transactionwhose ledger-9well_formedcheck returnsErrstill aborts replay atledger/helpers/src/ledger_9/context.rs:412-417, reproducing the same outage for current ledger-9 history. Apply the same replay-only fallback to the ledger-9 implementation.Useful? React with 👍 / 👎.