Skip to content
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

Add rebuilding of wasmstore as a part of node initialization #2314

Merged
merged 17 commits into from
Jun 11, 2024
Merged

Add rebuilding of wasmstore as a part of node initialization #2314

merged 17 commits into from
Jun 11, 2024

Conversation

ganeshvanahalli
Copy link
Contributor

@ganeshvanahalli ganeshvanahalli commented May 17, 2024 •
edited
Loading

Helper geth PR- OffchainLabs/go-ethereum#326
Resolves NIT-2503

@cla-bot cla-bot bot added the s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA. label May 17, 2024
@ganeshvanahalli ganeshvanahalli requested a review from tsahee May 17, 2024 21:28
@ganeshvanahalli ganeshvanahalli marked this pull request as ready for review May 17, 2024 21:28
tsahee
tsahee reviewed May 21, 2024
View reviewed changes
arbos/programs/programs.go Outdated Show resolved Hide resolved
cmd/nitro/init.go Outdated Show resolved Hide resolved
cmd/nitro/init.go Outdated Show resolved Hide resolved
util/wasmstorerebuilder/wasmstorerebuilder.go Outdated Show resolved Hide resolved
util/wasmstorerebuilder/wasmstorerebuilder.go Outdated Show resolved Hide resolved
util/wasmstorerebuilder/wasmstorerebuilder.go Outdated Show resolved Hide resolved
system_tests/program_test.go Show resolved Hide resolved
tsahee
tsahee reviewed Jun 4, 2024
View reviewed changes
Copy link
Contributor

@tsahee tsahee left a comment

Choose a reason for hiding this comment

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

I should have seen these earlier.
However - we are getting close. Great Job!

cmd/nitro/init.go Show resolved Hide resolved
arbos/programs/programs.go Outdated Show resolved Hide resolved
arbos/programs/programs.go Outdated Show resolved Hide resolved
execution/gethexec/wasmstorerebuilder.go Outdated Show resolved Hide resolved
execution/gethexec/wasmstorerebuilder.go Outdated Show resolved Hide resolved
execution/gethexec/wasmstorerebuilder.go Outdated Show resolved Hide resolved
@ganeshvanahalli ganeshvanahalli requested a review from tsahee June 5, 2024 16:12
tsahee
tsahee reviewed Jun 7, 2024
View reviewed changes
Copy link
Contributor

@tsahee tsahee left a comment

Choose a reason for hiding this comment

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

small things remaining.. but overall very good. Gonna label it and ask a review from one more

cmd/nitro/init.go Outdated Show resolved Hide resolved
execution/gethexec/wasmstorerebuilder.go Outdated Show resolved Hide resolved
magicxyyz
magicxyyz requested changes Jun 8, 2024
View reviewed changes
Copy link
Contributor

@magicxyyz magicxyyz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

great work! I've added comments to state retrieving + some nitpicking in test, otherwise looks good

execution/gethexec/wasmstorerebuilder.go Outdated Show resolved Hide resolved
execution/gethexec/wasmstorerebuilder.go Outdated Show resolved Hide resolved
execution/gethexec/wasmstorerebuilder.go Outdated Show resolved Hide resolved
system_tests/program_test.go Outdated Show resolved Hide resolved
system_tests/program_test.go Outdated Show resolved Hide resolved
system_tests/program_test.go Outdated Show resolved Hide resolved
magicxyyz
magicxyyz previously approved these changes Jun 10, 2024
View reviewed changes
Copy link
Contributor

@magicxyyz magicxyyz left a comment

Choose a reason for hiding this comment

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

LGTM

magicxyyz
magicxyyz approved these changes Jun 11, 2024
View reviewed changes
Copy link
Contributor

@magicxyyz magicxyyz left a comment

Choose a reason for hiding this comment

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

LGTM

@ganeshvanahalli ganeshvanahalli merged commit c5fab47 into master Jun 11, 2024
11 checks passed
@ganeshvanahalli ganeshvanahalli deleted the wasmstore-rebuilder branch June 11, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsahee tsahee tsahee left review comments

@magicxyyz magicxyyz magicxyyz approved these changes

@anodar anodar Awaiting requested review from anodar

@amsanghi amsanghi Awaiting requested review from amsanghi

Assignees
No one assigned
Labels
design-approved s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ganeshvanahalli @magicxyyz @tsahee