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.

Sign up for GitHub

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

switch to use the helper method to create positions #1787

Merged
merged 1 commit into from
Jun 27, 2024
Merged

switch to use the helper method to create positions #1787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
78 changes: 40 additions & 38 deletions protocol/daemons/liquidation/client/sub_task_runner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,18 @@ package client_test

import (
"context"
"cosmossdk.io/log"
"math/big"
"testing"

"cosmossdk.io/log"

"github.com/dydxprotocol/v4-chain/protocol/daemons/flags"
"github.com/dydxprotocol/v4-chain/protocol/daemons/liquidation/api"
"github.com/dydxprotocol/v4-chain/protocol/daemons/liquidation/client"
"github.com/dydxprotocol/v4-chain/protocol/dtypes"
"github.com/dydxprotocol/v4-chain/protocol/mocks"
"github.com/dydxprotocol/v4-chain/protocol/testutil/constants"
"github.com/dydxprotocol/v4-chain/protocol/testutil/grpc"
testutil "github.com/dydxprotocol/v4-chain/protocol/testutil/util"
blocktimetypes "github.com/dydxprotocol/v4-chain/protocol/x/blocktime/types"
clobtypes "github.com/dydxprotocol/v4-chain/protocol/x/clob/types"
perptypes "github.com/dydxprotocol/v4-chain/protocol/x/perpetuals/types"
Expand Down Expand Up @@ -283,17 +285,17 @@ func TestRunLiquidationDaemonTaskLoop(t *testing.T) {
{
Id: &constants.Carl_Num0,
AssetPositions: []*satypes.AssetPosition{
{
AssetId: 0,
Quantums: dtypes.NewInt(55_000_000_000), // $55,000
},
testutil.CreateSingleAssetPosition(
0,
big.NewInt(55_000_000_000), // $55,000
),
},
PerpetualPositions: []*satypes.PerpetualPosition{
{
PerpetualId: 0,
Quantums: dtypes.NewInt(-100_000_000), // -1 BTC
FundingIndex: dtypes.NewInt(10_000),
},
testutil.CreateSinglePerpetualPosition(
0,
big.NewInt(-100_000_000), // -1 BTC
big.NewInt(10_000),
),
Comment on lines +288 to +298
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor using testutil functions for position creation:

The replacement of direct instantiation with testutil.CreateSingleAssetPosition and testutil.CreateSinglePerpetualPosition is a significant improvement. This change not only reduces redundancy but also enhances the maintainability and readability of the test code by centralizing the creation logic.

Consider adding a brief comment above these utility function calls to explain what each parameter represents, especially for magic numbers like 0 or large integers like 55_000_000_000, to enhance code readability.

+ // Create a single asset position with specified parameters
  testutil.CreateSingleAssetPosition(
      0, // Asset ID
      big.NewInt(55_000_000_000), // Position value in USD
  ),

+ // Create a single perpetual position with specified parameters
  testutil.CreateSinglePerpetualPosition(
      0, // Perpetual ID
      big.NewInt(-100_000_000), // Position size in BTC
      big.NewInt(10_000), // Funding index
  ),

Also applies to: 368-378, 448-458, 526-536

},
},
},
Expand Down Expand Up @@ -363,17 +365,17 @@ func TestRunLiquidationDaemonTaskLoop(t *testing.T) {
{
Id: &constants.Dave_Num0,
AssetPositions: []*satypes.AssetPosition{
{
AssetId: 0,
Quantums: dtypes.NewInt(-45_000_000_000), // -$45,000
},
testutil.CreateSingleAssetPosition(
0,
big.NewInt(-45_000_000_000), // -$45,000
),
},
PerpetualPositions: []*satypes.PerpetualPosition{
{
PerpetualId: 0,
Quantums: dtypes.NewInt(100_000_000), // 1 BTC
FundingIndex: dtypes.NewInt(-10_000),
},
testutil.CreateSinglePerpetualPosition(
0,
big.NewInt(100_000_000), // 1 BTC
big.NewInt(-10_000),
),
},
},
},
Expand Down Expand Up @@ -443,17 +445,17 @@ func TestRunLiquidationDaemonTaskLoop(t *testing.T) {
{
Id: &constants.Carl_Num0,
AssetPositions: []*satypes.AssetPosition{
{
AssetId: 0,
Quantums: dtypes.NewInt(54_999_000_000), // $54,999
},
testutil.CreateSingleAssetPosition(
0,
big.NewInt(54_999_000_000), // $54,999
),
},
PerpetualPositions: []*satypes.PerpetualPosition{
{
PerpetualId: 0,
Quantums: dtypes.NewInt(-100_000_000), // -1 BTC
FundingIndex: dtypes.NewInt(-10_000),
},
testutil.CreateSinglePerpetualPosition(
0,
big.NewInt(-100_000_000), // -1 BTC
big.NewInt(-10_000),
),
},
},
},
Expand Down Expand Up @@ -521,17 +523,17 @@ func TestRunLiquidationDaemonTaskLoop(t *testing.T) {
{
Id: &constants.Dave_Num0,
AssetPositions: []*satypes.AssetPosition{
{
AssetId: 0,
Quantums: dtypes.NewInt(-44_999_000_000), // -$44,999
},
testutil.CreateSingleAssetPosition(
0,
big.NewInt(-44_999_000_000), // -$44,999
),
},
PerpetualPositions: []*satypes.PerpetualPosition{
{
PerpetualId: 0,
Quantums: dtypes.NewInt(100_000_000), // 1 BTC
FundingIndex: dtypes.NewInt(10_000),
},
testutil.CreateSinglePerpetualPosition(
0,
big.NewInt(100_000_000), // 1 BTC
big.NewInt(10_000),
),
},
},
},
Expand Down
28 changes: 15 additions & 13 deletions protocol/x/clob/client/cli/liquidations_cli_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,19 @@ import (
"fmt"
"math"

appflags "github.com/dydxprotocol/v4-chain/protocol/app/flags"
"math/big"
"testing"

appflags "github.com/dydxprotocol/v4-chain/protocol/app/flags"

networktestutil "github.com/cosmos/cosmos-sdk/testutil/network"
sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
"github.com/dydxprotocol/v4-chain/protocol/dtypes"
"github.com/dydxprotocol/v4-chain/protocol/lib"
testutil "github.com/dydxprotocol/v4-chain/protocol/testutil/util"
assettypes "github.com/dydxprotocol/v4-chain/protocol/x/assets/types"
"github.com/dydxprotocol/v4-chain/protocol/x/clob/client/testutil"
clob_testutil "github.com/dydxprotocol/v4-chain/protocol/x/clob/client/testutil"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

Clarify the specific utility of clob_testutil.

The review comment is valid. There are no comments or documentation explaining the specific utility of clob_testutil in the liquidations_cli_test.go file. Adding such explanations would help maintain clarity and prevent confusion.

  • File: protocol/x/clob/client/cli/liquidations_cli_test.go
  • Lines: 21
Analysis chain

Clarify the specific utility of clob_testutil.

It's important to maintain clear documentation or comments explaining the specific utilities provided by clob_testutil as opposed to testutil, especially since both are used in this file. This helps maintain clarity and prevents confusion.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for documentation or comments regarding `clob_testutil` usage.
rg --type python --files-with-matches 'clob_testutil' | xargs rg 'clob_testutil' --context 5

Length of output: 3231

Script:

#!/bin/bash
# Description: Check for documentation or comments regarding `clob_testutil` usage in Go files.
rg --type go --files-with-matches 'clob_testutil' | xargs rg 'clob_testutil' --context 5

Length of output: 1480

"github.com/dydxprotocol/v4-chain/protocol/x/clob/types"
epochstypes "github.com/dydxprotocol/v4-chain/protocol/x/epochs/types"
feetierstypes "github.com/dydxprotocol/v4-chain/protocol/x/feetiers/types"
Expand Down Expand Up @@ -128,7 +129,7 @@ func (s *LiquidationsIntegrationTestSuite) SetupSuite() {
s.cfg.GenesisState[types.ModuleName] = buf

// Set the balances in the genesis state.
s.cfg.GenesisState[banktypes.ModuleName] = testutil.CreateBankGenesisState(
s.cfg.GenesisState[banktypes.ModuleName] = clob_testutil.CreateBankGenesisState(
s.T(),
s.cfg,
liqTestInitialSubaccountModuleAccBalance,
Expand All @@ -147,16 +148,17 @@ func (s *LiquidationsIntegrationTestSuite) SetupSuite() {
satypes.Subaccount{
Id: &satypes.SubaccountId{Owner: s.validatorAddress.String(), Number: liqTestSubaccountNumberOne},
AssetPositions: []*satypes.AssetPosition{
{
AssetId: assettypes.AssetUsdc.Id,
Quantums: dtypes.NewInt(-45_001_000_000), // -$45,001
},
testutil.CreateSingleAssetPosition(
assettypes.AssetUsdc.Id,
big.NewInt(-45_001_000_000), // -$45,001
),
},
PerpetualPositions: []*satypes.PerpetualPosition{
{
PerpetualId: 0,
Quantums: dtypes.NewInt(100_000_000), // 1 BTC
},
testutil.CreateSinglePerpetualPosition(
0,
big.NewInt(100_000_000), // 1 BTC
big.NewInt(0),
),
},
},
)
Expand Down Expand Up @@ -211,7 +213,7 @@ func (s *LiquidationsIntegrationTestSuite) TestCLILiquidations() {
subticks := types.Subticks(50_000_000_000)

// Place the maker order that should be filled by the liquidation order.
_, err = testutil.MsgPlaceOrderExec(
_, err = clob_testutil.MsgPlaceOrderExec(
ctx,
s.validatorAddress,
liqTestSubaccountNumberZero,
Expand Down
39 changes: 20 additions & 19 deletions protocol/x/clob/e2e/app_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package clob_test

import (
"fmt"
"math/big"
"sync"
"testing"
"time"
Expand All @@ -15,7 +16,6 @@ import (
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
auth "github.com/cosmos/cosmos-sdk/x/auth/types"
"github.com/dydxprotocol/v4-chain/protocol/app/config"
"github.com/dydxprotocol/v4-chain/protocol/dtypes"
"github.com/dydxprotocol/v4-chain/protocol/testutil/rand"
"gopkg.in/typ.v4/slices"

Expand All @@ -25,6 +25,7 @@ import (
clobtestutils "github.com/dydxprotocol/v4-chain/protocol/testutil/clob"
"github.com/dydxprotocol/v4-chain/protocol/testutil/constants"
testtx "github.com/dydxprotocol/v4-chain/protocol/testutil/tx"
testutil "github.com/dydxprotocol/v4-chain/protocol/testutil/util"
clobtypes "github.com/dydxprotocol/v4-chain/protocol/x/clob/types"
epochtypes "github.com/dydxprotocol/v4-chain/protocol/x/epochs/types"
feetierstypes "github.com/dydxprotocol/v4-chain/protocol/x/feetiers/types"
Expand Down Expand Up @@ -525,35 +526,35 @@ func TestHydrationWithMatchPreBlocker(t *testing.T) {
require.Equal(t, satypes.Subaccount{
Id: &constants.Carl_Num0,
AssetPositions: []*satypes.AssetPosition{
{
AssetId: 0,
Quantums: dtypes.NewInt(100_000_000_000 - 50_000_000_000),
},
testutil.CreateSingleAssetPosition(
0,
big.NewInt(100_000_000_000-50_000_000_000),
),
},
PerpetualPositions: []*satypes.PerpetualPosition{
{
PerpetualId: 0,
Quantums: dtypes.NewInt(100_000_000),
FundingIndex: dtypes.NewInt(0),
},
testutil.CreateSinglePerpetualPosition(
0,
big.NewInt(100_000_000),
big.NewInt(0),
),
},
}, carl)

dave := tApp.App.SubaccountsKeeper.GetSubaccount(ctx, constants.Dave_Num0)
require.Equal(t, satypes.Subaccount{
Id: &constants.Dave_Num0,
AssetPositions: []*satypes.AssetPosition{
{
AssetId: 0,
Quantums: dtypes.NewInt(500_000_000_000 + 50_000_000_000),
},
testutil.CreateSingleAssetPosition(
0,
big.NewInt(500_000_000_000+50_000_000_000),
),
},
PerpetualPositions: []*satypes.PerpetualPosition{
{
PerpetualId: 0,
Quantums: dtypes.NewInt(-100_000_000),
FundingIndex: dtypes.NewInt(0),
},
testutil.CreateSinglePerpetualPosition(
0,
big.NewInt(-100_000_000),
big.NewInt(0),
),
},
}, dave)

Expand Down
Loading
Loading