added single pool query #4549

nicolaslara · 2023-03-09T12:33:10Z

Closes: #4481

What is the purpose of the change

Estimate swap queries can't be executed via LCD endpoints due to an issue with grpc gateway that prevents (repeated?) nested structures from being specified as query params.

This adds simple queries for estimating the price when using a single pool. This also simplifies querying in most cases (where users just care about one pool)

Brief Changelog

Added two new queries:
- /osmosis.poolmanager.v1beta1.Query/EstimateSinglePoolSwapExactAmountOut
- /osmosis.poolmanager.v1beta1.Query/EstimateSinglePoolSwapExactAmountIn

Testing and Verifying

Added tests that check that the results of those queries is the same as the underlying implementations
All other tests pass

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? (yes /)
How is the feature or change documented? (not applicable / specification (x/<module>/spec/) / Osmosis docs repo / not documented)

ValarDragon · 2023-03-13T20:07:05Z

proto/osmosis/poolmanager/v1beta1/query.proto

 message EstimateSwapExactAmountInRequest {
-  // TODO: CHANGE THIS TO RESERVED IN A PATCH RELEASE
-  string sender = 1 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
+  reserved 1;
+  reserved "sender";


Is this query in the stargate whitelist? If so, then seems important to reason about if this would cause a state break on mainnet. My first pass thought is it wouldn't, and should be safe. This is because the only gas difference would come from the rust side. But the Rust side is fixed.

It is. It's also potentially used by contracts.

One place that may be state breaking is https://github.com/osmosis-labs/osmosis/pull/4549/files#diff-417b3334c7fd0aefd80aafa70e8feb77a81586dc63b266105aa25bad2ab09a2fL53. It was (unnecessarily) checking that the sender wasn't empty. So a call with an empty sender would've failed before and would pass now.

good catch, this leads me to think that we might want to add queries included in stargate queries to e2e testing, since we can mistakenly merge a query PR and have it state breaking. Going to create an issue to track this and further discuss this

ValarDragon

LGTM, but feels like we should try to get this backported / into a patch release

ValarDragon · 2023-03-13T20:09:19Z

In a second non-state compatible PR, lets add these new queries to the whitelist?

proto/osmosis/poolmanager/v1beta1/query.proto

Co-authored-by: Matt, Park <[email protected]>

x/poolmanager/client/cli/query.go

proto/osmosis/poolmanager/v1beta1/query.proto

mattverse

Just one question I have in the change in the cli test, otherwise LGTM

x/poolmanager/client/cli/cli_test.go

nicolaslara · 2023-03-20T08:53:27Z

Since this is not gonna be state compatible anyway, added the stargate whitelist queries here.

Should we maybe backport this without the sender change so that thosse queries are available in a minor release? I think that's not really necessary, as these can still be queried from typescript via the rpc endpoints

mattverse · 2023-03-20T10:47:53Z

Should we maybe backport this without the sender change so that thosse queries are available in a minor release? I think that's not really necessary, as these can still be queried from typescript via the rpc endpoints

I don't think it should be necessary as well, it has been in the current state for years, I personally think it can wait

…ug (#4735) * link (#4708) * daily check for broken links (#4712) * Update README.md * Create check-broken-links.yml * Update check-broken-links.yml * Update check-broken-links.yml * Update check-broken-links.yml * Update check-broken-links.yml * Update check-broken-links.yml * Update check-broken-links.yml * Update README.md * Update AutoStake seed from .net to .com (#4711) Co-authored-by: AutoStake <[email protected]> * [tests/e2e]: simplify the logic for adding accounts to genesis sdk side and integrate into e2e (#4706) * update logic add account genesis * clean unused package * golint * XCS + Registries integration: IBC forward generation using registries (#4694) * initial attempt at splitting registries. Issues with circular deps * properly split registries * added reverse prefix map * channel validation done by registries * integrating registry into xcs and fixing the tests * Printing error context on failure * full integration with unwrapping * remove replace * removed unused deps * clippy * x86 bytecode * fmt after lints * gofumpt * fix re-export * lint * added test-contract addr * added missing newline * remove recovery states * x86 bytecode * new bytecode * added single pool query (#4549) * added single pool query * added changelog * remove empty tests as they are invalid * removed sender and made reserved * remove and reserve unused sender * removed sender * actually remove sender * querygen * move impl to correct file * more flexible querygen * Update proto/osmosis/poolmanager/v1beta1/query.proto Co-authored-by: Matt, Park <[email protected]> * Update proto/osmosis/poolmanager/v1beta1/query.proto Co-authored-by: Matt, Park <[email protected]> * Update proto/osmosis/poolmanager/v1beta1/query.proto Co-authored-by: Matt, Park <[email protected]> * Update proto/osmosis/poolmanager/v1beta1/query.proto Co-authored-by: Matt, Park <[email protected]> * protos * added cli * fixed command parsing * single line * lint * fix cli * run proto gen * added whitelisted query * fix test for v15 --------- Co-authored-by: Matt, Park <[email protected]> * Revert "[tests/e2e]: simplify the logic for adding accounts to genesis sdk side and integrate into e2e (#4706)" (#4727) This reverts commit a9725d6. * ProtoRev: Adding Txs to CLI (#4567) * init set hot routes cli command * adding remaining txs * few more tests * comments * Update upgrade_test.go (#4734) * refactor tests and fix incentive forfeiting bug * update create position tx * feat: whitelist poolmanager Pool, CL Params and ClaimableFees queries (#4725) * feat: whitelist poolmanager Pool, CL Params and ClaimableFees queries * lint * conflict * fix e2e query parsing * increase gas --------- Co-authored-by: Ruslan Akhtariev <[email protected]> Co-authored-by: Master Pi <[email protected]> Co-authored-by: AutoStake <[email protected]> Co-authored-by: AutoStake <[email protected]> Co-authored-by: Nguyen Thanh Nhan <[email protected]> Co-authored-by: Nicolas Lara <[email protected]> Co-authored-by: Matt, Park <[email protected]> Co-authored-by: David Terpay <[email protected]> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: Roman <[email protected]>

* utilize new position id in position key * add position id to migration response * regen proto * modify withdraw pos in e2e * cmd order * avoid store key formatting with fmt.Sprintf * set default NextPositionId to 1 * lint * return positionId from CreatePos E2E * utilize bytes.Buffer to build keys * Revert "utilize bytes.Buffer to build keys" This reverts commit 0c1bc13. * utilize sprintf for key * remove extra positions logic * initial push * regen proto * Update go.mod * lints * fix sim * add posid to sim collectincentive * e2e test fix * lint * clean up * remove extra fields * regen proto * only use pos id for fees * update cli and e2e * fee accum updates * modify claim to use only positionId * fix functional fees test * pool to position id map * add pool store to delete position * fix conflicts from main merge * test updates * lints / cleanup * add position Id to create position response * regen proto * regen proto * switch order * regen proto * add error comment to collectFees * move key format to types * rename to positionFields * Remove unnecessary test field * revert refactor to functional fees test * define custom errors * remove duplicate events emitted * correct validatePositionUpdate inputs * Update x/concentrated-liquidity/position.go Co-authored-by: Roman <[email protected]> * update README for KV stores * other readme updates * feat: query position by id (added to stargate whitelist) (#4718) * feat(CL): position by id query (stargate whitelisted) * fix test * Update app/upgrades/v15/upgrade_test.go --------- Co-authored-by: Adam Tucker <[email protected]> * initial push * remove position age as input param * [CL][bugfix] Refactor incentives tests and fix incentive forfeiting bug (#4735) * link (#4708) * daily check for broken links (#4712) * Update README.md * Create check-broken-links.yml * Update check-broken-links.yml * Update check-broken-links.yml * Update check-broken-links.yml * Update check-broken-links.yml * Update check-broken-links.yml * Update check-broken-links.yml * Update README.md * Update AutoStake seed from .net to .com (#4711) Co-authored-by: AutoStake <[email protected]> * [tests/e2e]: simplify the logic for adding accounts to genesis sdk side and integrate into e2e (#4706) * update logic add account genesis * clean unused package * golint * XCS + Registries integration: IBC forward generation using registries (#4694) * initial attempt at splitting registries. Issues with circular deps * properly split registries * added reverse prefix map * channel validation done by registries * integrating registry into xcs and fixing the tests * Printing error context on failure * full integration with unwrapping * remove replace * removed unused deps * clippy * x86 bytecode * fmt after lints * gofumpt * fix re-export * lint * added test-contract addr * added missing newline * remove recovery states * x86 bytecode * new bytecode * added single pool query (#4549) * added single pool query * added changelog * remove empty tests as they are invalid * removed sender and made reserved * remove and reserve unused sender * removed sender * actually remove sender * querygen * move impl to correct file * more flexible querygen * Update proto/osmosis/poolmanager/v1beta1/query.proto Co-authored-by: Matt, Park <[email protected]> * Update proto/osmosis/poolmanager/v1beta1/query.proto Co-authored-by: Matt, Park <[email protected]> * Update proto/osmosis/poolmanager/v1beta1/query.proto Co-authored-by: Matt, Park <[email protected]> * Update proto/osmosis/poolmanager/v1beta1/query.proto Co-authored-by: Matt, Park <[email protected]> * protos * added cli * fixed command parsing * single line * lint * fix cli * run proto gen * added whitelisted query * fix test for v15 --------- Co-authored-by: Matt, Park <[email protected]> * Revert "[tests/e2e]: simplify the logic for adding accounts to genesis sdk side and integrate into e2e (#4706)" (#4727) This reverts commit a9725d6. * ProtoRev: Adding Txs to CLI (#4567) * init set hot routes cli command * adding remaining txs * few more tests * comments * Update upgrade_test.go (#4734) * refactor tests and fix incentive forfeiting bug * update create position tx * feat: whitelist poolmanager Pool, CL Params and ClaimableFees queries (#4725) * feat: whitelist poolmanager Pool, CL Params and ClaimableFees queries * lint * conflict * fix e2e query parsing * increase gas --------- Co-authored-by: Ruslan Akhtariev <[email protected]> Co-authored-by: Master Pi <[email protected]> Co-authored-by: AutoStake <[email protected]> Co-authored-by: AutoStake <[email protected]> Co-authored-by: Nguyen Thanh Nhan <[email protected]> Co-authored-by: Nicolas Lara <[email protected]> Co-authored-by: Matt, Park <[email protected]> Co-authored-by: David Terpay <[email protected]> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: Roman <[email protected]> * refactor collect incentives test * remove any final references to freezing * merge main * add back ParsePositionFromBz * add comments to test * add forfeit incentives withdraw test case * add negative duration validation and test * clean up * add back multi-position test * regen proto --------- Co-authored-by: czarcas7ic <[email protected]> Co-authored-by: Roman <[email protected]> Co-authored-by: alpo <[email protected]> Co-authored-by: Ruslan Akhtariev <[email protected]> Co-authored-by: Master Pi <[email protected]> Co-authored-by: AutoStake <[email protected]> Co-authored-by: AutoStake <[email protected]> Co-authored-by: Nguyen Thanh Nhan <[email protected]> Co-authored-by: Nicolas Lara <[email protected]> Co-authored-by: Matt, Park <[email protected]> Co-authored-by: David Terpay <[email protected]>

added single pool query

e87f1ce

github-actions bot added C:CLI C:x/poolmanager labels Mar 9, 2023

added changelog

7278e48

nicolaslara added the V:state/compatible/no_backport State machine compatible PR, depends on prior breaks label Mar 9, 2023

nicolaslara added 8 commits March 9, 2023 13:44

remove empty tests as they are invalid

149c32f

removed sender and made reserved

ee46ae9

remove and reserve unused sender

12ba6cf

removed sender

9fd94a0

actually remove sender

01d7ba5

querygen

44679b2

move impl to correct file

e85903d

more flexible querygen

0569a5b

p0mvn approved these changes Mar 11, 2023

View reviewed changes

mattverse self-requested a review March 13, 2023 11:08

ValarDragon reviewed Mar 13, 2023

View reviewed changes

ValarDragon approved these changes Mar 13, 2023

View reviewed changes

nicolaslara added V:state/breaking State machine breaking PR and removed V:state/compatible/no_backport State machine compatible PR, depends on prior breaks labels Mar 14, 2023

mattverse reviewed Mar 15, 2023

View reviewed changes

nicolaslara and others added 6 commits March 15, 2023 10:22

Update proto/osmosis/poolmanager/v1beta1/query.proto

a760155

Co-authored-by: Matt, Park <[email protected]>

Update proto/osmosis/poolmanager/v1beta1/query.proto

65a00e1

Co-authored-by: Matt, Park <[email protected]>

Update proto/osmosis/poolmanager/v1beta1/query.proto

909f440

Co-authored-by: Matt, Park <[email protected]>

Update proto/osmosis/poolmanager/v1beta1/query.proto

5662ab3

Co-authored-by: Matt, Park <[email protected]>

Merge branch 'main' into nicolas/simple-swap-endpoint

4866d00

protos

95de2fe

mattverse reviewed Mar 15, 2023

View reviewed changes

x/poolmanager/client/cli/query.go Show resolved Hide resolved

mattverse reviewed Mar 15, 2023

View reviewed changes

x/poolmanager/client/cli/query.go Show resolved Hide resolved

added cli

27115c2

mattverse reviewed Mar 15, 2023

View reviewed changes

proto/osmosis/poolmanager/v1beta1/query.proto Outdated Show resolved Hide resolved

nicolaslara added 3 commits March 15, 2023 15:28

fixed command parsing

bf8b9d4

single line

a3c9056

lint

76a6293

mattverse approved these changes Mar 16, 2023

View reviewed changes

x/poolmanager/client/cli/cli_test.go Outdated Show resolved Hide resolved

nicolaslara added 4 commits March 16, 2023 10:49

fix cli

bf00a07

Merge branch 'main' into nicolas/simple-swap-endpoint

e6f7ecc

run proto gen

8408858

added whitelisted query

2755382

fix test for v15

5a7bf12

github-actions bot added the C:app-wiring Changes to the app folder label Mar 20, 2023

Merge branch 'main' into nicolas/simple-swap-endpoint

79b8fd2

nicolaslara merged commit 695c3c9 into main Mar 24, 2023

nicolaslara deleted the nicolas/simple-swap-endpoint branch March 24, 2023 14:56

nicolaslara mentioned this pull request Mar 28, 2023

feat: x/cosmwasmpool proto and query boilerplate #4764

Merged

github-actions bot mentioned this pull request Apr 15, 2024

Broken Links Detected #8046

Closed

github-actions bot mentioned this pull request May 1, 2024

Broken Links Detected #8183

Closed

github-actions bot mentioned this pull request Oct 15, 2024

Broken Links Detected notional-labs/osmosis#75

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added single pool query #4549

added single pool query #4549

nicolaslara commented Mar 9, 2023

ValarDragon Mar 13, 2023 •

edited

Loading

nicolaslara Mar 14, 2023

mattverse Mar 15, 2023 •

edited

Loading

ValarDragon left a comment

ValarDragon commented Mar 13, 2023

mattverse left a comment

nicolaslara commented Mar 20, 2023 •

edited

Loading

mattverse commented Mar 20, 2023

added single pool query #4549

added single pool query #4549

Conversation

nicolaslara commented Mar 9, 2023

What is the purpose of the change

Brief Changelog

Testing and Verifying

Documentation and Release Note

ValarDragon Mar 13, 2023 • edited Loading

Choose a reason for hiding this comment

nicolaslara Mar 14, 2023

Choose a reason for hiding this comment

mattverse Mar 15, 2023 • edited Loading

Choose a reason for hiding this comment

ValarDragon left a comment

Choose a reason for hiding this comment

ValarDragon commented Mar 13, 2023

mattverse left a comment

Choose a reason for hiding this comment

nicolaslara commented Mar 20, 2023 • edited Loading

mattverse commented Mar 20, 2023

ValarDragon Mar 13, 2023 •

edited

Loading

mattverse Mar 15, 2023 •

edited

Loading

nicolaslara commented Mar 20, 2023 •

edited

Loading