diff --git a/sei-cosmos/types/coin.go b/sei-cosmos/types/coin.go index af54d6e53e..8a5edcb595 100644 --- a/sei-cosmos/types/coin.go +++ b/sei-cosmos/types/coin.go @@ -358,16 +358,30 @@ func (coins Coins) safeAdd(coinsB Coins) Coins { // DenomsSubsetOf returns true if receiver's denom set // is subset of coinsB's denoms. +// +// Both coin sets — the receiver and coinsB — are assumed to be sorted ascending +// by denomination and free of zero amounts (the Coins invariant enforced by +// Validate/IsValid). The receiver must be sorted too: the merge walk only +// advances forward through coinsB, so an unsorted receiver can produce a false +// negative. Under the sorted invariant a denom is "present" iff it appears, so +// this merge walk can run in O(len(coins) + len(coinsB)). func (coins Coins) DenomsSubsetOf(coinsB Coins) bool { - // more denoms in B than in receiver + // more denoms in receiver than in B => cannot be a subset if len(coins) > len(coinsB) { return false } + indexB := 0 for _, coin := range coins { - if coinsB.AmountOf(coin.Denom).IsZero() { + // advance B until it reaches or passes coin's denom + for indexB < len(coinsB) && coinsB[indexB].Denom < coin.Denom { + indexB++ + } + if indexB == len(coinsB) || coinsB[indexB].Denom != coin.Denom { return false } + // matched; both lists are duplicate-free so B never re-matches this denom + indexB++ } return true diff --git a/sei-cosmos/types/coin_test.go b/sei-cosmos/types/coin_test.go index f1e55b41c3..da0ff5be9f 100644 --- a/sei-cosmos/types/coin_test.go +++ b/sei-cosmos/types/coin_test.go @@ -932,6 +932,56 @@ func (s *coinTestSuite) TestCoinsIsAllGTE() { s.Require().False(sdk.Coins{{"xxx", one}, {"yyy", one}}.IsAllGTE(sdk.Coins{{testDenom2, one}, {"ccc", one}, {"yyy", one}, {"zzz", one}})) } +func (s *coinTestSuite) TestCoinsDenomsSubsetOf() { + one := sdk.OneInt() + two := sdk.NewInt(2) + + // empty receiver is a subset of anything (including empty) + s.Require().True(sdk.Coins{}.DenomsSubsetOf(sdk.Coins{})) + s.Require().True(sdk.Coins{}.DenomsSubsetOf(sdk.Coins{{testDenom1, one}})) + + // a larger receiver can never be a subset + s.Require().False(sdk.Coins{{testDenom1, one}}.DenomsSubsetOf(sdk.Coins{})) + + // amounts are irrelevant, only denom membership matters + s.Require().True(sdk.Coins{{testDenom1, one}}.DenomsSubsetOf(sdk.Coins{{testDenom1, one}})) + s.Require().True(sdk.Coins{{testDenom1, two}}.DenomsSubsetOf(sdk.Coins{{testDenom1, one}})) + + // proper subset (receiver denoms all present in B) + s.Require().True(sdk.Coins{{testDenom1, one}}.DenomsSubsetOf(sdk.Coins{{testDenom1, one}, {testDenom2, one}})) + s.Require().True(sdk.Coins{{testDenom1, one}, {testDenom2, one}}.DenomsSubsetOf(sdk.Coins{{testDenom1, one}, {testDenom2, one}})) + s.Require().True(sdk.Coins{{testDenom1, one}, {testDenom3, one}}.DenomsSubsetOf(sdk.Coins{{testDenom1, one}, {testDenom2, one}, {testDenom3, one}})) + + // receiver contains a denom missing from B + s.Require().False(sdk.Coins{{testDenom1, one}}.DenomsSubsetOf(sdk.Coins{{testDenom2, one}})) + s.Require().False(sdk.Coins{{testDenom1, one}, {testDenom2, one}}.DenomsSubsetOf(sdk.Coins{{testDenom1, one}, {testDenom3, one}})) + // same length, one denom differs (guards the merge-walk termination) + s.Require().False(sdk.Coins{{testDenom2, one}, {testDenom3, one}}.DenomsSubsetOf(sdk.Coins{{testDenom1, one}, {testDenom2, one}})) + // receiver denom sorts after everything in B + s.Require().False(sdk.Coins{{"zzz", one}}.DenomsSubsetOf(sdk.Coins{{testDenom1, one}, {testDenom2, one}})) + + // large sorted inputs: subset and non-subset + const n = 5000 + full := make(sdk.Coins, n) + for i := 0; i < n; i++ { + full[i] = sdk.NewCoin(fmt.Sprintf("coin%08d", i), one) + } + s.Require().NoError(full.Validate(), "generated coins must satisfy the sorted/no-dup invariant") + + half := make(sdk.Coins, 0, n/2) + for i := 0; i < n; i += 2 { + half = append(half, full[i]) + } + s.Require().True(half.DenomsSubsetOf(full)) + + // flip one denom to something not in full => no longer a subset + notSubset := make(sdk.Coins, len(half)) + copy(notSubset, half) + notSubset[len(notSubset)-1] = sdk.NewCoin("zzz99999999", one) + s.Require().True(notSubset.IsValid()) + s.Require().False(notSubset.DenomsSubsetOf(full)) +} + func (s *coinTestSuite) TestNewCoins() { tenatom := sdk.NewInt64Coin("atom", 10) tenbtc := sdk.NewInt64Coin("btc", 10) diff --git a/sei-cosmos/x/feegrant/basic_fee.go b/sei-cosmos/x/feegrant/basic_fee.go index 3df5d264e6..854ced6437 100644 --- a/sei-cosmos/x/feegrant/basic_fee.go +++ b/sei-cosmos/x/feegrant/basic_fee.go @@ -7,6 +7,10 @@ import ( var _ FeeAllowanceI = (*BasicAllowance)(nil) +// MaxAllowanceDenoms bounds the number of coins allowed in a single allowance +// spend-limit list. +const MaxAllowanceDenoms = 100 + // Accept can use fee payment requested as well as timestamp of the current block // to determine whether or not to process this. This is checked in // Keeper.UseGrantedFees and the return values should match how it is handled there. @@ -38,6 +42,9 @@ func (a *BasicAllowance) Accept(ctx sdk.Context, fee sdk.Coins, _ []sdk.Msg) (bo // ValidateBasic implements FeeAllowance and enforces basic sanity checks func (a BasicAllowance) ValidateBasic() error { if a.SpendLimit != nil { + if len(a.SpendLimit) > MaxAllowanceDenoms { + return sdkerrors.Wrapf(ErrTooManyDenoms, "spend limit has %d denoms, max %d", len(a.SpendLimit), MaxAllowanceDenoms) + } if !a.SpendLimit.IsValid() { return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "send amount is invalid: %s", a.SpendLimit) } diff --git a/sei-cosmos/x/feegrant/basic_fee_test.go b/sei-cosmos/x/feegrant/basic_fee_test.go index 5a8ead554b..4b6d546b41 100644 --- a/sei-cosmos/x/feegrant/basic_fee_test.go +++ b/sei-cosmos/x/feegrant/basic_fee_test.go @@ -1,6 +1,7 @@ package feegrant_test import ( + "fmt" "testing" "time" @@ -148,3 +149,62 @@ func TestBasicFeeValidAllow(t *testing.T) { }) } } + +// sortedCoins builds a valid, sorted, duplicate-free Coins list of n denoms +// (zero-padded so lexical order matches numeric order). +func sortedCoins(n int) sdk.Coins { + coins := make(sdk.Coins, n) + for i := 0; i < n; i++ { + coins[i] = sdk.NewInt64Coin(fmt.Sprintf("coin%08d", i), 1) + } + return coins.Sort() +} + +func TestBasicAllowanceMaxDenoms(t *testing.T) { + atCap := &feegrant.BasicAllowance{SpendLimit: sortedCoins(feegrant.MaxAllowanceDenoms)} + require.NoError(t, atCap.ValidateBasic()) + + overCap := &feegrant.BasicAllowance{SpendLimit: sortedCoins(feegrant.MaxAllowanceDenoms + 1)} + err := overCap.ValidateBasic() + require.Error(t, err) + require.ErrorIs(t, err, feegrant.ErrTooManyDenoms) +} + +func TestPeriodicAllowanceMaxDenoms(t *testing.T) { + atCap := sortedCoins(feegrant.MaxAllowanceDenoms) + over := sortedCoins(feegrant.MaxAllowanceDenoms + 1) + + // Each case keeps the other allowance fields under cap so that a single + // over-cap field is the only thing that can trip ErrTooManyDenoms. This + // isolates the periodic-specific checks; if they only oversized every field + // at once, the embedded Basic.ValidateBasic() would short-circuit and the + // periodic checks would never be exercised. + cases := map[string]*feegrant.PeriodicAllowance{ + "basic spend limit over cap": { + Basic: feegrant.BasicAllowance{SpendLimit: over}, + PeriodSpendLimit: atCap, + PeriodCanSpend: atCap, + Period: time.Hour, + }, + "period spend limit over cap": { + Basic: feegrant.BasicAllowance{SpendLimit: atCap}, + PeriodSpendLimit: over, + PeriodCanSpend: atCap, + Period: time.Hour, + }, + "period can spend over cap": { + Basic: feegrant.BasicAllowance{SpendLimit: atCap}, + PeriodSpendLimit: atCap, + PeriodCanSpend: over, + Period: time.Hour, + }, + } + + for name, allowance := range cases { + t.Run(name, func(t *testing.T) { + err := allowance.ValidateBasic() + require.Error(t, err) + require.ErrorIs(t, err, feegrant.ErrTooManyDenoms) + }) + } +} diff --git a/sei-cosmos/x/feegrant/errors.go b/sei-cosmos/x/feegrant/errors.go index cb79f07f3a..ce19e425f2 100644 --- a/sei-cosmos/x/feegrant/errors.go +++ b/sei-cosmos/x/feegrant/errors.go @@ -22,4 +22,6 @@ var ( ErrNoMessages = sdkerrors.Register(DefaultCodespace, 6, "allowed messages are empty") // ErrMessageNotAllowed error if message is not allowed ErrMessageNotAllowed = sdkerrors.Register(DefaultCodespace, 7, "message not allowed") + // ErrTooManyDenoms error if an allowance coin list exceeds MaxAllowanceDenoms + ErrTooManyDenoms = sdkerrors.Register(DefaultCodespace, 8, "too many denominations in allowance") ) diff --git a/sei-cosmos/x/feegrant/periodic_fee.go b/sei-cosmos/x/feegrant/periodic_fee.go index 637a89e4b2..3dc146d1f8 100644 --- a/sei-cosmos/x/feegrant/periodic_fee.go +++ b/sei-cosmos/x/feegrant/periodic_fee.go @@ -79,12 +79,18 @@ func (a PeriodicAllowance) ValidateBasic() error { return err } + if len(a.PeriodSpendLimit) > MaxAllowanceDenoms { + return sdkerrors.Wrapf(ErrTooManyDenoms, "period spend limit has %d denoms, max %d", len(a.PeriodSpendLimit), MaxAllowanceDenoms) + } if !a.PeriodSpendLimit.IsValid() { return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "spend amount is invalid: %s", a.PeriodSpendLimit) } if !a.PeriodSpendLimit.IsAllPositive() { return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, "spend limit must be positive") } + if len(a.PeriodCanSpend) > MaxAllowanceDenoms { + return sdkerrors.Wrapf(ErrTooManyDenoms, "period can spend has %d denoms, max %d", len(a.PeriodCanSpend), MaxAllowanceDenoms) + } if !a.PeriodCanSpend.IsValid() { return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "can spend amount is invalid: %s", a.PeriodCanSpend) }