From ec3338959155cc262bbe2af96a168e6dc51b17d5 Mon Sep 17 00:00:00 2001 From: Sebastien Chapuis Date: Thu, 28 Mar 2024 11:12:17 +0100 Subject: [PATCH] Split other actions in 2 actions We need to fetch accounts first + Remove `best_tip_ledger` in `TransactionPool` --- ledger/src/scan_state/transaction_logic.rs | 18 +++ ledger/src/transaction_pool.rs | 86 +++++++------- node/src/action_kind.rs | 10 +- node/src/transaction_pool/mod.rs | 108 ++++++++++++------ .../transaction_pool_actions.rs | 16 ++- 5 files changed, 158 insertions(+), 80 deletions(-) diff --git a/ledger/src/scan_state/transaction_logic.rs b/ledger/src/scan_state/transaction_logic.rs index c36daa5e1..10e81066e 100644 --- a/ledger/src/scan_state/transaction_logic.rs +++ b/ledger/src/scan_state/transaction_logic.rs @@ -4541,6 +4541,24 @@ impl UserCommand { .collect() } + pub fn load_vks_from_ledger_accounts( + accounts: &BTreeMap, + ) -> HashMap { + accounts + .iter() + .filter_map(|(_, account)| { + let zkapp = account.zkapp.as_ref()?; + let vk = zkapp.verification_key.clone()?; + + // TODO: The account should contains the `WithHash` + let hash = vk.hash(); + let vk = WithHash { data: vk, hash }; + + Some((account.id(), vk)) + }) + .collect() + } + pub fn to_all_verifiable( ts: Vec>, load_vk_cache: F, diff --git a/ledger/src/transaction_pool.rs b/ledger/src/transaction_pool.rs index fde9bdb08..935f9179c 100644 --- a/ledger/src/transaction_pool.rs +++ b/ledger/src/transaction_pool.rs @@ -191,7 +191,7 @@ pub mod diff { fn preload_accounts( ledger: &Mask, - account_ids: &HashSet, + account_ids: &BTreeSet, ) -> HashMap { account_ids .iter() @@ -545,7 +545,7 @@ struct SenderState { pub enum RevalidateKind<'a> { EntirePool, - Subset(&'a HashSet), + Subset(&'a BTreeMap), } impl IndexedPool { @@ -1184,7 +1184,7 @@ impl IndexedPool { { let requires_revalidation = |account_id: &AccountId| match kind { RevalidateKind::EntirePool => true, - RevalidateKind::Subset(set) => set.contains(account_id), + RevalidateKind::Subset(set) => set.contains_key(account_id), }; let mut dropped = Vec::new(); @@ -1412,8 +1412,6 @@ pub struct TransactionPool { config: Config, batcher: (), best_tip_diff_relay: Option<()>, - #[serde(skip)] - best_tip_ledger: Option, verification_key_table: VkRefcountTable, } @@ -1428,7 +1426,6 @@ impl TransactionPool { config: todo!(), batcher: todo!(), best_tip_diff_relay: todo!(), - best_tip_ledger: todo!(), verification_key_table: todo!(), } } @@ -1438,9 +1435,6 @@ impl TransactionPool { } pub fn on_new_best_tip(&mut self, accounts: &BTreeMap) { - // let validation_ledger = new_best_tip; - // self.best_tip_ledger.replace(validation_ledger.clone()); - let dropped = self .pool .revalidate(RevalidateKind::EntirePool, |sender_id| { @@ -1494,10 +1488,32 @@ impl TransactionPool { list } + pub fn get_accounts_to_handle_transition_diff( + &self, + diff: &diff::BestTipDiff, + ) -> BTreeSet { + let diff::BestTipDiff { + new_commands, + removed_commands, + reorg_best_tip: _, + } = diff; + + new_commands + .iter() + .chain(removed_commands) + .flat_map(|cmd| cmd.data.forget_check().accounts_referenced()) + .chain( + self.locally_generated_uncommitted + .keys() + .map(|cmd| cmd.data.fee_payer()), + ) + .collect::>() + } + pub fn handle_transition_frontier_diff( &mut self, diff: &diff::BestTipDiff, - best_tip_ledger: Mask, + accounts: &BTreeMap, ) { let diff::BestTipDiff { new_commands, @@ -1506,7 +1522,6 @@ impl TransactionPool { } = diff; let global_slot = self.pool.global_slot_since_genesis(); - self.best_tip_ledger = Some(best_tip_ledger.clone()); let pool_max_size = self.config.pool_max_size; @@ -1538,20 +1553,14 @@ impl TransactionPool { .collect::>(); let dropped_commands = { - let accounts_to_check = new_commands - .iter() - .chain(removed_commands) - .flat_map(|cmd| cmd.data.forget_check().accounts_referenced()) - .collect::>(); - - let existing_account_states_by_id = - preload_accounts(&best_tip_ledger, &accounts_to_check); + let accounts_to_check = accounts; + let existing_account_states_by_id = accounts; let get_account = |id: &AccountId| { match existing_account_states_by_id.get(id) { Some(account) => account.clone(), None => { - if accounts_to_check.contains(id) { + if accounts_to_check.contains_key(id) { Account::empty() } else { // OCaml panic too, with same message @@ -1565,7 +1574,7 @@ impl TransactionPool { }; self.pool - .revalidate(RevalidateKind::Subset(&accounts_to_check), get_account) + .revalidate(RevalidateKind::Subset(accounts_to_check), get_account) }; let (committed_commands, dropped_commit_conflicts): (Vec<_>, Vec<_>) = { @@ -1610,10 +1619,7 @@ impl TransactionPool { remove_cmd(self) } else { let unchecked = &cmd.data; - match best_tip_ledger - .location_of_account(&unchecked.fee_payer()) - .and_then(|addr| best_tip_ledger.get(addr)) - { + match accounts.get(&unchecked.fee_payer()) { Some(account) => { match self.pool.add_from_gossip_exn( cmd, @@ -1643,9 +1649,15 @@ impl TransactionPool { } } + pub fn get_accounts_to_apply_diff(&self, diff: &diff::DiffVerified) -> BTreeSet { + let fee_payer = |cmd: &ValidCommandWithHash| cmd.data.fee_payer(); + diff.list.iter().map(fee_payer).collect() + } + fn apply( &mut self, diff: &diff::DiffVerified, + accounts: &BTreeMap, is_sender_local: bool, ) -> Result< ( @@ -1655,15 +1667,8 @@ impl TransactionPool { ), String, > { - let ledger = self.best_tip_ledger.as_ref().ok_or_else(|| { - "Got transaction pool diff when transitin frontier is unavailable, ignoring." - .to_string() - })?; - let fee_payer = |cmd: &ValidCommandWithHash| cmd.data.fee_payer(); - - let fee_payer_account_ids: HashSet<_> = diff.list.iter().map(fee_payer).collect(); - let fee_payer_accounts = preload_accounts(ledger, &fee_payer_account_ids); + let fee_payer_accounts = accounts; let check_command = |pool: &IndexedPool, cmd: &ValidCommandWithHash| { if pool.member(cmd) { @@ -1803,6 +1808,7 @@ impl TransactionPool { pub fn unsafe_apply( &mut self, diff: &diff::DiffVerified, + accounts: &BTreeMap, is_sender_local: bool, ) -> Result< ( @@ -1812,7 +1818,7 @@ impl TransactionPool { ), String, > { - let (decision, accepted, rejected) = self.apply(diff, is_sender_local)?; + let (decision, accepted, rejected) = self.apply(diff, accounts, is_sender_local)?; let accepted = accepted .into_iter() .map(|cmd| cmd.data.forget_check()) @@ -1844,7 +1850,11 @@ impl TransactionPool { } } - fn verify(&self, diff: Envelope) -> Result, String> { + fn verify( + &self, + diff: Envelope, + accounts: BTreeMap, + ) -> Result, String> { let well_formedness_errors: HashSet<_> = diff .data() .list @@ -1862,10 +1872,6 @@ impl TransactionPool { )); } - let ledger = self.best_tip_ledger.as_ref().ok_or_else(|| { - "We don't have a transition frontier at the moment, so we're unable to verify any transactions." - })?; - let cs = diff .data() .list @@ -1885,7 +1891,7 @@ impl TransactionPool { }) .collect(); - let ledger_vks = UserCommand::load_vks_from_ledger(account_ids, ledger); + let ledger_vks = UserCommand::load_vks_from_ledger_accounts(&accounts); let ledger_vks: HashMap<_, _> = ledger_vks .into_iter() .map(|(id, vk)| { diff --git a/node/src/action_kind.rs b/node/src/action_kind.rs index 3ce03b127..13463b74a 100644 --- a/node/src/action_kind.rs +++ b/node/src/action_kind.rs @@ -326,7 +326,9 @@ pub enum ActionKind { SnarkWorkVerifyPending, SnarkWorkVerifySuccess, TransactionPoolApplyTransitionFrontierDiff, + TransactionPoolApplyTransitionFrontierDiffWithAccounts, TransactionPoolApplyVerifiedDiff, + TransactionPoolApplyVerifiedDiffWithAccounts, TransactionPoolBestTipChanged, TransactionPoolBestTipChangedWithAccounts, TransactionPoolRebroadcast, @@ -398,7 +400,7 @@ pub enum ActionKind { } impl ActionKind { - pub const COUNT: u16 = 330; + pub const COUNT: u16 = 332; } impl std::fmt::Display for ActionKind { @@ -518,9 +520,15 @@ impl ActionKindGet for TransactionPoolAction { ActionKind::TransactionPoolBestTipChangedWithAccounts } Self::ApplyVerifiedDiff { .. } => ActionKind::TransactionPoolApplyVerifiedDiff, + Self::ApplyVerifiedDiffWithAccounts { .. } => { + ActionKind::TransactionPoolApplyVerifiedDiffWithAccounts + } Self::ApplyTransitionFrontierDiff { .. } => { ActionKind::TransactionPoolApplyTransitionFrontierDiff } + Self::ApplyTransitionFrontierDiffWithAccounts { .. } => { + ActionKind::TransactionPoolApplyTransitionFrontierDiffWithAccounts + } Self::Rebroadcast => ActionKind::TransactionPoolRebroadcast, } } diff --git a/node/src/transaction_pool/mod.rs b/node/src/transaction_pool/mod.rs index 36e13aeac..0d854d040 100644 --- a/node/src/transaction_pool/mod.rs +++ b/node/src/transaction_pool/mod.rs @@ -1,14 +1,7 @@ -use std::collections::BTreeMap; - -use ledger::{ - transaction_pool::{ - diff::{BestTipDiff, DiffVerified}, - ApplyDecision, - }, - Account, BaseLedger, Mask, -}; +use std::collections::{BTreeMap, BTreeSet}; + +use ledger::{transaction_pool::ApplyDecision, Account, AccountId, BaseLedger, Mask}; use mina_p2p_messages::v2::LedgerHash; -use serde::{Deserialize, Serialize}; use crate::{Service, Store}; @@ -19,7 +12,6 @@ pub use transaction_pool_actions::TransactionPoolAction; #[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] pub struct TransactionPoolState { pool: ledger::transaction_pool::TransactionPool, - // accounts_to_revalidate: Option>, } type TransactionPoolActionWithMeta = redux::ActionWithMeta; @@ -37,67 +29,111 @@ impl TransactionPoolState { let (action, meta) = action.split(); match action { - BestTipChanged { best_tip_hash } => { - // let account_ids = self.pool.get_accounts_to_revalidate_on_new_best_tip(); - // let best_tip: Mask = Mask::new_unattached(10); - // self.pool.on_new_best_tip(best_tip.clone()); - } + BestTipChanged { best_tip_hash: _ } => {} BestTipChangedWithAccounts { accounts } => { self.pool.on_new_best_tip(accounts); } ApplyVerifiedDiff { + best_tip_hash: _, + diff: _, + is_sender_local: _, + } => {} + ApplyVerifiedDiffWithAccounts { diff, is_sender_local, - } => match self.pool.unsafe_apply(diff, *is_sender_local) { + accounts, + } => match self.pool.unsafe_apply(diff, &accounts, *is_sender_local) { Ok((ApplyDecision::Accept, accepted, rejected)) => todo!(), Ok((ApplyDecision::Reject, accepted, rejected)) => todo!(), Err(_) => todo!(), }, ApplyTransitionFrontierDiff { - best_tip_hash, - diff, - } => { - // self.pool - // .handle_transition_frontier_diff(diff, best_tip.clone()); + best_tip_hash: _, + diff: _, + } => {} + ApplyTransitionFrontierDiffWithAccounts { diff, accounts } => { + self.pool.handle_transition_frontier_diff(diff, &accounts); } Rebroadcast => todo!(), } } } +fn load_accounts_from_best_tip_ledger( + store: &mut Store, + best_tip_hash: &LedgerHash, + account_ids: BTreeSet, +) -> BTreeMap { + let best_tip_mask = store.service.get_mask(&best_tip_hash).unwrap(); // TODO Handle error + + account_ids + .into_iter() + .filter_map(|account_id| { + best_tip_mask + .location_of_account(&account_id) + .and_then(|addr| { + best_tip_mask + .get(addr) + .map(|account| (account_id, *account)) + }) + }) + .collect::>() +} + pub fn transaction_pool_effects( store: &mut Store, action: TransactionPoolActionWithMeta, ) { - let (action, meta) = action.split(); + let (action, _meta) = action.split(); match action { TransactionPoolAction::BestTipChanged { best_tip_hash } => { let state = &store.state().transaction_pool; let account_ids = state.pool.get_accounts_to_revalidate_on_new_best_tip(); - let best_tip = store.service.get_mask(&best_tip_hash).unwrap(); // TODO Handle error - - let accounts = account_ids - .into_iter() - .filter_map(|account_id| { - best_tip - .location_of_account(&account_id) - .and_then(|addr| best_tip.get(addr).map(|account| (account_id, *account))) - }) - .collect::>(); + let accounts = load_accounts_from_best_tip_ledger(store, &best_tip_hash, account_ids); store.dispatch(TransactionPoolAction::BestTipChangedWithAccounts { accounts }); } - TransactionPoolAction::BestTipChangedWithAccounts { accounts } => todo!(), + TransactionPoolAction::BestTipChangedWithAccounts { accounts: _ } => {} TransactionPoolAction::ApplyVerifiedDiff { + best_tip_hash, diff, is_sender_local, - } => todo!(), + } => { + let state = &store.state().transaction_pool; + let account_ids = state.pool.get_accounts_to_apply_diff(&diff); + + let accounts = load_accounts_from_best_tip_ledger(store, &best_tip_hash, account_ids); + + store.dispatch(TransactionPoolAction::ApplyVerifiedDiffWithAccounts { + diff, + is_sender_local, + accounts, + }); + } + TransactionPoolAction::ApplyVerifiedDiffWithAccounts { + diff: _, + is_sender_local: _, + accounts: _, + } => {} TransactionPoolAction::ApplyTransitionFrontierDiff { best_tip_hash, diff, - } => todo!(), + } => { + let state = &store.state().transaction_pool; + let account_ids = state.pool.get_accounts_to_handle_transition_diff(&diff); + + let accounts = load_accounts_from_best_tip_ledger(store, &best_tip_hash, account_ids); + + store.dispatch( + TransactionPoolAction::ApplyTransitionFrontierDiffWithAccounts { diff, accounts }, + ); + } + TransactionPoolAction::ApplyTransitionFrontierDiffWithAccounts { + diff: _, + accounts: _, + } => {} TransactionPoolAction::Rebroadcast => todo!(), } } diff --git a/node/src/transaction_pool/transaction_pool_actions.rs b/node/src/transaction_pool/transaction_pool_actions.rs index ac8dc4855..264423862 100644 --- a/node/src/transaction_pool/transaction_pool_actions.rs +++ b/node/src/transaction_pool/transaction_pool_actions.rs @@ -16,16 +16,26 @@ pub enum TransactionPoolAction { accounts: BTreeMap, }, ApplyVerifiedDiff { + best_tip_hash: LedgerHash, + diff: DiffVerified, + /// Diff was crearted locally, or from remote peer ? + is_sender_local: bool, + }, + ApplyVerifiedDiffWithAccounts { diff: DiffVerified, is_sender_local: bool, + accounts: BTreeMap, }, ApplyTransitionFrontierDiff { best_tip_hash: LedgerHash, - // best_tip: Mask, diff: BestTipDiff, }, - // Rebroadcast locally generated pool items every 10 minutes. Do so for 50 - // minutes - at most 5 rebroadcasts - before giving up. + ApplyTransitionFrontierDiffWithAccounts { + diff: BestTipDiff, + accounts: BTreeMap, + }, + /// Rebroadcast locally generated pool items every 10 minutes. Do so for 50 + /// minutes - at most 5 rebroadcasts - before giving up. Rebroadcast, }