Skip to content

Commit

Permalink
Backport of PR #771 to 0.34 : big int parsing (#798)
Browse files Browse the repository at this point in the history
Replaced int64 with big.int

Co-authored-by: Lasaro <lasaro@informal.systems>
Co-authored-by: Sergio Mena <sergio@informal.systems>
Co-authored-by: Thane Thomson <connect@thanethomson.com>
  • Loading branch information
4 people authored May 11, 2023
1 parent 82c6c08 commit 30af8c2
Show file tree
Hide file tree
Showing 13 changed files with 399 additions and 64 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `[state/kvindex]` Querying event attributes that are bigger than int64 is now enabled.
([\#771](https://github.com/cometbft/cometbft/pull/771))
Original file line number Diff line number Diff line change
@@ -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))
11 changes: 11 additions & 0 deletions docs/app-dev/indexing-transactions.md
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,14 @@ is ignored and the data is retrieved as if `match_events=false`.
Additionally, if a node that was running Tendermint Core
when the data was first indexed, and switched to CometBFT, is queried, it will retrieve this previously indexed
data as if `match_events=false` (attributes can match the query conditions across different events on the same height).


# Event attribute value types

Users can use anything as an event value. However, if the event attribute 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.
14 changes: 14 additions & 0 deletions docs/core/subscription.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,20 @@ You can also use tags, given you had included them into DeliverTx
response, to query transaction results. See [Indexing
transactions](./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
(e.g., `10000000000000000000.0` is treated the same as `10000000000000000000.6`)
- When floating points do get converted to integers, they are always rounded down.
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
Expand Down
101 changes: 60 additions & 41 deletions libs/pubsub/query/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ package query

import (
"fmt"
"math/big"
"reflect"
"regexp"
"strconv"
Expand Down Expand Up @@ -151,16 +152,17 @@ 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:
Expand Down Expand Up @@ -298,11 +300,12 @@ 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
}
Expand Down Expand Up @@ -451,42 +454,58 @@ 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:
Expand Down
76 changes: 74 additions & 2 deletions libs/pubsub/query/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package query_test

import (
"fmt"
"math/big"
"testing"
"time"

Expand All @@ -11,6 +12,57 @@ import (
"github.com/tendermint/tendermint/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"
Expand Down Expand Up @@ -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
Expand All @@ -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)},
},
},
{
Expand Down
15 changes: 9 additions & 6 deletions state/indexer/block/kv/kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"errors"
"fmt"
"math/big"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -311,9 +312,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
}

Expand Down Expand Up @@ -385,15 +387,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
}

Expand Down
Loading

0 comments on commit 30af8c2

Please sign in to comment.