From eb3743845f3960ac6c2460626f47819a3719aedb Mon Sep 17 00:00:00 2001 From: Dzejkop Date: Tue, 17 Dec 2024 17:00:16 +0100 Subject: [PATCH] Validate user ops and payload counts --- world-chain-builder/Cargo.lock | 1 + .../crates/world/pbh/Cargo.toml | 1 + .../world/pbh/src/external_nullifier.rs | 30 ++-- .../crates/world/pool/src/error.rs | 4 +- .../crates/world/pool/src/lib.rs | 1 - .../crates/world/pool/src/payload.rs | 23 --- .../crates/world/pool/src/test_utils.rs | 36 +++-- .../crates/world/pool/src/validator.rs | 146 +++++++++--------- 8 files changed, 117 insertions(+), 125 deletions(-) delete mode 100644 world-chain-builder/crates/world/pool/src/payload.rs diff --git a/world-chain-builder/Cargo.lock b/world-chain-builder/Cargo.lock index 4bc8622..c27225a 100644 --- a/world-chain-builder/Cargo.lock +++ b/world-chain-builder/Cargo.lock @@ -13795,6 +13795,7 @@ name = "world-chain-builder-pbh" version = "0.1.0" dependencies = [ "alloy-rlp", + "bon", "chrono", "ethers-core 2.0.14 (git+https://github.com/gakonst/ethers-rs)", "semaphore", diff --git a/world-chain-builder/crates/world/pbh/Cargo.toml b/world-chain-builder/crates/world/pbh/Cargo.toml index c6df5f4..f56bca1 100644 --- a/world-chain-builder/crates/world/pbh/Cargo.toml +++ b/world-chain-builder/crates/world/pbh/Cargo.toml @@ -11,6 +11,7 @@ thiserror = { workspace = true } semaphore.workspace = true strum.workspace = true serde.workspace = true +bon.workspace = true [dev-dependencies] ethers-core.workspace = true diff --git a/world-chain-builder/crates/world/pbh/src/external_nullifier.rs b/world-chain-builder/crates/world/pbh/src/external_nullifier.rs index e3ad34b..5fb53ab 100644 --- a/world-chain-builder/crates/world/pbh/src/external_nullifier.rs +++ b/world-chain-builder/crates/world/pbh/src/external_nullifier.rs @@ -1,37 +1,25 @@ use std::str::FromStr; -use alloy_rlp::{Decodable, Encodable, Rlp}; +use alloy_rlp::{Decodable, Encodable}; +use bon::Builder; use semaphore::{hash_to_field, Field}; use strum::{Display, EnumString}; use thiserror::Error; use crate::date_marker::{DateMarker, DateMarkerParsingError}; -#[macro_export] -macro_rules! ext_nullifier { - ($mo:expr,$yr:expr,$no:expr) => {{ - let prefix = $crate::external_nullifier::Prefix::V1; - let date_marker = $crate::date_marker::DateMarker::new($yr, $mo); - - $crate::external_nullifier::ExternalNullifier::new(prefix, date_marker, $no) - }}; -} - -#[test] -fn whatever() { - ext_nullifier!(01, 2025, 1); -} - #[derive(Display, EnumString, Debug, Clone, Copy, PartialEq, Eq)] #[strum(serialize_all = "snake_case")] pub enum Prefix { V1, } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Builder, Debug, Clone, Copy, PartialEq, Eq)] pub struct ExternalNullifier { + #[builder(default = Prefix::V1)] pub prefix: Prefix, pub date_marker: DateMarker, + #[builder(default = 0)] pub nonce: u16, } @@ -117,13 +105,17 @@ impl FromStr for ExternalNullifier { impl Decodable for ExternalNullifier { fn decode(buf: &mut &[u8]) -> alloy_rlp::Result { - todo!() + let s: String = Decodable::decode(buf)?; + + s.parse::() + .map_err(|_| alloy_rlp::Error::Custom("Failed to parse string to external nullifier")) } } impl Encodable for ExternalNullifier { fn encode(&self, out: &mut dyn alloy_rlp::BufMut) { - todo!() + let s = self.to_string(); + Encodable::encode(&s, out) } } diff --git a/world-chain-builder/crates/world/pool/src/error.rs b/world-chain-builder/crates/world/pool/src/error.rs index ae10f5b..9dbc183 100644 --- a/world-chain-builder/crates/world/pool/src/error.rs +++ b/world-chain-builder/crates/world/pool/src/error.rs @@ -5,7 +5,7 @@ use reth_provider::ProviderError; use world_chain_builder_pbh::external_nullifier::ExternalNullifierParsingError; -#[derive(Debug, thiserror::Error)] +#[derive(Debug, thiserror::Error, PartialEq, Eq)] pub enum WorldChainTransactionPoolInvalid { #[error("nullifier has already been seen")] NullifierAlreadyExists, @@ -29,6 +29,8 @@ pub enum WorldChainTransactionPoolInvalid { InvalidRoot, #[error(transparent)] MalformedSignature(#[from] alloy_rlp::Error), + #[error("One or more user ops are missing pbh payloads")] + MissingPbhPayload, } #[derive(Debug, thiserror::Error)] diff --git a/world-chain-builder/crates/world/pool/src/lib.rs b/world-chain-builder/crates/world/pool/src/lib.rs index 6db5e68..e2b1d65 100644 --- a/world-chain-builder/crates/world/pool/src/lib.rs +++ b/world-chain-builder/crates/world/pool/src/lib.rs @@ -4,7 +4,6 @@ pub mod eip4337; pub mod error; pub mod noop; pub mod ordering; -pub mod payload; pub mod root; pub mod tx; pub mod validator; diff --git a/world-chain-builder/crates/world/pool/src/payload.rs b/world-chain-builder/crates/world/pool/src/payload.rs deleted file mode 100644 index f924d3f..0000000 --- a/world-chain-builder/crates/world/pool/src/payload.rs +++ /dev/null @@ -1,23 +0,0 @@ -use bon::builder; - -#[builder(on(String, into))] -pub fn build_pbh_tx( - #[builder(default = "test test test test test test test test test test test junk")] - mnemonic: String, - - user_ops: Vec, -) { -} - -#[builder(on(String, into))] -pub fn build_user_op() {} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn whatever() { - // build_pbh_tx().call(); - } -} diff --git a/world-chain-builder/crates/world/pool/src/test_utils.rs b/world-chain-builder/crates/world/pool/src/test_utils.rs index d7ef630..3467a46 100644 --- a/world-chain-builder/crates/world/pool/src/test_utils.rs +++ b/world-chain-builder/crates/world/pool/src/test_utils.rs @@ -20,7 +20,7 @@ use reth_primitives::PooledTransactionsElement; use reth_provider::test_utils::MockEthProvider; use revm_primitives::{hex, Address, TxKind}; use semaphore::identity::Identity; -use semaphore::poseidon_tree::{LazyPoseidonTree, PoseidonTree}; +use semaphore::poseidon_tree::LazyPoseidonTree; use semaphore::protocol::{generate_nullifier_hash, generate_proof}; use semaphore::{hash_to_field, Field}; use world_chain_builder_pbh::date_marker::DateMarker; @@ -173,16 +173,9 @@ pub fn user_op( } pub fn pbh_bundle( - ops: Vec<(PackedUserOperation, PbhPayload)>, + user_ops: Vec, + proofs: Vec, ) -> IPBHValidator::handleAggregatedOpsCall { - let mut user_ops = Vec::new(); - let mut proofs = Vec::new(); - - for (user_op, proof) in ops { - user_ops.push(user_op); - proofs.push(proof); - } - let mut signature_buff = Vec::new(); proofs.encode(&mut signature_buff); @@ -350,3 +343,26 @@ pub fn create_pbh_paylaod( proof, } } + +#[cfg(test)] +mod tests { + use test_case::test_case; + + use super::*; + + #[test_case(0, "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266")] + #[test_case(1, "0x70997970C51812dc3A010C7d01b50e0d17dc79C8")] + #[test_case(2, "0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC")] + #[test_case(3, "0x90F79bf6EB2c4f870365E785982E1f101E93b906")] + #[test_case(4, "0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65")] + #[test_case(5, "0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc")] + #[test_case(6, "0x976EA74026E726554dB657fA54763abd0C3a0aa9")] + #[test_case(7, "0x14dC79964da2C08b23698B3D3cc7Ca32193d9955")] + #[test_case(8, "0x23618e81E3f5cdF7f54C3d65f7FBc0aBf5B21E8f")] + #[test_case(9, "0xa0Ee7A142d267C1f36714E4a8F75612F20a79720")] + fn mnemonic_accounts(index: u32, exp_address: &str) { + let exp: Address = exp_address.parse().unwrap(); + + assert_eq!(exp, account(index)); + } +} diff --git a/world-chain-builder/crates/world/pool/src/validator.rs b/world-chain-builder/crates/world/pool/src/validator.rs index 6ea279d..98d865e 100644 --- a/world-chain-builder/crates/world/pool/src/validator.rs +++ b/world-chain-builder/crates/world/pool/src/validator.rs @@ -1,7 +1,7 @@ //! World Chain transaction pool types -use alloy_primitives::{Address, Bytes, U256}; +use alloy_primitives::{Address, U256}; use alloy_rlp::Decodable; -use alloy_sol_types::{SolCall, SolValue}; +use alloy_sol_types::SolCall; use rayon::iter::{IndexedParallelIterator, IntoParallelRefIterator, ParallelIterator}; use reth::transaction_pool::{ Pool, TransactionOrigin, TransactionValidationOutcome, TransactionValidationTaskExecutor, @@ -10,10 +10,8 @@ use reth::transaction_pool::{ use reth_optimism_node::txpool::OpTransactionValidator; use reth_primitives::{Block, SealedBlock, TransactionSigned}; use reth_provider::{BlockReaderIdExt, StateProviderFactory}; -use semaphore::hash_to_field; use semaphore::protocol::verify_proof; use world_chain_builder_pbh::date_marker::DateMarker; -use world_chain_builder_pbh::external_nullifier::ExternalNullifier; use world_chain_builder_pbh::payload::{PbhPayload, TREE_DEPTH}; use super::error::{TransactionValidationError, WorldChainTransactionPoolInvalid}; @@ -182,19 +180,17 @@ where .map_err(WorldChainTransactionPoolInvalid::from) .map_err(TransactionValidationError::from)?; + if pbh_payloads.len() != aggregated_ops.userOps.len() { + Err(WorldChainTransactionPoolInvalid::MissingPbhPayload)?; + } + pbh_payloads .par_iter() .zip(aggregated_ops.userOps) .try_for_each(|(payload, op)| { - let signal = alloy_primitives::keccak256( - <(Address, U256, Bytes) as SolValue>::abi_encode_packed(&( - op.sender, - op.nonce, - op.callData, - )), - ); + let signal = crate::eip4337::hash_user_op(&op); - self.validate_pbh_payload(&payload, hash_to_field(signal.as_ref()))?; + self.validate_pbh_payload(&payload, signal)?; Ok::<(), TransactionValidationError>(()) })?; @@ -236,30 +232,24 @@ where #[cfg(test)] pub mod tests { - use std::time::Instant; - - use alloy_primitives::Address; - use alloy_signer_local::coins_bip39::English; - use alloy_signer_local::PrivateKeySigner; use alloy_sol_types::SolCall; - use bon::builder; use chrono::{TimeZone, Utc}; use ethers_core::types::U256; use reth::transaction_pool::blobstore::InMemoryBlobStore; + use reth::transaction_pool::error::{InvalidPoolTransactionError, PoolError, PoolErrorKind}; use reth::transaction_pool::{ Pool, PoolTransaction as _, TransactionPool, TransactionValidator, }; use reth_primitives::{BlockBody, SealedBlock, SealedHeader}; use reth_provider::test_utils::{ExtendedAccount, MockEthProvider}; - use semaphore::identity::Identity; - use semaphore::poseidon_tree::PoseidonTree; use semaphore::Field; use test_case::test_case; - use world_chain_builder_pbh::ext_nullifier; + use world_chain_builder_pbh::date_marker::DateMarker; use world_chain_builder_pbh::external_nullifier::ExternalNullifier; use world_chain_builder_pbh::payload::{PbhPayload, Proof}; use super::WorldChainTransactionValidator; + use crate::error::WorldChainTransactionPoolInvalid; use crate::ordering::WorldChainOrdering; use crate::root::{LATEST_ROOT_SLOT, OP_WORLD_ID}; use crate::test_utils::{ @@ -268,56 +258,11 @@ pub mod tests { }; use crate::tx::WorldChainPooledTransaction; - #[builder(on(String, into))] - pub fn signer( - #[builder(default = "test test test test test test test test test test test junk")] - mnemonic: String, - #[builder(default = 0)] index: u32, - ) -> PrivateKeySigner { - let signer = alloy_signer_local::MnemonicBuilder::::default() - .phrase(&mnemonic) - .index(index) - .expect("Failed to set index") - .build() - .expect("Failed to create signer"); - - signer - } - - #[builder(on(String, into))] - pub fn account( - #[builder(default = "test test test test test test test test test test test junk")] - mnemonic: String, - #[builder(default = 0)] index: u32, - ) -> Address { - let signer = signer().mnemonic(mnemonic).index(index).call(); - - signer.address() - } - - #[test_case(0, "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266")] - #[test_case(1, "0x70997970C51812dc3A010C7d01b50e0d17dc79C8")] - #[test_case(2, "0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC")] - #[test_case(3, "0x90F79bf6EB2c4f870365E785982E1f101E93b906")] - #[test_case(4, "0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65")] - #[test_case(5, "0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc")] - #[test_case(6, "0x976EA74026E726554dB657fA54763abd0C3a0aa9")] - #[test_case(7, "0x14dC79964da2C08b23698B3D3cc7Ca32193d9955")] - #[test_case(8, "0x23618e81E3f5cdF7f54C3d65f7FBc0aBf5B21E8f")] - #[test_case(9, "0xa0Ee7A142d267C1f36714E4a8F75612F20a79720")] - fn mnemonic_accounts(index: u32, exp_address: &str) { - let exp: Address = exp_address.parse().unwrap(); - - assert_eq!(exp, account().index(index).call()); - } - async fn setup() -> Pool< WorldChainTransactionValidator, WorldChainOrdering, InMemoryBlobStore, > { - let start = Instant::now(); - let validator = world_chain_validator(); // TODO: Remove @@ -329,7 +274,7 @@ pub mod tests { // Fund 10 test accounts for acc in 0..10 { - let account_address = account().index(acc).call(); + let account_address = test_utils::account(acc); validator.inner.client().add_account( account_address, @@ -364,11 +309,23 @@ pub mod tests { Default::default(), ); - println!("Building the pool took {:?}", start.elapsed()); - pool } + fn downcast_pool_err(err: &PoolError) -> &WorldChainTransactionPoolInvalid { + let PoolErrorKind::InvalidTransaction(InvalidPoolTransactionError::Other(other)) = + &err.kind + else { + panic!("Invalid error kind!"); + }; + + other + .source() + .expect("Missing error source") + .downcast_ref::() + .expect("Failed to downcast") + } + #[tokio::test] async fn validate_noop_non_pbh() { const ACC: u32 = 0; @@ -410,8 +367,15 @@ pub mod tests { let pool = setup().await; - let user_op = test_utils::user_op().acc(USER_ACCOUNT).call(); - let bundle = test_utils::pbh_bundle(vec![user_op]); + let (user_op, proof) = test_utils::user_op() + .acc(USER_ACCOUNT) + .external_nullifier( + ExternalNullifier::builder() + .date_marker(DateMarker::from(chrono::Utc::now())) + .build(), + ) + .call(); + let bundle = test_utils::pbh_bundle(vec![user_op], vec![proof]); let calldata = bundle.abi_encode(); let tx = test_utils::eip1559() @@ -426,6 +390,46 @@ pub mod tests { .expect("Failed to add transaction"); } + #[tokio::test] + async fn validate_pbh_missing_proof_for_user_op() { + const BUNDLER_ACCOUNT: u32 = 9; + const USER_ACCOUNT: u32 = 0; + + let pool = setup().await; + + let (user_op, _proof) = test_utils::user_op() + .acc(USER_ACCOUNT) + .external_nullifier( + ExternalNullifier::builder() + .date_marker(DateMarker::from(chrono::Utc::now())) + .build(), + ) + .call(); + + let bundle = test_utils::pbh_bundle(vec![user_op], vec![]); + + let calldata = bundle.abi_encode(); + + let tx = test_utils::eip1559() + .to(PBH_TEST_VALIDATOR) + .input(calldata) + .call(); + + let tx = test_utils::eth_tx(BUNDLER_ACCOUNT, tx).await; + + let err = pool + .add_external_transaction(tx.clone().into()) + .await + .expect_err("Validation should fail because of missing proof"); + + let world_chain_err = downcast_pool_err(&err); + + assert_eq!( + world_chain_err, + &WorldChainTransactionPoolInvalid::MissingPbhPayload + ); + } + #[tokio::test] async fn validate_4337_bundle() { let validator = world_chain_validator();