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: cosmwasmpool state export #6666

Merged
merged 7 commits into from
Oct 12, 2023
Merged

fix: cosmwasmpool state export #6666

merged 7 commits into from
Oct 12, 2023

Conversation

czarcas7ic
Copy link
Member

@czarcas7ic czarcas7ic commented Oct 11, 2023

Closes: #XXX

What is the purpose of the change

When serializing into the any codec type, we were previously utilizing GetPools, which utilized the Pool interface in the cosmwasmpool module. This interface includes a wasmkeeper field which cannot be serialized.

For purposes of import/export logic, we need a separate method that allows us to serialize the pools without this field.

This PR adds a GetPoolsSerializable method that serializes into the CosmwasmPool interface instead of the Pool interface. This gives us all the information we need for importing and exporting.

Testing and Verifying

Ran a state export on the cosmwasmpool module only, here is an excerpt, implying it works:

{
  "app_hash": "",
  "app_state": {
    "cosmwasmpool": {
      "params": {
        "code_id_whitelist": [
          "148"
        ],
        "pool_migration_limit": "20"
      },
      "pools": [
        {
          "@type": "/osmosis.cosmwasmpool.v1beta1.CosmWasmPool",
          "code_id": "148",
          "contract_address": "osmo15ns40n0pctl80d7l7praluufcywderupvgcl8xjg4gzvvhgv4vqq78vk7n",
          "instantiate_msg": "eyJwb29sX2Fzc2V0X2Rlbm9tcyI6WyJ7cG9vbF9hc3NldF9kZW5vbXM6W2liYy84MjQyQUQyNDAwODAzMkU0NTdEMkUxMkQ0NjU4OEZEMzlGQjU0RkIyOTY4MEM2Q>
          "pool_id": "1175"
        },
        {
          "@type": "/osmosis.cosmwasmpool.v1beta1.CosmWasmPool",
          "code_id": "148",
          "contract_address": "osmo136f4pv283yywv3t56d5zdkhq43uucw462rt3qfpm2s84vvr7rrasn3kllg",
          "instantiate_msg": "eyJwb29sX2Fzc2V0X2Rlbm9tcyI6WyJ7XCJwb29sX2Fzc2V0X2Rlbm9tc1wiOiBbXCJpYmMvRDE4OTMzNUM2RTRBNjhCNTEzQzEwQUIyMjdCRjFDMUQzOEM3NDY3N>
          "pool_id": "1176"
        },
        ...

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes?
  • Changelog entry added to Unreleased section of CHANGELOG.md?

Where is the change documented?

  • Specification (x/{module}/README.md)
  • Osmosis documentation site
  • Code comments?
  • N/A

@czarcas7ic czarcas7ic added the V:state/compatible/backport State machine compatible PR, should be backported label Oct 11, 2023
@czarcas7ic
Copy link
Member Author

@devbot-wizard help

@czarcas7ic czarcas7ic added the A:backport/v19.x backport patches to v19.x branch label Oct 11, 2023
Comment on lines 46 to 48
func (p CosmWasmPool) GetId() uint64 {
panic("CosmWasmPool.GetId not implemented")
return p.PoolId
}
Copy link
Member Author

@czarcas7ic czarcas7ic Oct 11, 2023

Choose a reason for hiding this comment

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

Highlighting that we had to implement this method, otherwise when we call SetPool in InitGenesis with a CosmWasmPool instead of a Pool, we need access to the GetId method. It should be fine to implement this though, since its defined on the CosmWasmPool struct and there is no need for the wasmkeeper to retrieve this.

@czarcas7ic czarcas7ic marked this pull request as ready for review October 12, 2023 00:05
@czarcas7ic czarcas7ic added the A:backport/v20.x backport patches to v20.x branch label Oct 12, 2023
Copy link
Member

@mattverse mattverse left a comment

Choose a reason for hiding this comment

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

Nice, Can we add a test case for this? (specifically calling export genesis with cosmwasm pools)?

@czarcas7ic
Copy link
Member Author

@mattverse added a test here dea8e71

reverted the change and confirmed test fails without this change

@czarcas7ic czarcas7ic merged commit d4582d4 into main Oct 12, 2023
1 check passed
@czarcas7ic czarcas7ic deleted the adam/fix-cwpool-export-gen branch October 12, 2023 17:11
mergify bot pushed a commit that referenced this pull request Oct 12, 2023
* use CosmWasmExtension

* add changelog

* just implement getId for deserialization

* separate method for serializable pools

* fix accident

* add test

(cherry picked from commit d4582d4)

# Conflicts:
#	CHANGELOG.md
#	x/cosmwasmpool/genesis.go
#	x/cosmwasmpool/genesis_test.go
mergify bot pushed a commit that referenced this pull request Oct 12, 2023
* use CosmWasmExtension

* add changelog

* just implement getId for deserialization

* separate method for serializable pools

* fix accident

* add test

(cherry picked from commit d4582d4)

# Conflicts:
#	CHANGELOG.md
czarcas7ic added a commit that referenced this pull request Oct 12, 2023
* fix: cosmwasmpool state export (#6666)

* use CosmWasmExtension

* add changelog

* just implement getId for deserialization

* separate method for serializable pools

* fix accident

* add test

(cherry picked from commit d4582d4)

# Conflicts:
#	CHANGELOG.md
#	x/cosmwasmpool/genesis.go
#	x/cosmwasmpool/genesis_test.go

* fix merge conflicts

---------

Co-authored-by: Adam Tucker <adam@osmosis.team>
Co-authored-by: Adam Tucker <adamleetucker@outlook.com>
czarcas7ic added a commit that referenced this pull request Oct 12, 2023
* fix: cosmwasmpool state export (#6666)

* use CosmWasmExtension

* add changelog

* just implement getId for deserialization

* separate method for serializable pools

* fix accident

* add test

(cherry picked from commit d4582d4)

# Conflicts:
#	CHANGELOG.md

* fix merge conflict

---------

Co-authored-by: Adam Tucker <adam@osmosis.team>
Co-authored-by: Adam Tucker <adamleetucker@outlook.com>
pysel pushed a commit that referenced this pull request Oct 17, 2023
* use CosmWasmExtension

* add changelog

* just implement getId for deserialization

* separate method for serializable pools

* fix accident

* add test
@github-actions github-actions bot mentioned this pull request Feb 1, 2024
@github-actions github-actions bot mentioned this pull request Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v19.x backport patches to v19.x branch A:backport/v20.x backport patches to v20.x branch V:state/compatible/backport State machine compatible PR, should be backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants