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.

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

Fix genesis import with predictable addresses #1630

Merged
merged 1 commit into from
Sep 18, 2023
Merged

Fix genesis import with predictable addresses #1630

merged 1 commit into from
Sep 18, 2023

Conversation

alpe
Copy link
Contributor

@alpe alpe commented Sep 18, 2023

Resolves #1629

@alpe alpe requested a review from pinosu September 18, 2023 04:26
@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Merging #1630 (7dc2d0c) into main (7dba5c7) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1630   +/-   ##
=======================================
  Coverage   56.66%   56.66%           
=======================================
  Files          64       64           
  Lines        8917     8917           
=======================================
  Hits         5053     5053           
  Misses       3475     3475           
  Partials      389      389           
Files Changed Coverage Δ
x/wasm/keeper/genesis.go 88.40% <100.00%> (ø)

Copy link
Contributor

@pinosu pinosu left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@alpe alpe merged commit c12e85e into main Sep 18, 2023
8 checks passed
@alpe alpe deleted the 1629_genesis branch September 18, 2023 08:24
@czarcas7ic
Copy link

This fix doesn't confirm that contracts with predictable addresses were properly exported though right?

@alpe
Copy link
Contributor Author

alpe commented Sep 20, 2023

This fix doesn't confirm that contracts with predictable addresses were properly exported though right?

All contracts are stored in the same model and exported. This fix ensures that predictable addresses do not cause issues on genesis import. The contract id sequence had a constraint for sanity that ensured contractID_Seq > count(contract). With any predictable address though the count(contract) is always > contractID_Seq. Therefore the fix is either
a) bump contractID_Seq for every contract instantiation
b) export count(contract) +1 as contractID_Seq
c) replace the import constraint with something better

My motivation for c)
a) comes with some extra storage IO and gas costs
b) creates a different state on export/import in the store and will cause tests to fail
c) the constraint this is only a safety net to fail fast on inconsistent manual editions to the genesis. We achieve the same by checking that the next contract address is available and not used

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.

Ensure genesis import works with both address generators
3 participants