-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: Add version to BeaconEngineMessage
FCU
#12089
base: main
Are you sure you want to change the base?
Conversation
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.
this is great tysm
but I'd like to introduce all payload id changes separately
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
use crate::OpPayloadAttributes; | ||
use alloy_primitives::{address, b256, bytes, FixedBytes}; | ||
use alloy_rpc_types_engine::PayloadAttributes; | ||
use std::str::FromStr; | ||
|
||
#[test] | ||
fn test_payload_id_parity_op_geth() { |
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.
these are great though, so please open this as a separate one.
/// Version 1 | ||
V1, | ||
V1 = 1, | ||
/// Version 2 | ||
/// | ||
/// Added in the Shanghai hardfork. | ||
V2, | ||
V2 = 2, | ||
/// Version 3 | ||
/// | ||
/// Added in the Cancun hardfork. | ||
V3, | ||
#[default] | ||
V3 = 3, | ||
/// Version 4 | ||
/// | ||
/// Added in the Prague hardfork. | ||
V4, | ||
V4 = 4, |
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 we can solve this like this
Sounds good, @mattsse Just reverted the changes related to the payload id |
BeaconEngineMessage
FCU
This PR addresses #12071.
Trying to get this over the finish line. Debating whether
Option<EngineApiMessageVersion>
would be more appropriate as there are some places where we have no access to theEngineApiMessageVersion
without changing other interfaces, and the version is only relevant when starting a build process on Optimism.I have defaulted to
EngineApiMessageVersion::V3
in these places, but happy to change. Open to any feedback on the approach