[CL][bugfix] Refactor incentives tests and fix incentive forfeiting b…

…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]>

tests/e2e/configurer/chain/commands.go

@@ -69,17 +69,21 @@ func (n *NodeConfig) CreateConcentratedPool(from, denom1, denom2 string, tickSpa
 	return poolID
 }
 
-func (n *NodeConfig) CreateConcentratedPosition(from, lowerTick, upperTick string, token0, token1 string, token0MinAmt, token1MinAmt int64, freezeDuration string, poolId uint64) uint64 {
+func (n *NodeConfig) CreateConcentratedPosition(from, lowerTick, upperTick string, token0, token1 string, token0MinAmt, token1MinAmt int64, poolId uint64) uint64 {
 	n.LogActionF("creating concentrated position")
 
-	cmd := []string{"osmosisd", "tx", "concentratedliquidity", "create-position", lowerTick, upperTick, token0, token1, fmt.Sprintf("%d", token0MinAmt), fmt.Sprintf("%d", token1MinAmt), freezeDuration, fmt.Sprintf("--from=%s", from), fmt.Sprintf("--pool-id=%d", poolId)}
-	outBuf, _, err := n.containerManager.ExecTxCmd(n.t, n.chainId, n.Name, cmd)
+	cmd := []string{"osmosisd", "tx", "concentratedliquidity", "create-position", lowerTick, upperTick, token0, token1, fmt.Sprintf("%d", token0MinAmt), fmt.Sprintf("%d", token1MinAmt), fmt.Sprintf("--from=%s", from), fmt.Sprintf("--pool-id=%d", poolId), "-o json"}
+	outJson, _, err := n.containerManager.ExecTxCmdWithSuccessString(n.t, n.chainId, n.Name, cmd, "code\":0")
 	require.NoError(n.t, err)
 
-	prefix := "position_id\",\"value\":\""
-	startIndex := strings.Index(outBuf.String(), prefix) + len(prefix)
-	endIndex := startIndex + strings.Index(outBuf.String()[startIndex:], "\"}")
-	positionID, err := strconv.ParseUint(outBuf.String()[startIndex:endIndex], 10, 64)
+	var txResponse map[string]interface{}
+	err = json.Unmarshal(outJson.Bytes(), &txResponse)
+	require.NoError(n.t, err)
+
+	positionIDString, err := GetPositionID(txResponse)
+	require.NoError(n.t, err)
+
+	positionID, err := strconv.ParseUint(positionIDString, 10, 64)
 	require.NoError(n.t, err)
 
 	n.LogActionF("successfully created concentrated position from %s to %s", lowerTick, upperTick)
@@ -97,7 +101,7 @@ func (n *NodeConfig) StoreWasmCode(wasmFile, from string) {
 
 func (n *NodeConfig) WithdrawPosition(from, liquidityOut string, positionId uint64) {
 	n.LogActionF("withdrawing liquidity from position")
-	cmd := []string{"osmosisd", "tx", "concentratedliquidity", "withdraw-position", fmt.Sprint(positionId), liquidityOut, fmt.Sprintf("--from=%s", from)}
+	cmd := []string{"osmosisd", "tx", "concentratedliquidity", "withdraw-position", fmt.Sprint(positionId), liquidityOut, fmt.Sprintf("--from=%s", from), "--gas=auto", "--gas-prices=0.1uosmo", "--gas-adjustment=1.3"}
 	_, _, err := n.containerManager.ExecTxCmd(n.t, n.chainId, n.Name, cmd)
 	require.NoError(n.t, err)
 	n.LogActionF("successfully withdrew %s liquidity from position %d", liquidityOut, positionId)
@@ -447,3 +451,48 @@ func (n *NodeConfig) Status() (resultStatus, error) {
 	}
 	return result, nil
 }
+
+func GetPositionID(responseJson map[string]interface{}) (string, error) {
+	logs, ok := responseJson["logs"].([]interface{})
+	if !ok {
+		return "", fmt.Errorf("logs field not found in response")
+	}
+
+	if len(logs) == 0 {
+		return "", fmt.Errorf("empty logs field in response")
+	}
+
+	log, ok := logs[0].(map[string]interface{})
+	if !ok {
+		return "", fmt.Errorf("invalid format of logs field")
+	}
+
+	events, ok := log["events"].([]interface{})
+	if !ok {
+		return "", fmt.Errorf("events field not found in logs")
+	}
+
+	for _, event := range events {
+		attributes, ok := event.(map[string]interface{})["attributes"].([]interface{})
+		if !ok {
+			return "", fmt.Errorf("attributes field not found in event")
+		}
+
+		for _, attr := range attributes {
+			switch v := attr.(type) {
+			case map[string]interface{}:
+				if v["key"] == "position_id" {
+					positionID, ok := v["value"].(string)
+					if !ok {
+						return "", fmt.Errorf("invalid format of position_id field")
+					}
+					return positionID, nil
+				}
+			default:
+				return "", fmt.Errorf("invalid type for attributes field")
+			}
+		}
+	}
+
+	return "", fmt.Errorf("position_id field not found in response")
+}

tests/e2e/e2e_test.go

@@ -197,7 +197,6 @@ func (s *IntegrationTestSuite) TestConcentratedLiquidity() {
 		denom1                    string = "uosmo"
 		tickSpacing               uint64 = 1
 		precisionFactorAtPriceOne int64  = -1
-		freezeDuration                   = time.Duration(time.Second)
 		swapFee                          = "0.01"
 	)
 
@@ -230,15 +229,15 @@ func (s *IntegrationTestSuite) TestConcentratedLiquidity() {
 	address3 := node.CreateWalletAndFund("addr3", fundTokens)
 
 	// Create 2 positions for address1: overlap together, overlap with 2 address3 positions
-	addr1PosId := node.CreateConcentratedPosition(address1, "[-1200]", "400", fmt.Sprintf("1000%s", denom0), fmt.Sprintf("1000%s", denom1), 0, 0, freezeDuration.String(), poolID)
-	node.CreateConcentratedPosition(address1, "[-400]", "400", fmt.Sprintf("1000%s", denom0), fmt.Sprintf("1000%s", denom1), 0, 0, freezeDuration.String(), poolID)
+	addr1PosId := node.CreateConcentratedPosition(address1, "[-1200]", "400", fmt.Sprintf("1000%s", denom0), fmt.Sprintf("1000%s", denom1), 0, 0, poolID)
+	node.CreateConcentratedPosition(address1, "[-400]", "400", fmt.Sprintf("1000%s", denom0), fmt.Sprintf("1000%s", denom1), 0, 0, poolID)
 
 	// Create 1 position for address2: does not overlap with anything, ends at maximum
-	addr2PosId := node.CreateConcentratedPosition(address2, "2200", fmt.Sprintf("%d", maxTick), fmt.Sprintf("1000%s", denom0), fmt.Sprintf("1000%s", denom1), 0, 0, freezeDuration.String(), poolID)
+	addr2PosId := node.CreateConcentratedPosition(address2, "2200", fmt.Sprintf("%d", maxTick), fmt.Sprintf("1000%s", denom0), fmt.Sprintf("1000%s", denom1), 0, 0, poolID)
 
 	// Create 2 positions for address3: overlap together, overlap with 2 address1 positions, one position starts from minimum
-	addr3PosId := node.CreateConcentratedPosition(address3, "[-1600]", "[-200]", fmt.Sprintf("1000%s", denom0), fmt.Sprintf("1000%s", denom1), 0, 0, freezeDuration.String(), poolID)
-	node.CreateConcentratedPosition(address3, fmt.Sprintf("[%d]", minTick), "1400", fmt.Sprintf("1000%s", denom0), fmt.Sprintf("1000%s", denom1), 0, 0, freezeDuration.String(), poolID)
+	addr3PosId := node.CreateConcentratedPosition(address3, "[-1600]", "[-200]", fmt.Sprintf("1000%s", denom0), fmt.Sprintf("1000%s", denom1), 0, 0, poolID)
+	node.CreateConcentratedPosition(address3, fmt.Sprintf("[%d]", minTick), "1400", fmt.Sprintf("1000%s", denom0), fmt.Sprintf("1000%s", denom1), 0, 0, poolID)
 
 	// get newly created positions
 	positionsAddress1 := node.QueryConcentratedPositions(address1)

x/concentrated-liquidity/client/cli/tx.go

@@ -35,9 +35,9 @@ func NewCreateConcentratedPoolCmd() (*osmocli.TxCliDesc, *clmodel.MsgCreateConce
 
 func NewCreatePositionCmd() (*osmocli.TxCliDesc, *types.MsgCreatePosition) {
 	return &osmocli.TxCliDesc{
-		Use:                 "create-position [lower-tick] [upper-tick] [token-0] [token-1] [token-0-min-amount] [token-1-min-amount] [freeze-duration]",
+		Use:                 "create-position [lower-tick] [upper-tick] [token-0] [token-1] [token-0-min-amount] [token-1-min-amount]",
 		Short:               "create or add to existing concentrated liquidity position",
-		Example:             "create-position [-69082] 69082 1000000000uosmo 10000000uion 0 0 24h --pool-id 1 --from val --chain-id osmosis-1",
+		Example:             "create-position [-69082] 69082 1000000000uosmo 10000000uion 0 0 --pool-id 1 --from val --chain-id osmosis-1",
 		CustomFlagOverrides: poolIdFlagOverride,
 		Flags:               osmocli.FlagDesc{RequiredFlags: []*flag.FlagSet{FlagSetJustPoolId()}},
 	}, &types.MsgCreatePosition{}

x/concentrated-liquidity/export_test.go

@@ -244,7 +244,7 @@ func PrepareAccumAndClaimRewards(accum accum.AccumulatorObject, positionKey stri
 	return prepareAccumAndClaimRewards(accum, positionKey, growthOutside)
 }
 
-func (k Keeper) ClaimAllIncentivesForPosition(ctx sdk.Context, positionId uint64) (sdk.Coins, error) {
+func (k Keeper) ClaimAllIncentivesForPosition(ctx sdk.Context, positionId uint64) (sdk.Coins, sdk.Coins, error) {
 	return k.claimAllIncentivesForPosition(ctx, positionId)
 }
 

x/concentrated-liquidity/incentives.go

@@ -510,34 +510,39 @@ func prepareAccumAndClaimRewards(accum accum.AccumulatorObject, positionKey stri
 }
 
 // claimAllIncentivesForPosition claims and returns all the incentives for a given position.
-// It takes in a `forfeitIncentives` boolean to indicate whether the accrued incentives should be forfeited, in which case it
-// redeposits the accrued rewards back into the accumulator as additional rewards for other participants.
-func (k Keeper) claimAllIncentivesForPosition(ctx sdk.Context, positionId uint64) (sdk.Coins, error) {
+// It claims all the incentives that the position is eligible for and forfeits the rest by redepositing them back
+// into the accumulator (effectively redistributing them to the other LPs).
+//
+// Returns the amount of successfully claimed incentives and the amount of forfeited incentives.
+// Returns error if the position/uptime accumulators don't exist, or if there is an issue that arises while claiming.
+func (k Keeper) claimAllIncentivesForPosition(ctx sdk.Context, positionId uint64) (sdk.Coins, sdk.Coins, error) {
 	// Retrieve the position with the given ID.
 	position, err := k.GetPosition(ctx, positionId)
 	if err != nil {
-		return sdk.Coins{}, err
+		return sdk.Coins{}, sdk.Coins{}, err
 	}
 
+	// TODO: add validation to ensure this is never negative
 	positionAge := ctx.BlockTime().Sub(position.JoinTime)
 
 	// Retrieve the uptime accumulators for the position's pool.
 	uptimeAccumulators, err := k.getUptimeAccumulators(ctx, position.PoolId)
 	if err != nil {
-		return sdk.Coins{}, err
+		return sdk.Coins{}, sdk.Coins{}, err
 	}
 
 	// Compute uptime growth outside of the range between lower tick and upper tick
 	uptimeGrowthOutside, err := k.GetUptimeGrowthOutsideRange(ctx, position.PoolId, position.LowerTick, position.UpperTick)
 	if err != nil {
-		return sdk.Coins{}, err
+		return sdk.Coins{}, sdk.Coins{}, err
 	}
 
 	// Create a variable to hold the name of the position.
 	positionName := string(types.KeyPositionId(positionId))
 
-	// Create a variable to hold the total collected incentives for the position.
+	// Create variables to hold the total collected and forfeited incentives for the position.
 	collectedIncentivesForPosition := sdk.Coins{}
+	forfeitedIncentivesForPosition := sdk.Coins{}
 
 	supportedUptimes := types.SupportedUptimes
 
@@ -546,27 +551,30 @@ func (k Keeper) claimAllIncentivesForPosition(ctx sdk.Context, positionId uint64
 		// Check if the accumulator contains the position.
 		hasPosition, err := uptimeAccum.HasPosition(positionName)
 		if err != nil {
-			return sdk.Coins{}, err
+			return sdk.Coins{}, sdk.Coins{}, err
 		}
 
 		// If the accumulator contains the position, claim the position's incentives.
 		if hasPosition {
 			collectedIncentivesForUptime, err := prepareAccumAndClaimRewards(uptimeAccum, positionName, uptimeGrowthOutside[uptimeIndex])
 			if err != nil {
-				return sdk.Coins{}, err
+				return sdk.Coins{}, sdk.Coins{}, err
 			}
 
 			// If the claimed incentives are forfeited, deposit them back into the accumulator to be distributed
 			// to other qualifying positions.
 			if positionAge < supportedUptimes[uptimeIndex] {
 				uptimeAccum.AddToAccumulator(sdk.NewDecCoinsFromCoins(collectedIncentivesForUptime...))
+
+				forfeitedIncentivesForPosition = forfeitedIncentivesForPosition.Add(collectedIncentivesForUptime...)
+				continue
 			}
 
 			collectedIncentivesForPosition = collectedIncentivesForPosition.Add(collectedIncentivesForUptime...)
 		}
 	}
 
-	return collectedIncentivesForPosition, nil
+	return collectedIncentivesForPosition, forfeitedIncentivesForPosition, nil
 }
 
 // collectIncentives collects incentives for all uptime accumulators for the specified position id.
@@ -583,7 +591,8 @@ func (k Keeper) collectIncentives(ctx sdk.Context, owner sdk.AccAddress, positio
 	}
 
 	// Claim all incentives for the position.
-	collectedIncentivesForPosition, err := k.claimAllIncentivesForPosition(ctx, position.PositionId)
+	// TODO: consider returning forfeited rewards as well
+	collectedIncentivesForPosition, _, err := k.claimAllIncentivesForPosition(ctx, position.PositionId)
 	if err != nil {
 		return sdk.Coins{}, err
 	}