Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(oppool): use efficient pack length #371

Merged
merged 2 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions crates/pool/src/mempool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,11 @@ impl PoolOperation {
})
}

/// Returns the size of the operation in bytes.
pub fn size(&self) -> usize {
self.uo.pack().len()
/// Compute the amount of heap memory the PoolOperation takes up.
pub fn mem_size(&self) -> usize {
std::mem::size_of::<Self>()
+ self.uo.heap_size()
+ self.entities_needing_stake.len() * std::mem::size_of::<EntityType>()
}

fn entity_address(&self, entity: EntityType) -> Option<Address> {
Expand Down
40 changes: 29 additions & 11 deletions crates/pool/src/mempool/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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)
}

Expand Down Expand Up @@ -361,8 +361,8 @@ impl OrderedPoolOperation {
&self.po.uo
}

fn size(&self) -> usize {
self.po.size()
fn mem_size(&self) -> usize {
std::mem::size_of::<OrderedPoolOperation>() + self.po.mem_size()
}
}

Expand Down Expand Up @@ -664,7 +664,14 @@ mod tests {
}

assert_eq!(pool.address_count(sender), 1);
assert_eq!(pool.pool_size, po1.size());
assert_eq!(
pool.pool_size,
OrderedPoolOperation {
po: Arc::new(po1),
submission_id: 0
}
.mem_size()
);
}

#[test]
Expand All @@ -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.size());
assert_eq!(
pool.pool_size,
OrderedPoolOperation {
po: Arc::new(po2),
submission_id: 0
}
.mem_size()
);
}

#[test]
Expand All @@ -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 * size_of_op(),
max_size_of_pool_bytes: 20 * mem_size_of_ordered_pool_op(),
}
}

fn size_of_op() -> usize {
create_op(Address::random(), 1, 1).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 {
Expand Down
20 changes: 10 additions & 10 deletions crates/sim/src/estimation/estimation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand Down
7 changes: 4 additions & 3 deletions crates/sim/src/gas/gas.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::sync::Arc;

use ethers::{
abi::AbiEncode,
prelude::gas_oracle::GasCategory,
types::{Address, Chain, U256},
};
Expand Down Expand Up @@ -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 {
Expand Down
62 changes: 58 additions & 4 deletions crates/types/src/user_operation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()),
]))
Expand Down Expand Up @@ -60,8 +63,25 @@ 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 {
dancoombs marked this conversation as resolved.
Show resolved Hide resolved
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)
}

/// 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()
}

/// 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());
Expand Down Expand Up @@ -100,9 +120,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::*;

Expand Down Expand Up @@ -229,4 +259,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()
);
}
}
Loading