Skip to content

Commit 2617ea9

Browse files
committed
feat(consensus): Prevalidate blocks in reducers not enabling conditions
Also use error type for validation failure instead of boolean.
1 parent 384d24c commit 2617ea9

File tree

6 files changed

+72
-19
lines changed

6 files changed

+72
-19
lines changed

node/src/action_kind.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ pub enum ActionKind {
166166
ConsensusBlockSnarkVerifyError,
167167
ConsensusBlockSnarkVerifyPending,
168168
ConsensusBlockSnarkVerifySuccess,
169+
ConsensusBlockValidateError,
170+
ConsensusBlockValidateSuccess,
169171
ConsensusDetectForkRange,
170172
ConsensusLongRangeForkResolve,
171173
ConsensusP2pBestTipUpdate,
@@ -716,7 +718,7 @@ pub enum ActionKind {
716718
}
717719

718720
impl ActionKind {
719-
pub const COUNT: u16 = 599;
721+
pub const COUNT: u16 = 601;
720722
}
721723

722724
impl std::fmt::Display for ActionKind {
@@ -857,6 +859,8 @@ impl ActionKindGet for ConsensusAction {
857859
fn kind(&self) -> ActionKind {
858860
match self {
859861
Self::BlockReceived { .. } => ActionKind::ConsensusBlockReceived,
862+
Self::BlockValidateSuccess { .. } => ActionKind::ConsensusBlockValidateSuccess,
863+
Self::BlockValidateError { .. } => ActionKind::ConsensusBlockValidateError,
860864
Self::BlockChainProofUpdate { .. } => ActionKind::ConsensusBlockChainProofUpdate,
861865
Self::BlockSnarkVerifyPending { .. } => ActionKind::ConsensusBlockSnarkVerifyPending,
862866
Self::BlockSnarkVerifySuccess { .. } => ActionKind::ConsensusBlockSnarkVerifySuccess,

node/src/consensus/consensus_actions.rs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use snark::block_verify::SnarkBlockVerifyError;
99

1010
use crate::consensus::ConsensusBlockStatus;
1111
use crate::snark::block_verify::SnarkBlockVerifyId;
12+
use crate::state::BlockValidationError;
1213

1314
pub type ConsensusActionWithMeta = redux::ActionWithMeta<ConsensusAction>;
1415
pub type ConsensusActionWithMetaRef<'a> = redux::ActionWithMeta<&'a ConsensusAction>;
@@ -24,6 +25,13 @@ pub enum ConsensusAction {
2425
block: Arc<MinaBlockBlockStableV2>,
2526
chain_proof: Option<(Vec<StateHash>, ArcBlockWithHash)>,
2627
},
28+
BlockValidateSuccess {
29+
hash: StateHash,
30+
},
31+
BlockValidateError {
32+
hash: StateHash,
33+
error: BlockValidationError,
34+
},
2735
BlockChainProofUpdate {
2836
hash: StateHash,
2937
chain_proof: (Vec<StateHash>, ArcBlockWithHash),
@@ -69,10 +77,14 @@ impl redux::EnablingCondition<crate::State> for ConsensusAction {
6977
hash: hash.clone(),
7078
block: block.clone()
7179
};
72-
!block.is_genesis()
73-
&& !state.consensus.blocks.contains_key(hash)
74-
&& state.prevalidate_block(&block)
80+
!block.is_genesis() && !state.consensus.blocks.contains_key(hash)
7581
},
82+
ConsensusAction::BlockValidateSuccess { hash }
83+
| ConsensusAction::BlockValidateError { hash, .. } => state
84+
.consensus
85+
.blocks
86+
.get(hash)
87+
.map_or(false, |block| block.status.is_received()),
7688
ConsensusAction::BlockChainProofUpdate { hash, .. } => {
7789
(state.consensus.best_tip.as_ref() == Some(hash)
7890
&& state.consensus.best_tip_chain_proof.is_none())
@@ -87,7 +99,7 @@ impl redux::EnablingCondition<crate::State> for ConsensusAction {
8799
.consensus
88100
.blocks
89101
.get(hash)
90-
.map_or(false, |block| block.status.is_received())
102+
.map_or(false, |block| block.status.is_validated())
91103
&& state.snark.block_verify.jobs.contains(*req_id)
92104
},
93105
ConsensusAction::BlockSnarkVerifySuccess { hash } => {

node/src/consensus/consensus_reducer.rs

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use openmina_core::{
2-
block::BlockHash,
2+
block::{ArcBlockWithHash, BlockHash},
33
bug_condition,
44
consensus::{is_short_range_fork, long_range_fork_take, short_range_fork_take},
55
};
@@ -48,9 +48,33 @@ impl ConsensusState {
4848
);
4949

5050
// Dispatch
51+
let (dispatcher, state) = state_context.into_dispatcher_and_state();
52+
53+
let hash = hash.clone();
54+
let block = ArcBlockWithHash {
55+
hash: hash.clone(),
56+
block: block.clone(),
57+
};
58+
match state.prevalidate_block(&block) {
59+
Ok(()) => {
60+
dispatcher.push(ConsensusAction::BlockValidateSuccess { hash });
61+
}
62+
Err(error) => {
63+
dispatcher.push(ConsensusAction::BlockValidateError { hash, error });
64+
}
65+
}
66+
}
67+
ConsensusAction::BlockValidateSuccess { hash } => {
68+
let Some(block) = state.blocks.get_mut(hash) else {
69+
return;
70+
};
71+
block.status = ConsensusBlockStatus::Validated;
72+
73+
// Dispatch
74+
let block = (hash.clone(), block.block.clone()).into();
5175
let dispatcher = state_context.into_dispatcher();
5276
dispatcher.push(SnarkBlockVerifyAction::Init {
53-
block: (hash.clone(), block.clone()).into(),
77+
block,
5478
on_init: redux::callback!(
5579
on_received_block_snark_verify_init((hash: BlockHash, req_id: SnarkBlockVerifyId)) -> crate::Action {
5680
ConsensusAction::BlockSnarkVerifyPending { hash, req_id }
@@ -65,6 +89,9 @@ impl ConsensusState {
6589
}),
6690
});
6791
}
92+
ConsensusAction::BlockValidateError { hash, .. } => {
93+
state.blocks.remove(hash);
94+
}
6895
ConsensusAction::BlockChainProofUpdate { hash, chain_proof } => {
6996
if state.best_tip.as_ref() == Some(hash) {
7097
state.best_tip_chain_proof = Some(chain_proof.clone());

node/src/consensus/consensus_state.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ pub enum ConsensusBlockStatus {
4444
Received {
4545
time: redux::Timestamp,
4646
},
47+
Validated,
4748
SnarkVerifyPending {
4849
time: redux::Timestamp,
4950
req_id: SnarkBlockVerifyId,
@@ -73,6 +74,10 @@ impl ConsensusBlockStatus {
7374
matches!(self, Self::Received { .. })
7475
}
7576

77+
pub fn is_validated(&self) -> bool {
78+
matches!(self, Self::Validated)
79+
}
80+
7681
pub fn is_snark_verify_pending(&self) -> bool {
7782
matches!(self, Self::SnarkVerifyPending { .. })
7883
}
@@ -167,6 +172,7 @@ impl ConsensusState {
167172
};
168173
match &candidate.status {
169174
ConsensusBlockStatus::Received { .. } => false,
175+
ConsensusBlockStatus::Validated => false,
170176
ConsensusBlockStatus::SnarkVerifyPending { .. } => false,
171177
ConsensusBlockStatus::SnarkVerifySuccess { .. } => false,
172178
ConsensusBlockStatus::ForkRangeDetected { .. } => false,

node/src/p2p/peer/p2p_peer_actions.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,6 @@ use super::*;
22

33
impl redux::EnablingCondition<crate::State> for P2pPeerAction {
44
fn is_enabled(&self, state: &crate::State, time: redux::Timestamp) -> bool {
5-
if let P2pPeerAction::BestTipUpdate { best_tip, .. } = self {
6-
if !state.prevalidate_block(best_tip) {
7-
return false;
8-
}
9-
}
105
state.p2p.is_enabled(self, time)
116
}
127
}

node/src/state.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,15 @@ pub struct State {
7373
applied_actions_count: u64,
7474
}
7575

76+
#[derive(Serialize, Deserialize, Debug, Clone)]
77+
pub enum BlockValidationError {
78+
GenesisNotReady,
79+
TooEarly,
80+
TooLate,
81+
ConstantsMismatch,
82+
GenesisMismatch,
83+
}
84+
7685
// Substate accessors that will be used in reducers
7786
use openmina_core::{impl_substate_access, SubstateAccess};
7887

@@ -356,15 +365,15 @@ impl State {
356365
})
357366
}
358367

359-
pub fn prevalidate_block(&self, block: &ArcBlockWithHash) -> bool {
368+
pub fn prevalidate_block(&self, block: &ArcBlockWithHash) -> Result<(), BlockValidationError> {
360369
let Some((genesis, cur_global_slot)) =
361370
None.or_else(|| Some((self.genesis_block()?, self.cur_global_slot()?)))
362371
else {
363372
// we don't have genesis block. This should be impossible
364373
// because we don't even know chain_id before we have genesis
365374
// block, so we can't be connected to any peers from which
366375
// we would receive a block.
367-
return false;
376+
return Err(BlockValidationError::GenesisNotReady);
368377
};
369378

370379
// received_at_valid_time
@@ -375,23 +384,23 @@ impl State {
375384
let delta = genesis.constants().delta.as_u32();
376385
if cur_global_slot < block_global_slot {
377386
// Too_early
378-
return false;
387+
return Err(BlockValidationError::TooEarly);
379388
} else if cur_global_slot.saturating_sub(block_global_slot) > delta {
380389
// Too_late
381-
return false;
390+
return Err(BlockValidationError::TooLate);
382391
}
383392
}
384393

385394
if block.constants() != genesis.constants() {
386-
return false;
395+
return Err(BlockValidationError::ConstantsMismatch);
387396
}
388397

389398
if block.header().genesis_state_hash() != genesis.hash() {
390-
return false;
399+
return Err(BlockValidationError::GenesisMismatch);
391400
}
392401

393402
// TODO(binier): more checks.
394-
true
403+
Ok(())
395404
}
396405

397406
pub fn should_log_node_id(&self) -> bool {

0 commit comments

Comments
 (0)