Skip to content

Conversation

roberto-bayardo
Copy link
Collaborator

@roberto-bayardo roberto-bayardo commented Oct 12, 2022

Transaction (aka "Message") data is confusingly & unnecessarily referenced directly by the StateTransition struct even though it already has a pointer to the source of that data (st.msg). This was being done inconsistently : some attributes were copied over others weren't. This PR removes all such direct refereences so that tx data is accessed consistently.

(I found this aspect of this implementation confusing when adding the MaxFeePerDataGas() field)

Note: This is a generic refactoring sort of PR, which causes us to diverge a bit more significantly from upstream. But since we're making pretty heavy changes to this file already, I'm thinking it won't add signficant burden when it's time to merge.

@roberto-bayardo roberto-bayardo marked this pull request as ready for review October 12, 2022 19:47
@roberto-bayardo roberto-bayardo requested a review from Inphi October 12, 2022 19:48
@roberto-bayardo roberto-bayardo force-pushed the roberto/state-transition-cleanup branch from 6b518ea to 867c4d3 Compare October 12, 2022 19:52
@roberto-bayardo
Copy link
Collaborator Author

Hmm, perhaps I should be (also) submitting this to the upstream repo.

@roberto-bayardo
Copy link
Collaborator Author

I've got another cleanup PR I've submitted to the main repo here: ethereum#25977

Copy link
Collaborator

@Inphi Inphi left a comment

Choose a reason for hiding this comment

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

LGTM!

@roberto-bayardo roberto-bayardo merged commit 91322eb into eip-4844 Oct 13, 2022
@roberto-bayardo roberto-bayardo deleted the roberto/state-transition-cleanup branch October 13, 2022 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants