From f667d3fcab8c961ae3921a8d53254c6cfc0a824b Mon Sep 17 00:00:00 2001 From: Jasmina Malicevic Date: Tue, 16 May 2023 12:25:53 +0200 Subject: [PATCH] pubsub/kvindexer:support for big numbers - v2 (#797) * Applied Michaels patch * Added corner case tests, failing curently * Support for big floats and ints added * Added new util file * Fixed linter error * added internal package * Revert "added internal package" This reverts commit ef7f2b4bf0ef8be6dc058715eccfa02b61a583ec. * added internal/indexer * Moved utils to internal * Fixed linter * Updated docs * Applied @sergio-mena s PR comments * Fixed linter * Return with error in compare float * Changelog entries * Apply lasaroj's comments. Co-authored-by: Lasaro * applied some PR comments * updated docs Co-authored-by: Sergio Mena * Added errors and logger * Fixed linter * Fixed sentence in comment * Removed changelog * Avoid converting to string when parsing int to float * Added unexpected types to error messages * Added comment on the 8atom regex in pubsub --------- Co-authored-by: Lasaro Co-authored-by: Sergio Mena --- docs/app-dev/indexing-transactions.md | 9 ++ docs/core/subscription.md | 19 ++++ internal/indexer/indexer_utils.go | 119 ++++++++++++++++++++++ libs/pubsub/query/query.go | 32 +++--- libs/pubsub/query/query_test.go | 78 +++++++++++++++ libs/pubsub/query/syntax/parser.go | 35 +++++-- node/setup.go | 2 + state/indexer/block.go | 3 + state/indexer/block/kv/kv.go | 100 ++++++++++++++----- state/indexer/block/kv/kv_test.go | 137 ++++++++++++++++++++++++++ state/indexer/block/kv/util.go | 24 +++-- state/indexer/block/null/null.go | 4 + state/indexer/mocks/block_indexer.go | 7 ++ state/indexer/query_range.go | 21 +++- state/indexer/sink/psql/backport.go | 6 ++ state/txindex/indexer.go | 5 + state/txindex/kv/kv.go | 102 +++++++++++++------ state/txindex/kv/kv_test.go | 79 +++++++++++++++ state/txindex/kv/utils.go | 23 +++-- state/txindex/mocks/tx_indexer.go | 9 +- state/txindex/null/null.go | 6 ++ 21 files changed, 726 insertions(+), 94 deletions(-) create mode 100644 internal/indexer/indexer_utils.go diff --git a/docs/app-dev/indexing-transactions.md b/docs/app-dev/indexing-transactions.md index d6ee4e0ceb7..a2ca5a5b18a 100644 --- a/docs/app-dev/indexing-transactions.md +++ b/docs/app-dev/indexing-transactions.md @@ -276,3 +276,12 @@ 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 event attribute value is a number, the following needs to be taken into account: + +- Negative numbers will not be properly retrieved when querying the indexer. +- Event values are converted to big floats (from the `big/math` package). The precision of the floating point number is set to the bit length +of the integer it is supposed to represent, so that there is no loss of information due to insufficient precision. This was not present before CometBFT v0.38.x and all float values were ignored. +- As of CometBFT v0.38.x, queries can contain floating point numbers as well. +- Note that comparing to floats can be imprecise with a high number of decimals. \ No newline at end of file diff --git a/docs/core/subscription.md b/docs/core/subscription.md index beb5f4599f6..b6be5f3b36f 100644 --- a/docs/core/subscription.md +++ b/docs/core/subscription.md @@ -40,6 +40,25 @@ 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 considerations that need to be taken into account +when querying events with numeric values. + +- Queries convert all numeric event values to `big.Float` , provided by `math/big`. Integers +are converted into a float with a precision equal to the number of bits needed +to represent this integer. This is done to avoid precision loss for big integers when they +are converted with the default precision (`64`). +- When comparing two values, if either one of them is a float, the other one will be represented +as a big float. Integers are again parsed as big floats with a precision equal to the number +of bits required to represent them. +- As with all floating point comparisons, comparing floats with decimal values can lead to imprecise +results. +- Queries cannot include negative numbers + +Prior to version `v0.38.x`, floats were not supported as query parameters. + ## ValidatorSetUpdates When validator set changes, ValidatorSetUpdates event is published. The diff --git a/internal/indexer/indexer_utils.go b/internal/indexer/indexer_utils.go new file mode 100644 index 00000000000..c6caaee6f85 --- /dev/null +++ b/internal/indexer/indexer_utils.go @@ -0,0 +1,119 @@ +package indexer + +import ( + "fmt" + "math/big" + + "github.com/cometbft/cometbft/state/indexer" +) + +// If the actual event value is a float, we get the condition and parse it as a float +// to compare against +func compareFloat(op1 *big.Float, op2 interface{}) (int, bool, error) { + switch opVal := op2.(type) { + case *big.Int: + vF := new(big.Float) + vF.SetInt(opVal) + cmp := op1.Cmp(vF) + return cmp, false, nil + + case *big.Float: + return op1.Cmp(opVal), true, nil + default: + return -1, false, fmt.Errorf("unable to parse arguments, bad type: %T", op2) + } +} + +// If the event value we compare against the condition (op2) is an integer +// we convert the int to float with a precision equal to the number of bits +// needed to represent the integer to avoid rounding issues with floats +// where 100 would equal to 100.2 because 100.2 is rounded to 100, while 100.7 +// would be rounded to 101. +func compareInt(op1 *big.Int, op2 interface{}) (int, bool, error) { + + switch opVal := op2.(type) { + case *big.Int: + return op1.Cmp(opVal), false, nil + case *big.Float: + vF := new(big.Float) + vF.SetInt(op1) + return vF.Cmp(opVal), true, nil + default: + return -1, false, fmt.Errorf("unable to parse arguments, unexpected type: %T", op2) + } +} + +func CheckBounds(ranges indexer.QueryRange, v interface{}) (bool, error) { + // These functions fetch the lower and upper bounds of the query + // It is expected that for x > 5, the value of lowerBound is 6. + // This is achieved by adding one to the actual lower bound. + // For a query of x < 5, the value of upper bound is 4. + // This is achieved by subtracting one from the actual upper bound. + + // For integers this behavior will work. However, for floats, we cannot simply add/sub 1. + // Query :x < 5.5 ; x = 5 should match the query. If we subtracted one as for integers, + // the upperBound would be 4.5 and x would not match. Thus we do not subtract anything for + // floating point bounds. + + // We can rewrite these functions to not add/sub 1 but the function handles also time arguments. + // To be sure we are not breaking existing queries that compare time, and as we are planning to replace + // the indexer in the future, we adapt the code here to handle floats as a special case. + lowerBound := ranges.LowerBoundValue() + upperBound := ranges.UpperBoundValue() + + // *Explanation for the isFloat condition below.* + // In LowerBoundValue(), for floating points, we cannot simply add 1 due to the reasons explained in + // in the comment at the beginning. The same is true for subtracting one for UpperBoundValue(). + // That means that for integers, if the condition is >=, cmp will be either 0 or 1 + // ( cmp == -1 should always be false). + // But if the lowerBound is a float, we have not subtracted one, so returning a 0 + // is correct only if ranges.IncludeLowerBound is true. + // example int: x < 100; upperBound = 99; if x.Cmp(99) == 0 the condition holds + // example float: x < 100.0; upperBound = 100.0; if x.Cmp(100) ==0 then returning x + // would be wrong. + switch vVal := v.(type) { + case *big.Int: + if lowerBound != nil { + cmp, isFloat, err := compareInt(vVal, lowerBound) + if err != nil { + return false, err + } + if cmp == -1 || (isFloat && cmp == 0 && !ranges.IncludeLowerBound) { + return false, err + } + } + if upperBound != nil { + cmp, isFloat, err := compareInt(vVal, upperBound) + if err != nil { + return false, err + } + if cmp == 1 || (isFloat && cmp == 0 && !ranges.IncludeUpperBound) { + return false, err + } + } + + case *big.Float: + if lowerBound != nil { + cmp, isFloat, err := compareFloat(vVal, lowerBound) + if err != nil { + return false, err + } + if cmp == -1 || (cmp == 0 && isFloat && !ranges.IncludeLowerBound) { + return false, err + } + } + if upperBound != nil { + cmp, isFloat, err := compareFloat(vVal, upperBound) + if err != nil { + return false, err + } + if cmp == 1 || (cmp == 0 && isFloat && !ranges.IncludeUpperBound) { + return false, err + } + } + + default: + return false, fmt.Errorf("invalid argument type in query: %T", v) + } + return true, nil +} diff --git a/libs/pubsub/query/query.go b/libs/pubsub/query/query.go index e1db675adc1..8382d40ce3f 100644 --- a/libs/pubsub/query/query.go +++ b/libs/pubsub/query/query.go @@ -10,8 +10,8 @@ package query import ( "fmt" + "math/big" "regexp" - "strconv" "strings" "time" @@ -218,13 +218,23 @@ func compileCondition(cond syntax.Condition) (condition, error) { return out, nil } -// TODO(creachadair): The existing implementation allows anything number shaped -// to be treated as a number. This preserves the parts of that behavior we had -// tests for, but we should probably get rid of that. +// We use this regex to support queries of the from "8atom", "6.5stake", +// which are actively used in production. +// The regex takes care of removing the non-number suffix. var extractNum = regexp.MustCompile(`^\d+(\.\d+)?`) -func parseNumber(s string) (float64, error) { - return strconv.ParseFloat(extractNum.FindString(s), 64) +func parseNumber(s string) (*big.Float, error) { + intVal := new(big.Int) + if _, ok := intVal.SetString(s, 10); !ok { + f, _, err := big.ParseFloat(extractNum.FindString(s), 10, 125, big.ToNearestEven) + if err != nil { + return nil, err + } + return f, err + } + f, _, err := big.ParseFloat(extractNum.FindString(s), 10, uint(intVal.BitLen()), big.ToNearestEven) + return f, err + } // A map of operator ⇒ argtype ⇒ match-constructor. @@ -248,7 +258,7 @@ var opTypeMap = map[syntax.Token]map[syntax.Token]func(interface{}) func(string) syntax.TNumber: func(v interface{}) func(string) bool { return func(s string) bool { w, err := parseNumber(s) - return err == nil && w == v.(float64) + return err == nil && w.Cmp(v.(*big.Float)) == 0 } }, syntax.TDate: func(v interface{}) func(string) bool { @@ -268,7 +278,7 @@ var opTypeMap = map[syntax.Token]map[syntax.Token]func(interface{}) func(string) syntax.TNumber: func(v interface{}) func(string) bool { return func(s string) bool { w, err := parseNumber(s) - return err == nil && w < v.(float64) + return err == nil && w.Cmp(v.(*big.Float)) < 0 } }, syntax.TDate: func(v interface{}) func(string) bool { @@ -288,7 +298,7 @@ var opTypeMap = map[syntax.Token]map[syntax.Token]func(interface{}) func(string) syntax.TNumber: func(v interface{}) func(string) bool { return func(s string) bool { w, err := parseNumber(s) - return err == nil && w <= v.(float64) + return err == nil && w.Cmp(v.(*big.Float)) <= 0 } }, syntax.TDate: func(v interface{}) func(string) bool { @@ -308,7 +318,7 @@ var opTypeMap = map[syntax.Token]map[syntax.Token]func(interface{}) func(string) syntax.TNumber: func(v interface{}) func(string) bool { return func(s string) bool { w, err := parseNumber(s) - return err == nil && w > v.(float64) + return err == nil && w.Cmp(v.(*big.Float)) > 0 } }, syntax.TDate: func(v interface{}) func(string) bool { @@ -328,7 +338,7 @@ var opTypeMap = map[syntax.Token]map[syntax.Token]func(interface{}) func(string) syntax.TNumber: func(v interface{}) func(string) bool { return func(s string) bool { w, err := parseNumber(s) - return err == nil && w >= v.(float64) + return err == nil && w.Cmp(v.(*big.Float)) >= 0 } }, syntax.TDate: func(v interface{}) func(string) bool { diff --git a/libs/pubsub/query/query_test.go b/libs/pubsub/query/query_test.go index 68b270895ce..2c8fcc557ed 100644 --- a/libs/pubsub/query/query_test.go +++ b/libs/pubsub/query/query_test.go @@ -173,6 +173,84 @@ var apiTypeEvents = []types.Event{ }, } +func TestBigNumbers(t *testing.T) { + + apiBigNumTest := map[string][]string{ + "big.value": { + "99999999999999999999", + }, + "big2.value": { + "18446744073709551615", // max(uint64) == 18446744073709551615 + }, + "big.floatvalue": { + "99999999999999999999.10", + }, + "big2.floatvalue": { + "18446744073709551615.6", // max(uint64) == 18446744073709551615 + }, + } + + testCases := []struct { + s string + events map[string][]string + matches bool + }{ + + // Test cases for values that exceed the capacity if int64/float64. + {`big.value >= 99999999999999999999`, + apiBigNumTest, + true}, + {`big.value > 99999999999999999998`, + apiBigNumTest, + true}, + {`big2.value <= 18446744073709551615`, + apiBigNumTest, true}, + {`big.floatvalue >= 99999999999999999999`, + apiBigNumTest, + true}, + {`big.floatvalue > 99999999999999999998.10`, + apiBigNumTest, + true}, + {`big.floatvalue > 99999999999999999998`, + apiBigNumTest, + true}, + {`big2.floatvalue <= 18446744073709551615.6`, + apiBigNumTest, + true}, + {`big2.floatvalue <= 18446744073709551615.6`, + apiBigNumTest, + true}, + {`big2.floatvalue >= 18446744073709551615`, + apiBigNumTest, + true}, + {`big2.floatvalue >= 12.5`, + apiBigNumTest, + true}, + {`big.value >= 10`, + apiBigNumTest, + true}, + } + + for i, tc := range testCases { + t.Run(fmt.Sprintf("%02d", i+1), func(t *testing.T) { + c, err := query.New(tc.s) + if err != nil { + t.Fatalf("NewCompiled %#q: unexpected error: %v", tc.s, err) + } + + got, err := c.Matches(tc.events) + if err != nil { + t.Errorf("Query: %#q\nInput: %+v\nMatches: got error %v", + tc.s, tc.events, err) + } + if got != tc.matches { + t.Errorf("Query: %#q\nInput: %+v\nMatches: got %v, want %v", + tc.s, tc.events, got, tc.matches) + } + }) + } +} + func TestCompiledMatches(t *testing.T) { var ( txDate = "2017-01-01" diff --git a/libs/pubsub/query/syntax/parser.go b/libs/pubsub/query/syntax/parser.go index a100ec79c73..26c8554908a 100644 --- a/libs/pubsub/query/syntax/parser.go +++ b/libs/pubsub/query/syntax/parser.go @@ -3,8 +3,7 @@ package syntax import ( "fmt" "io" - "math" - "strconv" + "math/big" "strings" "time" ) @@ -68,17 +67,35 @@ func (a *Arg) String() string { } } -// Number returns the value of the argument text as a number, or a NaN if the +// Number returns the value of the argument text as a number, or nil if the // text does not encode a valid number value. -func (a *Arg) Number() float64 { +func (a *Arg) Number() *big.Float { if a == nil { - return -1 + return nil } - v, err := strconv.ParseFloat(a.text, 64) - if err == nil && v >= 0 { - return v + intVal := new(big.Int) + if _, ok := intVal.SetString(a.text, 10); !ok { + f, _, err := big.ParseFloat(a.text, 10, 125, big.ToNearestEven) + if err != nil { + return nil + } + return f + } + // If it is indeed a big integer, we make sure to convert it to a float with enough precision + // to represent all the bits + bitLen := uint(intVal.BitLen()) + var f *big.Float + var err error + if bitLen <= 64 { + f, _, err = big.ParseFloat(a.text, 10, 0, big.ToNearestEven) + } else { + f, _, err = big.ParseFloat(a.text, 10, bitLen, big.ToNearestEven) + } + if err != nil { + return nil } - return math.NaN() + return f + } // Time returns the value of the argument text as a time, or the zero value if diff --git a/node/setup.go b/node/setup.go index ce965a1e9ea..39df1fba939 100644 --- a/node/setup.go +++ b/node/setup.go @@ -154,6 +154,8 @@ func createAndStartIndexerService( return nil, nil, nil, err } + txIndexer.SetLogger(logger.With("module", "txindex")) + blockIndexer.SetLogger(logger.With("module", "txindex")) indexerService := txindex.NewIndexerService(txIndexer, blockIndexer, eventBus, false) indexerService.SetLogger(logger.With("module", "txindex")) diff --git a/state/indexer/block.go b/state/indexer/block.go index b79d66f9a3c..4044d2aaa7b 100644 --- a/state/indexer/block.go +++ b/state/indexer/block.go @@ -3,6 +3,7 @@ package indexer import ( "context" + "github.com/cometbft/cometbft/libs/log" "github.com/cometbft/cometbft/libs/pubsub/query" "github.com/cometbft/cometbft/types" ) @@ -21,4 +22,6 @@ type BlockIndexer interface { // Search performs a query for block heights that match a given FinalizeBlock // event search criteria. Search(ctx context.Context, q *query.Query) ([]int64, error) + + SetLogger(l log.Logger) } diff --git a/state/indexer/block/kv/kv.go b/state/indexer/block/kv/kv.go index 989d8eb727f..689758e5b60 100644 --- a/state/indexer/block/kv/kv.go +++ b/state/indexer/block/kv/kv.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "math/big" "sort" "strconv" "strings" @@ -14,6 +15,8 @@ import ( dbm "github.com/cometbft/cometbft-db" abci "github.com/cometbft/cometbft/abci/types" + idxutil "github.com/cometbft/cometbft/internal/indexer" + "github.com/cometbft/cometbft/libs/log" "github.com/cometbft/cometbft/libs/pubsub/query" "github.com/cometbft/cometbft/libs/pubsub/query/syntax" "github.com/cometbft/cometbft/state/indexer" @@ -31,6 +34,7 @@ type BlockerIndexer struct { // Add unique event identifier to use when querying // Matching will be done both on height AND eventSeq eventSeq int64 + log log.Logger } func New(store dbm.DB) *BlockerIndexer { @@ -39,6 +43,10 @@ func New(store dbm.DB) *BlockerIndexer { } } +func (idx *BlockerIndexer) SetLogger(l log.Logger) { + idx.log = l +} + // Has returns true if the given height has been indexed. An error is returned // upon database query failure. func (idx *BlockerIndexer) Has(height int64) (bool, error) { @@ -285,20 +293,48 @@ LOOP: continue } - if _, ok := qr.AnyBound().(int64); ok { - v, err := strconv.ParseInt(eventValue, 10, 64) - if err != nil { - continue LOOP + if _, ok := qr.AnyBound().(*big.Float); ok { + v := new(big.Int) + v, ok := v.SetString(eventValue, 10) + var vF *big.Float + if !ok { + // The precision here is 125. For numbers bigger than this, the value + // will not be parsed properly + vF, _, err = big.ParseFloat(eventValue, 10, 125, big.ToNearestEven) + if err != nil { + continue LOOP + } } if qr.Key != types.BlockHeightKey { keyHeight, err := parseHeightFromEventKey(it.Key()) - if err != nil || !checkHeightConditions(heightInfo, keyHeight) { + if err != nil { + idx.log.Error("failure to parse height from key:", err) + continue LOOP + } + withinHeight, err := checkHeightConditions(heightInfo, keyHeight) + if err != nil { + idx.log.Error("failure checking for height bounds:", err) + continue LOOP + } + if !withinHeight { continue LOOP } } - if checkBounds(qr, v) { - idx.setTmpHeights(tmpHeights, it) + + var withinBounds bool + var err error + if !ok { + withinBounds, err = idxutil.CheckBounds(qr, vF) + } else { + withinBounds, err = idxutil.CheckBounds(qr, v) + } + if err != nil { + idx.log.Error("failed to parse bounds:", err) + } else { + if withinBounds { + idx.setTmpHeights(tmpHeights, it) + } } } @@ -356,21 +392,6 @@ func (idx *BlockerIndexer) setTmpHeights(tmpHeights map[string][]byte, it dbm.It } -func checkBounds(ranges indexer.QueryRange, v int64) bool { - include := true - lowerBound := ranges.LowerBoundValue() - upperBound := ranges.UpperBoundValue() - if lowerBound != nil && v < lowerBound.(int64) { - include = false - } - - if upperBound != nil && v > upperBound.(int64) { - include = false - } - - return include -} - // match returns all matching heights that meet a given query condition and start // key. An already filtered result (filteredHeights) is provided such that any // non-intersecting matches are removed. @@ -404,7 +425,16 @@ func (idx *BlockerIndexer) match( for ; it.Valid(); it.Next() { keyHeight, err := parseHeightFromEventKey(it.Key()) - if err != nil || !checkHeightConditions(heightInfo, keyHeight) { + if err != nil { + idx.log.Error("failure to parse height from key:", err) + continue + } + withinHeight, err := checkHeightConditions(heightInfo, keyHeight) + if err != nil { + idx.log.Error("failure checking for height bounds:", err) + continue + } + if !withinHeight { continue } @@ -432,10 +462,21 @@ func (idx *BlockerIndexer) match( defer it.Close() for ; it.Valid(); it.Next() { + keyHeight, err := parseHeightFromEventKey(it.Key()) - if err != nil || !checkHeightConditions(heightInfo, keyHeight) { + if err != nil { + idx.log.Error("failure to parse height from key:", err) + continue + } + withinHeight, err := checkHeightConditions(heightInfo, keyHeight) + if err != nil { + idx.log.Error("failure checking for height bounds:", err) + continue + } + if !withinHeight { continue } + idx.setTmpHeights(tmpHeights, it) select { @@ -470,7 +511,16 @@ func (idx *BlockerIndexer) match( if strings.Contains(eventValue, c.Arg.Value()) { keyHeight, err := parseHeightFromEventKey(it.Key()) - if err != nil || !checkHeightConditions(heightInfo, keyHeight) { + if err != nil { + idx.log.Error("failure to parse height from key:", err) + continue + } + withinHeight, err := checkHeightConditions(heightInfo, keyHeight) + if err != nil { + idx.log.Error("failure checking for height bounds:", err) + continue + } + if !withinHeight { continue } idx.setTmpHeights(tmpHeights, it) diff --git a/state/indexer/block/kv/kv_test.go b/state/indexer/block/kv/kv_test.go index 3275b45f441..2f4e3f085ac 100644 --- a/state/indexer/block/kv/kv_test.go +++ b/state/indexer/block/kv/kv_test.go @@ -300,3 +300,140 @@ func TestBlockIndexerMulti(t *testing.T) { }) } } + +func TestBigInt(t *testing.T) { + + bigInt := "10000000000000000000" + bigFloat := bigInt + ".76" + bigFloatLower := bigInt + ".1" + bigIntSmaller := "9999999999999999999" + store := db.NewPrefixDB(db.NewMemDB(), []byte("block_events")) + indexer := blockidxkv.New(store) + + require.NoError(t, indexer.Index(types.EventDataNewBlockEvents{ + Height: 1, + Events: []abci.Event{ + {}, + { + Type: "end_event", + Attributes: []abci.EventAttribute{ + { + Key: "foo", + Value: "100", + Index: true, + }, + { + Key: "bar", + Value: bigFloat, + Index: true, + }, + { + Key: "bar_lower", + Value: bigFloatLower, + Index: true, + }, + }, + }, + { + Type: "end_event", + Attributes: []abci.EventAttribute{ + { + Key: "foo", + Value: bigInt, + Index: true, + }, + { + Key: "bar", + Value: "500", + Index: true, + }, + { + Key: "bla", + Value: "500.5", + Index: true, + }, + }, + }, + }, + }, + )) + + testCases := map[string]struct { + q *query.Query + results []int64 + }{ + + "query return all events from a height - exact": { + q: query.MustCompile("block.height = 1"), + results: []int64{1}, + }, + "query return all events from a height - exact (deduplicate height)": { + q: query.MustCompile("block.height = 1 AND block.height = 2"), + results: []int64{1}, + }, + "query return all events from a height - range": { + q: query.MustCompile("block.height < 2 AND block.height > 0 AND block.height > 0"), + results: []int64{1}, + }, + "query matches fields with big int and height - no match": { + q: query.MustCompile("end_event.foo = " + bigInt + " AND end_event.bar = 500 AND block.height = 2"), + results: []int64{}, + }, + "query matches fields with big int with less and height - no match": { + q: query.MustCompile("end_event.foo <= " + bigInt + " AND end_event.bar = 500 AND block.height = 2"), + results: []int64{}, + }, + "query matches fields with big int and height - match": { + q: query.MustCompile("end_event.foo = " + bigInt + " AND end_event.bar = 500 AND block.height = 1"), + results: []int64{1}, + }, + "query matches big int in range": { + q: query.MustCompile("end_event.foo = " + bigInt), + results: []int64{1}, + }, + "query matches big int in range with float with equality ": { + q: query.MustCompile("end_event.bar >= " + bigInt), + results: []int64{1}, + }, + "query matches big int in range with float ": { + q: query.MustCompile("end_event.bar > " + bigInt), + results: []int64{1}, + }, + "query matches big int in range with float lower dec point ": { + q: query.MustCompile("end_event.bar_lower > " + bigInt), + results: []int64{1}, + }, + "query matches big int in range with float with less - found": { + q: query.MustCompile("end_event.foo <= " + bigInt), + results: []int64{1}, + }, + "query matches big int in range with float with less with height range - found": { + q: query.MustCompile("end_event.foo <= " + bigInt + " AND block.height > 0"), + results: []int64{1}, + }, + "query matches big int in range with float with less - not found": { + q: query.MustCompile("end_event.foo < " + bigInt + " AND end_event.foo > 100"), + results: []int64{}, + }, + "query does not parse float": { + q: query.MustCompile("end_event.bla >= 500"), + results: []int64{1}, + }, + "query condition float": { + q: query.MustCompile("end_event.bla < " + bigFloat), + results: []int64{1}, + }, + "query condition big int plus one": { + q: query.MustCompile("end_event.foo > " + bigIntSmaller), + results: []int64{1}, + }, + } + for name, tc := range testCases { + tc := tc + t.Run(name, func(t *testing.T) { + results, err := indexer.Search(context.Background(), tc.q) + require.NoError(t, err) + require.Equal(t, tc.results, results) + }) + } +} diff --git a/state/indexer/block/kv/util.go b/state/indexer/block/kv/util.go index 6da9d7912e6..a20a0fd81e7 100644 --- a/state/indexer/block/kv/util.go +++ b/state/indexer/block/kv/util.go @@ -3,10 +3,12 @@ package kv import ( "encoding/binary" "fmt" + "math/big" "strconv" "github.com/google/orderedcode" + idxutil "github.com/cometbft/cometbft/internal/indexer" "github.com/cometbft/cometbft/libs/pubsub/query/syntax" "github.com/cometbft/cometbft/state/indexer" "github.com/cometbft/cometbft/types" @@ -161,10 +163,13 @@ func dedupHeight(conditions []syntax.Condition) (dedupConditions []syntax.Condit if found || heightRangeExists { continue } - heightCondition = append(heightCondition, c) - heightInfo.height = int64(c.Arg.Number()) - - found = true + hFloat := c.Arg.Number() + if hFloat != nil { + h, _ := hFloat.Int64() + heightInfo.height = h + heightCondition = append(heightCondition, c) + found = true + } } else { heightInfo.onlyHeightEq = false heightRangeExists = true @@ -191,15 +196,16 @@ func dedupHeight(conditions []syntax.Condition) (dedupConditions []syntax.Condit return dedupConditions, heightInfo, found } -func checkHeightConditions(heightInfo HeightInfo, keyHeight int64) bool { +func checkHeightConditions(heightInfo HeightInfo, keyHeight int64) (bool, error) { if heightInfo.heightRange.Key != "" { - if !checkBounds(heightInfo.heightRange, keyHeight) { - return false + withinBounds, err := idxutil.CheckBounds(heightInfo.heightRange, big.NewInt(keyHeight)) + if err != nil || !withinBounds { + return false, err } } else { if heightInfo.height != 0 && keyHeight != heightInfo.height { - return false + return false, nil } } - return true + return true, nil } diff --git a/state/indexer/block/null/null.go b/state/indexer/block/null/null.go index a8d63cd76ea..0a62a4273e0 100644 --- a/state/indexer/block/null/null.go +++ b/state/indexer/block/null/null.go @@ -4,6 +4,7 @@ import ( "context" "errors" + "github.com/cometbft/cometbft/libs/log" "github.com/cometbft/cometbft/libs/pubsub/query" "github.com/cometbft/cometbft/state/indexer" "github.com/cometbft/cometbft/types" @@ -25,3 +26,6 @@ func (idx *BlockerIndexer) Index(types.EventDataNewBlockEvents) error { func (idx *BlockerIndexer) Search(context.Context, *query.Query) ([]int64, error) { return []int64{}, nil } + +func (idx *BlockerIndexer) SetLogger(log.Logger) { +} diff --git a/state/indexer/mocks/block_indexer.go b/state/indexer/mocks/block_indexer.go index c17a6532247..60ee57dc5f3 100644 --- a/state/indexer/mocks/block_indexer.go +++ b/state/indexer/mocks/block_indexer.go @@ -5,6 +5,8 @@ package mocks import ( context "context" + log "github.com/cometbft/cometbft/libs/log" + mock "github.com/stretchr/testify/mock" query "github.com/cometbft/cometbft/libs/pubsub/query" @@ -81,6 +83,11 @@ func (_m *BlockIndexer) Search(ctx context.Context, q *query.Query) ([]int64, er return r0, r1 } +// SetLogger provides a mock function with given fields: l +func (_m *BlockIndexer) SetLogger(l log.Logger) { + _m.Called(l) +} + type mockConstructorTestingTNewBlockIndexer interface { mock.TestingT Cleanup(func()) diff --git a/state/indexer/query_range.go b/state/indexer/query_range.go index e3cfdc6fda0..4cb57f072d3 100644 --- a/state/indexer/query_range.go +++ b/state/indexer/query_range.go @@ -1,6 +1,7 @@ package indexer import ( + "math/big" "time" "github.com/cometbft/cometbft/libs/pubsub/query/syntax" @@ -44,7 +45,17 @@ func (qr QueryRange) LowerBoundValue() interface{} { switch t := qr.LowerBound.(type) { case int64: return t + 1 - + case *big.Int: + tmp := new(big.Int) + return tmp.Add(t, big.NewInt(1)) + + case *big.Float: + // For floats we cannot simply add one as the float to float + // comparison is more finegrained. + // When comparing to integers, adding one is also incorrect: + // example: x >100.2 ; x = 101 float increased to 101.2 and condition + // is not satisfied + return t case time.Time: return t.Unix() + 1 @@ -67,7 +78,11 @@ func (qr QueryRange) UpperBoundValue() interface{} { switch t := qr.UpperBound.(type) { case int64: return t - 1 - + case *big.Int: + tmp := new(big.Int) + return tmp.Sub(t, big.NewInt(1)) + case *big.Float: + return t case time.Time: return t.Unix() - 1 @@ -182,7 +197,7 @@ func conditionArg(c syntax.Condition) interface{} { } switch c.Arg.Type { case syntax.TNumber: - return int64(c.Arg.Number()) + return c.Arg.Number() case syntax.TTime, syntax.TDate: return c.Arg.Time() default: diff --git a/state/indexer/sink/psql/backport.go b/state/indexer/sink/psql/backport.go index 01d7e1bc994..81a59ce1ffb 100644 --- a/state/indexer/sink/psql/backport.go +++ b/state/indexer/sink/psql/backport.go @@ -17,6 +17,8 @@ import ( "context" "errors" + "github.com/cometbft/cometbft/libs/log" + abci "github.com/cometbft/cometbft/abci/types" "github.com/cometbft/cometbft/libs/pubsub/query" "github.com/cometbft/cometbft/state/txindex" @@ -58,6 +60,8 @@ func (BackportTxIndexer) Search(context.Context, *query.Query) ([]*abci.TxResult return nil, errors.New("the TxIndexer.Search method is not supported") } +func (BackportTxIndexer) SetLogger(log.Logger) {} + // BlockIndexer returns a bridge that implements the CometBFT v0.34 block // indexer interface, using the Postgres event sink as a backing store. func (es *EventSink) BlockIndexer() BackportBlockIndexer { @@ -85,3 +89,5 @@ func (b BackportBlockIndexer) Index(block types.EventDataNewBlockEvents) error { func (BackportBlockIndexer) Search(context.Context, *query.Query) ([]int64, error) { return nil, errors.New("the BlockIndexer.Search method is not supported") } + +func (BackportBlockIndexer) SetLogger(log.Logger) {} diff --git a/state/txindex/indexer.go b/state/txindex/indexer.go index a70c461c2f6..083e8288e5e 100644 --- a/state/txindex/indexer.go +++ b/state/txindex/indexer.go @@ -4,6 +4,8 @@ import ( "context" "errors" + "github.com/cometbft/cometbft/libs/log" + abci "github.com/cometbft/cometbft/abci/types" "github.com/cometbft/cometbft/libs/pubsub/query" ) @@ -26,6 +28,9 @@ type TxIndexer interface { // Search allows you to query for transactions. Search(ctx context.Context, q *query.Query) ([]*abci.TxResult, error) + + //Set Logger + SetLogger(l log.Logger) } // Batch groups together multiple Index operations to be performed at the same time. diff --git a/state/txindex/kv/kv.go b/state/txindex/kv/kv.go index 17440530428..ec0d1096b70 100644 --- a/state/txindex/kv/kv.go +++ b/state/txindex/kv/kv.go @@ -5,14 +5,18 @@ import ( "context" "encoding/hex" "fmt" + "math/big" "strconv" "strings" + "github.com/cometbft/cometbft/libs/log" + "github.com/cosmos/gogoproto/proto" dbm "github.com/cometbft/cometbft-db" abci "github.com/cometbft/cometbft/abci/types" + idxutil "github.com/cometbft/cometbft/internal/indexer" "github.com/cometbft/cometbft/libs/pubsub/query" "github.com/cometbft/cometbft/libs/pubsub/query/syntax" "github.com/cometbft/cometbft/state/indexer" @@ -32,6 +36,8 @@ type TxIndex struct { store dbm.DB // Number the events in the event list eventSeq int64 + + log log.Logger } // NewTxIndex creates new KV indexer. @@ -41,6 +47,10 @@ func NewTxIndex(store dbm.DB) *TxIndex { } } +func (txi *TxIndex) SetLogger(l log.Logger) { + txi.log = l +} + // Get gets transaction from the TxIndex storage and returns it or nil if the // transaction is not found. func (txi *TxIndex) Get(hash []byte) (*abci.TxResult, error) { @@ -368,10 +378,18 @@ func (txi *TxIndex) match( // If we have a height range in a query, we need only transactions // for this height keyHeight, err := extractHeightFromKey(it.Key()) - if err != nil || !checkHeightConditions(heightInfo, keyHeight) { + if err != nil { + txi.log.Error("failure to parse height from key:", err) + continue + } + withinBounds, err := checkHeightConditions(heightInfo, keyHeight) + if err != nil { + txi.log.Error("failure checking for height bounds:", err) + continue + } + if !withinBounds { continue } - txi.setTmpHashes(tmpHashes, it) // Potentially exit early. select { @@ -396,7 +414,16 @@ func (txi *TxIndex) match( EXISTS_LOOP: for ; it.Valid(); it.Next() { keyHeight, err := extractHeightFromKey(it.Key()) - if err != nil || !checkHeightConditions(heightInfo, keyHeight) { + if err != nil { + txi.log.Error("failure to parse height from key:", err) + continue + } + withinBounds, err := checkHeightConditions(heightInfo, keyHeight) + if err != nil { + txi.log.Error("failure checking for height bounds:", err) + continue + } + if !withinBounds { continue } txi.setTmpHashes(tmpHashes, it) @@ -430,7 +457,16 @@ func (txi *TxIndex) match( if strings.Contains(extractValueFromKey(it.Key()), c.Arg.Value()) { keyHeight, err := extractHeightFromKey(it.Key()) - if err != nil || !checkHeightConditions(heightInfo, keyHeight) { + if err != nil { + txi.log.Error("failure to parse height from key:", err) + continue + } + withinBounds, err := checkHeightConditions(heightInfo, keyHeight) + if err != nil { + txi.log.Error("failure checking for height bounds:", err) + continue + } + if !withinBounds { continue } txi.setTmpHashes(tmpHashes, it) @@ -514,20 +550,45 @@ LOOP: continue } - if _, ok := qr.AnyBound().(int64); ok { - v, err := strconv.ParseInt(extractValueFromKey(it.Key()), 10, 64) - if err != nil { - continue LOOP + if _, ok := qr.AnyBound().(*big.Float); ok { + v := new(big.Int) + v, ok := v.SetString(extractValueFromKey(it.Key()), 10) + var vF *big.Float + if !ok { + vF, _, err = big.ParseFloat(extractValueFromKey(it.Key()), 10, 125, big.ToNearestEven) + if err != nil { + continue LOOP + } + } if qr.Key != types.TxHeightKey { keyHeight, err := extractHeightFromKey(it.Key()) - if err != nil || !checkHeightConditions(heightInfo, keyHeight) { - continue LOOP + if err != nil { + txi.log.Error("failure to parse height from key:", err) + continue + } + withinBounds, err := checkHeightConditions(heightInfo, keyHeight) + if err != nil { + txi.log.Error("failure checking for height bounds:", err) + continue + } + if !withinBounds { + continue } - } - if checkBounds(qr, v) { - txi.setTmpHashes(tmpHashes, it) + var withinBounds bool + var err error + if !ok { + withinBounds, err = idxutil.CheckBounds(qr, vF) + } else { + withinBounds, err = idxutil.CheckBounds(qr, v) + } + if err != nil { + txi.log.Error("failed to parse bounds:", err) + } else { + if withinBounds { + txi.setTmpHashes(tmpHashes, it) + } } // XXX: passing time in a ABCI Events is not yet implemented @@ -658,18 +719,3 @@ func startKey(fields ...interface{}) []byte { } return b.Bytes() } - -func checkBounds(ranges indexer.QueryRange, v int64) bool { - include := true - lowerBound := ranges.LowerBoundValue() - upperBound := ranges.UpperBoundValue() - if lowerBound != nil && v < lowerBound.(int64) { - include = false - } - - if upperBound != nil && v > upperBound.(int64) { - include = false - } - - return include -} diff --git a/state/txindex/kv/kv_test.go b/state/txindex/kv/kv_test.go index e406c65ef71..5fbca71b8bc 100644 --- a/state/txindex/kv/kv_test.go +++ b/state/txindex/kv/kv_test.go @@ -357,6 +357,7 @@ func TestTxSearchOneTxWithMultipleSameTagsButDifferentValues(t *testing.T) { require.NoError(t, err) testCases := []struct { + name string q string found bool }{ @@ -633,6 +634,84 @@ func benchmarkTxIndex(txsCount int64, b *testing.B) { } } +func TestBigInt(t *testing.T) { + indexer := NewTxIndex(db.NewMemDB()) + + bigInt := "10000000000000000000" + bigIntPlus1 := "10000000000000000001" + bigFloat := bigInt + ".76" + bigFloatLower := bigInt + ".1" + bigFloatSmaller := "9999999999999999999" + ".1" + bigIntSmaller := "9999999999999999999" + + txResult := txResultWithEvents([]abci.Event{ + {Type: "account", Attributes: []abci.EventAttribute{{Key: "number", Value: bigInt, Index: true}}}, + {Type: "account", Attributes: []abci.EventAttribute{{Key: "number", Value: bigFloatSmaller, Index: true}}}, + {Type: "account", Attributes: []abci.EventAttribute{{Key: "number", Value: bigIntPlus1, Index: true}}}, + {Type: "account", Attributes: []abci.EventAttribute{{Key: "number", Value: bigFloatLower, Index: true}}}, + {Type: "account", Attributes: []abci.EventAttribute{{Key: "owner", Value: "/Ivan/", Index: true}}}, + {Type: "", Attributes: []abci.EventAttribute{{Key: "not_allowed", Value: "Vlad", Index: true}}}, + }) + hash := types.Tx(txResult.Tx).Hash() + + err := indexer.Index(txResult) + + require.NoError(t, err) + + txResult2 := txResultWithEvents([]abci.Event{ + {Type: "account", Attributes: []abci.EventAttribute{{Key: "number", Value: bigFloat, Index: true}}}, + {Type: "account", Attributes: []abci.EventAttribute{{Key: "number", Value: bigFloat, Index: true}, {Key: "amount", Value: "5", Index: true}}}, + {Type: "account", Attributes: []abci.EventAttribute{{Key: "number", Value: bigIntSmaller, Index: true}}}, + {Type: "account", Attributes: []abci.EventAttribute{{Key: "number", Value: bigInt, Index: true}, {Key: "amount", Value: "3", Index: true}}}}) + + txResult2.Tx = types.Tx("NEW TX") + txResult2.Height = 2 + txResult2.Index = 2 + + hash2 := types.Tx(txResult2.Tx).Hash() + + err = indexer.Index(txResult2) + require.NoError(t, err) + testCases := []struct { + q string + txRes *abci.TxResult + resultsLength int + }{ + // search by hash + {fmt.Sprintf("tx.hash = '%X'", hash), txResult, 1}, + // search by hash (lower) + {fmt.Sprintf("tx.hash = '%x'", hash), txResult, 1}, + {fmt.Sprintf("tx.hash = '%x'", hash2), txResult2, 1}, + // search by exact match (one key) - bigint + {"account.number >= " + bigInt, nil, 2}, + // search by exact match (one key) - bigint range + {"account.number >= " + bigInt + " AND tx.height > 0", nil, 2}, + {"account.number >= " + bigInt + " AND tx.height > 0 AND account.owner = '/Ivan/'", nil, 0}, + // Floats are not parsed + {"account.number >= " + bigInt + " AND tx.height > 0 AND account.amount > 4", txResult2, 1}, + {"account.number >= " + bigInt + " AND tx.height > 0 AND account.amount = 5", txResult2, 1}, + {"account.number >= " + bigInt + " AND account.amount <= 5", txResult2, 1}, + {"account.number > " + bigFloatSmaller + " AND account.amount = 3", txResult2, 1}, + {"account.number < " + bigInt + " AND tx.height >= 1", nil, 2}, + {"account.number < " + bigInt + " AND tx.height = 1", nil, 1}, + {"account.number < " + bigInt + " AND tx.height = 2", nil, 1}, + } + + ctx := context.Background() + + for _, tc := range testCases { + tc := tc + t.Run(tc.q, func(t *testing.T) { + results, err := indexer.Search(ctx, query.MustCompile(tc.q)) + assert.NoError(t, err) + assert.Len(t, results, tc.resultsLength) + if tc.resultsLength > 0 && tc.txRes != nil { + assert.True(t, proto.Equal(results[0], tc.txRes)) + } + }) + } +} + func BenchmarkTxIndex1(b *testing.B) { benchmarkTxIndex(1, b) } func BenchmarkTxIndex500(b *testing.B) { benchmarkTxIndex(500, b) } func BenchmarkTxIndex1000(b *testing.B) { benchmarkTxIndex(1000, b) } diff --git a/state/txindex/kv/utils.go b/state/txindex/kv/utils.go index 40e2b5d4c42..753b64e4d2a 100644 --- a/state/txindex/kv/utils.go +++ b/state/txindex/kv/utils.go @@ -2,7 +2,9 @@ package kv import ( "fmt" + "math/big" + idxutil "github.com/cometbft/cometbft/internal/indexer" cmtsyntax "github.com/cometbft/cometbft/libs/pubsub/query/syntax" "github.com/cometbft/cometbft/state/indexer" "github.com/cometbft/cometbft/types" @@ -59,9 +61,13 @@ func dedupHeight(conditions []cmtsyntax.Condition) (dedupConditions []cmtsyntax. if heightRangeExists || found { continue } - found = true - heightCondition = append(heightCondition, c) - heightInfo.height = int64(c.Arg.Number()) + hFloat := c.Arg.Number() + if hFloat != nil { + h, _ := hFloat.Int64() + heightInfo.height = h + found = true + heightCondition = append(heightCondition, c) + } } else { heightInfo.onlyHeightEq = false heightRangeExists = true @@ -87,15 +93,16 @@ func dedupHeight(conditions []cmtsyntax.Condition) (dedupConditions []cmtsyntax. return dedupConditions, heightInfo } -func checkHeightConditions(heightInfo HeightInfo, keyHeight int64) bool { +func checkHeightConditions(heightInfo HeightInfo, keyHeight int64) (bool, error) { if heightInfo.heightRange.Key != "" { - if !checkBounds(heightInfo.heightRange, keyHeight) { - return false + withinBounds, err := idxutil.CheckBounds(heightInfo.heightRange, big.NewInt(keyHeight)) + if err != nil || !withinBounds { + return false, err } } else { if heightInfo.height != 0 && keyHeight != heightInfo.height { - return false + return false, nil } } - return true + return true, nil } diff --git a/state/txindex/mocks/tx_indexer.go b/state/txindex/mocks/tx_indexer.go index fb50fd96dd9..dcffdb3ab38 100644 --- a/state/txindex/mocks/tx_indexer.go +++ b/state/txindex/mocks/tx_indexer.go @@ -5,9 +5,11 @@ package mocks import ( context "context" - query "github.com/cometbft/cometbft/libs/pubsub/query" + log "github.com/cometbft/cometbft/libs/log" mock "github.com/stretchr/testify/mock" + query "github.com/cometbft/cometbft/libs/pubsub/query" + txindex "github.com/cometbft/cometbft/state/txindex" types "github.com/cometbft/cometbft/abci/types" @@ -98,6 +100,11 @@ func (_m *TxIndexer) Search(ctx context.Context, q *query.Query) ([]*types.TxRes return r0, r1 } +// SetLogger provides a mock function with given fields: l +func (_m *TxIndexer) SetLogger(l log.Logger) { + _m.Called(l) +} + type mockConstructorTestingTNewTxIndexer interface { mock.TestingT Cleanup(func()) diff --git a/state/txindex/null/null.go b/state/txindex/null/null.go index c44a39ebea7..49338154c39 100644 --- a/state/txindex/null/null.go +++ b/state/txindex/null/null.go @@ -4,6 +4,8 @@ import ( "context" "errors" + "github.com/cometbft/cometbft/libs/log" + abci "github.com/cometbft/cometbft/abci/types" "github.com/cometbft/cometbft/libs/pubsub/query" "github.com/cometbft/cometbft/state/txindex" @@ -32,3 +34,7 @@ func (txi *TxIndex) Index(_ *abci.TxResult) error { func (txi *TxIndex) Search(_ context.Context, _ *query.Query) ([]*abci.TxResult, error) { return []*abci.TxResult{}, nil } + +func (txi *TxIndex) SetLogger(log.Logger) { + +}