JoinPoolNoSwap simulation #3242

pysel · 2022-11-04T06:55:44Z

Closes: #2956 (actually this)

What is the purpose of the change

Adds a simulating query of JoinPoolNoSwap

Testing and Verifying

tests added

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? no
How is the feature or change documented? changelog

georgemc98 · 2022-11-04T16:42:22Z

@apollo-sturdy

This looks good to me. Can we change the name SimJoinPoolNoSwap to CalcJoinPoolNoSwap?

pysel · 2022-11-05T05:02:23Z

fixed minor mistakes, now ready for review @apollo-sturdy

apollo-sturdy · 2022-11-09T17:12:25Z

fixed minor mistakes, now ready for review @apollo-sturdy

Hi. Thanks very much. I now had a chance to look at this. After more thought, this approach doesn't solve for our use case. The reason is that we currently have no way to know what to input to MsgJoinPool argument ShareOutAmount, which is what we want this query to address. So the query should be able to answer the question:

Given some amount of assets that we want to provide liquidity with via MsgJoinPool, what should we pass in as ShareOutAmount?

I should have noticed that @p0mvn's description of the requirements written here doesn't match what we need, since we don't know how to get the shareOutAmount in the first place.

@p0mvn Do you have any suggestions how to solve this? One option would of course be to change the API of MsgJoinPool to the more common one of just taking in the assets the user wants to provide liquidity with. Is this something you would be willing to consider?

georgemc98 · 2022-11-09T21:44:12Z

Hi @rusakh, did something change recently with the make proto-all command? I've been running make proto-all with no issues, but today the command is not generating the pg.go files. I implemented CalcJoinPoolNoSwapShares to @apollo-sturdy specifications and added table driven tests for it, but I cannot produce the pb.go file, even after restarting Docker. The CalcJoinPoolNoSwapShares query should pass the tests once the pb.go files are generated.

pysel · 2022-11-10T05:19:26Z

Hi @georgemc98. That is odd, make proto-all works on my machine well, however, after I generate files, in gamm/types/query.pb.go I get a few errors, for example in:

size := m.TokensIn.Size()

the error is:

m.TokensIn.Size undefined (type "github.com/cosmos/cosmos-sdk/types".Coins has no field or method Size)

georgemc98 · 2022-11-10T13:23:39Z

Hi @georgemc98. That is odd, make proto-all works on my machine well, however, after I generate files, in gamm/types/query.pb.go I get a few errors, for example in:
size := m.TokensIn.Size()
the error is:
m.TokensIn.Size undefined (type "github.com/cosmos/cosmos-sdk/types".Coins has no field or method Size)

I changed the tokens_in type. I think that should fix the issue. @rusakh

pysel · 2022-11-10T15:13:30Z

Hi @georgemc98. That is odd, make proto-all works on my machine well, however, after I generate files, in gamm/types/query.pb.go I get a few errors, for example in:
size := m.TokensIn.Size()
the error is:
m.TokensIn.Size undefined (type "github.com/cosmos/cosmos-sdk/types".Coins has no field or method Size)
I changed the tokens_in type. I think that should fix the issue. @rusakh

oh, yes it did, thank you very much! I will continue working on it as soon as possible

georgemc98 · 2022-11-10T15:17:04Z

Hi @georgemc98. That is odd, make proto-all works on my machine well, however, after I generate files, in gamm/types/query.pb.go I get a few errors, for example in:
size := m.TokensIn.Size()
the error is:
m.TokensIn.Size undefined (type "github.com/cosmos/cosmos-sdk/types".Coins has no field or method Size)
I changed the tokens_in type. I think that should fix the issue. @rusakh
oh, yes it did, thank you very much! I will continue working on it as soon as possible

Thanks! I fixed the unit tests and I am going to write one additional test. I'll have it done in a few hours.

georgemc98 · 2022-11-11T15:27:06Z

Hi @georgemc98. That is odd, make proto-all works on my machine well, however, after I generate files, in gamm/types/query.pb.go I get a few errors, for example in:
size := m.TokensIn.Size()
the error is:
m.TokensIn.Size undefined (type "github.com/cosmos/cosmos-sdk/types".Coins has no field or method Size)
I changed the tokens_in type. I think that should fix the issue. @rusakh
oh, yes it did, thank you very much! I will continue working on it as soon as possible

