-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
perf(op-receipts): reuse l1_block_info
for multiple receipts
#13781
Conversation
Ok(OpReceiptBuilder::new( | ||
&self.inner.eth_api.provider().chain_spec(), | ||
tx, | ||
meta, | ||
receipt, | ||
&receipts, | ||
l1_block_info.clone(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Main motivation as L1BlockInfo
is 312 bytes and this clone
can scale to 100k+ transactions/block for high-performance chains.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
// We must clear this cache as different L2 transactions can have different | ||
// L1 costs. A potential improvement here is to only clear the cache if the | ||
// new transaction input has changed, since otherwise the L1 cost wouldn't. | ||
l1_block_info.clear_tx_l1_cost(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-.-
I knew that introducing this behaviour would end causing problems somewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a dangerous but manageable one 🙏.
This PR allows reusing
L1BlockInfo
when building OP receipts for multiple transactions (of the same block). Came across this as we built fast pre-confirmations for users 🙏.