From 726f1077888a622f454cb54e2dbd2a753f19d2ec Mon Sep 17 00:00:00 2001 From: Alex Miao Date: Wed, 20 Sep 2023 11:18:57 -0700 Subject: [PATCH 1/2] fix(uo): fix user operation encoding and size calculations --- crates/pool/src/mempool/mod.rs | 9 ++-- crates/pool/src/mempool/pool.rs | 12 ++--- crates/sim/src/estimation/estimation.rs | 20 ++++---- crates/sim/src/gas/gas.rs | 7 +-- crates/types/src/user_operation.rs | 63 +++++++++++++++++++++++-- 5 files changed, 85 insertions(+), 26 deletions(-) diff --git a/crates/pool/src/mempool/mod.rs b/crates/pool/src/mempool/mod.rs index 259c62473..c81cbb14a 100644 --- a/crates/pool/src/mempool/mod.rs +++ b/crates/pool/src/mempool/mod.rs @@ -185,9 +185,12 @@ impl PoolOperation { }) } - /// Returns the size of the operation in bytes. - pub fn size(&self) -> usize { - self.uo.pack().len() + /// Compute the amount of memory the PoolOperation takes up. + pub fn mem_size(&self) -> usize { + std::mem::size_of::() + + self.uo.mem_size() + + std::mem::size_of::>() + + self.entities_needing_stake.len() * std::mem::size_of::() } fn entity_address(&self, entity: EntityType) -> Option
{ diff --git a/crates/pool/src/mempool/pool.rs b/crates/pool/src/mempool/pool.rs index 7d962d95e..5b61528d4 100644 --- a/crates/pool/src/mempool/pool.rs +++ b/crates/pool/src/mempool/pool.rs @@ -362,7 +362,7 @@ impl OrderedPoolOperation { } fn size(&self) -> usize { - self.po.size() + self.po.mem_size() } } @@ -664,7 +664,7 @@ mod tests { } assert_eq!(pool.address_count(sender), 1); - assert_eq!(pool.pool_size, po1.size()); + assert_eq!(pool.pool_size, po1.mem_size()); } #[test] @@ -687,7 +687,7 @@ mod tests { assert_eq!(pool.address_count(sender), 1); assert_eq!(pool.address_count(paymaster1), 0); assert_eq!(pool.address_count(paymaster2), 1); - assert_eq!(pool.pool_size, po2.size()); + assert_eq!(pool.pool_size, po2.mem_size()); } #[test] @@ -712,12 +712,12 @@ mod tests { chain_id: 1, max_userops_per_sender: 16, min_replacement_fee_increase_percentage: 10, - max_size_of_pool_bytes: 20 * size_of_op(), + max_size_of_pool_bytes: 20 * mem_size_of_op(), } } - fn size_of_op() -> usize { - create_op(Address::random(), 1, 1).size() + fn mem_size_of_op() -> usize { + create_op(Address::random(), 1, 1).mem_size() } fn create_op(sender: Address, nonce: usize, max_fee_per_gas: usize) -> PoolOperation { diff --git a/crates/sim/src/estimation/estimation.rs b/crates/sim/src/estimation/estimation.rs index 5bdd055a3..9c11bf23c 100644 --- a/crates/sim/src/estimation/estimation.rs +++ b/crates/sim/src/estimation/estimation.rs @@ -472,13 +472,13 @@ mod tests { let u_o = user_op.max_fill(&settings); - let u_o_packed = u_o.pack(); - let length_in_words = (u_o_packed.len() + 31) / 32; + let u_o_encoded = u_o.encode(); + let length_in_words = (u_o_encoded.len() + 31) / 32; //computed by mapping through the calldata bytes //and adding to the value either 4 or 16 depending //if the byte is non-zero - let call_data_cost = 4316; + let call_data_cost = 3936; let result = U256::from(FIXED) / U256::from(BUNDLE_SIZE) + call_data_cost @@ -513,13 +513,13 @@ mod tests { let u_o = user_op.max_fill(&settings); - let u_o_packed = u_o.pack(); - let length_in_words = (u_o_packed.len() + 31) / 32; + let u_o_encoded = u_o.encode(); + let length_in_words = (u_o_encoded.len() + 31) / 32; //computed by mapping through the calldata bytes //and adding to the value either 4 or 16 depending //if the byte is non-zero - let call_data_cost = 4316; + let call_data_cost = 3936; let result = U256::from(FIXED) / U256::from(BUNDLE_SIZE) + call_data_cost @@ -556,13 +556,13 @@ mod tests { let u_o = user_op.max_fill(&settings); - let u_o_packed = u_o.pack(); - let length_in_words = (u_o_packed.len() + 31) / 32; + let u_o_encoded: Bytes = u_o.encode().into(); + let length_in_words = (u_o_encoded.len() + 31) / 32; //computed by mapping through the calldata bytes //and adding to the value either 4 or 16 depending //if the byte is non-zero - let call_data_cost = 4316; + let call_data_cost = 3936; let result = U256::from(FIXED) / U256::from(BUNDLE_SIZE) + call_data_cost @@ -1117,7 +1117,7 @@ mod tests { let estimation = estimator.estimate_op_gas(user_op).await.unwrap(); // this number uses the same logic as the pre_verification tests - assert_eq!(estimation.pre_verification_gas, U256::from(43656)); + assert_eq!(estimation.pre_verification_gas, U256::from(43296)); // 30000 GAS_FEE_TRANSER_COST increased by default 10% assert_eq!(estimation.verification_gas_limit, U256::from(33000)); diff --git a/crates/sim/src/gas/gas.rs b/crates/sim/src/gas/gas.rs index c3ae9353b..edbc257c5 100644 --- a/crates/sim/src/gas/gas.rs +++ b/crates/sim/src/gas/gas.rs @@ -1,6 +1,7 @@ use std::sync::Arc; use ethers::{ + abi::AbiEncode, prelude::gas_oracle::GasCategory, types::{Address, Chain, U256}, }; @@ -124,9 +125,9 @@ pub fn user_operation_max_gas_cost(uo: &UserOperation, chain_id: u64) -> U256 { fn calc_static_pre_verification_gas(op: &UserOperation) -> U256 { let ov = GasOverheads::default(); - let packed = op.pack(); - let length_in_words = (packed.len() + 31) / 32; - let call_data_cost: U256 = packed + let encoded_op = op.clone().encode(); + let length_in_words = encoded_op.len() / 32; // size of packed user op is always a multiple of 32 bytes + let call_data_cost: U256 = encoded_op .iter() .map(|&x| { if x == 0 { diff --git a/crates/types/src/user_operation.rs b/crates/types/src/user_operation.rs index 8189f58d2..2c310fce6 100644 --- a/crates/types/src/user_operation.rs +++ b/crates/types/src/user_operation.rs @@ -10,6 +10,9 @@ use crate::{ UserOperation, }; +/// Number of bytes in the fixed size portion of an ABI encoded user operation +const PACKED_USER_OPERATION_FIXED_LEN: usize = 480; + /// Unique identifier for a user operation from a given sender #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] pub struct UserOperationId { @@ -24,7 +27,7 @@ impl UserOperation { /// It does not include the signature field. pub fn op_hash(&self, entry_point: Address, chain_id: u64) -> H256 { keccak256(encode(&[ - Token::FixedBytes(keccak256(self.pack()).to_vec()), + Token::FixedBytes(keccak256(self.pack_for_hash()).to_vec()), Token::Address(entry_point), Token::Uint(chain_id.into()), ])) @@ -60,8 +63,26 @@ impl UserOperation { } } - /// Packs the user operation into a byte array, consistent with the entrypoint contract's expectation - pub fn pack(&self) -> Bytes { + /// Efficient calculation of the size of a packed user operation + pub fn abi_encoded_size(&self) -> usize { + PACKED_USER_OPERATION_FIXED_LEN + + pad_len(&self.init_code) + + pad_len(&self.call_data) + + pad_len(&self.paymaster_and_data) + + pad_len(&self.signature) + } + + /// Calculates the size of the user operation in memory + pub fn mem_size(&self) -> usize { + std::mem::size_of::() + + self.init_code.len() + + self.call_data.len() + + self.paymaster_and_data.len() + + self.signature.len() + } + + /// Gets the byte array representation of the user operation to be used in the signature + pub fn pack_for_hash(&self) -> Bytes { let hash_init_code = keccak256(self.init_code.clone()); let hash_call_data = keccak256(self.call_data.clone()); let hash_paymaster_and_data = keccak256(self.paymaster_and_data.clone()); @@ -100,9 +121,19 @@ impl UserOperation { } } +/// Calculates the size a byte array padded to the next largest multiple of 32 +fn pad_len(b: &Bytes) -> usize { + (b.len() + 31) & !31 +} + #[cfg(test)] mod tests { - use ethers::types::{Bytes, U256}; + use std::str::FromStr; + + use ethers::{ + abi::AbiEncode, + types::{Bytes, U256}, + }; use super::*; @@ -229,4 +260,28 @@ mod tests { .unwrap() ); } + + #[test] + fn test_abi_encoded_size() { + let user_operation = UserOperation { + sender: "0xe29a7223a7e040d70b5cd460ef2f4ac6a6ab304d" + .parse() + .unwrap(), + nonce: U256::from_dec_str("3937668929043450082210854285941660524781292117276598730779").unwrap(), + init_code: Bytes::default(), + call_data: Bytes::from_str("0x5194544700000000000000000000000058440a3e78b190e5bd07905a08a60e30bb78cb5b000000000000000000000000000000000000000000000000000009184e72a000000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000").unwrap(), + call_gas_limit: 40_960.into(), + verification_gas_limit: 75_099.into(), + pre_verification_gas: 46_330.into(), + max_fee_per_gas: 105_000_000.into(), + max_priority_fee_per_gas: 105_000_000.into(), + paymaster_and_data: Bytes::from_str("0xc03aac639bb21233e0139381970328db8bceeb6700006508996f000065089a9b0000000000000000000000000000000000000000ca7517be4e51ca2cde69bc44c4c3ce00ff7f501ce4ee1b3c6b2a742f579247292e4f9a672522b15abee8eaaf1e1487b8e3121d61d42ba07a47f5ccc927aa7eb61b").unwrap(), + signature: Bytes::from_str("0x00000000f8a0655423f2dfbb104e0ff906b7b4c64cfc12db0ac5ef0fb1944076650ce92a1a736518e5b6cd46c6ff6ece7041f2dae199fb4c8e7531704fbd629490b712dc1b").unwrap(), + }; + + assert_eq!( + user_operation.clone().encode().len(), + user_operation.abi_encoded_size() + ); + } } From 55e780a4c3e7ed507d546055b2de2eaeb378aee6 Mon Sep 17 00:00:00 2001 From: Alex Miao Date: Wed, 20 Sep 2023 16:01:59 -0700 Subject: [PATCH 2/2] fix(pool): fix memory size calculation --- crates/pool/src/mempool/mod.rs | 5 ++-- crates/pool/src/mempool/pool.rs | 40 ++++++++++++++++++++++-------- crates/types/src/user_operation.rs | 7 +++--- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/crates/pool/src/mempool/mod.rs b/crates/pool/src/mempool/mod.rs index c81cbb14a..89817bb09 100644 --- a/crates/pool/src/mempool/mod.rs +++ b/crates/pool/src/mempool/mod.rs @@ -185,11 +185,10 @@ impl PoolOperation { }) } - /// Compute the amount of memory the PoolOperation takes up. + /// Compute the amount of heap memory the PoolOperation takes up. pub fn mem_size(&self) -> usize { std::mem::size_of::() - + self.uo.mem_size() - + std::mem::size_of::>() + + self.uo.heap_size() + self.entities_needing_stake.len() * std::mem::size_of::() } diff --git a/crates/pool/src/mempool/pool.rs b/crates/pool/src/mempool/pool.rs index 5b61528d4..a2e58a875 100644 --- a/crates/pool/src/mempool/pool.rs +++ b/crates/pool/src/mempool/pool.rs @@ -165,7 +165,7 @@ impl PoolInner { .filter(|(bn, _)| *bn < block_number) { if let Some((op, _)) = self.mined_at_block_number_by_hash.remove(&hash) { - self.cache_size -= op.size(); + self.cache_size -= op.mem_size(); } self.mined_hashes_with_block_numbers.remove(&(bn, hash)); } @@ -268,7 +268,7 @@ impl PoolInner { let hash = pool_op .uo() .op_hash(self.config.entry_point, self.config.chain_id); - self.pool_size += pool_op.size(); + self.pool_size += pool_op.mem_size(); self.by_hash.insert(hash, pool_op.clone()); self.by_id.insert(pool_op.uo().id(), pool_op.clone()); self.best.insert(pool_op); @@ -293,7 +293,7 @@ impl PoolInner { self.by_id.remove(&op.uo().id()); self.best.remove(&op); if let Some(block_number) = block_number { - self.cache_size += op.size(); + self.cache_size += op.mem_size(); self.mined_at_block_number_by_hash .insert(hash, (op.clone(), block_number)); self.mined_hashes_with_block_numbers @@ -303,7 +303,7 @@ impl PoolInner { self.decrement_address_count(e.address); } - self.pool_size -= op.size(); + self.pool_size -= op.mem_size(); Some(op.po) } @@ -361,8 +361,8 @@ impl OrderedPoolOperation { &self.po.uo } - fn size(&self) -> usize { - self.po.mem_size() + fn mem_size(&self) -> usize { + std::mem::size_of::() + self.po.mem_size() } } @@ -664,7 +664,14 @@ mod tests { } assert_eq!(pool.address_count(sender), 1); - assert_eq!(pool.pool_size, po1.mem_size()); + assert_eq!( + pool.pool_size, + OrderedPoolOperation { + po: Arc::new(po1), + submission_id: 0 + } + .mem_size() + ); } #[test] @@ -687,7 +694,14 @@ mod tests { assert_eq!(pool.address_count(sender), 1); assert_eq!(pool.address_count(paymaster1), 0); assert_eq!(pool.address_count(paymaster2), 1); - assert_eq!(pool.pool_size, po2.mem_size()); + assert_eq!( + pool.pool_size, + OrderedPoolOperation { + po: Arc::new(po2), + submission_id: 0 + } + .mem_size() + ); } #[test] @@ -712,12 +726,16 @@ mod tests { chain_id: 1, max_userops_per_sender: 16, min_replacement_fee_increase_percentage: 10, - max_size_of_pool_bytes: 20 * mem_size_of_op(), + max_size_of_pool_bytes: 20 * mem_size_of_ordered_pool_op(), } } - fn mem_size_of_op() -> usize { - create_op(Address::random(), 1, 1).mem_size() + fn mem_size_of_ordered_pool_op() -> usize { + OrderedPoolOperation { + po: Arc::new(create_op(Address::random(), 1, 1)), + submission_id: 1, + } + .mem_size() } fn create_op(sender: Address, nonce: usize, max_fee_per_gas: usize) -> PoolOperation { diff --git a/crates/types/src/user_operation.rs b/crates/types/src/user_operation.rs index 2c310fce6..7580ad1f1 100644 --- a/crates/types/src/user_operation.rs +++ b/crates/types/src/user_operation.rs @@ -72,10 +72,9 @@ impl UserOperation { + pad_len(&self.signature) } - /// Calculates the size of the user operation in memory - pub fn mem_size(&self) -> usize { - std::mem::size_of::() - + self.init_code.len() + /// Compute the amount of heap memory the UserOperation takes up. + pub fn heap_size(&self) -> usize { + self.init_code.len() + self.call_data.len() + self.paymaster_and_data.len() + self.signature.len()