[InternalReview][MsgFungifyChargedPositions] #5091
Conversation
| @@ -257,13 +255,6 @@ func (server msgServer) FungifyChargedPositions(goCtx context.Context, msg *type | |||
| sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory), | |||
| sdk.NewAttribute(sdk.AttributeKeySender, msg.Sender), | |||
| ), | |||
| sdk.NewEvent( | |||
p0mvn
changed the title
p0mvn
added
V:state/compatible/no_backport
github-actions
bot
added
the
C:x/gamm
p0mvn
changed the title
| // - positions are not in the same pool | ||
| // - positions are all not fully charged | ||
| // - positions are not in the same tick range | ||
| // - all positions are unlocked | ||
| func (k Keeper) validatePositionsAndGetTotalLiquidity(ctx sdk.Context, owner sdk.AccAddress, positionIds []uint64) (uint64, int64, int64, sdk.Dec, error) { |
There was a problem hiding this comment.
This function is only used to validate for fungify right? If so, can we be more specific with the method name so we know what this is validating for?
There was a problem hiding this comment.
I don't think this has to be married to fungify. Methods should be reusable. The spec already outlines what is being validated if someone would like to reuse it
| // GetCmdAllPools return all pools available across Osmosis modules. | ||
| func GetCmdAllPools() (*osmocli.QueryDescriptor, *queryproto.AllPoolsRequest) { | ||
| return &osmocli.QueryDescriptor{ | ||
| Use: "all-pools", | ||
| Short: "Query all pools on the Osmosis chain", | ||
| Long: "{{.Short}}", | ||
| }, &queryproto.AllPoolsRequest{} | ||
| } | ||
|
|
There was a problem hiding this comment.
Just a note here, under QueryDescriptor I think we need to add hasPagination to true. This will add the corresponding flags and such, but I don't think its properly paginates out of the box (there is no osmocli query that currently uses it, and any paginated query we actually rely on uses to old cli setup).
There was a problem hiding this comment.
Pagination isn't implemented for this query. Since they are they only known client, spoke to Jon and decided not to added for now due to time constraints
* [InternalReview][MsgFungifyChargedPositions] * comment * remove nit * comment * comment * revert launch json * lint * fix ctx * updates * CLI fix * revert me * Revert "revert me" This reverts commit 8acf2ce. * remove debug event * fix test * fix e2e * clean up comments * updates * updates * message server * fix test * update README * README update * README * tick spacing 100 * negative rewards * comments * comments
Closes: #XXX
What is the purpose of the change
https://app.clickup.com/37420681/v/dc/13nzm9-19302/13nzm9-35382
Review of
MsgFungifyChargedPositionsThis is part 1 only, will be continuing with more internal review.
Documentation and Release Note
Unreleasedsection inCHANGELOG.md? no