fix: 🎨 move to bottom function invalid orderId -> count relationship

Author: soring323

modules/apps/100-atomic-swap/keeper/atomic_order.go

@@ -123,28 +123,17 @@ func GetBidIDFromBytes(bz []byte) uint64 {
 }
 
 func (k Keeper) MoveOrderToBottom(ctx sdk.Context, orderId string) error {
-	// Step 1: Retrieve the item based on the given ID.
+	// Step 1: Retrieve the order based on the given ID.
 	order, found := k.GetAtomicOrder(ctx, orderId)
 	if !found {
 		return types.ErrNotFoundOrder
 	}
-	// Step 2: Remove the item from its current position.
+	// Step 2: Remove the original order from its position.
 	k.RemoveOrder(ctx, orderId)
-
-	// Step 3: Get the current count (which will be the new position for the order).
-	newCount := k.GetAtomicOrderCount(ctx)
-
-	// Step 4: Set the order at its new position.
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.OTCOrderBookKey)
-	bz := k.cdc.MustMarshal(&order)
-	store.Set(GetOrderIDBytes(newCount), bz)
-
-	// Step 5: Update the orderId -> count relationship.
-	k.SetAtomicOrderCountToOrderID(ctx, orderId, newCount)
-
-	// Step 6: Increment the total order count.
-	k.SetAtomicOrderCount(ctx, newCount+1)
-
+	// Step 3: Append the order to the end of the list.
+	// (Since AppendAtomicOrder manages the order count and index mapping,
+	// we can reuse this function to simplify our logic.)
+	k.AppendAtomicOrder(ctx, order)
 	return nil
 }
 

modules/apps/100-atomic-swap/keeper/atomic_order_test.go

@@ -59,7 +59,6 @@ func (suite *KeeperTestSuite) TestMoveOrderToBottom() {
 			CancelTimestamp: int64(i),
 		})
 	}
-
 	// Move the third order to the bottom
 	err := k.MoveOrderToBottom(ctx, orderIDs[2])
 	suite.Require().NoError(err)
@@ -73,6 +72,15 @@ func (suite *KeeperTestSuite) TestMoveOrderToBottom() {
 	movedOrderCount := k.GetAtomicOrderCountByOrderId(ctx, orderIDs[2])
 	lastOrderCount := k.GetAtomicOrderCount(ctx) - 1
 	suite.Require().Equal(movedOrderCount, lastOrderCount)
+
+	// Verify that all other orders have maintained their original sequence
+	for i := 0; i < len(orderIDs)-1; i++ { // Exclude the last order (since it was moved)
+		if i < 2 { // For orders before the moved order
+			suite.Require().Equal(ordersAfterMove[i].Id, orderIDs[i])
+		} else { // For orders after the moved order
+			suite.Require().Equal(ordersAfterMove[i].Id, orderIDs[i+1])
+		}
+	}
 }
 
 func (suite *KeeperTestSuite) TestTrimExcessOrders() {

modules/apps/100-atomic-swap/keeper/msg_server.go

@@ -154,8 +154,7 @@ func (k Keeper) OnReceivedMake(ctx sdk.Context, packet channeltypes.Packet, orde
 		Maker:  msg,
 	}
 
-	k.SetAtomicOrder(ctx, order)
-
+	k.AppendAtomicOrder(ctx, order)
 	ctx.EventManager().EmitTypedEvents(msg)
 	ctx.EventManager().EmitEvent(
 		sdk.NewEvent(

modules/apps/101-interchain-swap/keeper/interchain_liquidity_pool.go

@@ -10,15 +10,6 @@ import (
 	"github.com/sideprotocol/ibcswap/v6/modules/apps/101-interchain-swap/types"
 )
 
-// // SetInterchainLiquidityPool set a specific interchainLiquidityPool in the store from its index
-// func (k Keeper) SetInterchainLiquidityPool(ctx sdk.Context, interchainLiquidityPool types.InterchainLiquidityPool) {
-// 	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.InterchainLiquidityPoolKeyPrefix))
-// 	b := k.cdc.MustMarshal(&interchainLiquidityPool)
-// 	store.Set(types.InterchainLiquidityPoolKey(
-// 		interchainLiquidityPool.Id,
-// 	), b)
-// }
-
 // SetInterchainLiquidityPool set a specific interchainLiquidityPool in the store from its index
 func (k Keeper) SetInitialPoolAssets(ctx sdk.Context, poolId string, tokens sdk.Coins) {
 	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(poolId))
@@ -71,24 +62,24 @@ func (k Keeper) GetInitialPoolAssets(ctx sdk.Context, poolId string) sdk.Coins {
 	return tokens
 }
 
-// GetInterchainLiquidityPool returns a interchainLiquidityPool from its index
-func (k Keeper) GetInterchainLiquidityPool(
-	ctx sdk.Context,
-	poolId string,
+// // GetInterchainLiquidityPool returns a interchainLiquidityPool from its index
+// func (k Keeper) GetInterchainLiquidityPool(
+// 	ctx sdk.Context,
+// 	poolId string,
 
-) (val types.InterchainLiquidityPool, found bool) {
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.InterchainLiquidityPoolKeyPrefix))
+// ) (val types.InterchainLiquidityPool, found bool) {
 
-	b := store.Get(types.InterchainLiquidityPoolKey(
-		poolId,
-	))
-	if b == nil {
-		return val, false
-	}
+// 	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.InterchainLiquidityPoolKeyPrefix))
+// 	b := store.Get(types.InterchainLiquidityPoolKey(
+// 		poolId,
+// 	))
+// 	if b == nil {
+// 		return val, false
+// 	}
 
-	k.cdc.MustUnmarshal(b, &val)
-	return val, true
-}
+// 	k.cdc.MustUnmarshal(b, &val)
+// 	return val, true
+// }
 
 // RemoveInterchainLiquidityPool removes a interchainLiquidityPool from the store
 func (k Keeper) RemoveInterchainLiquidityPool(
@@ -102,50 +93,32 @@ func (k Keeper) RemoveInterchainLiquidityPool(
 	))
 }
 
