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

[TRA-115] decommission vaults at the beginning of a block #1264

Merged
merged 4 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
7 changes: 7 additions & 0 deletions protocol/x/vault/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ import (
"github.com/dydxprotocol/v4-chain/protocol/x/vault/keeper"
)

func BeginBlocker(
ctx sdk.Context,
keeper *keeper.Keeper,
) {
keeper.DecommissionNonPositiveEquityVaults(ctx)
}

func EndBlocker(
ctx sdk.Context,
keeper *keeper.Keeper,
Expand Down
83 changes: 83 additions & 0 deletions protocol/x/vault/keeper/vault.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package keeper

import (
"math/big"

"cosmossdk.io/store/prefix"
storetypes "cosmossdk.io/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/dydxprotocol/v4-chain/protocol/lib/log"
satypes "github.com/dydxprotocol/v4-chain/protocol/x/subaccounts/types"
"github.com/dydxprotocol/v4-chain/protocol/x/vault/types"
)

// GetVaultEquity returns the equity of a vault (in quote quantums).
func (k Keeper) GetVaultEquity(
ctx sdk.Context,
vaultId types.VaultId,
) (*big.Int, error) {
netCollateral, _, _, err := k.subaccountsKeeper.GetNetCollateralAndMarginRequirements(
ctx,
satypes.Update{
SubaccountId: *vaultId.ToSubaccountId(),
},
)
if err != nil {
return nil, err
}
return netCollateral, nil
}

// DecommissionVaults decommissions all vaults with positive shares and non-positive equity.
func (k Keeper) DecommissionNonPositiveEquityVaults(
ctx sdk.Context,
) {
// Iterate through all vaults.
totalSharesIterator := k.getTotalSharesIterator(ctx)
defer totalSharesIterator.Close()
for ; totalSharesIterator.Valid(); totalSharesIterator.Next() {
var totalShares types.NumShares
k.cdc.MustUnmarshal(totalSharesIterator.Value(), &totalShares)

// Skip if TotalShares is non-positive.
totalSharesRat, err := totalShares.ToBigRat()
if err != nil || totalSharesRat.Sign() <= 0 {
continue
}
Comment on lines +42 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

is it positive for totalShares to be negative? How could that happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

negative shouldn't happen, but just added this check here to be future-proof

Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, will this have to be updated when withdrawal logic is added? It's possible for a vault to have 0 shares and 0 equity in that case right? Non-blocking as that isn't part of Phase 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah 0 shares is possible. will think about that


// Get vault equity.
vaultId, err := types.GetVaultIdFromStateKey(totalSharesIterator.Key())
if err != nil {
log.ErrorLogWithError(ctx, "Failed to get vault ID from state key", err)
continue
}
equity, err := k.GetVaultEquity(ctx, *vaultId)
if err != nil {
log.ErrorLogWithError(ctx, "Failed to get vault equity", err)
continue
}

// Decommission vault if equity is non-positive.
if equity.Sign() <= 0 {
k.DecommissionVault(ctx, *vaultId)
}
}
}

// DecommissionVault decommissions a vault by deleting its total shares and owner shares.
func (k Keeper) DecommissionVault(
Copy link
Contributor

Choose a reason for hiding this comment

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

iiuc, if a user attempts to deposit in this vault before it's decommissioned, it will be recreated in the same block its decommissioned and a new vault will be created right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep! correct

Copy link
Contributor

@vincentwschau vincentwschau Mar 28, 2024

Choose a reason for hiding this comment

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

Is that the case? If a user deposits to the vault before begin block (when decomissioning occurs), then it would have happened in the previous block. What re-creates it in the new block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

deposit tx is processed after begin block. so if a vault is ready to be decommissioned, begin block would decommission it and then when deposit tx is processed, it will be created again.

if a user deposits before begin block, then that deposit tx is processed in the previous block

ctx sdk.Context,
vaultId types.VaultId,
) {
// Delete TotalShares of the vault.
totalSharesStore := prefix.NewStore(ctx.KVStore(k.storeKey), []byte(types.TotalSharesKeyPrefix))
totalSharesStore.Delete(vaultId.ToStateKey())

// Delete all OwnerShares of the vault.
ownerSharesStore := k.getVaultOwnerSharesStore(ctx, vaultId)
ownerSharesIterator := storetypes.KVStorePrefixIterator(ownerSharesStore, []byte{})
defer ownerSharesIterator.Close()
for ; ownerSharesIterator.Valid(); ownerSharesIterator.Next() {
ownerSharesStore.Delete(ownerSharesIterator.Key())
}
}
26 changes: 0 additions & 26 deletions protocol/x/vault/keeper/vault_info.go

This file was deleted.

224 changes: 224 additions & 0 deletions protocol/x/vault/keeper/vault_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
package keeper_test
Copy link
Contributor

Choose a reason for hiding this comment

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

non-blocking nit: Both tests here can be broken up into separate test cases w/ descriptions which would be more helpful for someone running the test if anything fails.


import (
"math/big"
"testing"

"github.com/cometbft/cometbft/types"
"github.com/dydxprotocol/v4-chain/protocol/dtypes"
testapp "github.com/dydxprotocol/v4-chain/protocol/testutil/app"
"github.com/dydxprotocol/v4-chain/protocol/testutil/constants"
assettypes "github.com/dydxprotocol/v4-chain/protocol/x/assets/types"
satypes "github.com/dydxprotocol/v4-chain/protocol/x/subaccounts/types"
vaulttypes "github.com/dydxprotocol/v4-chain/protocol/x/vault/types"
"github.com/stretchr/testify/require"
)

func TestDecommissionNonPositiveEquityVaults(t *testing.T) {
tests := map[string]struct {
/* --- Setup --- */
// Vault IDs.
vaultIds []vaulttypes.VaultId
// Total shares of above vaults.
totalShares []*big.Rat
// Equities of above vaults.
equities []*big.Int

/* --- Expectations --- */
// Whether the vaults are decommissioned.
decommissioned []bool
}{
"Decommission no vault": {
vaultIds: []vaulttypes.VaultId{
constants.Vault_Clob_0,
constants.Vault_Clob_1,
},
totalShares: []*big.Rat{
big.NewRat(7, 1),
big.NewRat(7, 1),
},
equities: []*big.Int{
big.NewInt(1),
big.NewInt(1),
},
decommissioned: []bool{
false,
false,
},
},
"Decommission one vault": {
vaultIds: []vaulttypes.VaultId{
constants.Vault_Clob_0,
constants.Vault_Clob_1,
},
totalShares: []*big.Rat{
big.NewRat(7, 1),
big.NewRat(7, 1),
},
equities: []*big.Int{
big.NewInt(1),
big.NewInt(0),
},
decommissioned: []bool{
false,
true, // this vault should be decommissioned.
},
},
"Decommission two vaults": {
vaultIds: []vaulttypes.VaultId{
constants.Vault_Clob_0,
constants.Vault_Clob_1,
},
totalShares: []*big.Rat{
big.NewRat(7, 1),
big.NewRat(7, 1),
},
equities: []*big.Int{
big.NewInt(0),
big.NewInt(-1),
},
decommissioned: []bool{
true,
true,
},
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
// Initialize vaults with their equities.
tApp := testapp.NewTestAppBuilder(t).WithGenesisDocFn(func() (genesis types.GenesisDoc) {
genesis = testapp.DefaultGenesis()
testapp.UpdateGenesisDocWithAppStateForModule(
&genesis,
func(genesisState *satypes.GenesisState) {
subaccounts := []satypes.Subaccount{}
for i, vaultId := range tc.vaultIds {
if tc.equities[i].Sign() != 0 {
subaccounts = append(
subaccounts,
satypes.Subaccount{
Id: vaultId.ToSubaccountId(),
AssetPositions: []*satypes.AssetPosition{
{
AssetId: assettypes.AssetUsdc.Id,
Quantums: dtypes.NewIntFromBigInt(tc.equities[i]),
},
},
},
)
}
}
genesisState.Subaccounts = subaccounts
},
)
return genesis
}).Build()
ctx := tApp.InitChain()
k := tApp.App.VaultKeeper

// Set total shares and owner shares for all vaults.
testOwner := constants.Alice_Num0.Owner
for i, vaultId := range tc.vaultIds {
err := k.SetTotalShares(
ctx,
vaultId,
vaulttypes.BigRatToNumShares(tc.totalShares[i]),
)
require.NoError(t, err)
err = k.SetOwnerShares(
ctx,
vaultId,
testOwner,
vaulttypes.BigRatToNumShares(big.NewRat(7, 1)),
)
require.NoError(t, err)
}

// Decommission all vaults.
k.DecommissionNonPositiveEquityVaults(ctx)

// Check that total shares and owner shares are deleted for decommissioned
// vaults and not deleted for non-decommissioned vaults.
for i, decommissioned := range tc.decommissioned {
_, exists := k.GetTotalShares(ctx, tc.vaultIds[i])
require.Equal(t, !decommissioned, exists)
_, exists = k.GetOwnerShares(ctx, tc.vaultIds[i], testOwner)
require.Equal(t, !decommissioned, exists)
}
})
}

}

