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

Full history of contract code migrations #195

Merged
merged 4 commits into from
Jul 20, 2020
Merged

Full history of contract code migrations #195

merged 4 commits into from
Jul 20, 2020

Conversation

alpe
Copy link
Member

@alpe alpe commented Jul 16, 2020

Resolves #130

A contract history is stored within ContractInfo for:

  • Init
  • Migrate
  • Genesis import

Note: The genesis import/ export was more complex than storing some history data within the entity.

Moving the history out of the ContractInfo makes more sense to me to optimize the workflow and save gas. I will add a ticket to have this as a second step.


For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@alpe alpe requested a review from ethanfrey as a code owner July 16, 2020 15:07
Copy link
Member

@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.

Looks good.

I also agree to store the code history under a different key.
And then have a separate query endpoint that just returns the history.

I don't think we have to split the history into multiple keys, as it is only used on init and migrate, which are relatively heavy/privledged ops, and not loaded on execute (which should remain cheap)

"source": "https://example.com",
"builder": "foo/bar:tag",
"instantiate_config": {
"type": 1,
Copy link
Member

Choose a reason for hiding this comment

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

I thought you updated this to "Nobody" / "Everybody" instead of numbers? Or is that PR not merged?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, this is an old configuration I had before. Good 👁️ . It also shows that the field is not validated proper I will fix this

x/wasm/internal/keeper/querier.go Show resolved Hide resolved
@CosmWasm CosmWasm deleted a comment from codecov bot Jul 17, 2020
@CosmWasm CosmWasm deleted a comment from codecov bot Jul 17, 2020
@alpe
Copy link
Member Author

alpe commented Jul 17, 2020

Pushed some additional tests. They are not really related to this feature but red areas on the critical path.... should make codecov happy 😆

@alpe alpe mentioned this pull request Jul 17, 2020
3 tasks
@codecov
Copy link

codecov bot commented Jul 17, 2020

Codecov Report

Merging #195 into master will increase coverage by 0.20%.
The diff coverage is 60.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #195      +/-   ##
==========================================
+ Coverage   71.71%   71.91%   +0.20%     
==========================================
  Files          27       27              
  Lines        2584     2610      +26     
==========================================
+ Hits         1853     1877      +24     
- Misses        621      623       +2     
  Partials      110      110              
Impacted Files Coverage Δ
x/wasm/internal/types/types.go 22.88% <7.40%> (-9.23%) ⬇️
x/wasm/internal/keeper/querier.go 38.84% <85.71%> (+2.70%) ⬆️
x/wasm/internal/keeper/test_fuzz.go 95.12% <85.71%> (-4.88%) ⬇️
x/wasm/internal/keeper/genesis.go 93.44% <100.00%> (+0.22%) ⬆️
x/wasm/internal/keeper/keeper.go 93.62% <100.00%> (+0.01%) ⬆️
x/wasm/internal/types/genesis.go 95.00% <100.00%> (+0.55%) ⬆️
x/wasm/internal/types/msg.go 50.00% <100.00%> (+7.54%) ⬆️
x/wasm/internal/types/params.go 57.69% <100.00%> (+1.28%) ⬆️
x/wasm/internal/types/test_fixtures.go 93.90% <100.00%> (+2.38%) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 774f6d7...a9f31c7. Read the comment docs.

Copy link
Member

@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.

Looks good to merge with the extra tests

x/wasm/internal/types/params.go Show resolved Hide resolved
@ethanfrey ethanfrey merged commit 65e4e3a into master Jul 20, 2020
@ethanfrey ethanfrey deleted the update_history branch July 20, 2020 12:29
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.

Full history of contract code migrations
2 participants