From 9de9de671e20abadcbbe10716186ee0e77e8f497 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Wed, 10 Jul 2024 11:14:17 -0700 Subject: [PATCH] feat(x/precisebank): Display 0 reserve balance to module consumers (#1958) Module reserve represents fractional balances, so it should be hidden to consumers to not have a misleading total balance that doubles the fractional balances. This modifies GetBalance() and SpendableCoin() to always return zero coins when fetching the reserve address balance for fractional amounts. --- x/precisebank/keeper/burn_integration_test.go | 8 +-- x/precisebank/keeper/view.go | 14 ++++ x/precisebank/keeper/view_integration_test.go | 71 +++++++++++++++++++ x/precisebank/keeper/view_test.go | 70 ++++++++++++++++++ 4 files changed, 159 insertions(+), 4 deletions(-) diff --git a/x/precisebank/keeper/burn_integration_test.go b/x/precisebank/keeper/burn_integration_test.go index c569f69e..6f10e852 100644 --- a/x/precisebank/keeper/burn_integration_test.go +++ b/x/precisebank/keeper/burn_integration_test.go @@ -261,7 +261,7 @@ func (suite *burnIntegrationTestSuite) TestBurnCoins_Remainder() { // Burn 0.1 until balance is 0 for { - reserveBalBefore := suite.Keeper.GetBalance( + reserveBalBefore := suite.BankKeeper.GetBalance( suite.Ctx, reserveAddr, types.IntegerCoinDenom, @@ -291,7 +291,7 @@ func (suite *burnIntegrationTestSuite) TestBurnCoins_Remainder() { moduleAddr, types.ExtendedCoinDenom, ) - reserveBalAfter := suite.Keeper.GetBalance( + reserveBalAfter := suite.BankKeeper.GetBalance( suite.Ctx, reserveAddr, types.IntegerCoinDenom, @@ -357,7 +357,7 @@ func (suite *burnIntegrationTestSuite) TestBurnCoins_Spread_Remainder() { // Burn 0.1 from each account for _, addr := range addrs { - reserveBalBefore := suite.Keeper.GetBalance( + reserveBalBefore := suite.BankKeeper.GetBalance( suite.Ctx, reserveAddr, types.IntegerCoinDenom, @@ -395,7 +395,7 @@ func (suite *burnIntegrationTestSuite) TestBurnCoins_Spread_Remainder() { addr, types.ExtendedCoinDenom, ) - reserveBalAfter := suite.Keeper.GetBalance( + reserveBalAfter := suite.BankKeeper.GetBalance( suite.Ctx, reserveAddr, types.IntegerCoinDenom, diff --git a/x/precisebank/keeper/view.go b/x/precisebank/keeper/view.go index 38a428e0..3c6b83db 100644 --- a/x/precisebank/keeper/view.go +++ b/x/precisebank/keeper/view.go @@ -1,6 +1,7 @@ package keeper import ( + sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/kava-labs/kava/x/precisebank/types" ) @@ -13,6 +14,14 @@ func (k Keeper) GetBalance( addr sdk.AccAddress, denom string, ) sdk.Coin { + // Module balance should display as empty for extended denom. Module + // balances are **only** for the reserve which backs the fractional + // balances. Returning the backing balances if querying extended denom would + // result in a double counting of the fractional balances. + if denom == types.ExtendedCoinDenom && addr.Equals(k.ak.GetModuleAddress(types.ModuleName)) { + return sdk.NewCoin(denom, sdkmath.ZeroInt()) + } + // Pass through to x/bank for denoms except ExtendedCoinDenom if denom != types.ExtendedCoinDenom { return k.bk.GetBalance(ctx, addr, denom) @@ -41,6 +50,11 @@ func (k Keeper) SpendableCoin( addr sdk.AccAddress, denom string, ) sdk.Coin { + // Same as GetBalance, extended denom balances are transparent to consumers. + if denom == types.ExtendedCoinDenom && addr.Equals(k.ak.GetModuleAddress(types.ModuleName)) { + return sdk.NewCoin(denom, sdkmath.ZeroInt()) + } + // Pass through to x/bank for denoms except ExtendedCoinDenom if denom != types.ExtendedCoinDenom { return k.bk.SpendableCoin(ctx, addr, denom) diff --git a/x/precisebank/keeper/view_integration_test.go b/x/precisebank/keeper/view_integration_test.go index 9d3866b9..a2f38895 100644 --- a/x/precisebank/keeper/view_integration_test.go +++ b/x/precisebank/keeper/view_integration_test.go @@ -129,3 +129,74 @@ func (suite *viewIntegrationTestSuite) TestKeeper_SpendableCoin() { }) } } + +func (suite *viewIntegrationTestSuite) TestKeeper_HiddenReserve() { + // Reserve balances should not be shown to consumers of x/precisebank, as it + // represents the fractional balances of accounts. + + moduleAddr := authtypes.NewModuleAddress(types.ModuleName) + addr1 := sdk.AccAddress{1} + + // Make the reserve hold a non-zero balance + // Mint fractional coins to an account, which should cause a mint of 1 + // integer coin to the reserve to back it. + extCoin := sdk.NewCoin(types.ExtendedCoinDenom, types.ConversionFactor().AddRaw(1000)) + unrelatedCoin := sdk.NewCoin("unrelated", sdk.NewInt(1000)) + suite.MintToAccount( + addr1, + sdk.NewCoins( + extCoin, + unrelatedCoin, + ), + ) + + // Check underlying x/bank balance for reserve + reserveIntCoin := suite.BankKeeper.GetBalance(suite.Ctx, moduleAddr, types.IntegerCoinDenom) + suite.Require().Equal( + sdkmath.NewInt(1), + reserveIntCoin.Amount, + "reserve should hold 1 integer coin", + ) + + tests := []struct { + name string + giveAddr sdk.AccAddress + giveDenom string + wantAmount sdkmath.Int + }{ + { + "reserve account - hidden extended denom", + moduleAddr, + types.ExtendedCoinDenom, + sdkmath.ZeroInt(), + }, + { + "reserve account - visible integer denom", + moduleAddr, + types.IntegerCoinDenom, + sdkmath.OneInt(), + }, + { + "user account - visible extended denom", + addr1, + types.ExtendedCoinDenom, + extCoin.Amount, + }, + { + "user account - visible integer denom", + addr1, + types.IntegerCoinDenom, + extCoin.Amount.Quo(types.ConversionFactor()), + }, + } + + for _, tt := range tests { + suite.Run(tt.name, func() { + coin := suite.Keeper.GetBalance(suite.Ctx, tt.giveAddr, tt.giveDenom) + suite.Require().Equal(tt.wantAmount.Int64(), coin.Amount.Int64()) + + spendableCoin := suite.Keeper.SpendableCoin(suite.Ctx, tt.giveAddr, tt.giveDenom) + suite.Require().Equal(tt.wantAmount.Int64(), spendableCoin.Amount.Int64()) + }) + } +} diff --git a/x/precisebank/keeper/view_test.go b/x/precisebank/keeper/view_test.go index fd8bd401..1a4e7820 100644 --- a/x/precisebank/keeper/view_test.go +++ b/x/precisebank/keeper/view_test.go @@ -5,6 +5,7 @@ import ( sdkmath "cosmossdk.io/math" sdk "github.com/cosmos/cosmos-sdk/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/kava-labs/kava/x/precisebank/types" "github.com/stretchr/testify/require" ) @@ -90,6 +91,13 @@ func TestKeeper_GetBalance(t *testing.T) { // Set fractional balance in store before query tk.keeper.SetFractionalBalance(tk.ctx, addr, tt.giveFractionalBal) + // Checks address if its a reserve denom + if tt.giveDenom == types.ExtendedCoinDenom { + tk.ak.EXPECT().GetModuleAddress(types.ModuleName). + Return(authtypes.NewModuleAddress(types.ModuleName)). + Once() + } + if tt.giveDenom == types.ExtendedCoinDenom { // No balance pass through tk.bk.EXPECT(). @@ -198,6 +206,13 @@ func TestKeeper_SpendableCoin(t *testing.T) { // Set fractional balance in store before query tk.keeper.SetFractionalBalance(tk.ctx, addr, tt.giveFractionalBal) + // If its a reserve denom, module address is checked + if tt.giveDenom == types.ExtendedCoinDenom { + tk.ak.EXPECT().GetModuleAddress(types.ModuleName). + Return(authtypes.NewModuleAddress(types.ModuleName)). + Once() + } + if tt.giveDenom == types.ExtendedCoinDenom { // No balance pass through tk.bk.EXPECT(). @@ -224,3 +239,58 @@ func TestKeeper_SpendableCoin(t *testing.T) { }) } } + +func TestHiddenReserve(t *testing.T) { + // Reserve balances should not be shown to consumers of x/precisebank, as it + // represents the fractional balances of accounts. + + tk := NewMockedTestData(t) + + moduleAddr := authtypes.NewModuleAddress(types.ModuleName) + + // No mock bankkeeper expectations, which means the zero coin is returned + // directly for reserve address. So the mock bankkeeper doesn't need to have + // a handler for getting underlying balance. + + tests := []struct { + name string + denom string + expectedBalance sdk.Coin + }{ + {"akava", types.ExtendedCoinDenom, sdk.NewCoin(types.ExtendedCoinDenom, sdkmath.ZeroInt())}, + {"ukava", types.IntegerCoinDenom, sdk.NewCoin(types.IntegerCoinDenom, sdkmath.NewInt(1))}, + {"unrelated denom", "cat", sdk.NewCoin("cat", sdkmath.ZeroInt())}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // 2 calls for GetBalance and SpendableCoin, only for reserve coins + if tt.denom == "akava" { + tk.ak.EXPECT().GetModuleAddress(types.ModuleName). + Return(moduleAddr). + Twice() + } else { + // Passthrough to x/bank for non-reserve denoms + tk.bk.EXPECT(). + GetBalance(tk.ctx, moduleAddr, tt.denom). + Return(sdk.NewCoin(tt.denom, sdkmath.ZeroInt())). + Once() + + tk.bk.EXPECT(). + SpendableCoin(tk.ctx, moduleAddr, tt.denom). + Return(sdk.NewCoin(tt.denom, sdkmath.ZeroInt())). + Once() + } + + // GetBalance should return zero balance for reserve address + coin := tk.keeper.GetBalance(tk.ctx, moduleAddr, tt.denom) + require.Equal(t, tt.denom, coin.Denom) + require.Equal(t, sdkmath.ZeroInt(), coin.Amount) + + // SpendableCoin should return zero balance for reserve address + spendableCoin := tk.keeper.SpendableCoin(tk.ctx, moduleAddr, tt.denom) + require.Equal(t, tt.denom, spendableCoin.Denom) + require.Equal(t, sdkmath.ZeroInt(), spendableCoin.Amount) + }) + } +}