Skip to content

Commit 129433b

Browse files
binierbinier
committed
fix(consensus): transition frontier isn't always updated if it wasn't done right after consensus decision
1 parent 86e2b61 commit 129433b

File tree

5 files changed

+87
-58
lines changed

5 files changed

+87
-58
lines changed

‎node/src/action_kind.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ pub enum ActionKind {
156156
ConsensusP2pBestTipUpdate,
157157
ConsensusPrune,
158158
ConsensusShortRangeForkResolve,
159+
ConsensusTransitionFrontierSyncTargetUpdate,
159160
EventSourceNewEvent,
160161
EventSourceProcessEvents,
161162
EventSourceWaitForEvents,
@@ -660,7 +661,7 @@ pub enum ActionKind {
660661
}
661662

662663
impl ActionKind {
663-
pub const COUNT: u16 = 548;
664+
pub const COUNT: u16 = 549;
664665
}
665666

666667
impl std::fmt::Display for ActionKind {
@@ -796,6 +797,9 @@ impl ActionKindGet for ConsensusAction {
796797
Self::ShortRangeForkResolve { .. } => ActionKind::ConsensusShortRangeForkResolve,
797798
Self::LongRangeForkResolve { .. } => ActionKind::ConsensusLongRangeForkResolve,
798799
Self::BestTipUpdate { .. } => ActionKind::ConsensusBestTipUpdate,
800+
Self::TransitionFrontierSyncTargetUpdate => {
801+
ActionKind::ConsensusTransitionFrontierSyncTargetUpdate
802+
}
799803
Self::P2pBestTipUpdate { .. } => ActionKind::ConsensusP2pBestTipUpdate,
800804
Self::Prune => ActionKind::ConsensusPrune,
801805
}

‎node/src/consensus/consensus_actions.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::sync::Arc;
22

33
use mina_p2p_messages::v2::{MinaBlockBlockStableV2, StateHash};
44
use openmina_core::block::{ArcBlockWithHash, BlockWithHash};
5+
use openmina_core::consensus::consensus_take;
56
use openmina_core::{action_event, ActionEvent};
67
use serde::{Deserialize, Serialize};
78
use snark::block_verify::SnarkBlockVerifyError;
@@ -53,6 +54,7 @@ pub enum ConsensusAction {
5354
BestTipUpdate {
5455
hash: StateHash,
5556
},
57+
TransitionFrontierSyncTargetUpdate,
5658
P2pBestTipUpdate {
5759
best_tip: BlockWithHash<Arc<MinaBlockBlockStableV2>>,
5860
},
@@ -149,6 +151,24 @@ impl redux::EnablingCondition<crate::State> for ConsensusAction {
149151
.consensus
150152
.is_candidate_decided_to_use_as_tip(hash)
151153
},
154+
ConsensusAction::TransitionFrontierSyncTargetUpdate => {
155+
let Some(best_tip) = state.consensus.best_tip_block_with_hash() else {
156+
return false;
157+
};
158+
// do not need to update transition frontier sync target.
159+
if IntoIterator::into_iter([
160+
state.transition_frontier.best_tip(),
161+
state.transition_frontier.sync.best_tip(),
162+
])
163+
.flatten()
164+
.all(|b| b.hash() == best_tip.hash()
165+
|| !consensus_take(b.consensus_state(), best_tip.consensus_state(), b.hash(), best_tip.hash())) {
166+
return false;
167+
}
168+
169+
// has enough data
170+
state.consensus.best_tip_chain_proof(&state.transition_frontier).is_some()
171+
},
152172
ConsensusAction::P2pBestTipUpdate { .. } => true,
153173
ConsensusAction::Prune => {
154174
state.consensus.best_tip().is_some()

‎node/src/consensus/consensus_reducer.rs‎

Lines changed: 33 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use openmina_core::{
22
block::BlockHash,
3+
bug_condition,
34
consensus::{is_short_range_fork, long_range_fork_take, short_range_fork_take},
45
};
56
use snark::block_verify::{SnarkBlockVerifyAction, SnarkBlockVerifyError};
@@ -12,7 +13,7 @@ use crate::{
1213
},
1314
TransitionFrontierSyncAction,
1415
},
15-
Action, State, WatchedAccountsAction,
16+
WatchedAccountsAction,
1617
};
1718

1819
use super::{
@@ -78,7 +79,7 @@ impl ConsensusState {
7879
return;
7980
}
8081

81-
transition_frontier_new_best_tip_handler(global_state, dispatcher);
82+
dispatcher.push(ConsensusAction::TransitionFrontierSyncTargetUpdate);
8283
}
8384
ConsensusAction::BlockSnarkVerifyPending { req_id, hash } => {
8485
if let Some(block) = state.blocks.get_mut(hash) {
@@ -240,7 +241,36 @@ impl ConsensusState {
240241
});
241242
}
242243

243-
transition_frontier_new_best_tip_handler(global_state, dispatcher);
244+
dispatcher.push(ConsensusAction::TransitionFrontierSyncTargetUpdate);
245+
}
246+
ConsensusAction::TransitionFrontierSyncTargetUpdate => {
247+
let (dispatcher, state) = state_context.into_dispatcher_and_state();
248+
let Some(best_tip) = state.consensus.best_tip_block_with_hash() else {
249+
bug_condition!(
250+
"ConsensusAction::TransitionFrontierSyncTargetUpdate | no chosen best tip"
251+
);
252+
return;
253+
};
254+
255+
let Some((blocks_inbetween, root_block)) = state
256+
.consensus
257+
.best_tip_chain_proof(&state.transition_frontier)
258+
else {
259+
bug_condition!("ConsensusAction::TransitionFrontierSyncTargetUpdate | no best tip chain proof");
260+
return;
261+
};
262+
263+
let previous_root_snarked_ledger_hash = state
264+
.transition_frontier
265+
.root()
266+
.map(|b| b.snarked_ledger_hash().clone());
267+
268+
dispatcher.push(TransitionFrontierSyncAction::BestTipUpdate {
269+
previous_root_snarked_ledger_hash,
270+
best_tip,
271+
root_block,
272+
blocks_inbetween,
273+
});
244274
}
245275
ConsensusAction::P2pBestTipUpdate { best_tip } => {
246276
let dispatcher = state_context.into_dispatcher();
@@ -278,56 +308,3 @@ impl ConsensusState {
278308
}
279309
}
280310
}
281-
282-
fn transition_frontier_new_best_tip_handler(
283-
state: &State,
284-
dispatcher: &mut redux::Dispatcher<Action, State>,
285-
) {
286-
let Some(best_tip) = state.consensus.best_tip_block_with_hash() else {
287-
return;
288-
};
289-
let pred_hash = best_tip.pred_hash();
290-
291-
let Some((blocks_inbetween, root_block)) =
292-
state.consensus.best_tip_chain_proof.clone().or_else(|| {
293-
let old_best_tip = state.transition_frontier.best_tip()?;
294-
let mut iter = state.transition_frontier.best_chain.iter();
295-
if old_best_tip.hash() == pred_hash {
296-
if old_best_tip.height() > old_best_tip.constants().k.as_u32() {
297-
iter.next();
298-
}
299-
let root_block = iter.next()?.block_with_hash().clone();
300-
let hashes = iter.map(|b| b.hash().clone()).collect();
301-
Some((hashes, root_block))
302-
} else if old_best_tip.pred_hash() == pred_hash {
303-
let root_block = iter.next()?.block_with_hash().clone();
304-
let hashes = iter.rev().skip(1).rev().map(|b| b.hash().clone()).collect();
305-
Some((hashes, root_block))
306-
} else {
307-
None
308-
}
309-
})
310-
else {
311-
return;
312-
};
313-
314-
if !state.transition_frontier.sync.is_pending() && !state.transition_frontier.sync.is_synced() {
315-
dispatcher.push(TransitionFrontierSyncAction::Init {
316-
best_tip,
317-
root_block,
318-
blocks_inbetween,
319-
});
320-
} else {
321-
let previous_root_snarked_ledger_hash = state
322-
.transition_frontier
323-
.root()
324-
.map(|b| b.snarked_ledger_hash().clone());
325-
326-
dispatcher.push(TransitionFrontierSyncAction::BestTipUpdate {
327-
previous_root_snarked_ledger_hash,
328-
best_tip,
329-
root_block,
330-
blocks_inbetween,
331-
});
332-
}
333-
}

