-
Notifications
You must be signed in to change notification settings - Fork 35
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.
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
Unable to fund state sync relayer when using rootchain originated native token #411
Merged
ZeroEkkusu
merged 14 commits into
0xPolygon:dev
from
Stefan-Ethernal:EVM-865-unable-to-fund-relayer-when-using-rootchain-originated-native-token
Oct 31, 2023
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
366c197
Premine function implementation
Stefan-Ethernal bd06c5b
Fix slither
Stefan-Ethernal 6baee8d
Unit tests
Stefan-Ethernal 579029f
Use correct forge-std revision
Stefan-Ethernal 96f761f
Address comments (part 1)
Stefan-Ethernal ffc6b22
Remove blank line
Stefan-Ethernal 5f491bf
Pass tests again
Stefan-Ethernal 9f63d58
Introduce genesisBalances field for storing premine balances
Stefan-Ethernal 308282e
Rename premine to addGenesisBalance
Stefan-Ethernal 49f8f02
Remove nativeTokenRoot function and make field public
Stefan-Ethernal 32e63d4
Track genesis balances in the CustomSupernetManager
Stefan-Ethernal 2d1dcc0
Rename event
Stefan-Ethernal 485e3c6
Remove getGenesisBalance function
Stefan-Ethernal 206a2e5
Minor fix
Stefan-Ethernal File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ZeroEkkusu Is this change safe to be made? The expectation for this PR is that the chain needs to be started from the genesis in order to be able to use the premine functionality however, I'm wondering what would happen for the already running chains when they apply this change and is it ok to rename it or revert it just in case to be backward compatible. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only in the sense that if someone's using the struct in their code, they'd need to update all
.validator
occurrences to.addr
, but it doesn't change anything on the bytecode level.The actual issue is that appending the
uint256 balance
shifts the storage layout. Maybe we could create amapping
somewhere instead and record it there.See my following comment as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT about such change? I've introduced an array which stores premine balances and exposed a function that retrieves it (because we are going to need it on the client-side, it is easier that way than having mapping and fetching balances by
address
key, one by one):9f63d58
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re your comment/post at the end:
Do you mean that a restart will be required anyway? If so, different storage layouts don't matter because those would be fresh deployments with clean state.
If not:
This also shifts the storage layout of
CustomSupernetManager
(because there'sGenesisSet private _genesis
in it).We can have the array defined and updated in the
CustomSupernetManager
directly (after all other storage variables and 1 subtracted from the gap), if that works for you.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be required in case one uses rootchain originated token as a native token. Otherwise we don't expect chain reset (redeployment is not expected in that case).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case we'll need to align the storage layouts. Does the solution I proposed work for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that should work. I'll change it that way most likely. 👍