From ee83b4cda85c1e7a35dc9a5fc0defd52e25c0b81 Mon Sep 17 00:00:00 2001 From: Bruno Deferrari Date: Fri, 22 Mar 2024 14:15:38 -0300 Subject: [PATCH] feat(bootstrap): Reimplement snarked ledger sync progress estimation --- ...on_frontier_sync_ledger_snarked_actions.rs | 19 ++++---- ...on_frontier_sync_ledger_snarked_effects.rs | 7 +-- ...on_frontier_sync_ledger_snarked_reducer.rs | 27 +++++++---- ...tion_frontier_sync_ledger_snarked_state.rs | 47 ++++++++++++++++--- 4 files changed, 69 insertions(+), 31 deletions(-) diff --git a/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_actions.rs b/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_actions.rs index 54aa773e8..e76e10394 100644 --- a/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_actions.rs +++ b/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_actions.rs @@ -110,6 +110,7 @@ pub enum TransitionFrontierSyncLedgerSnarkedAction { }, ChildAccountsAccepted { address: LedgerAddress, + count: u64, sender: PeerId, }, ChildAccountsRejected { @@ -403,16 +404,18 @@ impl redux::EnablingCondition for TransitionFrontierSyncLedgerSnar .map_or(false, |s| s.is_success()), TransitionFrontierSyncLedgerSnarkedAction::ChildAccountsAccepted { address, + count, sender, } => { - state - .transition_frontier - .sync - .ledger() - .and_then(|s| s.snarked()?.fetch_pending()?.get(address)) - .and_then(|s| s.attempts.get(sender)) - // TODO(tizoc): check if expected response kind is correct. - .map_or(false, |s| s.is_success()) + *count > 0 + && state + .transition_frontier + .sync + .ledger() + .and_then(|s| s.snarked()?.fetch_pending()?.get(address)) + .and_then(|s| s.attempts.get(sender)) + // TODO(tizoc): check if expected response kind is correct. + .map_or(false, |s| s.is_success()) } TransitionFrontierSyncLedgerSnarkedAction::ChildAccountsRejected { .. } => true, // TODO(sync): implement TransitionFrontierSyncLedgerSnarkedAction::Success => state diff --git a/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_effects.rs b/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_effects.rs index f633424a4..a49c1cd00 100644 --- a/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_effects.rs +++ b/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_effects.rs @@ -342,12 +342,6 @@ impl TransitionFrontierSyncLedgerSnarkedAction { let left_hash_stale = current_left_hash != *left_hash; let right_hash_stale = current_right_hash != *right_hash; - //println!( - // "+++ sync address={} (depth={}) stale=({left_hash_stale} {right_hash_stale})", - // address.to_string(), - // address.length(), - //); - store.dispatch( TransitionFrontierSyncLedgerSnarkedAction::ChildHashesAccepted { address: address.clone(), @@ -417,6 +411,7 @@ impl TransitionFrontierSyncLedgerSnarkedAction { store.dispatch( TransitionFrontierSyncLedgerSnarkedAction::ChildAccountsAccepted { address: address.clone(), + count: accounts.len() as u64, sender: *sender, }, ); diff --git a/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_reducer.rs b/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_reducer.rs index 6bc57e249..a6bb1d1ba 100644 --- a/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_reducer.rs +++ b/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_reducer.rs @@ -110,6 +110,8 @@ impl TransitionFrontierSyncLedgerSnarkedState { let Self::Pending { pending_num_accounts, num_accounts, + num_accounts_accepted, + num_hashes_accepted, queue, .. } = self @@ -118,6 +120,8 @@ impl TransitionFrontierSyncLedgerSnarkedState { }; *num_accounts = *accepted_num_accounts; + *num_accounts_accepted = 0; + *num_hashes_accepted = 0; *pending_num_accounts = None; // We know at which node to begin querying, so we skip all the intermediary depths @@ -240,6 +244,7 @@ impl TransitionFrontierSyncLedgerSnarkedState { let Self::Pending { queue, pending_addresses: pending, + num_hashes_accepted, .. } = self else { @@ -247,10 +252,14 @@ impl TransitionFrontierSyncLedgerSnarkedState { }; // Once hashes are accepted, we can consider this query fulfilled - // TODO: add helpers to the state, self.fulfill_query(address); pending.remove(address); - // TODO: add helpers to the state, self.enqueue_query(address, expected_hash); + // TODO(tizoc): for non-stale hashes, we can consider the full subtree + // as accepted. Given the value of `num_accounts` and the position + // in the tree we could estimate how many accounts and hashes + // from that subtree will be skipped and add them to the count. + *num_hashes_accepted += 2; + let (left, right) = hashes; if *left_hash_stale { @@ -265,28 +274,26 @@ impl TransitionFrontierSyncLedgerSnarkedState { expected_hash: right.clone(), }); } - - println!( - "+++ sync queue size={} address={} (depth={}) stale=({left_hash_stale} {right_hash_stale})", - queue.len(), - address.to_string(), - address.length(), - ); } TransitionFrontierSyncLedgerSnarkedAction::ChildHashesRejected { .. } => { // TODO(tizoc): should this be reflected in the state somehow? } TransitionFrontierSyncLedgerSnarkedAction::ChildAccountsReceived { .. } => {} TransitionFrontierSyncLedgerSnarkedAction::ChildAccountsAccepted { - address, .. + address, + count, + .. } => { let Self::Pending { pending_addresses: pending, + num_accounts_accepted, .. } = self else { return; }; + + *num_accounts_accepted += count; pending.remove(address); } TransitionFrontierSyncLedgerSnarkedAction::ChildAccountsRejected { .. } => { diff --git a/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_state.rs b/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_state.rs index ce97091d5..12f8a85a4 100644 --- a/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_state.rs +++ b/node/src/transition_frontier/sync/ledger/snarked/transition_frontier_sync_ledger_snarked_state.rs @@ -4,13 +4,13 @@ use mina_p2p_messages::v2::LedgerHash; use redux::Timestamp; use serde::{Deserialize, Serialize}; -use crate::ledger::LedgerAddress; +use crate::ledger::{tree_height_for_num_accounts, LedgerAddress}; use crate::p2p::channels::rpc::P2pRpcId; use crate::p2p::PeerId; use crate::rpc::LedgerSyncProgress; use crate::transition_frontier::sync::ledger::SyncLedgerTarget; -use super::PeerLedgerQueryError; +use super::{PeerLedgerQueryError, ACCOUNT_SUBTREE_HEIGHT}; static SYNC_PENDING_EMPTY: BTreeMap = BTreeMap::new(); @@ -23,6 +23,7 @@ pub enum TransitionFrontierSyncLedgerSnarkedState { target: SyncLedgerTarget, num_accounts: u64, num_accounts_accepted: u64, + num_hashes_accepted: u64, /// Queue of addresses to query and the expected contents hash queue: VecDeque, /// Pending ongoing address queries and their attempts @@ -116,6 +117,7 @@ impl TransitionFrontierSyncLedgerSnarkedState { target, num_accounts: 0, num_accounts_accepted: 0, + num_hashes_accepted: 0, queue: vec![LedgerQueryQueued::NumAccounts].into(), pending_addresses: Default::default(), pending_num_accounts: Default::default(), @@ -187,12 +189,43 @@ impl TransitionFrontierSyncLedgerSnarkedState { } } - // TODO(sync): reimplement for new sync algorithm pub fn estimation(&self) -> Option { - return Some(LedgerSyncProgress { - fetched: 1, - estimation: 1, - }); + match self { + TransitionFrontierSyncLedgerSnarkedState::Pending { + num_accounts, + num_accounts_accepted, + num_hashes_accepted, + .. + } if *num_accounts > 0 => { + // TODO(tizoc): this approximation is very rough, could be improved + let tree_height = tree_height_for_num_accounts(*num_accounts); + let fill_ratio = (*num_accounts as f64) / 2f64.powf(tree_height as f64); + let num_hashes_estimate = 2u64.pow((tree_height - ACCOUNT_SUBTREE_HEIGHT) as u32); + let num_hashes_estimate = (num_hashes_estimate as f64 * fill_ratio).ceil() as u64; + let fetched = *num_accounts_accepted + num_hashes_accepted; + let estimation = fetched.max(*num_accounts + num_hashes_estimate); + + println!( + "{:?}", + LedgerSyncProgress { + fetched, + estimation, + } + ); + + Some(LedgerSyncProgress { + fetched, + estimation, + }) + } + TransitionFrontierSyncLedgerSnarkedState::Success { .. } => { + return Some(LedgerSyncProgress { + fetched: 1, + estimation: 1, + }) + } + _ => None, + } } pub fn peer_num_account_query_get(