Skip to content

fix: add docs and serde attrs to ExecutionPayload v3 fields#4302

Merged
mattsse merged 2 commits intomainfrom
dan/skip-none-blob-gas
Aug 22, 2023
Merged

fix: add docs and serde attrs to ExecutionPayload v3 fields#4302
mattsse merged 2 commits intomainfrom
dan/skip-none-blob-gas

Conversation

@Rjected
Copy link
Member

@Rjected Rjected commented Aug 21, 2023

Adds #[serde(default, skip_serializing_if = "Option::is_none")] to the ExecutionPayload V3 fields, similar to the withdrawals field

@Rjected Rjected added C-docs An addition or correction to our documentation M-eip This change relates to the implementation of an EIP E-cancun Related to the Cancun network upgrade labels Aug 21, 2023
@Rjected Rjected requested a review from mattsse as a code owner August 21, 2023 21:39
@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Merging #4302 (24d2e75) into main (34b68de) will decrease coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

Files Changed Coverage Δ
crates/rpc/rpc-types/src/eth/engine/payload.rs 85.29% <100.00%> (ø)

... and 18 files with indirect coverage changes

Flag Coverage Δ
integration-tests 16.83% <0.00%> (+<0.01%) ⬆️
unit-tests 63.71% <100.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
reth binary 26.12% <ø> (ø)
blockchain tree 81.82% <ø> (-0.75%) ⬇️
pipeline 90.07% <ø> (ø)
storage (db) 74.71% <ø> (-0.06%) ⬇️
trie 94.85% <ø> (+0.03%) ⬆️
txpool 48.22% <ø> (ø)
networking 77.51% <ø> (-0.04%) ⬇️
rpc 58.83% <100.00%> (+0.01%) ⬆️
consensus 63.09% <ø> (-0.44%) ⬇️
revm 32.01% <ø> (ø)
payload builder 6.82% <ø> (ø)
primitives 86.06% <ø> (-0.09%) ⬇️

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, validation is separate

@mattsse
Copy link
Collaborator

mattsse commented Aug 21, 2023

failing existing test, that needs to be updated,

perhaps also a good idea to serialize if None?

@Rjected
Copy link
Member Author

Rjected commented Aug 21, 2023

perhaps also a good idea to serialize if None?

hmm, I'm inclined to change the test instead and keep the current serde flags, since we already have a convention of not serializing fields from later forks (for example withdrawals)

@Rjected Rjected force-pushed the dan/skip-none-blob-gas branch from b7ac438 to 24d2e75 Compare August 21, 2023 22:47
@mattsse mattsse added this pull request to the merge queue Aug 22, 2023
Merged via the queue into main with commit 8d25aa3 Aug 22, 2023
@mattsse mattsse deleted the dan/skip-none-blob-gas branch August 22, 2023 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-docs An addition or correction to our documentation E-cancun Related to the Cancun network upgrade M-eip This change relates to the implementation of an EIP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants