Skip to content

Commit

Permalink
Remove unnecessary ProductKeeper abstraction (#1765)
Browse files Browse the repository at this point in the history
  • Loading branch information
BrendanChou authored Jun 25, 2024
1 parent f7984a6 commit 0c9637e
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 68 deletions.
9 changes: 4 additions & 5 deletions protocol/testing/e2e/gov/add_new_market_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,13 @@ import (
delaymsgtypes "github.com/dydxprotocol/v4-chain/protocol/x/delaymsg/types"
perptypes "github.com/dydxprotocol/v4-chain/protocol/x/perpetuals/types"
pricestypes "github.com/dydxprotocol/v4-chain/protocol/x/prices/types"
satypes "github.com/dydxprotocol/v4-chain/protocol/x/subaccounts/types"
"github.com/stretchr/testify/require"
)

const (
NumBlocksAfterTradingEnabled = 50
TestMarketId = 1001
// Expected response log when a order is submitted but oracle price is zero.
// https://github.com/dydxprotocol/v4-chain/blob/5ee11ed/protocol/x/perpetuals/keeper/perpetual.go#L1514-L1517
ExpectedPlaceOrderCheckTxResponseLog = "recovered: type: perpetual, id: 1001: product position is not updatable"
)

var (
Expand Down Expand Up @@ -392,9 +390,10 @@ func TestAddNewMarketProposal(t *testing.T) {
require.Conditionf(t, resp.IsErr, "Expected CheckTx to error. Response: %+v", resp)
require.Contains(t,
resp.Log,
ExpectedPlaceOrderCheckTxResponseLog,
satypes.ErrProductPositionNotUpdatable.Error(),
"expected CheckTx response log to contain: %s, got: %s",
ExpectedPlaceOrderCheckTxResponseLog, resp.Log,
satypes.ErrProductPositionNotUpdatable,
resp.Log,
)

// Advance to the next block and check chain is not halted.
Expand Down
44 changes: 16 additions & 28 deletions protocol/x/subaccounts/keeper/subaccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,32 +432,6 @@ func (k Keeper) CanUpdateSubaccounts(
return success, successPerUpdate, err
}

func checkPositionUpdatable(
ctx sdk.Context,
pk types.ProductKeeper,
p types.PositionSize,
) (
err error,
) {
updatable, err := pk.IsPositionUpdatable(
ctx,
p.GetId(),
)
if err != nil {
return err
}

if !updatable {
return errorsmod.Wrapf(
types.ErrProductPositionNotUpdatable,
"type: %v, id: %d",
p.GetProductType(),
p.GetId(),
)
}
return nil
}

// internalCanUpdateSubaccounts will validate all `updates` to the relevant subaccounts and compute
// if any of the updates led to an isolated perpetual position being opened or closed.
// The `updates` do not have to contain `Subaccounts` with unique `SubaccountIds`.
Expand Down Expand Up @@ -587,18 +561,32 @@ func (k Keeper) internalCanUpdateSubaccounts(
for i, u := range settledUpdates {
// Check all updated perps are updatable.
for _, perpUpdate := range u.PerpetualUpdates {
err := checkPositionUpdatable(ctx, k.perpetualsKeeper, perpUpdate)
updatable, err := k.perpetualsKeeper.IsPositionUpdatable(ctx, perpUpdate.GetId())
if err != nil {
return false, nil, err
}
if !updatable {
return false, nil, errorsmod.Wrapf(
types.ErrProductPositionNotUpdatable,
"type: perpetual, id: %d",
perpUpdate.GetId(),
)
}
}

// Check all updated assets are updatable.
for _, assetUpdate := range u.AssetUpdates {
err := checkPositionUpdatable(ctx, k.assetsKeeper, assetUpdate)
updatable, err := k.assetsKeeper.IsPositionUpdatable(ctx, assetUpdate.GetId())
if err != nil {
return false, nil, err
}
if !updatable {
return false, nil, errorsmod.Wrapf(
types.ErrProductPositionNotUpdatable,
"type: asset, id: %d",
assetUpdate.GetId(),
)
}
}

// Get the new collateralization and margin requirements with the update applied.
Expand Down
16 changes: 8 additions & 8 deletions protocol/x/subaccounts/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,14 @@ import (
pricestypes "github.com/dydxprotocol/v4-chain/protocol/x/prices/types"
)

// ProductKeeper represents a generic interface for a keeper
// of a product.
type ProductKeeper interface {
type AssetsKeeper interface {
IsPositionUpdatable(
ctx sdk.Context,
id uint32,
) (
updatable bool,
err error,
)
}

type AssetsKeeper interface {
ProductKeeper
ConvertAssetToCoin(
ctx sdk.Context,
assetId uint32,
Expand All @@ -37,7 +31,13 @@ type AssetsKeeper interface {
}

type PerpetualsKeeper interface {
ProductKeeper
IsPositionUpdatable(
ctx sdk.Context,
id uint32,
) (
updatable bool,
err error,
)
GetPerpetual(
ctx sdk.Context,
perpetualId uint32,
Expand Down
27 changes: 0 additions & 27 deletions protocol/x/subaccounts/types/position_size.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,13 @@ import (
"github.com/dydxprotocol/v4-chain/protocol/dtypes"
)

const (
AssetProductType = "asset"
PerpetualProductType = "perpetual"
UnknownProductTYpe = "unknown"
)

// PositionSize is an interface for expressing the size of a position
type PositionSize interface {
// Returns true if and only if the position size is positive.
GetIsLong() bool
// Returns the signed position size in big.Int.
GetBigQuantums() *big.Int
GetId() uint32
GetProductType() string
}

type PositionUpdate struct {
Expand Down Expand Up @@ -74,10 +67,6 @@ func (m *AssetPosition) GetIsLong() bool {
return m.GetBigQuantums().Sign() > 0
}

func (m *AssetPosition) GetProductType() string {
return AssetProductType
}

func (m *PerpetualPosition) GetId() uint32 {
return m.GetPerpetualId()
}
Expand Down Expand Up @@ -110,10 +99,6 @@ func (m *PerpetualPosition) GetIsLong() bool {
return m.GetBigQuantums().Sign() > 0
}

func (m *PerpetualPosition) GetProductType() string {
return PerpetualProductType
}

func (au AssetUpdate) GetIsLong() bool {
return au.GetBigQuantums().Sign() > 0
}
Expand All @@ -126,10 +111,6 @@ func (au AssetUpdate) GetId() uint32 {
return au.AssetId
}

func (au AssetUpdate) GetProductType() string {
return AssetProductType
}

func (pu PerpetualUpdate) GetBigQuantums() *big.Int {
return pu.BigQuantumsDelta
}
Expand All @@ -142,10 +123,6 @@ func (pu PerpetualUpdate) GetIsLong() bool {
return pu.GetBigQuantums().Sign() > 0
}

func (pu PerpetualUpdate) GetProductType() string {
return PerpetualProductType
}

func (pu PositionUpdate) GetId() uint32 {
return pu.Id
}
Expand All @@ -161,7 +138,3 @@ func (pu PositionUpdate) SetBigQuantums(bigQuantums *big.Int) {
func (pu PositionUpdate) GetBigQuantums() *big.Int {
return pu.BigQuantums
}
func (pu PositionUpdate) GetProductType() string {
// PositionUpdate is generic and doesn't have a product type.
return UnknownProductTYpe
}

0 comments on commit 0c9637e

Please sign in to comment.