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

[TRA-70] Add state migrations for isolated markets #1155

Merged
merged 6 commits into from
Mar 8, 2024
Merged

[TRA-70] Add state migrations for isolated markets #1155

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a78c68b
wip upgrade handler
shrenujb Mar 6, 2024
7d63b0a
Merge branch 'main' into tra70
shrenujb Mar 7, 2024
142696a
fix lint and gen mocks
shrenujb Mar 7, 2024
321628d
address comments, add tests
shrenujb Mar 8, 2024
f7d05e3
add restriction to only set type if current is UNSPECIFIED
shrenujb Mar 8, 2024
19a71ad
parametrize all set perp market type tests
shrenujb Mar 8, 2024
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
4 changes: 3 additions & 1 deletion protocol/app/upgrades.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ package app
import (
"fmt"

v5_0_0 "github.com/dydxprotocol/v4-chain/protocol/app/upgrades/v5.0.0"

upgradetypes "cosmossdk.io/x/upgrade/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/dydxprotocol/v4-chain/protocol/app/upgrades"
v5_0_0 "github.com/dydxprotocol/v4-chain/protocol/app/upgrades/v5.0.0"
)

var (
Expand All @@ -29,6 +30,7 @@ func (app *App) setupUpgradeHandlers() {
v5_0_0.CreateUpgradeHandler(
app.ModuleManager,
app.configurator,
app.PerpetualsKeeper,
),
)
}
Expand Down
25 changes: 25 additions & 0 deletions protocol/app/upgrades/v5.0.0/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,44 @@ import (
"context"
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"

perptypes "github.com/dydxprotocol/v4-chain/protocol/x/perpetuals/types"

upgradetypes "cosmossdk.io/x/upgrade/types"
"github.com/cosmos/cosmos-sdk/types/module"
"github.com/dydxprotocol/v4-chain/protocol/lib"
)

// Set all existing perpetuals to cross market type
func perpetualsUpgrade(
ctx sdk.Context,
perpetualsKeeper perptypes.PerpetualsKeeper,
) {
// Set all perpetuals to cross market type
perpetuals := perpetualsKeeper.GetAllPerpetuals(ctx)
for _, p := range perpetuals {
_, err := perpetualsKeeper.SetPerpetualMarketType(
ctx, p.GetId(),
perptypes.PerpetualMarketType_PERPETUAL_MARKET_TYPE_CROSS)
if err != nil {
panic(fmt.Sprintf("failed to set perpetual market type for perpetual %d: %s", p.GetId(), err))
}
}
}

