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

feat/spike: CosmWasm Pool Type and Module #4675

Closed
wants to merge 20 commits into from
Closed

Conversation

p0mvn
Copy link
Member

@p0mvn p0mvn commented Mar 20, 2023

Closes: #4389

What is the purpose of the change

The CosmWasm Pool Module is an extension for the Osmosis pools, aiming to create a custom module that allows users to create and manage liquidity pools backed by CosmWasm smart contracts. The feature enables developers to build and deploy custom smart contracts that can be integrated with the rest of the pool types on the Osmosis chain.

The module is built on top of the CosmWasm smart contracting platform, which provides a secure and efficient way to develop and execute WebAssembly (Wasm) smart contracts on the Cosmos SDK.

Having pools in CosmWasm provides several benefits, one of which is avoiding the need for chain upgrades when introducing new functionalities or modifying existing ones related to liquidity pools. This advantage is particularly important in the context of speed of development and iteration.

An example of a CosmWasm pool type: https://github.com/osmosis-labs/transmuter

Brief Changelog

  • created new SDK module - cosmwasmpool
  • wired it up to the app
  • made cosmwasmpool implement poolmanagertypes.PoolModule
  • created new Pool - x/cosmwasm/model.Pool of type poolmanagertypes.CosmWasm
  • made new pool extend poolmanagertypes.PoolI
  • implemented MsgCreateCosmWasmPool
  • implemented cosmwasmpool.Keeper.InitializePool
  • implemented pool interface extension CosmWasmExtension
  • implemented helpers for sending query and sudo message to a cosmwasm contract
  • implemented all PoolI and CosmWasm extension methods
  • implemented all poolmanagertypes.PoolModule
  • test
    • pool.GetSwapFee()
    • pool.SpotPrice()
    • cosmwasmpoolkeeper.InitializePool()
      • full coverage with mocks
      • test with real transmuter pool
    • cosmwasmpoolkeeper.CalcOutAmtGivenIn()
    • cosmwasmpoolkeeper.SwapOutAmtGivenIn()
      • ran into bug where contract balance is not updated upon joining the pool
  • document state entries
  • document messages
  • document queries

Testing and Verifying

TBD after getting concept-ACK from @iboss-ptk

Open Quesitons

  • What to do with the creatorAddress in InitializePool?

  • How to resolve codec issues around PoolI

Currently, getting the following:

go test -mod=readonly -tags='ledger test_ledger_mock norace' github.com/osmosis-labs/osmosis/v15/x/mint/client/cli
--- FAIL: TestIntegrationTestSuite (5.07s)
    cli_test.go:29: setting up integration test suite
    network.go:173: acquiring test network lock
    network.go:178: created temporary directory: /tmp/TestIntegrationTestSuite1806756216/001/osmosis-code-test361649419
    network.go:187: preparing test network...
    network.go:381: starting test network...
    network.go:383: 
                Error Trace:    /root/go/pkg/mod/github.com/osmosis-labs/[email protected]/testutil/network/network.go:383
                                                        /root/osmosis/x/mint/client/cli/cli_test.go:33
                                                        /root/go/pkg/mod/github.com/stretchr/[email protected]/suite/suite.go:154
                                                        /root/osmosis/x/mint/client/cli/cli_test.go:124
                Error:          Received unexpected error:
                                unable to create codec descriptor: unable to get proto name for implementing type / for interface osmosis.poolmanager.v1beta1.PoolI
                Test:           TestIntegrationTestSuite
FAIL
FAIL    github.com/osmosis-labs/osmosis/v15/x/mint/client/cli   5.137s
FAIL

The reason is because the struct implementing PoolI is a Go Struct. This is done so that Pool struct can have a field for WasmKeeper. Otherwise, the pool struct is unable to interact with the contract for querying spot price and other pool operations.

UPDATE: #4675 (comment)

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? yes
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? TBD
  • How is the feature or change documented? yes

