-
Notifications
You must be signed in to change notification settings - Fork 115
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
} | ||
|
||
// 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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep! correct There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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()) | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,223 @@ | ||
package keeper_test | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
} | ||
}) | ||
} | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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