@rusakh The test pass on my end and we are ready to merge. Should we change this from a Draft so it can start being reviewed?

pysel · 2022-11-11T18:38:42Z

thank you @georgemc98, opened!

p0mvn · 2022-11-11T22:09:19Z

Can we fix CI here?

x/gamm/keeper/export_test.go

x/gamm/keeper/grpc_query.go

Co-authored-by: Roman <[email protected]>

p0mvn · 2022-11-12T01:46:45Z

x/gamm/keeper/grpc_query.go

@@ -288,6 +288,29 @@ func (q Querier) CalcExitPoolCoinsFromShares(ctx context.Context, req *types.Que
 	return &types.QueryCalcExitPoolCoinsFromSharesResponse{TokensOut: exitCoins}, nil
 }

+//  CalcJoinPoolNoSwapShares returns the amount of shares you'd get if joined a pool without a swap and tokens which need to be provided
+func (q Querier) CalcJoinPoolNoSwapShares(ctx context.Context, req *types.QueryCalcJoinPoolNoSwapSharesRequest) (*types.QueryCalcJoinPoolNoSwapSharesResponse, error) {


Can we please add this and your previous query to this test:

osmosis/x/gamm/client/cli/query_test.go

Line 26 in 0423e96

func (s *QueryTestSuite) TestQueriesNeverAlterState() {

It ensures that queries don't alter state. This would be very useful for a stargate query

Got it, I'll change this ASAP

p0mvn · 2022-11-12T01:47:21Z

x/gamm/keeper/grpc_query.go

+	}
+
+	sdkCtx := sdk.UnwrapSDKContext(ctx)
+	pool, err := q.GetPoolAndPoke(sdkCtx, req.PoolId)


This method calls PokePool() which may update pool weights for a balancer pool. Is this acceptable for a stargate query?

CC: @ValarDragon

Seems like it's fine: #2972 (comment)

So then this PR should be able to be merged?

@p0mvn

* autocli copy from cosmos * reset * save * SimJoinPoolNoSwap * docs * JoinPoolNoSwap simulation * more fixes * fix docs and name * fix: change tokens_in type * test: fix TestCalcJoinPoolNoSwapShares test * chore: remove whitespace Co-authored-by: Roman <[email protected]> * chore: change query name in comment Co-authored-by: Roman <[email protected]> * feat: build query.pb.go Co-authored-by: Ruslan Akhtariev <[email protected]> Co-authored-by: George McAskill <[email protected]> Co-authored-by: Roman <[email protected]> (cherry picked from commit fa2f483)

* autocli copy from cosmos * reset * save * SimJoinPoolNoSwap * docs * JoinPoolNoSwap simulation * more fixes * fix docs and name * fix: change tokens_in type * test: fix TestCalcJoinPoolNoSwapShares test * chore: remove whitespace Co-authored-by: Roman <[email protected]> * chore: change query name in comment Co-authored-by: Roman <[email protected]> * feat: build query.pb.go Co-authored-by: Ruslan Akhtariev <[email protected]> Co-authored-by: George McAskill <[email protected]> Co-authored-by: Roman <[email protected]> (cherry picked from commit fa2f483) Co-authored-by: Ruslan Akhtariev <[email protected]>

github-actions bot added the C:x/gamm Changes, features and bugs related to the gamm module. label Nov 4, 2022

pysel changed the title ~~Issue2956 cont~~ JoinPoolNoSwap simulation Nov 5, 2022

pysel marked this pull request as ready for review November 5, 2022 05:01

pysel requested a review from a team November 5, 2022 05:01

Ruslan Akhtariev and others added 4 commits November 6, 2022 12:59

autocli copy from cosmos

4bccd64

Merge branch 'osmosis-labs:main' into main

952805b

reset

0b36043

Merge branch 'osmosis-labs:main' into main

772a339

Ruslan Akhtariev and others added 5 commits November 10, 2022 11:33

save

3a412be

SimJoinPoolNoSwap

44d5e7e

docs

d256d6a

JoinPoolNoSwap simulation

21507ac

Merge branch 'osmosis-labs:main' into main

13ee4cc

pysel force-pushed the issue2956_cont branch from 1ab5665 to 8a73512 Compare November 10, 2022 04:37

more fixes

8c1842a

pysel force-pushed the issue2956_cont branch from f3aefe1 to 8c1842a Compare November 10, 2022 05:12

pysel marked this pull request as draft November 10, 2022 05:14

fix docs and name

b0a7e02

fix: change tokens_in type

d27f5f5

test: fix TestCalcJoinPoolNoSwapShares test

1885ab3

Merge branch 'osmosis-labs:main' into main

92bb3e0

georgemc98 mentioned this pull request Nov 11, 2022

Add CalcJoinPoolShares and CalcExitPoolCoinsFromShares pool queries to Stargate whitelist #3217

Merged

pysel marked this pull request as ready for review November 11, 2022 18:38

Merge branch 'main' into issue2956_cont

d82a187

p0mvn reviewed Nov 11, 2022

View reviewed changes

x/gamm/keeper/export_test.go Outdated Show resolved Hide resolved

x/gamm/keeper/grpc_query.go Outdated Show resolved Hide resolved

p0mvn added V:state/compatible/backport State machine compatible PR, should be backported A:backport/v13.x backport patches to v13.x branch labels Nov 11, 2022

georgemc98 and others added 3 commits November 11, 2022 17:24

chore: remove whitespace

ad4ad23

Co-authored-by: Roman <[email protected]>

chore: change query name in comment

b570872

Co-authored-by: Roman <[email protected]>

feat: build query.pb.go

6b8bc7a

p0mvn reviewed Nov 12, 2022

View reviewed changes

p0mvn approved these changes Nov 14, 2022

View reviewed changes

p0mvn merged commit fa2f483 into osmosis-labs:main Nov 14, 2022

mergify bot mentioned this pull request Nov 14, 2022

JoinPoolNoSwap simulation (backport #3242) #3376

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JoinPoolNoSwap simulation #3242

JoinPoolNoSwap simulation #3242

pysel commented Nov 4, 2022 •

edited

Loading

georgemc98 commented Nov 4, 2022

pysel commented Nov 5, 2022

apollo-sturdy commented Nov 9, 2022 •

edited

Loading

georgemc98 commented Nov 9, 2022

pysel commented Nov 10, 2022

georgemc98 commented Nov 10, 2022

pysel commented Nov 10, 2022

georgemc98 commented Nov 10, 2022

georgemc98 commented Nov 11, 2022

pysel commented Nov 11, 2022

p0mvn commented Nov 11, 2022

p0mvn Nov 12, 2022

georgemc98 Nov 12, 2022

p0mvn Nov 12, 2022

p0mvn Nov 12, 2022

apollo-sturdy Nov 14, 2022 •

edited

Loading

JoinPoolNoSwap simulation #3242

JoinPoolNoSwap simulation #3242

Conversation

pysel commented Nov 4, 2022 • edited Loading

What is the purpose of the change

Testing and Verifying

Documentation and Release Note

georgemc98 commented Nov 4, 2022

pysel commented Nov 5, 2022

apollo-sturdy commented Nov 9, 2022 • edited Loading

georgemc98 commented Nov 9, 2022

pysel commented Nov 10, 2022

georgemc98 commented Nov 10, 2022

pysel commented Nov 10, 2022

georgemc98 commented Nov 10, 2022

georgemc98 commented Nov 11, 2022

pysel commented Nov 11, 2022

p0mvn commented Nov 11, 2022

p0mvn Nov 12, 2022

Choose a reason for hiding this comment

georgemc98 Nov 12, 2022

Choose a reason for hiding this comment

p0mvn Nov 12, 2022

Choose a reason for hiding this comment

p0mvn Nov 12, 2022

Choose a reason for hiding this comment

apollo-sturdy Nov 14, 2022 • edited Loading

Choose a reason for hiding this comment

pysel commented Nov 4, 2022 •

edited

Loading

apollo-sturdy commented Nov 9, 2022 •

edited

Loading

apollo-sturdy Nov 14, 2022 •

edited

Loading