func CreateUpgradeHandler(
mm *module.Manager,
configurator module.Configurator,
perpetualsKeeper perptypes.PerpetualsKeeper,
) upgradetypes.UpgradeHandler {
return func(ctx context.Context, plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
sdkCtx := lib.UnwrapSDKContext(ctx, "app/upgrades")
sdkCtx.Logger().Info(fmt.Sprintf("Running %s Upgrade...", UpgradeName))

// Set all perpetuals to cross market type
perpetualsUpgrade(sdkCtx, perpetualsKeeper)

Comment on lines +42 to +44
Copy link
Contributor

Choose a reason for hiding this comment

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

The call to perpetualsUpgrade within CreateUpgradeHandler does not handle potential errors. Consider capturing and handling any errors returned by perpetualsUpgrade to ensure that the upgrade process can respond appropriately to failures.

// TODO(TRA-93): Initialize `x/vault` module.

return mm.RunMigrations(ctx, configurator, vm)
Expand Down
40 changes: 40 additions & 0 deletions protocol/mocks/PerpetualsKeeper.go
View file
Open in desktop

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

27 changes: 27 additions & 0 deletions protocol/x/perpetuals/keeper/perpetual.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,33 @@ func (k Keeper) ModifyPerpetual(
return perpetual, nil
}

func (k Keeper) SetPerpetualMarketType(
shrenujb marked this conversation as resolved.
Show resolved Hide resolved
ctx sdk.Context,
perpetualId uint32,
marketType types.PerpetualMarketType,
) (types.Perpetual, error) {
if marketType == types.PerpetualMarketType_PERPETUAL_MARKET_TYPE_UNSPECIFIED {
return types.Perpetual{}, errorsmod.Wrap(
types.ErrInvalidMarketType,
fmt.Sprintf("invalid market type %v for perpetual %d", marketType, perpetualId),
)
}

// Get perpetual.
perpetual, err := k.GetPerpetual(ctx, perpetualId)
if err != nil {
return perpetual, err
}

// Modify perpetual.
perpetual.Params.MarketType = marketType

// Store the modified perpetual.
k.setPerpetual(ctx, perpetual)

return perpetual, nil
}

// GetPerpetual returns a perpetual from its id.
func (k Keeper) GetPerpetual(
ctx sdk.Context,
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [6-6]

The use of math/rand is noted in the imports. It's important to consider the context in which random values are used within this file. If the randomness is used in a security-sensitive context, such as generating unique identifiers or secrets that could affect the protocol's integrity, it's recommended to use crypto/rand instead of math/rand for cryptographically secure random number generation.

Consider replacing math/rand with crypto/rand if the randomness is used in security-sensitive contexts. This change enhances the security of the generated random values against predictability attacks.

Expand Down
44 changes: 44 additions & 0 deletions protocol/x/perpetuals/keeper/perpetual_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,50 @@ func TestModifyPerpetual_Failure(t *testing.T) {
}
}

func TestSetPerpetualMarketType_Success(t *testing.T) {
vincentwschau marked this conversation as resolved.
Show resolved Hide resolved
pc := keepertest.PerpetualsKeepers(t)
// Create liquidity tiers and perpetuals,
perp := keepertest.CreateLiquidityTiersAndNPerpetuals(t, pc.Ctx, pc.PerpetualsKeeper, pc.PricesKeeper, 1)[0]

_, err := pc.PerpetualsKeeper.SetPerpetualMarketType(
pc.Ctx,
perp.Params.Id,
types.PerpetualMarketType_PERPETUAL_MARKET_TYPE_CROSS,
)
require.NoError(t, err)

rst, err := pc.PerpetualsKeeper.GetPerpetual(
pc.Ctx,
perp.Params.Id,
)
require.NoError(t, err)
require.Equal(
t,
types.PerpetualMarketType_PERPETUAL_MARKET_TYPE_CROSS,
rst.Params.MarketType,
)
}

func TestSetPerpetualMarketType_Failure(t *testing.T) {
pc := keepertest.PerpetualsKeepers(t)
// Create liquidity tiers and perpetuals,
perp := keepertest.CreateLiquidityTiersAndNPerpetuals(t, pc.Ctx, pc.PerpetualsKeeper, pc.PricesKeeper, 1)[0]

_, err := pc.PerpetualsKeeper.SetPerpetualMarketType(
pc.Ctx,
perp.Params.Id,
types.PerpetualMarketType_PERPETUAL_MARKET_TYPE_UNSPECIFIED,
)
expectedErr := errorsmod.Wrap(
types.ErrInvalidMarketType,
fmt.Sprintf(
"invalid market type %v for perpetual %d",
types.PerpetualMarketType_PERPETUAL_MARKET_TYPE_UNSPECIFIED, perp.GetId(),
),
)
require.EqualError(t, err, expectedErr.Error())
}

func TestGetPerpetual_Success(t *testing.T) {
pc := keepertest.PerpetualsKeepers(t)
// Create liquidity tiers and perpetuals,
Expand Down
8 changes: 8 additions & 0 deletions protocol/x/perpetuals/types/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,12 @@ type PerpetualsKeeper interface {
ctx sdk.Context,
params Params,
) error
SetPerpetualMarketType(
ctx sdk.Context,
id uint32,
marketType PerpetualMarketType,
) (Perpetual, error)
GetAllPerpetuals(
ctx sdk.Context,
) []Perpetual
}
Loading