Skip to content

refactoring ideas for withdrawals contract #30

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Oct 28, 2024
Merged

refactoring ideas for withdrawals contract #30

merged 6 commits into from
Oct 28, 2024

Conversation

fjl
Copy link
Collaborator

@fjl fjl commented Oct 25, 2024 •
edited by lightclient
Loading

  • moves the read operation for 7002 and 7251 to after the record operation
  • refactor the offset calculation in the system operation to use bump-the-pointer style

@fjl fjl force-pushed the withdrawals-refactor branch 2 times, most recently from f069172 to 6c2aec9 Compare October 25, 2024 21:37
fjl added 3 commits October 27, 2024 14:45
@fjl @lightclient
withdrawals: move read operation
6eac752 
This is to simplify the control flow.
@fjl @lightclient
withdrawals: rename pk[0:32] -> pk1 in stack comments
419c391
@fjl @lightclient
withdrawals: bump-the-pointer style output formatting
2ecad76 
This changes the output section to a different style where the current
output pointer is tracked near top of stack, and gets incremented after
each write to memory.

I'm not sure if it's better this way.
lightclient
lightclient approved these changes Oct 28, 2024
View reviewed changes
Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

SGTM - I made the same refactor for consolidations.

@lightclient lightclient merged commit 1bc4999 into ethereum:main Oct 28, 2024
1 check passed
@fjl fjl mentioned this pull request Oct 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lightclient lightclient lightclient approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fjl @lightclient