@p0mvn p0mvn requested a review from iboss-ptk March 20, 2023 02:48
@github-actions github-actions bot added C:app-wiring Changes to the app folder C:CLI C:docs Improvements or additions to documentation C:x/gamm Changes, features and bugs related to the gamm module. C:x/poolmanager labels Mar 20, 2023
@p0mvn p0mvn added the V:state/breaking State machine breaking PR label Mar 20, 2023
Comment on lines +21 to +29
registry.RegisterInterface(
"osmosis.poolmanager.v1beta1.PoolI",
(*poolmanagertypes.PoolI)(nil),
&CosmWasmPool{},
)
registry.RegisterInterface(
"osmosis.cosmwasmpool.v1beta1.CosmWasmExtension",
(*types.CosmWasmExtension)(nil),
&CosmWasmPool{},
Copy link
Member Author

Choose a reason for hiding this comment

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

This code block is the reason why fdc16a4 is needed.

I'm open to suggestions on how to reduce the need for the boilerplate in fdc16a4

The way this works is that there is a proto-generated CosmWasmPool model that is written to state. It only implements PoolI and CosmWasmExtensions for registering interfaces.

There is also a Go struct wrapper Pool. It is not proto-generated because it must contain WasmKeeper for interacting with the contract. As a result, I'm unable to RegisterInterfaces for Pool and have to rely on the verbose hack in fdc16a4

Copy link
Contributor

@iboss-ptk iboss-ptk left a comment

Choose a reason for hiding this comment

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

What to do with the creatorAddress in InitializePool?

  • creatorAddress should essentially be the same as contract initiator I suppose? Unless I'm missing anything.

referring to question asked in Slack:

Use Go mocks for the wasmKeeper and contractKeeper to mock out the responses of instantiate msg, query msg and sudo msg in tests

  • I think this makes a lot of sense for as unit tests

What is the ETA on the contract that I can use for testing this?

  • I will make transmuter conform pool interface within this week. Will quickly release and document new beaker features and back to this task :)

please also refer to:
#4389 (comment)

@p0mvn p0mvn force-pushed the roman/cosmwasm-pool branch from fdc16a4 to 8804f74 Compare March 26, 2023 18:48
@github-actions github-actions bot removed the C:x/gamm Changes, features and bugs related to the gamm module. label Mar 26, 2023
Comment on lines 328 to 335
// Assert that pool and swapper balances are changes
// TODO / Question for Boss: this check is failint because the originalPoolBalance is empty.
// I would expect it to equal to coins that were added to the pool in the beginning of the test
// via the join pool message (tc.initialCoins).
expectedPoolBalances := originalPoolBalances.Add(tc.tokenIn).Sub(sdk.NewCoins(tc.expectedTokenOut))
afterSwapPoolBalances := s.App.BankKeeper.GetAllBalances(s.Ctx, sdk.AccAddress(pool.GetContractAddress()))

s.Require().Equal(expectedPoolBalances.String(), afterSwapPoolBalances.String())
Copy link
Member Author

Choose a reason for hiding this comment

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

@iboss-ptk it seems to me that pool's balances are not updated upon joining the pool. Should this be handles inside a contract? That is when a user executes join message via contract, the pool's balance is updated with the joined tokens?

@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you!

@github-actions github-actions bot added the Stale label Apr 14, 2023
@github-actions github-actions bot closed this Apr 21, 2023
@p0mvn p0mvn reopened this May 26, 2023
@github-actions github-actions bot removed the C:docs Improvements or additions to documentation label May 26, 2023
@github-actions github-actions bot removed the Stale label May 27, 2023
@p0mvn
Copy link
Member Author

p0mvn commented May 28, 2023

Closing in favor of: #5327

@p0mvn p0mvn closed this May 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:no-changelog C:app-wiring Changes to the app folder C:CLI C:x/poolmanager V:state/breaking State machine breaking PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EPIC]: CosmWasm Pool Type
2 participants