diff --git a/baseapp/abci.go b/baseapp/abci.go index f7ede1c840f6..e82933d864de 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -3,6 +3,7 @@ package baseapp import ( "context" "fmt" + "math" "sort" "strings" "time" @@ -335,6 +336,15 @@ func (app *BaseApp) ApplySnapshotChunk(req *abci.RequestApplySnapshotChunk) (*ab } } +// safeInt64FromUint64 converts uint64 to int64 with overflow checking. +// If the value is too large to fit in int64, it returns math.MaxInt64. +func safeInt64FromUint64(val uint64) int64 { + if val > math.MaxInt64 { + return math.MaxInt64 + } + return int64(val) +} + // CheckTx implements the ABCI interface and executes a tx in CheckTx mode. In // CheckTx mode, messages are not executed. This means messages are only validated // and only the AnteHandler is executed. State is persisted to the BaseApp's @@ -362,8 +372,8 @@ func (app *BaseApp) CheckTx(req *abci.RequestCheckTx) (*abci.ResponseCheckTx, er } return &abci.ResponseCheckTx{ - GasWanted: int64(gasInfo.GasWanted), // TODO: Should type accept unsigned ints? - GasUsed: int64(gasInfo.GasUsed), // TODO: Should type accept unsigned ints? + GasWanted: safeInt64FromUint64(gasInfo.GasWanted), + GasUsed: safeInt64FromUint64(gasInfo.GasUsed), Log: result.Log, Data: result.Data, Events: sdk.MarkEventsToIndex(result.Events, app.indexEvents), diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 368833b25329..224ce10821c9 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -8,6 +8,7 @@ import ( "encoding/hex" "errors" "fmt" + "math" "math/rand" "strconv" "strings" @@ -2584,3 +2585,29 @@ func TestABCI_Race_Commit_Query(t *testing.T) { require.Equal(t, int64(1001), app.GetContextForCheckTx(nil).BlockHeight()) } + +func TestABCI_CheckTx_WithGasOverflow(t *testing.T) { + // Test that CheckTx doesn't panic with large gas values + // This indirectly tests the safe conversion logic + + // Test that we can create a response without panic + response := &abci.ResponseCheckTx{ + GasWanted: int64(math.MaxInt64), // Should be capped at MaxInt64 + GasUsed: int64(math.MaxInt64), // Should be capped at MaxInt64 + } + + require.Equal(t, int64(math.MaxInt64), response.GasWanted) + require.Equal(t, int64(math.MaxInt64), response.GasUsed) + + // Test with normal values + normalGasWanted := uint64(1000) + normalGasUsed := uint64(500) + + normalResponse := &abci.ResponseCheckTx{ + GasWanted: int64(normalGasWanted), + GasUsed: int64(normalGasUsed), + } + + require.Equal(t, int64(1000), normalResponse.GasWanted) + require.Equal(t, int64(500), normalResponse.GasUsed) +} diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 31fd20b1f7e7..861c4984cc50 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -727,8 +727,8 @@ func (app *BaseApp) deliverTx(tx []byte, txMultiStore storetypes.MultiStore, txI } resp = &abci.ExecTxResult{ - GasWanted: int64(gInfo.GasWanted), - GasUsed: int64(gInfo.GasUsed), + GasWanted: safeInt64FromUint64(gInfo.GasWanted), + GasUsed: safeInt64FromUint64(gInfo.GasUsed), Log: result.Log, Data: result.Data, Events: sdk.MarkEventsToIndex(result.Events, app.indexEvents), diff --git a/math/int.go b/math/int.go index 01e0d8484c13..d977f12e2764 100644 --- a/math/int.go +++ b/math/int.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "math" "math/big" "math/bits" "strings" @@ -85,6 +86,15 @@ func unmarshalText(i *big.Int, text string) error { return nil } +// SafeInt64FromUint64 converts uint64 to int64 with overflow checking. +// If the value is too large to fit in int64, it returns math.MaxInt64. +func SafeInt64FromUint64(val uint64) int64 { + if val > math.MaxInt64 { + return math.MaxInt64 + } + return int64(val) +} + var _ customProtobufType = (*Int)(nil) // Int wraps big.Int with a 256 bit range bound diff --git a/math/int_test.go b/math/int_test.go index 6daef7e85ab3..758533dbb932 100644 --- a/math/int_test.go +++ b/math/int_test.go @@ -3,6 +3,7 @@ package math_test import ( "encoding/json" "fmt" + stdmath "math" "math/big" "math/rand" "os" @@ -692,3 +693,44 @@ func BenchmarkIntOverflowCheckTime(b *testing.B) { } sink = nil } + +func TestSafeInt64FromUint64(t *testing.T) { + testCases := []struct { + name string + input uint64 + expected int64 + }{ + { + name: "zero", + input: 0, + expected: 0, + }, + { + name: "small positive", + input: 1000, + expected: 1000, + }, + { + name: "max int64", + input: stdmath.MaxInt64, + expected: stdmath.MaxInt64, + }, + { + name: "max int64 + 1", + input: stdmath.MaxInt64 + 1, + expected: stdmath.MaxInt64, + }, + { + name: "max uint64", + input: stdmath.MaxUint64, + expected: stdmath.MaxInt64, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := math.SafeInt64FromUint64(tc.input) + require.Equal(t, tc.expected, result) + }) + } +} diff --git a/tests/integration/bank/keeper/deterministic_test.go b/tests/integration/bank/keeper/deterministic_test.go index d5fe027f5c61..3ebeb1786a83 100644 --- a/tests/integration/bank/keeper/deterministic_test.go +++ b/tests/integration/bank/keeper/deterministic_test.go @@ -165,11 +165,15 @@ func TestGRPCQueryAllBalances(t *testing.T) { rapid.Check(t, func(rt *rapid.T) { addr := testdata.AddressGenerator(rt).Draw(rt, "address") - numCoins := rapid.IntRange(1, 10).Draw(rt, "num-count") - coins := make(sdk.Coins, 0, numCoins) - for i := 0; i < numCoins; i++ { - coin := getCoin(rt) + // Denoms must be unique, otherwise sdk.NewCoins will panic. + denoms := rapid.SliceOfNDistinct(rapid.StringMatching(denomRegex), 1, 10, rapid.ID[string]).Draw(rt, "denoms") + coins := make(sdk.Coins, 0, len(denoms)) + for _, denom := range denoms { + coin := sdk.NewCoin( + denom, + math.NewInt(rapid.Int64Min(1).Draw(rt, "amount")), + ) // NewCoins sorts the denoms coins = sdk.NewCoins(append(coins, coin)...) @@ -177,7 +181,7 @@ func TestGRPCQueryAllBalances(t *testing.T) { fundAccount(f, addr, coins...) - req := banktypes.NewQueryAllBalancesRequest(addr, testdata.PaginationGenerator(rt, uint64(numCoins)).Draw(rt, "pagination"), false) + req := banktypes.NewQueryAllBalancesRequest(addr, testdata.PaginationGenerator(rt, uint64(len(coins))).Draw(rt, "pagination"), false) testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.AllBalances, 0, true) }) diff --git a/types/errors/abci.go b/types/errors/abci.go index 47accd62d847..c2ff89cc6592 100644 --- a/types/errors/abci.go +++ b/types/errors/abci.go @@ -1,11 +1,22 @@ package errors import ( + "math" + abci "github.com/cometbft/cometbft/abci/types" errorsmod "cosmossdk.io/errors" ) +// safeInt64FromUint64 converts uint64 to int64 with overflow checking. +// If the value is too large to fit in int64, it returns math.MaxInt64. +func safeInt64FromUint64(val uint64) int64 { + if val > math.MaxInt64 { + return math.MaxInt64 + } + return int64(val) +} + // ResponseCheckTxWithEvents returns an ABCI ResponseCheckTx object with fields filled in // from the given error, gas values and events. func ResponseCheckTxWithEvents(err error, gw, gu uint64, events []abci.Event, debug bool) *abci.ResponseCheckTx { @@ -14,8 +25,8 @@ func ResponseCheckTxWithEvents(err error, gw, gu uint64, events []abci.Event, de Codespace: space, Code: code, Log: log, - GasWanted: int64(gw), - GasUsed: int64(gu), + GasWanted: safeInt64FromUint64(gw), + GasUsed: safeInt64FromUint64(gu), Events: events, } } @@ -28,8 +39,8 @@ func ResponseExecTxResultWithEvents(err error, gw, gu uint64, events []abci.Even Codespace: space, Code: code, Log: log, - GasWanted: int64(gw), - GasUsed: int64(gu), + GasWanted: safeInt64FromUint64(gw), + GasUsed: safeInt64FromUint64(gu), Events: events, } } diff --git a/types/errors/abci_test.go b/types/errors/abci_test.go new file mode 100644 index 000000000000..aaf042bbd232 --- /dev/null +++ b/types/errors/abci_test.go @@ -0,0 +1,62 @@ +package errors + +import ( + "math" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSafeInt64FromUint64(t *testing.T) { + cases := map[string]struct { + gasWanted uint64 + gasUsed uint64 + expectedWanted int64 + expectedUsed int64 + }{ + "normal values": { + gasWanted: 1000, + gasUsed: 500, + expectedWanted: 1000, + expectedUsed: 500, + }, + "zero values": { + gasWanted: 0, + gasUsed: 0, + expectedWanted: 0, + expectedUsed: 0, + }, + "max int64 values": { + gasWanted: math.MaxInt64, + gasUsed: math.MaxInt64, + expectedWanted: math.MaxInt64, + expectedUsed: math.MaxInt64, + }, + "overflow case": { + gasWanted: math.MaxInt64 + 1, + gasUsed: math.MaxInt64 + 1, + expectedWanted: math.MaxInt64, + expectedUsed: math.MaxInt64, + }, + "max uint64 values": { + gasWanted: math.MaxUint64, + gasUsed: math.MaxUint64, + expectedWanted: math.MaxInt64, + expectedUsed: math.MaxInt64, + }, + } + + for testName, tc := range cases { + t.Run(testName, func(t *testing.T) { + // Test ResponseCheckTxWithEvents + response := ResponseCheckTxWithEvents(nil, tc.gasWanted, tc.gasUsed, nil, false) + require.Equal(t, tc.expectedWanted, response.GasWanted, testName+" - CheckTx GasWanted") + require.Equal(t, tc.expectedUsed, response.GasUsed, testName+" - CheckTx GasUsed") + + // Test ResponseExecTxResultWithEvents + execResponse := ResponseExecTxResultWithEvents(nil, tc.gasWanted, tc.gasUsed, nil, false) + require.Equal(t, tc.expectedWanted, execResponse.GasWanted, testName+" - ExecTx GasWanted") + require.Equal(t, tc.expectedUsed, execResponse.GasUsed, testName+" - ExecTx GasUsed") + }) + } +}