Skip to content

Commit

Permalink
[CT-1202] logic to handle unauthorized maker orders when authenticato… (
Browse files Browse the repository at this point in the history
  • Loading branch information
jayy04 authored Oct 2, 2024
1 parent 672169b commit a2eacde
Show file tree
Hide file tree
Showing 17 changed files with 229 additions and 12 deletions.
1 change: 1 addition & 0 deletions protocol/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -1139,6 +1139,7 @@ func New(
app.StatsKeeper,
app.RewardsKeeper,
app.AffiliatesKeeper,
app.AccountPlusKeeper,
app.IndexerEventManager,
app.FullNodeStreamingManager,
txConfig.TxDecoder(),
Expand Down
20 changes: 19 additions & 1 deletion protocol/mocks/MemClobKeeper.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions protocol/testutil/keeper/accountplus.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/dydxprotocol/v4-chain/protocol/lib"
"github.com/dydxprotocol/v4-chain/protocol/x/revshare/types"
"github.com/dydxprotocol/v4-chain/protocol/x/accountplus/types"

storetypes "cosmossdk.io/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -16,7 +16,7 @@ import (
keeper "github.com/dydxprotocol/v4-chain/protocol/x/accountplus/keeper"
)

func TimestampNonceKeepers(t testing.TB) (
func AccountPlusKeepers(t testing.TB) (
ctx sdk.Context,
keeper *keeper.Keeper,
storeKey storetypes.StoreKey,
Expand All @@ -32,7 +32,7 @@ func TimestampNonceKeepers(t testing.TB) (
) []GenesisInitializer {
// Define necessary keepers here for unit tests
keeper, storeKey, mockTimeProvider =
createTimestampNonceKeeper(stateStore, db, cdc)
createAccountPlusKeeper(stateStore, db, cdc)

return []GenesisInitializer{keeper}
},
Expand All @@ -41,7 +41,7 @@ func TimestampNonceKeepers(t testing.TB) (
return ctx, keeper, storeKey, mockTimeProvider
}

func createTimestampNonceKeeper(
func createAccountPlusKeeper(
stateStore storetypes.CommitMultiStore,
db *dbm.MemDB,
cdc *codec.ProtoCodec,
Expand Down
13 changes: 12 additions & 1 deletion protocol/testutil/keeper/clob.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
streaming "github.com/dydxprotocol/v4-chain/protocol/streaming"
clobtest "github.com/dydxprotocol/v4-chain/protocol/testutil/clob"
"github.com/dydxprotocol/v4-chain/protocol/testutil/constants"
accountpluskeeper "github.com/dydxprotocol/v4-chain/protocol/x/accountplus/keeper"
affiliateskeeper "github.com/dydxprotocol/v4-chain/protocol/x/affiliates/keeper"
asskeeper "github.com/dydxprotocol/v4-chain/protocol/x/assets/keeper"
blocktimekeeper "github.com/dydxprotocol/v4-chain/protocol/x/blocktime/keeper"
Expand Down Expand Up @@ -51,6 +52,7 @@ type ClobKeepersTestContext struct {
SubaccountsKeeper *subkeeper.Keeper
AffiliatesKeeper *affiliateskeeper.Keeper
VaultKeeper *vaultkeeper.Keeper
AccountPlusKeeper *accountpluskeeper.Keeper
StoreKey storetypes.StoreKey
MemKey storetypes.StoreKey
Cdc *codec.ProtoCodec
Expand Down Expand Up @@ -91,7 +93,13 @@ func NewClobKeepersTestContextWithUninitializedMemStore(
stateStore,
db,
cdc,
registry)
registry,
)
ks.AccountPlusKeeper, _, _ = createAccountPlusKeeper(
stateStore,
db,
cdc,
)

stakingKeeper, _ := createStakingKeeper(
stateStore,
Expand Down Expand Up @@ -191,6 +199,7 @@ func NewClobKeepersTestContextWithUninitializedMemStore(
ks.AffiliatesKeeper,
ks.SubaccountsKeeper,
revShareKeeper,
ks.AccountPlusKeeper,
indexerEventManager,
indexerEventsTransientStoreKey,
)
Expand Down Expand Up @@ -231,6 +240,7 @@ func createClobKeeper(
affiliatesKeeper types.AffiliatesKeeper,
saKeeper *subkeeper.Keeper,
revShareKeeper types.RevShareKeeper,
accountplusKeeper types.AccountPlusKeeper,
indexerEventManager indexer_manager.IndexerEventManager,
indexerEventsTransientStoreKey storetypes.StoreKey,
) (*keeper.Keeper, storetypes.StoreKey, storetypes.StoreKey) {
Expand Down Expand Up @@ -262,6 +272,7 @@ func createClobKeeper(
statsKeeper,
rewardsKeeper,
affiliatesKeeper,
accountplusKeeper,
indexerEventManager,
streaming.NewNoopGrpcStreamingManager(),
constants.TestEncodingCfg.TxConfig.TxDecoder(),
Expand Down
9 changes: 8 additions & 1 deletion protocol/testutil/keeper/listing.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,13 @@ func ListingKeepers(
stateStore,
db,
cdc,
registry)
registry,
)
accountPlusKeeper, _, _ := createAccountPlusKeeper(
stateStore,
db,
cdc,
)
bankKeeper, _ = createBankKeeper(stateStore, db, cdc, accountsKeeper)
stakingKeeper, _ := createStakingKeeper(
stateStore,
Expand Down Expand Up @@ -154,6 +160,7 @@ func ListingKeepers(
affiliatesKeeper,
subaccountsKeeper,
revShareKeeper,
accountPlusKeeper,
indexerEventManager,
transientStoreKey,
)
Expand Down
4 changes: 4 additions & 0 deletions protocol/testutil/memclob/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,3 +528,7 @@ func (f *FakeMemClobKeeper) AddOrderToOrderbookSubaccountUpdatesCheck(
) satypes.UpdateResult {
return satypes.Success
}

func (f *FakeMemClobKeeper) MaybeValidateAuthenticators(ctx sdk.Context, txBytes []byte) error {
return nil
}
58 changes: 58 additions & 0 deletions protocol/x/accountplus/keeper/authenticators.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,69 @@ import (
"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
gogotypes "github.com/cosmos/gogoproto/types"
"github.com/dydxprotocol/v4-chain/protocol/lib/metrics"
"github.com/dydxprotocol/v4-chain/protocol/x/accountplus/lib"
"github.com/dydxprotocol/v4-chain/protocol/x/accountplus/types"
)

// MaybeValidateAuthenticators checks if the transaction has authenticators specified and if so,
// validates them. It returns an error if the authenticators are not valid or removed from state.
func (k Keeper) MaybeValidateAuthenticators(ctx sdk.Context, tx sdk.Tx) error {
// Check if the tx had authenticator specified.
specified, txOptions := lib.HasSelectedAuthenticatorTxExtensionSpecified(tx, k.cdc)
if !specified {
return nil
}

// The tx had authenticators specified.
// First make sure smart account flow is enabled.
if active := k.GetIsSmartAccountActive(ctx); !active {
return types.ErrSmartAccountNotActive
}

// Make sure txn is a SigVerifiableTx and get signers from the tx.
sigVerifiableTx, ok := tx.(authsigning.SigVerifiableTx)
if !ok {
return errors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type")
}

signers, err := sigVerifiableTx.GetSigners()
if err != nil {
return err
}

if len(signers) != 1 {
return errors.Wrap(types.ErrTxnHasMultipleSigners, "only one signer is allowed")
}

account := sdk.AccAddress(signers[0])

// Retrieve the selected authenticators from the extension and make sure they are valid, i.e. they
// are registered and not removed from state.
//
// Note that we only verify the existence of the authenticators here without actually
// runnning them. This is because all current authenticators are stateless and do not read/modify any states.
selectedAuthenticators := txOptions.GetSelectedAuthenticators()
for _, authenticatorId := range selectedAuthenticators {
_, err := k.GetInitializedAuthenticatorForAccount(
ctx,
account,
authenticatorId,
)
if err != nil {
return errors.Wrapf(
err,
"selected authenticator (%s, %d) is not registered or removed from state",
account.String(),
authenticatorId,
)
}
}
return nil
}

// AddAuthenticator adds an authenticator to an account, this function is used to add multiple
// authenticators such as SignatureVerifications and AllOfs
func (k Keeper) AddAuthenticator(
Expand Down
5 changes: 5 additions & 0 deletions protocol/x/accountplus/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,9 @@ var (
5,
"Error initializing authenticator",
)
ErrTxnHasMultipleSigners = errorsmod.Register(
ModuleName,
6,
"The transaction has multiple signers",
)
)
11 changes: 9 additions & 2 deletions protocol/x/clob/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1211,8 +1211,15 @@ func TestPrepareCheckState(t *testing.T) {
case *types.Operation_ShortTermOrderPlacement:
order := operation.GetShortTermOrderPlacement()
tempCtx, writeCache := setupCtx.CacheContext()
tempCtx = tempCtx.WithTxBytes(order.Order.GetOrderHash().ToBytes())
_, _, err := ks.ClobKeeper.PlaceShortTermOrder(

txBuilder := constants.TestEncodingCfg.TxConfig.NewTxBuilder()
err := txBuilder.SetMsgs(operation.GetShortTermOrderPlacement())
require.NoError(t, err)
bytes, err := constants.TestEncodingCfg.TxConfig.TxEncoder()(txBuilder.GetTx())
require.NoError(t, err)
tempCtx = tempCtx.WithTxBytes(bytes)

_, _, err = ks.ClobKeeper.PlaceShortTermOrder(
tempCtx,
order,
)
Expand Down
21 changes: 21 additions & 0 deletions protocol/x/clob/keeper/authenticators.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package keeper

import (
sdk "github.com/cosmos/cosmos-sdk/types"
)

// MaybeValidateAuthenticators checks if the transaction has authenticators specified and if so,
// validates them. It returns an error if the authenticators are not valid or removed from state.
func (k Keeper) MaybeValidateAuthenticators(ctx sdk.Context, txBytes []byte) error {
// Decode the tx from the tx bytes.
tx, err := k.txDecoder(txBytes)
if err != nil {
return err
}

// Perform a light-weight validation of the authenticators via the accountplus module.
//
// Note that alternatively we could have been calling the ante handler directly on this transaction,
// but there are some deadlock issues that are non-trivial to resolve.
return k.accountPlusKeeper.MaybeValidateAuthenticators(ctx, tx)
}
3 changes: 3 additions & 0 deletions protocol/x/clob/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type (
rewardsKeeper types.RewardsKeeper
affiliatesKeeper types.AffiliatesKeeper
revshareKeeper types.RevShareKeeper
accountPlusKeeper types.AccountPlusKeeper

indexerEventManager indexer_manager.IndexerEventManager
streamingManager streamingtypes.FullNodeStreamingManager
Expand Down Expand Up @@ -88,6 +89,7 @@ func NewKeeper(
statsKeeper types.StatsKeeper,
rewardsKeeper types.RewardsKeeper,
affiliatesKeeper types.AffiliatesKeeper,
accountPlusKeeper types.AccountPlusKeeper,
indexerEventManager indexer_manager.IndexerEventManager,
streamingManager streamingtypes.FullNodeStreamingManager,
txDecoder sdk.TxDecoder,
Expand All @@ -114,6 +116,7 @@ func NewKeeper(
statsKeeper: statsKeeper,
rewardsKeeper: rewardsKeeper,
affiliatesKeeper: affiliatesKeeper,
accountPlusKeeper: accountPlusKeeper,
indexerEventManager: indexerEventManager,
streamingManager: streamingManager,
memStoreInitialized: &atomic.Bool{}, // False by default.
Expand Down
22 changes: 20 additions & 2 deletions protocol/x/clob/keeper/liquidations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,16 @@ func TestPlacePerpetualLiquidation(t *testing.T) {

// Create all existing orders.
for _, order := range tc.existingOrders {
_, _, err := ks.ClobKeeper.PlaceShortTermOrder(ctx, &types.MsgPlaceOrder{Order: order})
msg := &types.MsgPlaceOrder{Order: order}

txBuilder := constants.TestEncodingCfg.TxConfig.NewTxBuilder()
err := txBuilder.SetMsgs(msg)
require.NoError(t, err)
bytes, err := constants.TestEncodingCfg.TxConfig.TxEncoder()(txBuilder.GetTx())
require.NoError(t, err)
ctx = ctx.WithTxBytes(bytes)

_, _, err = ks.ClobKeeper.PlaceShortTermOrder(ctx, msg)
require.NoError(t, err)
}

Expand Down Expand Up @@ -1306,7 +1315,16 @@ func TestPlacePerpetualLiquidation_PreexistingLiquidation(t *testing.T) {
require.NoError(t, err)
} else {
order := matchableOrder.MustGetOrder()
_, _, err := ks.ClobKeeper.PlaceShortTermOrder(ctx, &types.MsgPlaceOrder{Order: order.MustGetOrder()})
msg := &types.MsgPlaceOrder{Order: order}

txBuilder := constants.TestEncodingCfg.TxConfig.NewTxBuilder()
err := txBuilder.SetMsgs(msg)
require.NoError(t, err)
bytes, err := constants.TestEncodingCfg.TxConfig.TxEncoder()(txBuilder.GetTx())
require.NoError(t, err)
ctx = ctx.WithTxBytes(bytes)

_, _, err = ks.ClobKeeper.PlaceShortTermOrder(ctx, msg)
require.NoError(t, err)
}
}
Expand Down
19 changes: 18 additions & 1 deletion protocol/x/clob/keeper/orders_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,16 @@ func TestPlaceShortTermOrder(t *testing.T) {

// Create all existing orders.
for _, order := range tc.existingOrders {
_, _, err := ks.ClobKeeper.PlaceShortTermOrder(ctx, &types.MsgPlaceOrder{Order: order})
msg := &types.MsgPlaceOrder{Order: order}

txBuilder := constants.TestEncodingCfg.TxConfig.NewTxBuilder()
err := txBuilder.SetMsgs(msg)
require.NoError(t, err)
bytes, err := constants.TestEncodingCfg.TxConfig.TxEncoder()(txBuilder.GetTx())
require.NoError(t, err)
ctx = ctx.WithTxBytes(bytes)

_, _, err = ks.ClobKeeper.PlaceShortTermOrder(ctx, msg)
require.NoError(t, err)
}

Expand All @@ -632,6 +641,14 @@ func TestPlaceShortTermOrder(t *testing.T) {
ctx.MultiStore().SetTracer(traceDecoder)

msg := &types.MsgPlaceOrder{Order: tc.order}

txBuilder := constants.TestEncodingCfg.TxConfig.NewTxBuilder()
err = txBuilder.SetMsgs(msg)
require.NoError(t, err)
bytes, err := constants.TestEncodingCfg.TxConfig.TxEncoder()(txBuilder.GetTx())
require.NoError(t, err)
ctx = ctx.WithTxBytes(bytes)

orderSizeOptimisticallyFilledFromMatching,
orderStatus,
err := ks.ClobKeeper.PlaceShortTermOrder(ctx, msg)
Expand Down
Loading

0 comments on commit a2eacde

Please sign in to comment.