-
Couldn't load subscription status.
- Fork 52
Add base_meterBundle RPC for transaction bundle metering #142
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
base: main
Are you sure you want to change the base?
Conversation
Implements JSON-RPC endpoint to simulate and meter Optimism transaction bundles, returning detailed gas usage, execution time, and bundle hash using Flashbots methodology. Includes reusable meter_bundle() function for non-RPC contexts.
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.
Looks good to me, left some comments on things that stood out but aren't blockers to merging.
Can we add a sanity check e2e test to this? Happy to do it in a follow up PR
Add comprehensive tests following flashblocks-rpc pattern with funded test accounts from genesis.json. Tests cover bundle metering, gas calculations, state resolution, and error handling.
- Add tips-core dependency (default-features=false to avoid version conflicts) - Replace MeterBundleRequest with tips-core Bundle - Use BundleWithMetadata.bundle_hash() instead of manual calculation - Use bundle.block_number and bundle.min_timestamp for simulation - Update docs to clarify TIPS Bundle format - Add workaround for op-alloy 0.20/0.21 version mismatch (temporary until next reth release)
| let header = self | ||
| .provider | ||
| .sealed_header(bundle.block_number) |
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.
The biggest wart from moving to tips_core::Bundle is that specific block numbers now need to be provided in the request. The ability to specify "pending" or "latest" has been lost.
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.
@niran i think we could do BlockNumberOrTag::Latest.as_number()?
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.
Yep, that's what the code was doing before. But since the Bundle type has its own block number field, we can either just use Bundle's field, refactor the core types to make it easier to provide the block reference in the right way, or just avoid the core types altogether.
The types refactoring approach seems like the best option to me, but it's a little messy if we want to be able to maintain the main benefit that changes to bundle features are reflected in every crate that depends on tips-core. I think it'd be something like this in tips-core, which implements conversions between types to ensure they stay compatible. (Unfortunately, for other crates to inherit this, they'll have to remember to add similar conversion functions. Otherwise conversions will silently break.)
// Core bundle data without timing/targeting
pub struct BundleCore {
pub txs: Vec<Bytes>,
pub reverting_tx_hashes: Vec<TxHash>,
pub replacement_uuid: Option<String>,
pub dropping_tx_hashes: Vec<TxHash>,
}
// Timing/targeting metadata separate from core
pub struct TimingMetadata {
pub block_number: u64,
pub flashblock_number_min: Option<u64>,
pub flashblock_number_max: Option<u64>,
pub min_timestamp: Option<u64>,
pub max_timestamp: Option<u64>,
}
// Original Bundle type (unchanged for backward compatibility)
pub struct Bundle {
pub txs: Vec<Bytes>,
pub block_number: u64,
pub flashblock_number_min: Option<u64>,
pub flashblock_number_max: Option<u64>,
pub min_timestamp: Option<u64>,
pub max_timestamp: Option<u64>,
pub reverting_tx_hashes: Vec<TxHash>,
pub replacement_uuid: Option<String>,
pub dropping_tx_hashes: Vec<TxHash>,
}
// Decompose Bundle into parts
impl Bundle {
pub fn into_parts(self) -> (BundleCore, TimingMetadata) {
let core = BundleCore {
txs: self.txs,
reverting_tx_hashes: self.reverting_tx_hashes,
replacement_uuid: self.replacement_uuid,
dropping_tx_hashes: self.dropping_tx_hashes,
};
let timing = TimingMetadata {
block_number: self.block_number,
flashblock_number_min: self.flashblock_number_min,
flashblock_number_max: self.flashblock_number_max,
min_timestamp: self.min_timestamp,
max_timestamp: self.max_timestamp,
};
(core, timing)
}
pub fn from_parts(core: BundleCore, timing: TimingMetadata) -> Self {
Bundle {
txs: core.txs,
block_number: timing.block_number,
flashblock_number_min: timing.flashblock_number_min,
flashblock_number_max: timing.flashblock_number_max,
min_timestamp: timing.min_timestamp,
max_timestamp: timing.max_timestamp,
reverting_tx_hashes: core.reverting_tx_hashes,
replacement_uuid: core.replacement_uuid,
dropping_tx_hashes: core.dropping_tx_hashes,
}
}
pub fn core(&self) -> &BundleCore {
// For zero-copy access
}
}
// Standard conversions
impl From<Bundle> for BundleCore {
fn from(bundle: Bundle) -> Self {
bundle.into_parts().0
}
}
impl From<&Bundle> for BundleCore {
fn from(bundle: &Bundle) -> Self {
BundleCore {
txs: bundle.txs.clone(),
reverting_tx_hashes: bundle.reverting_tx_hashes.clone(),
replacement_uuid: bundle.replacement_uuid.clone(),
dropping_tx_hashes: bundle.dropping_tx_hashes.clone(),
}
}
}
Then the MeterBundleRequest would have its own handling for BlockNumberOrTag::Latest.as_number() with a method that extracts a TimingMetadata to maintain compatibility over time.
Whatever we decide here, I think we should do it as its own feature later.
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.
The biggest wart from moving to tips_core::Bundle is that specific block numbers now need to be provided in the request. The ability to specify "pending" or "latest" has been lost.
Any reason to not always sim against latest (or ideally pending/flashblock state)?
|
Once this is reviewed, I'm going to EDIT: turns out the merge base is editable! It's now based on main. |
Update tips-core to rev 27674ae which includes the metering response types. Remove local type definitions and use the types from tips-core instead.
Implements JSON-RPC endpoint to simulate and meter Optimism transaction bundles, returning detailed gas usage, execution time, and bundle hash using Flashbots methodology.
Includes reusable meter_bundle() function for non-RPC contexts.