func TestDecommissionVault(t *testing.T) {
tests := map[string]struct {
/* --- Setup --- */
// Vault ID.
vaultId vaulttypes.VaultId
// Whether total shares exists.
totalSharesExists bool
// Owners.
owners []string
}{
"Total shares doesn't exist, no owners": {
vaultId: constants.Vault_Clob_0,
},
"Total shares exists, no owners": {
vaultId: constants.Vault_Clob_0,
totalSharesExists: true,
},
"Total shares exists, one owner": {
vaultId: constants.Vault_Clob_1,
totalSharesExists: true,
owners: []string{constants.Alice_Num0.Owner},
},
"Total shares exists, two owners": {
vaultId: constants.Vault_Clob_1,
totalSharesExists: true,
owners: []string{
constants.Alice_Num0.Owner,
constants.Bob_Num0.Owner,
},
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
tApp := testapp.NewTestAppBuilder(t).Build()
ctx := tApp.InitChain()
k := tApp.App.VaultKeeper

shares := vaulttypes.BigRatToNumShares(
big.NewRat(7, 1),
)

if tc.totalSharesExists {
err := k.SetTotalShares(
ctx,
tc.vaultId,
shares,
)
require.NoError(t, err)
}
for _, owner := range tc.owners {
err := k.SetOwnerShares(
ctx,
tc.vaultId,
owner,
shares,
)
require.NoError(t, err)
}

// Decommission vault.
k.DecommissionVault(ctx, tc.vaultId)

// Check that total shares and owner shares are deleted.
_, exists := k.GetTotalShares(ctx, tc.vaultId)
require.Equal(t, false, exists)
for _, owner := range tc.owners {
_, exists = k.GetOwnerShares(ctx, tc.vaultId, owner)
require.Equal(t, false, exists)
}
})
}
}
11 changes: 11 additions & 0 deletions protocol/x/vault/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var (
_ module.HasGenesisBasics = AppModuleBasic{}

_ appmodule.AppModule = AppModule{}
_ appmodule.HasBeginBlocker = AppModule{}
_ appmodule.HasEndBlocker = AppModule{}
_ module.HasConsensusVersion = AppModule{}
_ module.HasGenesis = AppModule{}
Expand Down Expand Up @@ -147,6 +148,16 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw
// be set to 1.
func (AppModule) ConsensusVersion() uint64 { return 1 }

// BeginBlock executes all ABCI BeginBlock logic respective to the vault module.
func (am AppModule) BeginBlock(ctx context.Context) error {
defer telemetry.ModuleMeasureSince(am.Name(), time.Now(), telemetry.MetricKeyBeginBlocker)
BeginBlocker(
lib.UnwrapSDKContext(ctx, types.ModuleName),
&am.keeper,
)
return nil
}

// EndBlock executes all ABCI EndBlock logic respective to the vault module.
func (am AppModule) EndBlock(ctx context.Context) error {
defer telemetry.ModuleMeasureSince(am.Name(), time.Now(), telemetry.MetricKeyEndBlocker)
Expand Down
Loading