‎node/src/consensus/consensus_state.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use openmina_core::consensus::{
1212
};
1313

1414
use crate::snark::block_verify::SnarkBlockVerifyId;
15+
use crate::transition_frontier::TransitionFrontierState;
1516

1617
#[derive(Serialize, Deserialize, Debug, Clone)]
1718
pub enum ConsensusShortRangeForkDecision {
@@ -181,6 +182,32 @@ impl ConsensusState {
181182
} => decision.use_as_best_tip() && self.best_tip.as_ref() == Some(compared_with),
182183
}
183184
}
185+
186+
pub fn best_tip_chain_proof(
187+
&self,
188+
transition_frontier: &TransitionFrontierState,
189+
) -> Option<(Vec<StateHash>, ArcBlockWithHash)> {
190+
let best_tip = self.best_tip_block_with_hash()?;
191+
let pred_hash = best_tip.pred_hash();
192+
self.best_tip_chain_proof.clone().or_else(|| {
193+
let old_best_tip = transition_frontier.best_tip()?;
194+
let mut iter = transition_frontier.best_chain.iter();
195+
if old_best_tip.hash() == pred_hash {
196+
if old_best_tip.height() > old_best_tip.constants().k.as_u32() {
197+
iter.next();
198+
}
199+
let root_block = iter.next()?.block_with_hash().clone();
200+
let hashes = iter.map(|b| b.hash().clone()).collect();
201+
Some((hashes, root_block))
202+
} else if old_best_tip.pred_hash() == pred_hash {
203+
let root_block = iter.next()?.block_with_hash().clone();
204+
let hashes = iter.rev().skip(1).rev().map(|b| b.hash().clone()).collect();
205+
Some((hashes, root_block))
206+
} else {
207+
None
208+
}
209+
})
210+
}
184211
}
185212

186213
#[derive(Serialize, Debug, Clone, Copy)]

‎node/src/reducer.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use openmina_core::{bug_condition, error, Substate};
22
use p2p::{P2pAction, P2pEffectfulAction, P2pInitializeAction, P2pState};
33

4-
use crate::{Action, ActionWithMeta, EventSourceAction, P2p, State};
4+
use crate::{Action, ActionWithMeta, ConsensusAction, EventSourceAction, P2p, State};
55

66
pub fn reducer(
77
state: &mut State,
@@ -18,6 +18,7 @@ pub fn reducer(
1818
bug_condition!("{}", error);
1919
};
2020
}
21+
dispatcher.push(ConsensusAction::TransitionFrontierSyncTargetUpdate);
2122
}
2223
Action::EventSource(EventSourceAction::NewEvent { .. }) => {}
2324
Action::EventSource(_) => {}

0 commit comments

Comments
 (0)