v0.37 pubsub: Handle big ints (#771)

Replaced int64 with big.int

Co-authored-by: Lasaro <[email protected]>
Co-authored-by: Sergio Mena <[email protected]>

.changelog/unreleased/bug-fixes/771-kvindexer-parsing-big-ints.md

@@ -0,0 +1,2 @@
+- `[state/kvindex]` Querying event attributes that are bigger than int64 is now enabled. We are not supporting reading floats from the db into the indexer  nor parsing them into BigFloats to not introduce breaking changes in minor releases.
+  ([\#771](https://github.com/cometbft/cometbft/pull/771))

.changelog/unreleased/bug-fixes/771-pubsub-parsing-big-ints.md

@@ -0,0 +1,3 @@
+- `[pubsub]` Pubsub queries are now able to parse big integers (larger than int64). Very big floats 
+   are also properly parsed into very big integers instead of being truncated to int64. 
+  ([\#771](https://github.com/cometbft/cometbft/pull/771))

docs/app-dev/indexing-transactions.md

@@ -245,3 +245,10 @@ This behavior was fixed with CometBFT 0.34.26+. However, if the data was indexed
 Tendermint Core and not re-indexed, that data will be queried as if all the attributes within a height
 occurred within the same event.
 
+# Event attribute value types
+Users can use anything as an event value. However, if the even attrbute value is a number, the following restrictions apply:
+- Negative numbers will not be properly retrieved when querying the indexer
+- When querying the events using `tx_search` and `block_search`, the value given as part of the condition cannot be a float.
+- Any event value retrieved from the database will be represented as a `BigInt` (from `math/big`)
+- Floating point values are not read from the database even with the introduction of `BigInt`. This was intentionally done 
+to keep the same beheaviour as was historically present and not introduce breaking  changes. This will be fixed in the 0.38 series.

docs/core/subscription.md

@@ -40,6 +40,20 @@ You can also use tags, given you had included them into DeliverTx
 response, to query transaction results. See [Indexing
 transactions](../app-dev/indexing-transactions.md) for details.
 
+
+## Query parameter and event type restrictions
+
+While CometBFT imposes no restrictions on the application with regards to the type of 
+the event output, there are several restrictions when it comes to querying 
+events whose attribute values are numeric. 
+
+- Queries cannot include negative numbers
+- If floating points are compared to integers, they are converted to an integer
+- Floating point to floating point comparison leads to a loss of precision for very big floating point numbers
+(`10000000000000000000.0` is treated the same as `10000000000000000000.6`) 
+- When floating points do get converted to integers, they are not rounded, the decimal part is simply truncated.
+This has been done to preserve the behaviour present before introducing the support for BigInts in the query parameters. 
+
 ## ValidatorSetUpdates
 
 When validator set changes, ValidatorSetUpdates event is published. The

libs/pubsub/query/query.go

@@ -10,6 +10,7 @@ package query
 
 import (
 	"fmt"
+	"math/big"
 	"reflect"
 	"regexp"
 	"strconv"
@@ -152,16 +153,18 @@ func (q *Query) Conditions() ([]Condition, error) {
 
 				conditions = append(conditions, Condition{eventAttr, op, value})
 			} else {
-				value, err := strconv.ParseInt(number, 10, 64)
-				if err != nil {
-					err = fmt.Errorf(
-						"got %v while trying to parse %s as int64 (should never happen if the grammar is correct)",
-						err, number,
+				valueBig := new(big.Int)
+
+				_, ok := valueBig.SetString(number, 10)
+				if !ok {
+					err := fmt.Errorf(
+						"problem parsing %s as bigint (should never happen if the grammar is correct)",
+						number,
 					)
 					return nil, err
 				}
+				conditions = append(conditions, Condition{eventAttr, op, valueBig})
 
-				conditions = append(conditions, Condition{eventAttr, op, value})
 			}
 
 		case ruletime:
@@ -299,11 +302,13 @@ func (q *Query) Matches(events map[string][]string) (bool, error) {
 					return false, nil
 				}
 			} else {
-				value, err := strconv.ParseInt(number, 10, 64)
-				if err != nil {
-					err = fmt.Errorf(
-						"got %v while trying to parse %s as int64 (should never happen if the grammar is correct)",
-						err, number,
+				value := new(big.Int)
+				_, ok := value.SetString(number, 10)
+
+				if !ok {
+					err := fmt.Errorf(
+						"problem parsing %s as bigInt (should never happen if the grammar is correct)",
+						number,
 					)
 					return false, err
 				}
@@ -452,42 +457,59 @@ func matchValue(value string, op Operator, operand reflect.Value) (bool, error)
 			return v == operandFloat64, nil
 		}
 
-	case reflect.Int64:
-		var v int64
+	case reflect.Pointer:
 
-		operandInt := operand.Interface().(int64)
-		filteredValue := numRegex.FindString(value)
+		switch operand.Interface().(type) {
+		case *big.Int:
+			filteredValue := numRegex.FindString(value)
+			operandVal := operand.Interface().(*big.Int)
+			v := new(big.Int)
+			if strings.ContainsAny(filteredValue, ".") {
+				// We do this just to check whether the string can be parsed as a float
+				_, err := strconv.ParseFloat(filteredValue, 64)
+				if err != nil {
+					err = fmt.Errorf(
+						"got %v while trying to parse %s as float64 (should never happen if the grammar is correct)",
+						err, filteredValue,
+					)
+					return false, err
+				}
 
-		// if value looks like float, we try to parse it as float
-		if strings.ContainsAny(filteredValue, ".") {
-			v1, err := strconv.ParseFloat(filteredValue, 64)
-			if err != nil {
-				return false, fmt.Errorf("failed to convert value %v from event attribute to float64: %w", filteredValue, err)
-			}
+				// If yes, we get the int part of the  string.
+				// We could simply cast the float to an int and use that to create a big int but
+				// if it is a number bigger than int64, it will not be parsed properly.
+				// If we use bigFloat and convert that to a string, the values will be rounded which
+				// is not what we want either.
+				// Here we are simulating the behavior that int64(floatValue). This was the default behavior
+				// before introducing BigInts and we do not want to break the logic in minor releases.
+				_, ok := v.SetString(strings.Split(filteredValue, ".")[0], 10)
+				if !ok {
+					return false, fmt.Errorf("failed to convert value %s from float to big int", filteredValue)
+				}
+			} else {
+				// try our best to convert value from tags to big int
+				_, ok := v.SetString(filteredValue, 10)
+
+				if !ok {
+					return false, fmt.Errorf("failed to convert value %v from event attribute to big int", filteredValue)
+				}
 
-			v = int64(v1)
-		} else {
-			var err error
-			// try our best to convert value from tags to int64
-			v, err = strconv.ParseInt(filteredValue, 10, 64)
-			if err != nil {
-				return false, fmt.Errorf("failed to convert value %v from event attribute to int64: %w", filteredValue, err)
 			}
-		}
+			cmpRes := operandVal.Cmp(v)
+			switch op {
+			case OpLessEqual:
+				return cmpRes == 0 || cmpRes == 1, nil
+			case OpGreaterEqual:
+				return cmpRes == 0 || cmpRes == -1, nil
+			case OpLess:
+				return cmpRes == 1, nil
+			case OpGreater:
+				return cmpRes == -1, nil
+			case OpEqual:
+				return cmpRes == 0, nil
+			}
 
-		switch op {
-		case OpLessEqual:
-			return v <= operandInt, nil
-		case OpGreaterEqual:
-			return v >= operandInt, nil
-		case OpLess:
-			return v < operandInt, nil
-		case OpGreater:
-			return v > operandInt, nil
-		case OpEqual:
-			return v == operandInt, nil
 		}
-
 	case reflect.String:
 		switch op {
 		case OpEqual:

libs/pubsub/query/query_test.go

@@ -2,6 +2,7 @@ package query_test
 
 import (
 	"fmt"
+	"math/big"
 	"testing"
 	"time"
 
@@ -11,6 +12,57 @@ import (
 	"github.com/cometbft/cometbft/libs/pubsub/query"
 )
 
+func TestBigNumbers(t *testing.T) {
+	bigInt := "10000000000000000000"
+	bigIntAsFloat := "10000000000000000000.0"
+	bigFloat := "10000000000000000000.6"
+	bigFloatLowerRounding := "10000000000000000000.1"
+	doubleBigInt := "20000000000000000000"
+
+	testCases := []struct {
+		s        string
+		events   map[string][]string
+		err      bool
+		matches  bool
+		matchErr bool
+	}{
+
+		{"account.balance <= " + bigInt, map[string][]string{"account.balance": {bigInt}}, false, true, false},
+		{"account.balance <= " + bigInt, map[string][]string{"account.balance": {bigIntAsFloat}}, false, true, false},
+		{"account.balance <= " + doubleBigInt, map[string][]string{"account.balance": {bigInt}}, false, true, false},
+		{"account.balance <= " + bigInt, map[string][]string{"account.balance": {"10000000000000000001"}}, false, false, false},
+		{"account.balance <= " + doubleBigInt, map[string][]string{"account.balance": {bigFloat}}, false, true, false},
+		// To maintain compatibility with the old implementation which did a simple cast of float to int64, we do not round the float
+		// Thus both 10000000000000000000.6 and "10000000000000000000.1 are equal to 10000000000000000000
+		// and the test does not find a match
+		{"account.balance > " + bigInt, map[string][]string{"account.balance": {bigFloat}}, false, false, false},
+		{"account.balance > " + bigInt, map[string][]string{"account.balance": {bigFloatLowerRounding}}, true, false, false},
+		// This test should also find a match, but floats that are too big cannot be properly converted, thus
+		// 10000000000000000000.6 gets rounded to 10000000000000000000
+		{"account.balance > " + bigIntAsFloat, map[string][]string{"account.balance": {bigFloat}}, false, false, false},
+		{"account.balance > 11234.0", map[string][]string{"account.balance": {"11234.6"}}, false, true, false},
+		{"account.balance <= " + bigInt, map[string][]string{"account.balance": {"1000.45"}}, false, true, false},
+	}
+
+	for _, tc := range testCases {
+		q, err := query.New(tc.s)
+		if !tc.err {
+			require.Nil(t, err)
+		}
+		require.NotNil(t, q, "Query '%s' should not be nil", tc.s)
+
+		if tc.matches {
+			match, err := q.Matches(tc.events)
+			assert.Nil(t, err, "Query '%s' should not error on match %v", tc.s, tc.events)
+			assert.True(t, match, "Query '%s' should match %v", tc.s, tc.events)
+		} else {
+			match, err := q.Matches(tc.events)
+			assert.Equal(t, tc.matchErr, err != nil, "Unexpected error for query '%s' match %v", tc.s, tc.events)
+			assert.False(t, match, "Query '%s' should not match %v", tc.s, tc.events)
+		}
+	}
+}
+
 func TestMatches(t *testing.T) {
 	var (
 		txDate = "2017-01-01"
@@ -180,6 +232,10 @@ func TestConditions(t *testing.T) {
 	txTime, err := time.Parse(time.RFC3339, "2013-05-03T14:45:00Z")
 	require.NoError(t, err)
 
+	bigInt := new(big.Int)
+	bigInt, ok := bigInt.SetString("10000000000000000000", 10)
+	require.True(t, ok)
+
 	testCases := []struct {
 		s          string
 		conditions []query.Condition
@@ -193,8 +249,24 @@ func TestConditions(t *testing.T) {
 		{
 			s: "tx.gas > 7 AND tx.gas < 9",
 			conditions: []query.Condition{
-				{CompositeKey: "tx.gas", Op: query.OpGreater, Operand: int64(7)},
-				{CompositeKey: "tx.gas", Op: query.OpLess, Operand: int64(9)},
+				{CompositeKey: "tx.gas", Op: query.OpGreater, Operand: big.NewInt(7)},
+				{CompositeKey: "tx.gas", Op: query.OpLess, Operand: big.NewInt(9)},
+			},
+		},
+		{
+
+			s: "tx.gas > 7.5 AND tx.gas < 9",
+			conditions: []query.Condition{
+				{CompositeKey: "tx.gas", Op: query.OpGreater, Operand: 7.5},
+				{CompositeKey: "tx.gas", Op: query.OpLess, Operand: big.NewInt(9)},
+			},
+		},
+		{
+
+			s: "tx.gas > " + bigInt.String() + " AND tx.gas < 9",
+			conditions: []query.Condition{
+				{CompositeKey: "tx.gas", Op: query.OpGreater, Operand: bigInt},
+				{CompositeKey: "tx.gas", Op: query.OpLess, Operand: big.NewInt(9)},
 			},
 		},
 		{

state/indexer/block/kv/kv.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math/big"
 	"sort"
 	"strconv"
 	"strings"
@@ -293,9 +294,10 @@ LOOP:
 			continue
 		}
 
-		if _, ok := qr.AnyBound().(int64); ok {
-			v, err := strconv.ParseInt(eventValue, 10, 64)
-			if err != nil {
+		if _, ok := qr.AnyBound().(*big.Int); ok {
+			v := new(big.Int)
+			v, ok := v.SetString(eventValue, 10)
+			if !ok { // If the number was not int it might be a float but this behavior is kept the same as before the patch
 				continue LOOP
 			}
 
@@ -364,15 +366,16 @@ func (idx *BlockerIndexer) setTmpHeights(tmpHeights map[string][]byte, it dbm.It
 
 }
 
-func checkBounds(ranges indexer.QueryRange, v int64) bool {
+func checkBounds(ranges indexer.QueryRange, v *big.Int) bool {
 	include := true
 	lowerBound := ranges.LowerBoundValue()
 	upperBound := ranges.UpperBoundValue()
-	if lowerBound != nil && v < lowerBound.(int64) {
+
+	if lowerBound != nil && v.Cmp(lowerBound.(*big.Int)) == -1 {
 		include = false
 	}
 
-	if upperBound != nil && v > upperBound.(int64) {
+	if upperBound != nil && v.Cmp(upperBound.(*big.Int)) == 1 {
 		include = false
 	}