diff --git a/protocol/x/subaccounts/keeper/isolated_subaccount.go b/protocol/x/subaccounts/keeper/isolated_subaccount.go index 0424934aea..50798a3805 100644 --- a/protocol/x/subaccounts/keeper/isolated_subaccount.go +++ b/protocol/x/subaccounts/keeper/isolated_subaccount.go @@ -1,6 +1,8 @@ package keeper import ( + "math" + errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/dydxprotocol/v4-chain/protocol/lib" @@ -10,7 +12,6 @@ import ( // checkIsolatedSubaccountConstaints will validate all `updates` to the relevant subaccounts against // isolated subaccount constraints. -// The `updates` have to contain `Subaccounts` with unique `SubaccountIds`. // The input subaccounts must be settled. // // Returns a `success` value of `true` if all updates are valid. @@ -21,6 +22,7 @@ func (k Keeper) checkIsolatedSubaccountConstraints( ctx sdk.Context, settledUpdates []settledUpdate, perpetuals []perptypes.Perpetual, + uniqueSubaccounts bool, ) ( success bool, successPerUpdate []types.UpdateResult, @@ -36,8 +38,10 @@ func (k Keeper) checkIsolatedSubaccountConstraints( } for i, u := range settledUpdates { - if _, exists := idOfSettledUpdates[*u.SettledSubaccount.Id]; exists { - return false, nil, types.ErrNonUniqueUpdatesSubaccount + if uniqueSubaccounts { + if _, exists := idOfSettledUpdates[*u.SettledSubaccount.Id]; exists { + return false, nil, types.ErrNonUniqueUpdatesSubaccount + } } result, err := isValidIsolatedPerpetualUpdates(u, perpIdToMarketType) @@ -77,7 +81,7 @@ func isValidIsolatedPerpetualUpdates( // Check if the updates contain an update to an isolated perpetual. hasIsolatedUpdate := false - isolatedUpdatePerpetualId := uint32(0) + isolatedUpdatePerpetualId := uint32(math.MaxUint32) for _, perpetualUpdate := range settledUpdate.PerpetualUpdates { marketType, exists := perpIdToMarketType[perpetualUpdate.PerpetualId] if !exists { @@ -96,7 +100,7 @@ func isValidIsolatedPerpetualUpdates( // Check if the subaccount has a position in an isolated perpetual. // Assumes the subaccount itself does not violate the isolated perpetual constraints. isIsolatedSubaccount := false - isolatedPositionPerpetualId := uint32(0) + isolatedPositionPerpetualId := uint32(math.MaxUint32) hasPerpetualPositions := len(settledUpdate.SettledSubaccount.PerpetualPositions) > 0 for _, perpetualPosition := range settledUpdate.SettledSubaccount.PerpetualPositions { marketType, exists := perpIdToMarketType[perpetualPosition.PerpetualId] diff --git a/protocol/x/subaccounts/keeper/subaccount.go b/protocol/x/subaccounts/keeper/subaccount.go index f773d92f33..8df5cb0388 100644 --- a/protocol/x/subaccounts/keeper/subaccount.go +++ b/protocol/x/subaccounts/keeper/subaccount.go @@ -280,25 +280,29 @@ func (k Keeper) UpdateSubaccounts( return false, nil, err } + // Check if the updates satisfy the isolated perpetual constraints. + allPerps := k.perpetualsKeeper.GetAllPerpetuals(ctx) + success, successPerUpdate, err = k.checkIsolatedSubaccountConstraints( + ctx, + settledUpdates, + allPerps, + true, // uniqueSubaccounts + ) + if !success || err != nil { + return success, successPerUpdate, err + } + success, successPerUpdate, err = k.internalCanUpdateSubaccounts(ctx, settledUpdates, updateType) if !success || err != nil { return success, successPerUpdate, err } // Get a mapping from perpetual Id to current perpetual funding index. - allPerps := k.perpetualsKeeper.GetAllPerpetuals(ctx) perpIdToFundingIndex := make(map[uint32]dtypes.SerializableInt) for _, perp := range allPerps { perpIdToFundingIndex[perp.Params.Id] = perp.FundingIndex } - // Check if the updates satisfy the isolated perpetual constraints. - success, successPerUpdate, err = k.checkIsolatedSubaccountConstraints( - ctx, settledUpdates, allPerps) - if !success || err != nil { - return success, successPerUpdate, err - } - // Apply the updates to perpetual positions. UpdatePerpetualPositions( settledUpdates, @@ -385,6 +389,18 @@ func (k Keeper) CanUpdateSubaccounts( return false, nil, err } + // Check if the updates satisfy the isolated perpetual constraints. + allPerps := k.perpetualsKeeper.GetAllPerpetuals(ctx) + success, successPerUpdate, err = k.checkIsolatedSubaccountConstraints( + ctx, + settledUpdates, + allPerps, + false, // uniqueSubaccounts + ) + if !success || err != nil { + return success, successPerUpdate, err + } + return k.internalCanUpdateSubaccounts(ctx, settledUpdates, updateType) } diff --git a/protocol/x/subaccounts/keeper/subaccount_test.go b/protocol/x/subaccounts/keeper/subaccount_test.go index a963c77aff..49fcaab685 100644 --- a/protocol/x/subaccounts/keeper/subaccount_test.go +++ b/protocol/x/subaccounts/keeper/subaccount_test.go @@ -3638,6 +3638,130 @@ func TestCanUpdateSubaccounts(t *testing.T) { }, }, }, + "Isolated subaccounts - has update for both an isolated perpetual and non-isolated perpetual": { + assetPositions: testutil.CreateUsdcAssetPosition(big.NewInt(1_000_000_000_000)), + expectedSuccess: false, + expectedSuccessPerUpdate: []types.UpdateResult{types.ViolatesIsolatedSubaccountConstraints}, + perpetuals: []perptypes.Perpetual{ + constants.BtcUsd_NoMarginRequirement, + constants.IsoUsd_IsolatedMarket, + }, + updates: []types.Update{ + { + PerpetualUpdates: []types.PerpetualUpdate{ + { + PerpetualId: uint32(0), + BigQuantumsDelta: big.NewInt(-100_000_000), // -1 BTC + }, + { + PerpetualId: uint32(3), + BigQuantumsDelta: big.NewInt(1_000_000_000), // 1 ISO + }, + }, + }, + }, + }, + "Isolated subaccounts - has update for both 2 isolated perpetuals": { + assetPositions: testutil.CreateUsdcAssetPosition(big.NewInt(1_000_000_000_000)), + expectedSuccess: false, + expectedSuccessPerUpdate: []types.UpdateResult{types.ViolatesIsolatedSubaccountConstraints}, + perpetuals: []perptypes.Perpetual{ + constants.IsoUsd_IsolatedMarket, + constants.Iso2Usd_IsolatedMarket, + }, + updates: []types.Update{ + { + PerpetualUpdates: []types.PerpetualUpdate{ + { + PerpetualId: uint32(3), + BigQuantumsDelta: big.NewInt(-1_000_000_000), // 1 ISO + }, + { + PerpetualId: uint32(4), + BigQuantumsDelta: big.NewInt(10_000_000), // 1 ISO2 + }, + }, + }, + }, + }, + "Isolated subaccounts - subaccount with isolated perpetual position has update for non-isolated perpetual": { + assetPositions: testutil.CreateUsdcAssetPosition(big.NewInt(1_000_000_000_000)), + expectedSuccess: false, + expectedSuccessPerUpdate: []types.UpdateResult{types.ViolatesIsolatedSubaccountConstraints}, + perpetuals: []perptypes.Perpetual{ + constants.BtcUsd_NoMarginRequirement, + constants.IsoUsd_IsolatedMarket, + }, + perpetualPositions: []*types.PerpetualPosition{ + { + PerpetualId: uint32(3), + Quantums: dtypes.NewInt(1_000_000_000), // 1 ISO + FundingIndex: dtypes.NewInt(0), + }, + }, + updates: []types.Update{ + { + PerpetualUpdates: []types.PerpetualUpdate{ + { + PerpetualId: uint32(0), + BigQuantumsDelta: big.NewInt(-100_000_000), // -1 BTC + }, + }, + }, + }, + }, + "Isolated subaccounts - subaccount with isolated perpetual position has update for another isolated perpetual": { + assetPositions: testutil.CreateUsdcAssetPosition(big.NewInt(1_000_000_000_000)), + expectedSuccess: false, + expectedSuccessPerUpdate: []types.UpdateResult{types.ViolatesIsolatedSubaccountConstraints}, + perpetuals: []perptypes.Perpetual{ + constants.IsoUsd_IsolatedMarket, + constants.Iso2Usd_IsolatedMarket, + }, + perpetualPositions: []*types.PerpetualPosition{ + { + PerpetualId: uint32(3), + Quantums: dtypes.NewInt(1_000_000_000), // 1 ISO + FundingIndex: dtypes.NewInt(0), + }, + }, + updates: []types.Update{ + { + PerpetualUpdates: []types.PerpetualUpdate{ + { + PerpetualId: uint32(4), + BigQuantumsDelta: big.NewInt(-10_000_000), // -1 ISO2 + }, + }, + }, + }, + }, + "Isolated subaccounts - subaccount with non-isolated perpetual position has update for isolated perpetual": { + assetPositions: testutil.CreateUsdcAssetPosition(big.NewInt(1_000_000_000_000)), + expectedSuccess: false, + expectedSuccessPerUpdate: []types.UpdateResult{types.ViolatesIsolatedSubaccountConstraints}, + perpetuals: []perptypes.Perpetual{ + constants.BtcUsd_NoMarginRequirement, + constants.IsoUsd_IsolatedMarket, + }, + perpetualPositions: []*types.PerpetualPosition{ + { + PerpetualId: uint32(0), + Quantums: dtypes.NewInt(100_000_000), // 1 BTC + FundingIndex: dtypes.NewInt(0), + }, + }, + updates: []types.Update{ + { + PerpetualUpdates: []types.PerpetualUpdate{ + { + PerpetualId: uint32(3), + BigQuantumsDelta: big.NewInt(-1_000_000_000), // -1 ISO + }, + }, + }, + }, + }, } for name, tc := range tests {