-// // GetAllInterchainLiquidityPool returns all interchainLiquidityPool
-// func (k Keeper) GetAllInterchainLiquidityPool(ctx sdk.Context) (list []types.InterchainLiquidityPool) {
+// func (k Keeper) SetInterchainLiquidityPool(ctx sdk.Context, interchainLiquidityPool types.InterchainLiquidityPool) {
 // 	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.InterchainLiquidityPoolKeyPrefix))
-// 	iterator := sdk.KVStorePrefixIterator(store, []byte{})
-
-// 	defer iterator.Close()
-
-// 	for ; iterator.Valid(); iterator.Next() {
-// 		var val types.InterchainLiquidityPool
-// 		k.cdc.MustUnmarshal(iterator.Value(), &val)
-// 		list = append(list, val)
-// 	}
-// 	return
-// }
-
-// CurrentPoolCountKey stores the current number of pools.
-var CurrentPoolCountKey = []byte("CurrentPoolCount")
-
-func (k Keeper) SetInterchainLiquidityPool(ctx sdk.Context, interchainLiquidityPool types.InterchainLiquidityPool) {
-	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.InterchainLiquidityPoolKeyPrefix))
 
-	// Get current pool count
-	poolCount := k.GetPoolCount(ctx)
+// 	// Get current pool count
+// 	poolCount := k.GetPoolCount(ctx)
 
-	// Increment the count
-	poolCount++
+// 	// Increment the count
+// 	poolCount++
 
-	// Set the new count
-	k.SetPoolCount(ctx, poolCount)
+// 	// Set the new count
+// 	k.SetPoolCount(ctx, poolCount)
 
-	// Marshal the pool and set in store
-	b := k.cdc.MustMarshal(&interchainLiquidityPool)
-	store.Set(GetInterchainLiquidityPoolKey(poolCount), b)
+// 	// Marshal the pool and set in store
+// 	b := k.cdc.MustMarshal(&interchainLiquidityPool)
+// 	store.Set(GetInterchainLiquidityPoolKey(poolCount), b)
 
-	// Check if we exceed max pools
-	if poolCount > types.MaxPoolCount {
-		// Remove the oldest pool
-		store.Delete(GetInterchainLiquidityPoolKey(poolCount - types.MaxPoolCount))
-	}
-}
+// 	// Check if we exceed max pools
+// 	if poolCount > types.MaxPoolCount {
+// 		// Remove the oldest pool
+// 		store.Delete(GetInterchainLiquidityPoolKey(poolCount - types.MaxPoolCount))
+// 	}
+// }
 
 func (k Keeper) GetPoolCount(ctx sdk.Context) uint64 {
 	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.InterchainLiquidityPoolKeyPrefix))
-	b := store.Get(CurrentPoolCountKey)
+	b := store.Get(types.CurrentPoolCountKey)
 	if b == nil {
 		return 0
 	}
@@ -156,7 +129,7 @@ func (k Keeper) SetPoolCount(ctx sdk.Context, count uint64) {
 	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.InterchainLiquidityPoolKeyPrefix))
 	b := make([]byte, 8)
 	binary.BigEndian.PutUint64(b, count)
-	store.Set(CurrentPoolCountKey, b)
+	store.Set(types.CurrentPoolCountKey, b)
 }
 
 func GetInterchainLiquidityPoolKey(count uint64) []byte {
@@ -180,3 +153,66 @@ func (k Keeper) GetAllInterchainLiquidityPool(ctx sdk.Context) (list []types.Int
 	}
 	return
 }
