diff --git a/sei-cosmos/types/dec_coin.go b/sei-cosmos/types/dec_coin.go index 6997c019cb..1b8f468b81 100644 --- a/sei-cosmos/types/dec_coin.go +++ b/sei-cosmos/types/dec_coin.go @@ -179,16 +179,18 @@ func sanitizeDecCoins(decCoins []DecCoin) DecCoins { return newDecCoins.Sort() } -// NewDecCoinsFromCoins constructs a new coin set with decimal values -// from regular Coins. -func NewDecCoinsFromCoins(coins ...Coin) DecCoins { - decCoins := make(DecCoins, len(coins)) +// NewDecCoinsFromCoins constructs DecCoins from Coins, or an error if any +// amount exceeds the decimal conversion range. +func NewDecCoinsFromCoins(coins ...Coin) (DecCoins, error) { newCoins := NewCoins(coins...) + decCoins := make(DecCoins, len(newCoins)) for i, coin := range newCoins { + if !coin.Amount.CanConvertToDec() { + return nil, fmt.Errorf("coin %s amount exceeds decimal conversion range", coin) + } decCoins[i] = NewDecCoinFromCoin(coin) } - - return decCoins + return decCoins, nil } // String implements the Stringer interface for DecCoins. It returns a diff --git a/sei-cosmos/types/dec_coin_test.go b/sei-cosmos/types/dec_coin_test.go index 1159b174bd..f630830df6 100644 --- a/sei-cosmos/types/dec_coin_test.go +++ b/sei-cosmos/types/dec_coin_test.go @@ -1,6 +1,7 @@ package types_test import ( + "math/big" "strings" "testing" @@ -18,7 +19,26 @@ import ( func TestNewDecCoinConversionRange(t *testing.T) { require.Panics(t, func() { sdk.NewDecCoin("stake", maxInt()) }, "NewDecCoin must reject max Int") require.Panics(t, func() { sdk.NewDecCoinFromCoin(sdk.NewCoin("stake", maxInt())) }, "NewDecCoinFromCoin must reject max Int") - require.Panics(t, func() { sdk.NewDecCoinsFromCoins(sdk.NewCoin("stake", maxInt())) }, "NewDecCoinsFromCoins must reject max Int") + + require.False(t, sdk.Int{}.CanConvertToDec()) + require.False(t, maxInt().CanConvertToDec()) + _, err := sdk.NewDecCoinsFromCoins(sdk.NewCoin("stake", maxInt())) + require.Error(t, err) + + // Largest Int whose whole-coin Dec stays within maxDecBitLen (315): + // floor((2^315 - 1) / 10^18). + ten18 := new(big.Int).Exp(big.NewInt(10), big.NewInt(18), nil) + maxScaled := new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 315), big.NewInt(1)) + maxConvertible := sdk.NewIntFromBigInt(new(big.Int).Quo(maxScaled, ten18)) + require.True(t, maxConvertible.CanConvertToDec()) + decCoins, err := sdk.NewDecCoinsFromCoins(sdk.NewCoin("stake", maxConvertible)) + require.NoError(t, err) + require.Equal(t, maxConvertible.ToDec(), decCoins[0].Amount) + + oneAbove := maxConvertible.Add(sdk.OneInt()) + require.False(t, oneAbove.CanConvertToDec()) + _, err = sdk.NewDecCoinsFromCoins(sdk.NewCoin("stake", oneAbove)) + require.Error(t, err) } type decCoinTestSuite struct { @@ -293,7 +313,7 @@ func (s *decCoinTestSuite) TestSubDecCoins() { msg string }{ { - sdk.NewDecCoinsFromCoins(sdk.NewCoin("mytoken", sdk.NewInt(10)), sdk.NewCoin("btc", sdk.NewInt(20)), sdk.NewCoin("eth", sdk.NewInt(30))), + sdk.NewDecCoins(sdk.NewDecCoin("mytoken", sdk.NewInt(10)), sdk.NewDecCoin("btc", sdk.NewInt(20)), sdk.NewDecCoin("eth", sdk.NewInt(30))), true, "sorted coins should have passed", }, @@ -309,7 +329,8 @@ func (s *decCoinTestSuite) TestSubDecCoins() { }, } - decCoins := sdk.NewDecCoinsFromCoins(sdk.NewCoin("btc", sdk.NewInt(10)), sdk.NewCoin("eth", sdk.NewInt(15)), sdk.NewCoin("mytoken", sdk.NewInt(5))) + decCoins, err := sdk.NewDecCoinsFromCoins(sdk.NewCoin("btc", sdk.NewInt(10)), sdk.NewCoin("eth", sdk.NewInt(15)), sdk.NewCoin("mytoken", sdk.NewInt(5))) + s.Require().NoError(err) for _, tc := range tests { tc := tc diff --git a/sei-cosmos/types/int.go b/sei-cosmos/types/int.go index 45e521641b..2836c4d3a5 100644 --- a/sei-cosmos/types/int.go +++ b/sei-cosmos/types/int.go @@ -160,6 +160,15 @@ func (i Int) ToDec() Dec { return NewDecFromInt(i) } +// CanConvertToDec reports whether i fits in a whole-number Dec (i × 10^Precision). +func (i Int) CanConvertToDec() bool { + if i.i == nil { + return false + } + scaled := new(big.Int).Mul(i.BigInt(), precisionMultiplier(0)) + return scaled.BitLen() <= maxDecBitLen +} + // Int64 converts Int to int64 // Panics if the value is out of range func (i Int) Int64() int64 { diff --git a/sei-cosmos/x/auth/legacy/legacytx/stdtx.go b/sei-cosmos/x/auth/legacy/legacytx/stdtx.go index 3798017e7a..c065b37c09 100644 --- a/sei-cosmos/x/auth/legacy/legacytx/stdtx.go +++ b/sei-cosmos/x/auth/legacy/legacytx/stdtx.go @@ -73,7 +73,11 @@ func (fee StdFee) GasPrices() sdk.DecCoins { if fee.Gas > uint64(math.MaxInt64) { panic(fmt.Sprintf("gas %d exceeds max int64", fee.Gas)) } - return sdk.NewDecCoinsFromCoins(fee.Amount...).QuoDec(sdk.NewDec(int64(fee.Gas))) //nolint:gosec G115 -- bounds checked above + decAmount, err := sdk.NewDecCoinsFromCoins(fee.Amount...) + if err != nil { + panic(err) + } + return decAmount.QuoDec(sdk.NewDec(int64(fee.Gas))) //nolint:gosec G115 -- bounds checked above } // StdTx is the legacy transaction format for wrapping a Msg with Fee and Signatures. diff --git a/sei-cosmos/x/distribution/keeper/allocation.go b/sei-cosmos/x/distribution/keeper/allocation.go index 82bc95275f..204834442d 100644 --- a/sei-cosmos/x/distribution/keeper/allocation.go +++ b/sei-cosmos/x/distribution/keeper/allocation.go @@ -20,10 +20,13 @@ func (k Keeper) AllocateTokens( // (and distributed to the previous proposer) feeCollector := k.authKeeper.GetModuleAccount(ctx, k.feeCollectorName) feesCollectedInt := k.bankKeeper.GetAllBalances(ctx, feeCollector.GetAddress()) - feesCollected := sdk.NewDecCoinsFromCoins(feesCollectedInt...) + feesCollected, err := sdk.NewDecCoinsFromCoins(feesCollectedInt...) + if err != nil { + panic(err) + } // transfer collected fees to the distribution module account - err := k.bankKeeper.SendCoinsFromModuleToModule(ctx, k.feeCollectorName, types.ModuleName, feesCollectedInt) + err = k.bankKeeper.SendCoinsFromModuleToModule(ctx, k.feeCollectorName, types.ModuleName, feesCollectedInt) if err != nil { panic(err) } diff --git a/sei-cosmos/x/distribution/keeper/fee_pool.go b/sei-cosmos/x/distribution/keeper/fee_pool.go index 292754e915..02c23eb006 100644 --- a/sei-cosmos/x/distribution/keeper/fee_pool.go +++ b/sei-cosmos/x/distribution/keeper/fee_pool.go @@ -10,17 +10,22 @@ import ( func (k Keeper) DistributeFromFeePool(ctx sdk.Context, amount sdk.Coins, receiveAddr sdk.AccAddress) error { feePool := k.GetFeePool(ctx) + decAmount, err := sdk.NewDecCoinsFromCoins(amount...) + if err != nil { + return err + } + // NOTE the community pool isn't a module account, however its coins // are held in the distribution module account. Thus the community pool // must be reduced separately from the SendCoinsFromModuleToAccount call - newPool, negative := feePool.CommunityPool.SafeSub(sdk.NewDecCoinsFromCoins(amount...)) + newPool, negative := feePool.CommunityPool.SafeSub(decAmount) if negative { return types.ErrBadDistribution } feePool.CommunityPool = newPool - err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, receiveAddr, amount) + err = k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, receiveAddr, amount) if err != nil { return err } diff --git a/sei-cosmos/x/distribution/keeper/grpc_query_test.go b/sei-cosmos/x/distribution/keeper/grpc_query_test.go index 1cb6b1718f..7eb6c83a31 100644 --- a/sei-cosmos/x/distribution/keeper/grpc_query_test.go +++ b/sei-cosmos/x/distribution/keeper/grpc_query_test.go @@ -592,7 +592,9 @@ func (suite *KeeperTestSuite) TestGRPCCommunityPool() { suite.Require().Nil(err) req = &types.QueryCommunityPoolRequest{} - expPool = &types.QueryCommunityPoolResponse{Pool: sdk.NewDecCoinsFromCoins(amount...)} + pool, err := sdk.NewDecCoinsFromCoins(amount...) + suite.Require().NoError(err) + expPool = &types.QueryCommunityPoolResponse{Pool: pool} }, true, }, diff --git a/sei-cosmos/x/distribution/keeper/hooks.go b/sei-cosmos/x/distribution/keeper/hooks.go index ee00bc6f63..d125d12252 100644 --- a/sei-cosmos/x/distribution/keeper/hooks.go +++ b/sei-cosmos/x/distribution/keeper/hooks.go @@ -63,7 +63,11 @@ func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, _ sdk.ConsAddress, valAddr } } else { feePool := h.k.GetFeePool(ctx) - feePool.CommunityPool = feePool.CommunityPool.Add(sdk.NewDecCoinsFromCoins(coins...)...) + decCoins, err := sdk.NewDecCoinsFromCoins(coins...) + if err != nil { + panic(err) + } + feePool.CommunityPool = feePool.CommunityPool.Add(decCoins...) h.k.SetFeePool(ctx, feePool) } } diff --git a/sei-cosmos/x/distribution/keeper/keeper.go b/sei-cosmos/x/distribution/keeper/keeper.go index c3fe032ef1..1340c5a299 100644 --- a/sei-cosmos/x/distribution/keeper/keeper.go +++ b/sei-cosmos/x/distribution/keeper/keeper.go @@ -127,11 +127,16 @@ func (k Keeper) WithdrawValidatorCommission(ctx sdk.Context, valAddr sdk.ValAddr } commission, remainder := accumCommission.Commission.TruncateDecimal() + commissionDec, err := sdk.NewDecCoinsFromCoins(commission...) + if err != nil { + return nil, err + } + k.SetValidatorAccumulatedCommission(ctx, valAddr, types.ValidatorAccumulatedCommission{Commission: remainder}) // leave remainder to withdraw later // update outstanding outstanding := k.GetValidatorOutstandingRewards(ctx, valAddr).Rewards - k.SetValidatorOutstandingRewards(ctx, valAddr, types.ValidatorOutstandingRewards{Rewards: outstanding.Sub(sdk.NewDecCoinsFromCoins(commission...))}) + k.SetValidatorOutstandingRewards(ctx, valAddr, types.ValidatorOutstandingRewards{Rewards: outstanding.Sub(commissionDec)}) if !commission.IsZero() { accAddr := sdk.AccAddress(valAddr) @@ -169,12 +174,17 @@ func (k Keeper) GetTotalRewards(ctx sdk.Context) (totalRewards sdk.DecCoins) { // added to the pool. An error is returned if the amount cannot be sent to the // module account. func (k Keeper) FundCommunityPool(ctx sdk.Context, amount sdk.Coins, sender sdk.AccAddress) error { + decAmount, err := sdk.NewDecCoinsFromCoins(amount...) + if err != nil { + return err + } + if err := k.bankKeeper.SendCoinsFromAccountToModule(ctx, sender, types.ModuleName, amount); err != nil { return err } feePool := k.GetFeePool(ctx) - feePool.CommunityPool = feePool.CommunityPool.Add(sdk.NewDecCoinsFromCoins(amount...)...) + feePool.CommunityPool = feePool.CommunityPool.Add(decAmount...) k.SetFeePool(ctx, feePool) return nil diff --git a/sei-cosmos/x/distribution/keeper/keeper_test.go b/sei-cosmos/x/distribution/keeper/keeper_test.go index 1f317a822d..7fdee31a5c 100644 --- a/sei-cosmos/x/distribution/keeper/keeper_test.go +++ b/sei-cosmos/x/distribution/keeper/keeper_test.go @@ -304,7 +304,9 @@ func TestFundCommunityPool(t *testing.T) { err := app.DistrKeeper.FundCommunityPool(ctx, amount, addr[0]) assert.Nil(t, err) - assert.Equal(t, initPool.CommunityPool.Add(sdk.NewDecCoinsFromCoins(amount...)...), app.DistrKeeper.GetFeePool(ctx).CommunityPool) + wantPool, err := sdk.NewDecCoinsFromCoins(amount...) + require.NoError(t, err) + assert.Equal(t, initPool.CommunityPool.Add(wantPool...), app.DistrKeeper.GetFeePool(ctx).CommunityPool) assert.Empty(t, app.BankKeeper.GetAllBalances(ctx, addr[0])) } @@ -322,11 +324,28 @@ func TestFundCommunityPoolRejectsOutOfRangeAmount(t *testing.T) { coins := sdk.NewCoins(sdk.NewCoin("bigcoin", maxAmt)) require.NoError(t, apptesting.FundAccount(app.BankKeeper, ctx, addr[0], coins)) - require.Panics(t, func() { - _ = app.DistrKeeper.FundCommunityPool(ctx, coins, addr[0]) - }, "funding the community pool with an out-of-range amount must be rejected") + require.NotPanics(t, func() { + err := app.DistrKeeper.FundCommunityPool(ctx, coins, addr[0]) + require.Error(t, err) + }) // The stored fee pool must be unchanged after the rejected attempt. - require.NotPanics(t, func() { app.DistrKeeper.GetFeePool(ctx) }) require.True(t, app.DistrKeeper.GetFeePool(ctx).CommunityPool.IsZero()) + require.Equal(t, coins, app.BankKeeper.GetAllBalances(ctx, addr[0])) +} + +func TestDistributeFromFeePoolRejectsOutOfRangeAmount(t *testing.T) { + app := seiapp.Setup(t, false, false, false) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + + recipient := seiapp.AddTestAddrs(app, ctx, 1, sdk.ZeroInt())[0] + maxAmt := sdk.NewIntFromBigInt(new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 256), big.NewInt(1))) + coins := sdk.NewCoins(sdk.NewCoin("bigcoin", maxAmt)) + + require.NotPanics(t, func() { + err := app.DistrKeeper.DistributeFromFeePool(ctx, coins, recipient) + require.Error(t, err) + }) + require.True(t, app.DistrKeeper.GetFeePool(ctx).CommunityPool.IsZero()) + require.True(t, app.BankKeeper.GetAllBalances(ctx, recipient).IsZero()) } diff --git a/sei-cosmos/x/distribution/proposal_handler_test.go b/sei-cosmos/x/distribution/proposal_handler_test.go index 532535f019..fadbed266e 100644 --- a/sei-cosmos/x/distribution/proposal_handler_test.go +++ b/sei-cosmos/x/distribution/proposal_handler_test.go @@ -1,6 +1,7 @@ package distribution_test import ( + "math/big" "testing" tmproto "github.com/sei-protocol/sei-chain/sei-tendermint/proto/tendermint/types" @@ -42,7 +43,9 @@ func TestProposalHandlerPassed(t *testing.T) { require.True(t, app.BankKeeper.GetAllBalances(ctx, account.GetAddress()).IsZero()) feePool := app.DistrKeeper.GetFeePool(ctx) - feePool.CommunityPool = sdk.NewDecCoinsFromCoins(amount...) + poolCoins, err := sdk.NewDecCoinsFromCoins(amount...) + require.NoError(t, err) + feePool.CommunityPool = poolCoins app.DistrKeeper.SetFeePool(ctx, feePool) tp := testProposal(recipient, amount) @@ -70,3 +73,23 @@ func TestProposalHandlerFailed(t *testing.T) { balances := app.BankKeeper.GetAllBalances(ctx, recipient) require.True(t, balances.IsZero()) } + +func TestProposalHandlerRejectsOutOfRangeAmount(t *testing.T) { + app := seiapp.Setup(t, false, false, false) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + + recipient := delAddr1 + account := app.AccountKeeper.NewAccountWithAddress(ctx, recipient) + app.AccountKeeper.SetAccount(ctx, account) + + maxAmt := sdk.NewIntFromBigInt(new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 256), big.NewInt(1))) + huge := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, maxAmt)) + tp := testProposal(recipient, huge) + require.Error(t, tp.ValidateBasic(), "ValidateBasic must reject unconvertible spend amounts") + + hdlr := distribution.NewCommunityPoolSpendProposalHandler(app.DistrKeeper) + require.NotPanics(t, func() { + require.Error(t, hdlr(ctx, tp)) + }) + require.True(t, app.BankKeeper.GetAllBalances(ctx, recipient).IsZero()) +} diff --git a/sei-cosmos/x/distribution/types/msg.go b/sei-cosmos/x/distribution/types/msg.go index 1e4810dfd7..b5c6b7ca5c 100644 --- a/sei-cosmos/x/distribution/types/msg.go +++ b/sei-cosmos/x/distribution/types/msg.go @@ -158,6 +158,9 @@ func (msg MsgFundCommunityPool) ValidateBasic() error { if !msg.Amount.IsValid() { return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, msg.Amount.String()) } + if _, err := sdk.NewDecCoinsFromCoins(msg.Amount...); err != nil { + return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, err.Error()) + } if msg.Depositor == "" { return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, msg.Depositor) } diff --git a/sei-cosmos/x/distribution/types/msg_test.go b/sei-cosmos/x/distribution/types/msg_test.go index fa2a4d4f9e..b4b2c749c4 100644 --- a/sei-cosmos/x/distribution/types/msg_test.go +++ b/sei-cosmos/x/distribution/types/msg_test.go @@ -1,6 +1,7 @@ package types import ( + "math/big" "testing" "github.com/stretchr/testify/require" @@ -75,6 +76,7 @@ func TestMsgWithdrawValidatorCommission(t *testing.T) { // test ValidateBasic for MsgDepositIntoCommunityPool func TestMsgDepositIntoCommunityPool(t *testing.T) { + maxAmt := sdk.NewIntFromBigInt(new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 256), big.NewInt(1))) tests := []struct { amount sdk.Coins depositor sdk.AccAddress @@ -83,6 +85,7 @@ func TestMsgDepositIntoCommunityPool(t *testing.T) { {sdk.NewCoins(sdk.NewInt64Coin("uatom", 10000)), sdk.AccAddress{}, false}, {sdk.Coins{sdk.NewInt64Coin("uatom", 10), sdk.NewInt64Coin("uatom", 10)}, delAddr1, false}, {sdk.NewCoins(sdk.NewInt64Coin("uatom", 1000)), delAddr1, true}, + {sdk.NewCoins(sdk.NewCoin("uatom", maxAmt)), delAddr1, false}, } for i, tc := range tests { msg := NewMsgFundCommunityPool(tc.amount, tc.depositor) diff --git a/sei-cosmos/x/distribution/types/proposal.go b/sei-cosmos/x/distribution/types/proposal.go index 7ed9adb23f..b44794efa8 100644 --- a/sei-cosmos/x/distribution/types/proposal.go +++ b/sei-cosmos/x/distribution/types/proposal.go @@ -47,6 +47,9 @@ func (csp *CommunityPoolSpendProposal) ValidateBasic() error { if !csp.Amount.IsValid() { return ErrInvalidProposalAmount } + if _, err := sdk.NewDecCoinsFromCoins(csp.Amount...); err != nil { + return ErrInvalidProposalAmount + } if csp.Recipient == "" { return ErrEmptyProposalRecipient }