refactor(protocol/protocol): switch to nested structs#3058
Conversation
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Sentry. |
2e78dce to
26cf93e
Compare
emhane
left a comment
There was a problem hiding this comment.
nice, i like this approach. we do this in op-alloy to for payload attributes and sidecars
26cf93e to
8a89bf8
Compare
L1BlockInfo structs 8a89bf8 to
1e89811
Compare
a09174a to
95a4eeb
Compare
theochap
left a comment
There was a problem hiding this comment.
Approving to unblock. Feel free to go ahead and merge if my comment's not relevant
95a4eeb to
79fc6e6
Compare
79fc6e6 to
2eaf035
Compare
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
|
This pull request has been automatically marked as stale because it has been inactive for 3 weeks. |
488276e to
69f81df
Compare
69f81df to
0c3d49e
Compare
The `L1BlockInfo___` structs contains overlapping fields. This branch
factors out the non-deprecated fields e.g. `L1BlockInfoBedrockBase` and
embeds this as a field in `L1BlockInfoBedrock`:
```
pub struct L1BlockInfoBedrock {
#[serde(flatten)]
base: L1BlockInfoBedrockBase,
/// The fee overhead for L1 data. Deprecated in ecotone.
pub l1_fee_overhead: U256,
/// The fee scalar for L1 data. Deprecated in ecotone.
pub l1_fee_scalar: U256,
}
```
The purpose is reuse (think OOP inheritance) instead of repetition. As a
side-effect this increases encapsulation.
It establishes a partial order and a chain of fully embedded structs:
L1BlockInfoBedrockBase < L1BlockInfoEcotoneBase < L1BlockInfoIsthmus <
L1BlockInfoJovian
Further
L1BlockInfoBedrockBase < L1BlockInfoBedrock
and
L1BlockInfoEcotoneBase < L1BlockInfoEcotone
This is deemed necessary to get around deprecated fields in
`L1BlockInfoBedrock` and `L1BlockInfoEcotone`.
To hide the implementation details, constructors have been added and
destructuring is discouraged.
There is no single way to do this in Rust, but this is one way. A
similar way is used
[`op-alloy`](https://github.com/alloy-rs/op-alloy/blob/main/crates/rpc-types/src/transaction.rs).
The
L1BlockInfo___structs contains overlapping fields. This branch factors out the non-deprecated fields e.g.L1BlockInfoBedrockBaseand embeds this as a field inL1BlockInfoBedrock:The purpose is reuse (think OOP inheritance) instead of repetition. As a side-effect this increases encapsulation.
It establishes a partial order and a chain of fully embedded structs:
Further
and
This is deemed necessary to get around deprecated fields in
L1BlockInfoBedrockandL1BlockInfoEcotone.To hide the implementation details, constructors have been added and destructuring is discouraged.
There is no single way to do this in Rust, but this is one way. A similar way is used
op-alloy.