Merge pull request #1204 from maticnetwork/mardizzone/grpc

gRPC edge cases

bor/client/grpc/grpc_util_test.go

@@ -0,0 +1,53 @@
+package grpc
+
+import (
+	"math/big"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestToBlockNumArg(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		input    *big.Int
+		expected string
+	}{
+		{
+			name:     "Nil input",
+			input:    nil,
+			expected: "latest",
+		},
+		{
+			name:     "Positive number",
+			input:    big.NewInt(12345),
+			expected: "12345",
+		},
+		{
+			name:     "Zero",
+			input:    big.NewInt(0),
+			expected: "0",
+		},
+		{
+			name:     "Negative number",
+			input:    big.NewInt(-1),
+			expected: "pending",
+		},
+		{
+			name:     "Large negative number",
+			input:    big.NewInt(-1234567890),
+			expected: "<invalid -1234567890>",
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			result := ToBlockNumArg(tt.input)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}

bor/client/grpc/query.go

@@ -2,11 +2,15 @@ package grpc
 
 import (
 	"context"
+	"fmt"
 	"math/big"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/ethereum/go-ethereum/common/math"
 	ethTypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/log"
+	"github.com/ethereum/go-ethereum/rpc"
 
 	proto "github.com/maticnetwork/polyproto/bor"
 	protoutil "github.com/maticnetwork/polyproto/utils"
@@ -52,10 +56,16 @@ func (h *BorGRPCClient) GetVoteOnHash(ctx context.Context, startBlock uint64, en
 	return res.Response, nil
 }
 
-func (h *BorGRPCClient) HeaderByNumber(ctx context.Context, blockID uint64) (*ethTypes.Header, error) {
+func (h *BorGRPCClient) HeaderByNumber(ctx context.Context, blockID int64) (*ethTypes.Header, error) {
+
+	if blockID > math.MaxInt64 {
+		return nil, fmt.Errorf("blockID too large: %d", blockID)
+	}
+
+	blockNumberAsString := ToBlockNumArg(big.NewInt(blockID))
 
 	req := &proto.GetHeaderByNumberRequest{
-		Number: blockID,
+		Number: blockNumberAsString,
 	}
 
 	log.Info("Fetching header by number")
@@ -76,10 +86,16 @@ func (h *BorGRPCClient) HeaderByNumber(ctx context.Context, blockID uint64) (*et
 	return resp, nil
 }
 
-func (h *BorGRPCClient) BlockByNumber(ctx context.Context, blockID uint64) (*ethTypes.Block, error) {
+func (h *BorGRPCClient) BlockByNumber(ctx context.Context, blockID int64) (*ethTypes.Block, error) {
+
+	if blockID > math.MaxInt64 {
+		return nil, fmt.Errorf("blockID too large: %d", blockID)
+	}
+
+	blockNumberAsString := ToBlockNumArg(big.NewInt(blockID))
 
 	req := &proto.GetBlockByNumberRequest{
-		Number: blockID,
+		Number: blockNumberAsString,
 	}
 
 	log.Info("Fetching block by number")
@@ -153,3 +169,18 @@ func receiptResponseToTypesReceipt(receipt *proto.Receipt) *ethTypes.Receipt {
 		TransactionIndex:  uint(receipt.TransactionIndex),
 	}
 }
+
+func ToBlockNumArg(number *big.Int) string {
+	if number == nil {
+		return "latest"
+	}
+	if number.Sign() >= 0 {
+		return hexutil.EncodeBig(number)
+	}
+	// It's negative.
+	if number.IsInt64() {
+		return rpc.BlockNumber(number.Int64()).String()
+	}
+	// It's negative and large, which is invalid.
+	return fmt.Sprintf("<invalid %d>", number)
+}

go.mod

@@ -15,7 +15,7 @@ require (
 	github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
 	github.com/hashicorp/golang-lru v1.0.2
 	github.com/json-iterator/go v1.1.12
-	github.com/maticnetwork/polyproto v0.0.3
+	github.com/maticnetwork/polyproto v0.0.4
 	github.com/pborman/uuid v1.2.1
 	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.19.0

go.sum

@@ -2225,8 +2225,8 @@ github.com/maticnetwork/crand v1.0.2 h1:Af0tAivC8zrxXDpGWNWVT/0s1fOz8w0eRbahZgUR
 github.com/maticnetwork/crand v1.0.2/go.mod h1:/NRNL3bj2eYdqpWmoIP5puxndTpi0XRxpj5ZKxfHjyg=
 github.com/maticnetwork/heimdall v1.0.7/go.mod h1:+ANI5+VV28ahwfdl7oMzrcNwaTEs1Fn6z39BqBGcvaA=
 github.com/maticnetwork/polyproto v0.0.3-0.20230216113155-340ea926ca53/go.mod h1:e1mU2EXSwEpn5jM7GfNwu3AupsV6WAGoPFFfswXOF0o=
-github.com/maticnetwork/polyproto v0.0.3 h1:a69rIp97fcl3ABY4LlVX9B2t1qhLa0Jhny3HNOzReBU=
-github.com/maticnetwork/polyproto v0.0.3/go.mod h1:e1mU2EXSwEpn5jM7GfNwu3AupsV6WAGoPFFfswXOF0o=
+github.com/maticnetwork/polyproto v0.0.4 h1:qQ/qwcO6UNGS4mJlzlLJn1AUMfJK9Rqmf1v+KJgnPsk=
+github.com/maticnetwork/polyproto v0.0.4/go.mod h1:e1mU2EXSwEpn5jM7GfNwu3AupsV6WAGoPFFfswXOF0o=
 github.com/maticnetwork/tendermint v0.33.2 h1:R9M7jgAmON8K/LbzMvtWPDhtPkNcqzkUUHp1ict/h3s=
 github.com/maticnetwork/tendermint v0.33.2/go.mod h1:D2fcnxGk6bje+LoPwImuKSSYLiK7/G06IynGNDSEcJk=
 github.com/matryer/try v0.0.0-20161228173917-9ac251b645a2/go.mod h1:0KeJpeMD6o+O4hW7qJOT7vyQPKrWmj26uf5wMc/IiIs=

helper/call.go

@@ -464,11 +464,13 @@ func (c *ContractCaller) GetMaticChainBlock(blockNum *big.Int) (header *ethTypes
 
 	var latestBlock *ethTypes.Header
 
-	if c.MaticGrpcFlag && blockNum != nil {
-		if blockNum.Sign() < 0 {
-			blockNum = new(big.Int).Abs(blockNum)
+	if c.MaticGrpcFlag {
+		if blockNum == nil {
+			// LatestBlockNumber is BlockNumber(-2) in go-ethereum rpc
+			latestBlock, err = c.MaticGrpcClient.HeaderByNumber(ctx, -2)
+		} else {
+			latestBlock, err = c.MaticGrpcClient.HeaderByNumber(ctx, blockNum.Int64())
 		}
-		latestBlock, err = c.MaticGrpcClient.HeaderByNumber(ctx, blockNum.Uint64())
 	} else {
 		latestBlock, err = c.MaticChainClient.HeaderByNumber(ctx, blockNum)
 	}
@@ -885,8 +887,8 @@ func (c *ContractCaller) GetBlockByNumber(ctx context.Context, blockNumber uint6
 	var block *ethTypes.Block
 	var err error
 
-	if c.MaticGrpcFlag && big.NewInt(int64(blockNumber)) != nil {
-		block, err = c.MaticGrpcClient.BlockByNumber(ctx, blockNumber)
+	if c.MaticGrpcFlag {
+		block, err = c.MaticGrpcClient.BlockByNumber(ctx, int64(blockNumber))
 	} else {
 		block, err = c.MaticChainClient.BlockByNumber(ctx, big.NewInt(int64(blockNumber)))
 	}

integration-tests/bor_health.sh

@@ -11,5 +11,5 @@ do
     fi
 done
 
-echo $peers
-echo $block
+echo "$peers"
+echo "$block"

integration-tests/smoke_test.sh

@@ -18,27 +18,27 @@ do
         exit 1
     fi
 
-    if (( $balance > $balanceInit )); then
-        if [ $stateSyncFound != "true" ]; then
+    if (( balance > balanceInit )); then
+        if [ "$stateSyncFound" != "true" ]; then
             stateSyncTime=$(( SECONDS - start_time ))
             stateSyncFound="true"
         fi
     fi
 
     checkpointID=$(curl -sL http://localhost:1317/checkpoints/latest | jq .result.id)
 
-    if [ $checkpointID != "null" ]; then
-        if [ $checkpointFound != "true" ]; then
+    if [ "$checkpointID" != "null" ]; then
+        if [ "$checkpointFound" != "true" ]; then
             checkpointTime=$(( SECONDS - start_time ))
             checkpointFound="true"
         fi
     fi
 
-    if [ $stateSyncFound == "true" ]  && [ $checkpointFound == "true" ]; then
+    if [ "$stateSyncFound" == "true" ] && [ "$checkpointFound" == "true" ]; then
         break
     fi
 
 done
 echo "Both state sync and checkpoint went through. All tests have passed!"
-echo "Time taken for state sync: $(printf '%02dm:%02ds\n'  $(($stateSyncTime%3600/60)) $(($stateSyncTime%60)))"
-echo "Time taken for checkpoint: $(printf '%02dm:%02ds\n'  $(($checkpointTime%3600/60)) $(($checkpointTime%60)))"
+echo "Time taken for state sync: $(printf '%02dm:%02ds\n' $((stateSyncTime%3600/60)) $((stateSyncTime%60)))"
+echo "Time taken for checkpoint: $(printf '%02dm:%02ds\n' $((checkpointTime%3600/60)) $((checkpointTime%60)))"