From 5d3fb73ed58c7937bcf0e7244adce2b55fa27b59 Mon Sep 17 00:00:00 2001 From: Sebastien Chapuis Date: Sat, 31 Aug 2024 13:20:28 +0200 Subject: [PATCH 1/3] Fix `add_from_gossip_exn_impl` Don't mutate `by_sender` until ends of the method --- ledger/src/transaction_pool.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ledger/src/transaction_pool.rs b/ledger/src/transaction_pool.rs index 6ac0c24c6a..206a5b5ace 100644 --- a/ledger/src/transaction_pool.rs +++ b/ledger/src/transaction_pool.rs @@ -996,7 +996,7 @@ impl IndexedPool { consumed }; - match by_sender.state.as_mut() { + match by_sender.state.clone() { None => { if current_nonce != cmd_applicable_at_nonce { return Err(CommandError::InvalidNonce { @@ -1020,7 +1020,7 @@ impl IndexedPool { Ok((cmd.clone(), Self::make_queue())) } - Some((queued_cmds, reserved_currency)) => { + Some((mut queued_cmds, reserved_currency)) => { assert!(!queued_cmds.is_empty()); let queue_applicable_at_nonce = { let first = queued_cmds.front().unwrap(); @@ -1031,14 +1031,14 @@ impl IndexedPool { last.data.forget_check().expected_target_nonce() }; if queue_target_nonce == cmd_applicable_at_nonce { - *reserved_currency = consumed - .checked_add(reserved_currency) + let reserved_currency = consumed + .checked_add(&reserved_currency) .ok_or(CommandError::Overflow)?; - if *reserved_currency > balance.to_amount() { + if reserved_currency > balance.to_amount() { return Err(CommandError::InsufficientFunds { balance, - consumed: *reserved_currency, + consumed: reserved_currency, }); } @@ -1050,6 +1050,8 @@ impl IndexedPool { add_to_applicable_by_fee: false, }); + by_sender.state = Some((queued_cmds, reserved_currency)); + Ok((cmd.clone(), Self::make_queue())) } else if queue_applicable_at_nonce == current_nonce { if !cmd_applicable_at_nonce From a21be7f2504fb3eea749a5f278e1f8e14130193a Mon Sep 17 00:00:00 2001 From: Sebastien Chapuis Date: Mon, 2 Sep 2024 11:05:51 +0200 Subject: [PATCH 2/3] Don't panic when a VK is not found --- ledger/src/scan_state/transaction_logic.rs | 2 +- ledger/src/transaction_pool.rs | 4 +++- node/src/transaction_pool/mod.rs | 22 ++++++++++++++-------- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/ledger/src/scan_state/transaction_logic.rs b/ledger/src/scan_state/transaction_logic.rs index bdac6a3c59..f48882601f 100644 --- a/ledger/src/scan_state/transaction_logic.rs +++ b/ledger/src/scan_state/transaction_logic.rs @@ -4187,7 +4187,7 @@ pub mod zkapp_command { .find(|(id, _)| account_id == id) .map(|(_, key)| key.clone()) }) - .ok_or_else(|| "verification key not found in cache".to_string()) + .ok_or_else(|| format!("verification key not found in cache: {:?}", vk_hash)) })?; if !is_failed { for (account_id, vk) in cmd.extract_vks() { diff --git a/ledger/src/transaction_pool.rs b/ledger/src/transaction_pool.rs index 206a5b5ace..a5e3c784a4 100644 --- a/ledger/src/transaction_pool.rs +++ b/ledger/src/transaction_pool.rs @@ -33,6 +33,8 @@ pub enum TransactionPoolErrors { /// Invalid transactions, rejeceted diffs, etc... #[error("Transaction pool errors: {0:?}")] BatchedErrors(Vec), + #[error("{0:?}")] + LoadingVK(String), /// Errors that should panic the node (bugs in implementation) #[error("Unexpected error: {0}")] Unexpected(String), @@ -2163,7 +2165,7 @@ impl TransactionPool { from_unapplied_sequence::Cache::new(merged) }) - .map_err(TransactionPoolErrors::Unexpected)?; + .map_err(TransactionPoolErrors::LoadingVK)?; let diff = diff .into_iter() diff --git a/node/src/transaction_pool/mod.rs b/node/src/transaction_pool/mod.rs index d528d23d7e..3a5eacec46 100644 --- a/node/src/transaction_pool/mod.rs +++ b/node/src/transaction_pool/mod.rs @@ -182,11 +182,9 @@ impl TransactionPoolState { from_rpc: *from_rpc, }); } - Err(e) => match e { - TransactionPoolErrors::BatchedErrors(errors) => { + Err(e) => { + let dispatch_errors = |errors: Vec| { let dispatcher = state.into_dispatcher(); - let errors: Vec<_> = - errors.into_iter().map(|e| e.to_string()).collect(); dispatcher.push(TransactionPoolAction::VerifyError { errors: errors.clone(), }); @@ -196,11 +194,19 @@ impl TransactionPoolState { errors, }) } + }; + match e { + TransactionPoolErrors::BatchedErrors(errors) => { + let errors: Vec<_> = + errors.into_iter().map(|e| e.to_string()).collect(); + dispatch_errors(errors); + } + TransactionPoolErrors::LoadingVK(error) => dispatch_errors(vec![error]), + TransactionPoolErrors::Unexpected(es) => { + panic!("{es}") + } } - TransactionPoolErrors::Unexpected(es) => { - panic!("{es}") - } - }, + } } } TransactionPoolAction::VerifyError { .. } => { From 2326cb253c1d0c1bb76c2d35589fd4f8e0c9e470 Mon Sep 17 00:00:00 2001 From: Sebastien Chapuis Date: Mon, 2 Sep 2024 11:12:44 +0200 Subject: [PATCH 3/3] Add `TransactionPoolState::save_actions` --- node/src/transaction_pool/mod.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/node/src/transaction_pool/mod.rs b/node/src/transaction_pool/mod.rs index 3a5eacec46..e5c07831df 100644 --- a/node/src/transaction_pool/mod.rs +++ b/node/src/transaction_pool/mod.rs @@ -95,17 +95,20 @@ impl TransactionPoolState { id } + #[allow(dead_code)] + fn save_actions(state: &mut crate::Substate) { + let substate = state.get_substate_mut().unwrap(); + if substate.file.is_none() { + let mut file = std::fs::File::create("/tmp/pool.bin").unwrap(); + postcard::to_io(&state.get_state(), &mut file).unwrap(); + let substate = state.get_substate_mut().unwrap(); + substate.file = Some(file); + } + } + pub fn reducer(mut state: crate::Substate, action: &TransactionPoolAction) { - // Uncoment following block to save actions to `/tmp/pool.bin` - // { - // let substate = state.get_substate_mut().unwrap(); - // if substate.file.is_none() { - // let mut file = std::fs::File::create("/tmp/pool.bin").unwrap(); - // postcard::to_io(&state.get_state(), &mut file).unwrap(); - // let substate = state.get_substate_mut().unwrap(); - // substate.file = Some(file); - // } - // } + // Uncoment following line to save actions to `/tmp/pool.bin` + // Self::save_actions(&mut state); let substate = state.get_substate_mut().unwrap(); if let Some(file) = substate.file.as_mut() {