Skip to content

Commit

Permalink
Replica Validation Refactor (#2170)
Browse files Browse the repository at this point in the history 
* Create internal type

  * add `Proposal` and `ValidatedTransition` types
  * move validation to isolated methods.
  * add doc comments to methods
  * update error enum
  * update tests

* fix validate_proposal_success test

Replace references to `validate_proposal` /w
`ValidatedTransition`. Remove `validate_proposal_error_cases` as it
nightmarish to navigate that and error cases are more thoroughly tested
in unit tests now.

---------

Co-authored-by: tbro <tbro@users.noreply.github.com>
Co-authored-by: sveitser <sveitser@gmail.com>
  • Loading branch information
@tbro @sveitser
3 people authored Oct 24, 2024
1 parent ee6f7d4 commit 4144ba4
Show file tree
Hide file tree
Showing 5 changed files with 883 additions and 406 deletions.
164 changes: 21 additions & 143 deletions types/src/v0/impls/header.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1101,27 +1101,19 @@ impl ExplorerHeader<SeqTypes> for Header {

#[cfg(test)]
mod test_headers {

use std::sync::Arc;

use ethers::{
types::{Address, U256},
utils::Anvil,
};
use ethers::{types::Address, utils::Anvil};
use hotshot_types::{traits::signature_key::BuilderSignatureKey, vid::vid_scheme};
use jf_vid::VidScheme;

use sequencer_utils::test_utils::setup_test;
use v0_1::{BlockMerkleTree, FeeMerkleTree, L1Client};
use vbs::{
bincode_serializer::BincodeSerializer,
version::{StaticVersion, StaticVersionType},
BinarySerializer,
};
use vbs::{bincode_serializer::BincodeSerializer, version::StaticVersion, BinarySerializer};

use crate::{eth_signature_key::EthKeyPair, mock::MockStateCatchup};

use super::*;
use crate::{
eth_signature_key::EthKeyPair, v0::impls::instance_state::mock::MockStateCatchup,
validate_proposal, ProposalValidationError,
};

#[derive(Debug, Default)]
#[must_use]
Expand Down Expand Up @@ -1409,121 +1401,7 @@ mod test_headers {
}

#[async_std::test]
async fn test_validate_proposal_error_cases() {
// TODO add assertion for timestamp validation
let genesis = GenesisForTest::default().await;
let vid_common = vid_scheme(1).disperse([]).unwrap().common;

let mut validated_state = ValidatedState::default();
let mut block_merkle_tree = validated_state.block_merkle_tree.clone();

let mut parent_header = genesis.header.clone();
let mut parent_leaf = genesis.leaf.clone();
*parent_leaf.block_header_mut() = parent_header.clone();

// Populate the tree with an initial `push`.
block_merkle_tree.push(genesis.header.commit()).unwrap();
let block_merkle_tree_root = block_merkle_tree.commitment();
validated_state.block_merkle_tree = block_merkle_tree.clone();
*parent_header.block_merkle_tree_root_mut() = block_merkle_tree_root;
let mut proposal = parent_header.clone();
*proposal.timestamp_mut() = OffsetDateTime::now_utc().unix_timestamp() as u64;
*proposal.l1_head_mut() = 5;

let ver = StaticVersion::<0, 1>::version();

// Pass a different chain config to trigger a chain config validation error.
let state = validated_state
.apply_header(
&genesis.instance_state,
&genesis.instance_state.peers,
&parent_leaf,
&proposal,
ver,
)
.await
.unwrap()
.0;

let chain_config = ChainConfig {
chain_id: U256::zero().into(),
..Default::default()
};
let err = validate_proposal(&state, chain_config, &parent_leaf, &proposal, &vid_common)
.unwrap_err();

assert_eq!(
ProposalValidationError::InvalidChainConfig {
expected: format!("{:?}", chain_config),
proposal: format!("{:?}", proposal.chain_config())
},
err
);

// Advance `proposal.height` to trigger validation error.

let validated_state = validated_state
.apply_header(
&genesis.instance_state,
&genesis.instance_state.peers,
&parent_leaf,
&proposal,
ver,
)
.await
.unwrap()
.0;
let err = validate_proposal(
&validated_state,
genesis.instance_state.chain_config,
&parent_leaf,
&proposal,
&vid_common,
)
.unwrap_err();
assert_eq!(
ProposalValidationError::InvalidHeight {
parent_height: 0,
proposal_height: 0
},
err
);

// proposed `Header` root should include parent + parent.commit
*proposal.height_mut() += 1;

let validated_state = validated_state
.apply_header(
&genesis.instance_state,
&genesis.instance_state.peers,
&parent_leaf,
&proposal,
ver,
)
.await
.unwrap()
.0;

let err = validate_proposal(
&validated_state,
genesis.instance_state.chain_config,
&parent_leaf,
&proposal,
&vid_common,
)
.unwrap_err();
// Fails b/c `proposal` has not advanced from `parent`
assert_eq!(
ProposalValidationError::InvalidBlockRoot {
expected_root: validated_state.block_merkle_tree.commitment(),
proposal_root: proposal.block_merkle_tree_root()
},
err
);
}

#[async_std::test]
async fn test_validate_proposal_success() {
async fn test_proposal_validation_success() {
setup_test();

let anvil = Anvil::new().block_time(1u32).spawn();
Expand Down Expand Up @@ -1601,7 +1479,7 @@ mod test_headers {
let mut block_merkle_tree = proposal_state.block_merkle_tree.clone();
block_merkle_tree.push(proposal.commit()).unwrap();

let proposal_state = proposal_state
let _proposal_state = proposal_state
.apply_header(
&genesis_state,
&genesis_state.peers,
Expand All @@ -1612,23 +1490,23 @@ mod test_headers {
.await
.unwrap()
.0;
validate_proposal(
&proposal_state,
genesis.instance_state.chain_config,
&parent_leaf,
&proposal.clone(),
&vid_common,
)
.unwrap();

assert_eq!(
proposal_state.block_merkle_tree.commitment(),
proposal.block_merkle_tree_root()
);
// ValidatedTransition::new(
// proposal_state.clone(),
// &parent_leaf.block_header(),
// Proposal::new(&proposal, VidSchemeType::get_payload_byte_len(&vid_common)),
// )
// .validate()
// .unwrap();

// assert_eq!(
// proposal_state.block_merkle_tree.commitment(),
// proposal.block_merkle_tree_root()
// );
}

#[test]
fn verify_header_signature() {
fn verify_builder_signature() {
// simulate a fixed size hash by padding our message
let message = ";)";
let mut commitment = [0u8; 32];
Expand Down
13 changes: 13 additions & 0 deletions types/src/v0/impls/instance_state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,19 @@ impl NodeState {
)
}

#[cfg(any(test, feature = "testing"))]
pub fn mock_v2() -> Self {
use vbs::version::StaticVersion;

Self::new(
0,
ChainConfig::default(),
L1Client::new("http://localhost:3331".parse().unwrap(), 10000),
mock::MockStateCatchup::default(),
StaticVersion::<0, 2>::version(),
)
}

#[cfg(any(test, feature = "testing"))]
pub fn mock_v3() -> Self {
use vbs::version::StaticVersion;
Expand Down
5 changes: 1 addition & 4 deletions types/src/v0/impls/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,4 @@ pub use auction::SolverAuctionResultsProvider;
pub use fee_info::{retain_accounts, FeeError};
pub use instance_state::{mock, NodeState};
pub use state::ProposalValidationError;
pub use state::{
get_l1_deposits, validate_proposal, BuilderValidationError, StateValidationError,
ValidatedState,
};
pub use state::{get_l1_deposits, BuilderValidationError, StateValidationError, ValidatedState};
Loading

0 comments on commit 4144ba4

Please sign in to comment.