+
+// Sets the mapping between poolId and its count index
+func (k Keeper) SetPoolIdToCountMapping(ctx sdk.Context, poolId string, count uint64) {
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.PoolIdToCountKeyPrefix)
+	b := make([]byte, 8)
+	binary.BigEndian.PutUint64(b, count)
+	store.Set([]byte(poolId), b)
+}
+
+// Gets the count index of the poolId
+func (k Keeper) GetCountByPoolId(ctx sdk.Context, poolId string) (count uint64, found bool) {
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.PoolIdToCountKeyPrefix)
+	b := store.Get([]byte(poolId))
+	if b == nil {
+		return 0, false
+	}
+	return binary.BigEndian.Uint64(b), true
+}
+
+// Modified SetInterchainLiquidityPool
+func (k Keeper) SetInterchainLiquidityPool(ctx sdk.Context, interchainLiquidityPool types.InterchainLiquidityPool) {
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.InterchainLiquidityPoolKeyPrefix))
+
+	// Get current pool count
+	poolCount := k.GetPoolCount(ctx)
+
+	// Increment the count
+	poolCount++
+
+	// Set the new count
+	k.SetPoolCount(ctx, poolCount)
+
+	// Set the poolId to count mapping
+	k.SetPoolIdToCountMapping(ctx, interchainLiquidityPool.Id, poolCount)
+
+	// Marshal the pool and set in store
+	b := k.cdc.MustMarshal(&interchainLiquidityPool)
+	store.Set(GetInterchainLiquidityPoolKey(poolCount), b)
+
+	// Check if we exceed max pools
+	if poolCount > types.MaxPoolCount {
+		// Remove the oldest pool
+		store.Delete(GetInterchainLiquidityPoolKey(poolCount - types.MaxPoolCount))
+	}
+}
+
+// Modified GetInterchainLiquidityPool
+func (k Keeper) GetInterchainLiquidityPool(ctx sdk.Context, poolId string) (val types.InterchainLiquidityPool, found bool) {
+	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.InterchainLiquidityPoolKeyPrefix))
+
+	count, found := k.GetCountByPoolId(ctx, poolId)
+	if !found {
+		return val, false
+	}
+
+	b := store.Get(GetInterchainLiquidityPoolKey(count))
+	if b == nil {
+		return val, false
+	}
+
+	k.cdc.MustUnmarshal(b, &val)
+	return val, true
+}

modules/apps/101-interchain-swap/keeper/interchain_liquidity_pool_test.go

@@ -10,15 +10,38 @@ func (suite *KeeperTestSuite) TestSetAndGetInterchainLiquidityPool() {
 	ctx := suite.chainA.GetContext()
 	k := suite.chainA.GetSimApp().InterchainSwapKeeper
 
-	pool := types.InterchainLiquidityPool{
-		Id: "pool1",
+	// Set a few pools
+	poolCount := 5 // Let's say you want to set 5 pools for this test. Adjust as needed.
+	createdPools := make([]types.InterchainLiquidityPool, poolCount)
+
+	for i := 1; i <= poolCount; i++ {
+		pool := types.InterchainLiquidityPool{
+			Id: fmt.Sprintf("pool%d", i),
+		}
+		createdPools[i-1] = pool
+		k.SetInterchainLiquidityPool(ctx, pool)
 	}
 
-	k.SetInterchainLiquidityPool(ctx, pool)
+	// Get and verify each set pool
+	for _, createdPool := range createdPools {
+		retrievedPool, found := k.GetInterchainLiquidityPool(ctx, createdPool.Id)
+		suite.Require().True(found, "Expected to find the set pool.")
+		suite.Require().Equal(createdPool, retrievedPool, "The set pool did not match the retrieved pool.")
+	}
 
-	retrievedPool, found := k.GetInterchainLiquidityPool(ctx, pool.Id)
-	suite.Require().True(found, "Expected to find the set pool.")
-	suite.Require().Equal(pool, retrievedPool, "The set pool did not match the retrieved pool.")
+	// Now, let's test exceeding the max pool count (if applicable)
+	// You'd need to set pools greater than the `types.MaxPoolCount`, and then ensure that the oldest pools are no longer retrievable.
+	if poolCount < types.MaxPoolCount {
+		for i := poolCount + 1; i <= types.MaxPoolCount+1; i++ { // +1 to exceed max count by one
+			pool := types.InterchainLiquidityPool{
+				Id: fmt.Sprintf("pool%d", i),
+			}
+			k.SetInterchainLiquidityPool(ctx, pool)
+		}
+		// Now, the oldest pool (pool1) should not be found
+		_, found := k.GetInterchainLiquidityPool(ctx, "pool1")
+		suite.Require().False(found, "Expected not to find the oldest pool.")
+	}
 }
 
 func (suite *KeeperTestSuite) TestGetAllInterchainLiquidityPools() {

modules/apps/101-interchain-swap/keeper/msg_server_cancel_pool.go

@@ -72,7 +72,7 @@ func (k msgServer) CancelPool(ctx context.Context, msg *types.MsgCancelPoolReque
 
 	sdkCtx.EventManager().EmitEvent(
 		sdk.NewEvent(
-			types.EventTypeMakePool,
+			types.EventTypeCancelPool,
 			sdk.Attribute{
 				Key:   types.AttributeKeyPoolId,
 				Value: msg.PoolId,

modules/apps/101-interchain-swap/types/keys.go

@@ -30,7 +30,9 @@ const (
 
 var (
 	// PortKey defines the key to store the port ID in store
-	PortKey = []byte{0x01}
+	PortKey                = []byte{0x01}
+	PoolIdToCountKeyPrefix = []byte{0x02}
+	CurrentPoolCountKey    = []byte{0x03}
 )
 
 func KeyPrefix(p string) []byte {