Fix: deterministically fetch perp info from state (backport #2341) (#2354)

Co-authored-by: ttl33 <19664986+ttl33@users.noreply.github.com>

Author: mergify[bot]

protocol/testutil/constants/positions.go

@@ -73,6 +73,12 @@ var (
 		PerpetualId: 1,
 		Quantums:    dtypes.NewIntFromBigInt(BigNegMaxUint64()), // 18,446,744,070 ETH, -$55,340,232,210,000 notional.
 	}
+	// SOL positions
+	PerpetualPosition_OneSolLong = satypes.PerpetualPosition{
+		PerpetualId:  2,
+		Quantums:     dtypes.NewInt(100_000_000_000), // 1 SOL
+		FundingIndex: dtypes.NewInt(0),
+	}
 	// Long position for arbitrary isolated market
 	PerpetualPosition_OneISOLong = satypes.PerpetualPosition{
 		PerpetualId:  3,

protocol/x/subaccounts/keeper/subaccount.go

@@ -1027,29 +1027,32 @@ func (k Keeper) GetAllRelevantPerpetuals(
 	map[uint32]perptypes.PerpInfo,
 	error,
 ) {
-	subaccountIds := make(map[types.SubaccountId]struct{})
-	perpIds := make(map[uint32]struct{})
+	subaccountIdsSet := make(map[types.SubaccountId]struct{})
+	perpIdsSet := make(map[uint32]struct{})
 
 	// Add all relevant perpetuals in every update.
 	for _, update := range updates {
 		// If this subaccount has not been processed already, get all of its existing perpetuals.
-		if _, exists := subaccountIds[update.SubaccountId]; !exists {
+		if _, exists := subaccountIdsSet[update.SubaccountId]; !exists {
 			sa := k.GetSubaccount(ctx, update.SubaccountId)
 			for _, postition := range sa.PerpetualPositions {
-				perpIds[postition.PerpetualId] = struct{}{}
+				perpIdsSet[postition.PerpetualId] = struct{}{}
 			}
-			subaccountIds[update.SubaccountId] = struct{}{}
+			subaccountIdsSet[update.SubaccountId] = struct{}{}
 		}
 
 		// Add all perpetuals in the update.
 		for _, perpUpdate := range update.PerpetualUpdates {
-			perpIds[perpUpdate.GetId()] = struct{}{}
+			perpIdsSet[perpUpdate.GetId()] = struct{}{}
 		}
 	}
 
+	// Important: Sort the perpIds to ensure determinism!
+	sortedPerpIds := lib.GetSortedKeys[lib.Sortable[uint32]](perpIdsSet)
+
 	// Get all perpetual information from state.
-	perpetuals := make(map[uint32]perptypes.PerpInfo, len(perpIds))
-	for perpId := range perpIds {
+	perpetuals := make(map[uint32]perptypes.PerpInfo, len(sortedPerpIds))
+	for _, perpId := range sortedPerpIds {
 		perpetual,
 			price,
 			liquidityTier,

protocol/x/subaccounts/keeper/subaccount_test.go

@@ -10,13 +10,15 @@ import (
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	"github.com/dydxprotocol/v4-chain/protocol/lib"
 
+	storetypes "cosmossdk.io/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/dydxprotocol/v4-chain/protocol/dtypes"
 	indexerevents "github.com/dydxprotocol/v4-chain/protocol/indexer/events"
 	bank_testutil "github.com/dydxprotocol/v4-chain/protocol/testutil/bank"
 	big_testutil "github.com/dydxprotocol/v4-chain/protocol/testutil/big"
 	"github.com/dydxprotocol/v4-chain/protocol/testutil/constants"
+	keepertest "github.com/dydxprotocol/v4-chain/protocol/testutil/keeper"
 	testutil "github.com/dydxprotocol/v4-chain/protocol/testutil/keeper"
 	"github.com/dydxprotocol/v4-chain/protocol/testutil/nullify"
 	perptest "github.com/dydxprotocol/v4-chain/protocol/testutil/perpetuals"
@@ -5937,3 +5939,115 @@ func TestIsValidStateTransitionForUndercollateralizedSubaccount_ZeroMarginRequir
 		})
 	}
 }
+
+func TestGetAllRelevantPerpetuals_Deterministic(t *testing.T) {
+	tests := map[string]struct {
+		// state
+		perpetuals []perptypes.Perpetual
+
+		// subaccount state
+		assetPositions     []*types.AssetPosition
+		perpetualPositions []*types.PerpetualPosition
+
+		// updates
+		assetUpdates     []types.AssetUpdate
+		perpetualUpdates []types.PerpetualUpdate
+	}{
+		"Gas used is deterministic when erroring on gas usage": {
+			assetPositions: testutil.CreateUsdcAssetPosition(big.NewInt(10_000_000_001)), // $10,000.000001
+			perpetuals: []perptypes.Perpetual{
+				constants.BtcUsd_NoMarginRequirement,
+				constants.EthUsd_NoMarginRequirement,
+				constants.SolUsd_20PercentInitial_10PercentMaintenance,
+			},
+			perpetualPositions: []*types.PerpetualPosition{
+				&constants.PerpetualPosition_OneBTCLong,
+				&constants.PerpetualPosition_OneTenthEthLong,
+				&constants.PerpetualPosition_OneSolLong,
+			},
+			assetUpdates: []types.AssetUpdate{
+				{
+					AssetId:          constants.Usdc.Id,
+					BigQuantumsDelta: big.NewInt(1_000_000), // +1 USDC
+				},
+			},
+			perpetualUpdates: []types.PerpetualUpdate{
+				{
+					PerpetualId:      uint32(0),
+					BigQuantumsDelta: big.NewInt(-200_000_000), // -2 BTC
+				},
+				{
+					PerpetualId:      uint32(1),
+					BigQuantumsDelta: big.NewInt(250_000_000), // .25 ETH
+				},
+				{
+					PerpetualId:      uint32(2),
+					BigQuantumsDelta: big.NewInt(500_000_000), // .005 SOL
+				},
+			},
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			// Setup.
+			ctx, keeper, pricesKeeper, perpetualsKeeper, _, _, assetsKeeper, _, _ := keepertest.SubaccountsKeepers(
+				t,
+				true,
+			)
+			keepertest.CreateTestMarkets(t, ctx, pricesKeeper)
+			keepertest.CreateTestLiquidityTiers(t, ctx, perpetualsKeeper)
+			keepertest.CreateTestPerpetuals(t, ctx, perpetualsKeeper)
+			for _, p := range tc.perpetuals {
+				perpetualsKeeper.SetPerpetualForTest(ctx, p)
+			}
+			require.NoError(t, keepertest.CreateUsdcAsset(ctx, assetsKeeper))
+
+			subaccount := createNSubaccount(keeper, ctx, 1, big.NewInt(1_000))[0]
+			subaccount.PerpetualPositions = tc.perpetualPositions
+			subaccount.AssetPositions = tc.assetPositions
+			keeper.SetSubaccount(ctx, subaccount)
+			subaccountId := *subaccount.Id
+
+			update := types.Update{
+				SubaccountId:     subaccountId,
+				AssetUpdates:     tc.assetUpdates,
+				PerpetualUpdates: tc.perpetualUpdates,
+			}
+
+			// Execute.
+			gasUsedBefore := ctx.GasMeter().GasConsumed()
+			_, err := keeper.GetAllRelevantPerpetuals(ctx, []types.Update{update})
+			require.NoError(t, err)
+			gasUsedAfter := ctx.GasMeter().GasConsumed()
+
+			gasUsed := uint64(0)
+			// Run 100 times since it's highly unlikely gas usage is deterministic over 100 times if
+			// there's non-determinism.
+			for range 100 {
+				// divide by 2 so that the state read fails at least second to last time.
+				ctxWithLimitedGas := ctx.WithGasMeter(storetypes.NewGasMeter((gasUsedAfter - gasUsedBefore) / 2))
+
+				require.PanicsWithValue(
+					t,
+					storetypes.ErrorOutOfGas{Descriptor: "ReadPerByte"},
+					func() {
+						_, _ = keeper.GetAllRelevantPerpetuals(ctxWithLimitedGas, []types.Update{update})
+					},
+				)
+
+				if gasUsed == 0 {
+					gasUsed = ctxWithLimitedGas.GasMeter().GasConsumed()
+					require.Greater(t, gasUsed, uint64(0))
+				} else {
+					require.Equal(
+						t,
+						gasUsed,
+						ctxWithLimitedGas.GasMeter().GasConsumed(),
+						"Gas usage when out of gas is not deterministic",
+					)
+				}
+			}
+		})
+	}
+}