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

State export import #348

Merged
merged 25 commits into from
May 5, 2022
Merged

State export import #348

merged 25 commits into from
May 5, 2022

Conversation

alpe
Copy link
Contributor

@alpe alpe commented Apr 26, 2022

Based on the #340 spike and #344

This design comes with a new state_exporter_importer privilege that allow contracts to register for a custom in state export/import callback.

Design choices

No support for:

  • Zero height exports
  • No contract state updates via seed genesis configuration

Reviewer

Genesis changes #351 are included in this branch, now

A good start would be to follow the code in:

Please do Squash + Merge. The single commits do not add value in the history

Follow up tasks

#356
#357

Testing

State export import can be manually tested via:

# rm -rf ~/.tgrade
./contrib/local/setup_tgrade.sh
./contrib/local/start_node.sh
# run it for some blocks, then ctrl-c to stop the server

# export a state dump
tgrade export --height=-1 > dump.json
# check custom data model for valset contract

# import
tgrade unsafe-reset-all
cp -f dump.json ~/.tgrade/config/genesis.json
./contrib/local/start_node.sh

# check it starts and continues from stopped block

@alpe alpe changed the base branch from main to pr-340 April 26, 2022 15:01
@alpe alpe changed the base branch from pr-340 to main April 27, 2022 16:08
@alpe alpe changed the title DRAFT: State export import WIP: State export import Apr 28, 2022
@codecov-commenter
Copy link

codecov-commenter commented Apr 28, 2022

Codecov Report

Merging #348 (426ba69) into main (26116cd) will increase coverage by 0.64%.
The diff coverage is 72.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #348      +/-   ##
==========================================
+ Coverage   56.92%   57.57%   +0.64%     
==========================================
  Files          73       73              
  Lines        5205     5440     +235     
==========================================
+ Hits         2963     3132     +169     
- Misses       1954     1996      +42     
- Partials      288      312      +24     
Impacted Files Coverage Δ
x/poe/contract/tg4_stake.go 73.07% <0.00%> (-2.93%) ⬇️
x/poe/contract/tgrade_distribution.go 53.84% <0.00%> (-9.80%) ⬇️
x/poe/keeper/contracts.go 25.00% <ø> (ø)
x/poe/keeper/keeper.go 70.00% <ø> (ø)
x/poe/types/genesisio.go 0.00% <0.00%> (ø)
x/twasm/module.go 0.00% <0.00%> (ø)
x/twasm/types/contract_extension.go 47.50% <ø> (ø)
x/twasm/types/privilege.go 82.97% <ø> (ø)
x/poe/module.go 19.56% <20.00%> (-0.68%) ⬇️
x/poe/contract/contractio.go 60.00% <61.11%> (-1.12%) ⬇️
... and 19 more

@alpe alpe mentioned this pull request Apr 28, 2022
Copy link
Contributor

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

Nice Work! 🏅

I left a number of comments showing my thinking and areas that stood out to me as well. If you review them / answer them and are content with it, no need to wait another approval from me.

I didn't look at go style, just the business logic.

app/app_test.go Show resolved Hide resolved
app/app_test.go Outdated Show resolved Hide resolved
app/export.go Show resolved Hide resolved
app/export.go Show resolved Hide resolved
app/genesis_integration_test.go Outdated Show resolved Hide resolved
x/twasm/keeper/genesis.go Show resolved Hide resolved
x/twasm/keeper/genesis.go Show resolved Hide resolved
x/twasm/keeper/genesis.go Show resolved Hide resolved
x/twasm/types/codec.go Show resolved Hide resolved
x/twasm/types/genesis.go Show resolved Hide resolved
@alpe alpe force-pushed the 340-state_export_import branch from 5cd1ff5 to a2d530d Compare May 2, 2022 10:21
@alpe alpe changed the title WIP: State export import State export import May 2, 2022
@alpe alpe marked this pull request as ready for review May 2, 2022 11:31
@alpe alpe requested review from maurolacy and pinosu May 4, 2022 06:55
Copy link
Contributor

@maurolacy maurolacy left a comment

Choose a reason for hiding this comment

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

LGTM. Great work.

Didn't review everything in detail, as it's a really long PR, but followed your "review path" suggestion.

docs/proto/proto-docs.md Outdated Show resolved Hide resolved
repeated cosmwasm.wasm.v1.Model models = 1 [ (gogoproto.nullable) = false ];
}

// CustomModel contains the raw json data for a contract to seed it's state on
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// CustomModel contains the raw json data for a contract to seed it's state on
// CustomModel contains the raw json data for a contract to seed its state on

x/poe/keeper/genesis.go Show resolved Hide resolved
x/poe/keeper/genesis.go Show resolved Hide resolved
return nil
}
}
newCursor, err := v.doPageableQuery(ctx, ValsetQuery{ListActiveValidators: &ListValidatorsQuery{StartAfter: cursor.String()}}, &rsp)
Copy link
Contributor

@maurolacy maurolacy May 4, 2022

Choose a reason for hiding this comment

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

This is correct AFAIK. It's a little tricky with the pagination cursor, but this code is similar / equivalent to the one in TestListAllValidatorsViaCursor integration test (it just uses the doPageableQuery wrapper).

The cursor is a just copy of a field of the last element of the page (the operator address).

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 🥇 excellent work!

@alpe alpe merged commit edf38d7 into main May 5, 2022
@alpe alpe deleted the 340-state_export_import branch May 5, 2022 06:42
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.

PoE state export PoE Bootstrap from genesis export
5 participants