-
Notifications
You must be signed in to change notification settings - Fork 262
add nonce to state upgrades #1560
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
base: master
Are you sure you want to change the base?
Conversation
baa8c80
to
b6b48be
Compare
State upgrades are only meant to be used by existing Avalanche L1 networks in specific situations to make required network upgrades. For creating a new L1, the initial state (including balance, nonce, and code) of any account can be specified in the genesis configuration as part of the Changing the nonce of an existing account via state upgrade is generally very dangerous because, as you mentioned, it affects the replay protection for transactions sent by the account. It could also have many unintended consequences if that account were to send transactions on the chain prior to the state upgrade taking effect, etc. |
That's right, the genesis does cover already setting up a nonce from the beginning. But unfortunately there seems to be a size restriction for setting up the genesis with L1 which is the transaction size of the P-Chain on the mainnet. Thus a genesis may not exceed 64k on an L1. With that in mind the solution of using the genesis file will only work for fresh networks or small-scale migrations, but when it comes to bigger migrations it doesn't cover the needs. Let's say for instance one wants to migrate 10k EOAs with individual balances and nonce values. The addresses alone with 20bytes each would accumulate to 200k worth of data well exceeding the genesis limits.
I also fully agree with this. StateUpgrades are a dangerous tool if not used with the sufficient precaution, which also goes for the already existing functionality. Adding the possibility to alter the nonce is of a similar level of "dangerous territory" if not used with the same level of awareness. In the case of a migration it could simply be used as a tool to be executed right after genesis to do a full migration of an existing network including not only smart contracts and accounts but also the nonce values. |
Thank you for opening this PR, I think this can be good tool for chain migrations. However I also agree with @michaelkaplan13 that nonce modification can be very dangerous (i.e in case a tx is issued at the same time etc). I think the best way forward is to see what else might be needed for a full chain migration, test it and then see how we can make it a robust tool from it. As you mentioned, I think that tool should ensure the chain is at genesis (block.height == 0) or we should at least ensure modified account nonce's are at 0. Could you let us know when you successfully test your migration, probably based on this branch or any other branch that has all necessary changes? Then we can re-evaluate what's needed and how we can make it a reliable and generic tool for all possible chains. |
Thank you for the feedback. We understand the concerns regarding nonce modification and its potential risks. We're currently investigating our options and plan to create a proof of concept using a local network for testing. During this, we'll address the recommendation to ensure the chain is at genesis or that modified account nonces are at 0. We'll update this PR once we have more information and test results to share. |
b6b48be
to
786e48d
Compare
Why this should be merged
When migrating an existing blockchain to the Avalanche ecosystem and re-using the existing chain-id, the nonce is needed in order to preserve the correct nonce on the migration-target network. This is needed to ensure a complete migration and also preventing replay-attacks or hash-collisions.
How this works
Adds nonce as an option to state upgrades
How this was tested
Unit tests
Need to be documented?
Yes, this page needs to updated. I can provide a PR for that update also.
Need to update RELEASES.md?
Please advice