From af7a7f99a36f76165bd551822a9c5c00cf45a049 Mon Sep 17 00:00:00 2001 From: Tian Qin Date: Wed, 18 Sep 2024 22:36:00 -0400 Subject: [PATCH] add check on non-positive shares and redeemed quote quantums --- ...msg_server_withdraw_from_megavault_test.go | 47 +++++++++++++++++-- protocol/x/vault/keeper/withdraw.go | 28 ++++++----- 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/protocol/x/vault/keeper/msg_server_withdraw_from_megavault_test.go b/protocol/x/vault/keeper/msg_server_withdraw_from_megavault_test.go index f17c2cae92..93463c6cda 100644 --- a/protocol/x/vault/keeper/msg_server_withdraw_from_megavault_test.go +++ b/protocol/x/vault/keeper/msg_server_withdraw_from_megavault_test.go @@ -48,9 +48,9 @@ func TestMsgWithdrawFromMegavault(t *testing.T) { // Vaults. vaults []VaultSetup // Shares to withdraw. - sharesToWithdraw uint64 + sharesToWithdraw int64 // Minimum quote quantums to redeem. - minQuoteQuantums uint64 + minQuoteQuantums int64 /* --- Expectations --- */ // Whether DeliverTx should fail. @@ -101,6 +101,19 @@ func TestMsgWithdrawFromMegavault(t *testing.T) { expectedTotalShares: 0, expectedOwnerShares: 0, }, + "Success: Withdraw some unlocked shares (1% of total), No sub-vaults, Redeemed quantums rounds down to 0": { + mainVaultBalance: 99, + totalShares: 200, + owner: constants.AliceAccAddress.String(), + ownerTotalShares: 10, + ownerLockedShares: 5, + sharesToWithdraw: 2, + minQuoteQuantums: -1, + deliverTxFails: true, + redeemedQuoteQuantums: 0, // 99 * 2 / 200 = 0.99 ~= 0 (rounded down) + expectedTotalShares: 200, // unchanged + expectedOwnerShares: 10, // unchanged + }, "Failure: Withdraw more than locked shares": { mainVaultBalance: 100, totalShares: 500, @@ -113,6 +126,30 @@ func TestMsgWithdrawFromMegavault(t *testing.T) { expectedTotalShares: 500, // unchanged expectedOwnerShares: 100, // unchanged }, + "Failure: Withdraw zero shares": { + mainVaultBalance: 100, + totalShares: 500, + owner: constants.AliceAccAddress.String(), + ownerTotalShares: 100, + ownerLockedShares: 20, + sharesToWithdraw: 0, + minQuoteQuantums: 1, + deliverTxFails: true, + expectedTotalShares: 500, // unchanged + expectedOwnerShares: 100, // unchanged + }, + "Failure: Withdraw negative shares": { + mainVaultBalance: 100, + totalShares: 500, + owner: constants.AliceAccAddress.String(), + ownerTotalShares: 100, + ownerLockedShares: 20, + sharesToWithdraw: -1, + minQuoteQuantums: 1, + deliverTxFails: true, + expectedTotalShares: 500, // unchanged + expectedOwnerShares: 100, // unchanged + }, "Failure: Withdraw some unlocked shares (8% of total), one sub-vault, Redeemed quantums < Min quantums": { mainVaultBalance: 1_234, totalShares: 500, @@ -164,7 +201,7 @@ func TestMsgWithdrawFromMegavault(t *testing.T) { }, }, sharesToWithdraw: 4444, - minQuoteQuantums: 0, + minQuoteQuantums: -1, deliverTxFails: false, redeemedQuoteQuantums: 3_950, // 888_888 * 4444 / 1_000_000 ~= 3950 (sub-vault is skipped) expectedTotalShares: 995_556, // 1_000_000 - 4444 @@ -420,9 +457,9 @@ func TestMsgWithdrawFromMegavault(t *testing.T) { Number: 0, }, Shares: vaulttypes.NumShares{ - NumShares: dtypes.NewIntFromUint64(tc.sharesToWithdraw), + NumShares: dtypes.NewInt(tc.sharesToWithdraw), }, - MinQuoteQuantums: dtypes.NewIntFromUint64(tc.minQuoteQuantums), + MinQuoteQuantums: dtypes.NewInt(tc.minQuoteQuantums), } preMegavaultEquity, err := tApp.App.VaultKeeper.GetMegavaultEquity(ctx) diff --git a/protocol/x/vault/keeper/withdraw.go b/protocol/x/vault/keeper/withdraw.go index 9a732c75ad..30e4af6f70 100644 --- a/protocol/x/vault/keeper/withdraw.go +++ b/protocol/x/vault/keeper/withdraw.go @@ -123,7 +123,14 @@ func (k Keeper) WithdrawFromMegavault( sharesToWithdraw *big.Int, minQuoteQuantums *big.Int, ) (redeemedQuoteQuantums *big.Int, err error) { - // 1. Check that the owner is withdrawing less that or equal to their unlocked shares. + // 1. Check that shares to withdraw is positive and not more than unlocked shares. + if sharesToWithdraw.Sign() <= 0 { + return nil, errorsmod.Wrapf( + types.ErrInvalidSharesToWithdraw, + "sharesToWithdraw: %s", + sharesToWithdraw, + ) + } ownerShares, exists := k.GetOwnerShares(ctx, toSubaccount.Owner) if !exists { return nil, types.ErrOwnerNotFound @@ -161,7 +168,7 @@ func (k Keeper) WithdrawFromMegavault( if err != nil { log.ErrorLogWithError( ctx, - "Megavault withdrawal: failed to get vault ID from state key. Skipping this vault", + "Megavault withdrawal: error when getting vault ID from state key. Skipping this vault", err, ) continue @@ -173,7 +180,7 @@ func (k Keeper) WithdrawFromMegavault( if err != nil { log.ErrorLogWithError( ctx, - "Megavault withdrawal: failed to get perpetual and market. Skipping this vault", + "Megavault withdrawal: error when getting perpetual and market. Skipping this vault", err, "Vault ID", vaultId, @@ -184,7 +191,7 @@ func (k Keeper) WithdrawFromMegavault( if err != nil { log.ErrorLogWithError( ctx, - "Megavault withdrawal: failed to get vault leverage and equity. Skipping this vault", + "Megavault withdrawal: error when getting vault leverage and equity. Skipping this vault", err, "Vault ID", vaultId, @@ -204,7 +211,7 @@ func (k Keeper) WithdrawFromMegavault( if err != nil { log.ErrorLogWithError( ctx, - "Megavault withdrawal: failed to get vault withdrawal slippage. Skipping this vault", + "Megavault withdrawal: error when getting vault withdrawal slippage. Skipping this vault", err, "Vault ID", vaultId, @@ -228,7 +235,7 @@ func (k Keeper) WithdrawFromMegavault( if err != nil { log.ErrorLogWithError( ctx, - "Megavault withdrawal: failed to transfer from sub vault to main vault. Skipping this vault", + "Megavault withdrawal: error when transfering from sub vault to main vault. Skipping this vault", err, "Vault ID", vaultId, @@ -243,8 +250,8 @@ func (k Keeper) WithdrawFromMegavault( megavaultEquity.Add(megavaultEquity, equity) } - // 4. Return error if less than min quote quantums are redeemed. - if redeemedQuoteQuantums.Cmp(minQuoteQuantums) < 0 { + // 4. Return error if redeemed quantums are non-positive or less than min quantums. + if redeemedQuoteQuantums.Sign() <= 0 || redeemedQuoteQuantums.Cmp(minQuoteQuantums) < 0 { return nil, errorsmod.Wrapf( types.ErrInsufficientRedeemedQuoteQuantums, "redeemed quote quantums: %s, min quote quantums: %s", @@ -275,11 +282,10 @@ func (k Keeper) WithdrawFromMegavault( } // 6. Decrement total and owner shares. - err = k.SetTotalShares( + if err = k.SetTotalShares( ctx, types.BigIntToNumShares(new(big.Int).Sub(totalShares, sharesToWithdraw)), - ) - if err != nil { + ); err != nil { return nil, err } if ownerSharesAfterWithdrawal.Sign() == 0 {