Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary ProductKeeper abstraction #1765

Merged
merged 1 commit into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}
Loading