ethclient: include block hash from FilterQuery #17996 (#1354)

ethclient/ethclient.go

@@ -365,26 +365,52 @@ func (ec *Client) NonceAt(ctx context.Context, account common.Address, blockNumb
 // FilterLogs executes a filter query.
 func (ec *Client) FilterLogs(ctx context.Context, q ethereum.FilterQuery) ([]types.Log, error) {
 	var result []types.Log
-	err := ec.c.CallContext(ctx, &result, "eth_getLogs", toFilterArg(q))
+	arg, err := toFilterArg(q)
+	if err != nil {
+		return nil, err
+	}
+	err = ec.c.CallContext(ctx, &result, "eth_getLogs", arg)
 	return result, err
 }
 
 // SubscribeFilterLogs subscribes to the results of a streaming filter query.
 func (ec *Client) SubscribeFilterLogs(ctx context.Context, q ethereum.FilterQuery, ch chan<- types.Log) (ethereum.Subscription, error) {
-	return ec.c.EthSubscribe(ctx, ch, "logs", toFilterArg(q))
+	arg, err := toFilterArg(q)
+	if err != nil {
+		return nil, err
+	}
+	sub, err := ec.c.EthSubscribe(ctx, ch, "logs", arg)
+	if err != nil {
+		// Defensively prefer returning nil interface explicitly on error-path, instead
+		// of letting default golang behavior wrap it with non-nil interface that stores
+		// nil concrete type value.
+		return nil, err
+	}
+	return sub, nil
 }
 
-func toFilterArg(q ethereum.FilterQuery) interface{} {
+func toFilterArg(q ethereum.FilterQuery) (interface{}, error) {
 	arg := map[string]interface{}{
-		"fromBlock": toBlockNumArg(q.FromBlock),
-		"toBlock":   toBlockNumArg(q.ToBlock),
-		"address":   q.Addresses,
-		"topics":    q.Topics,
+		"address": q.Addresses,
+		"topics":  q.Topics,
 	}
 	if q.FromBlock == nil {
 		arg["fromBlock"] = "0x0"
 	}
-	return arg
+	if q.BlockHash != nil {
+		arg["blockHash"] = *q.BlockHash
+		if q.FromBlock != nil || q.ToBlock != nil {
+			return nil, errors.New("cannot specify both BlockHash and FromBlock/ToBlock")
+		}
+	} else {
+		if q.FromBlock == nil {
+			arg["fromBlock"] = "0x0"
+		} else {
+			arg["fromBlock"] = toBlockNumArg(q.FromBlock)
+		}
+		arg["toBlock"] = toBlockNumArg(q.ToBlock)
+	}
+	return arg, nil
 }
 
 // Pending State

ethclient/ethclient_test.go

@@ -17,7 +17,13 @@
 package ethclient
 
 import (
+	"fmt"
+	"math/big"
+	"reflect"
+	"testing"
+
 	ethereum "github.com/XinFinOrg/XDPoSChain"
+	"github.com/XinFinOrg/XDPoSChain/common"
 )
 
 // Verify that Client implements the ethereum interfaces.
@@ -34,3 +40,114 @@ var (
 	// _ = ethereum.PendingStateEventer(&Client{})
 	_ = ethereum.PendingContractCaller(&Client{})
 )
+
+func TestToFilterArg(t *testing.T) {
+	blockHashErr := fmt.Errorf("cannot specify both BlockHash and FromBlock/ToBlock")
+	addresses := []common.Address{
+		common.HexToAddress("0xD36722ADeC3EdCB29c8e7b5a47f352D701393462"),
+	}
+	blockHash := common.HexToHash(
+		"0xeb94bb7d78b73657a9d7a99792413f50c0a45c51fc62bdcb08a53f18e9a2b4eb",
+	)
+
+	for _, testCase := range []struct {
+		name   string
+		input  ethereum.FilterQuery
+		output interface{}
+		err    error
+	}{
+		{
+			"without BlockHash",
+			ethereum.FilterQuery{
+				Addresses: addresses,
+				FromBlock: big.NewInt(1),
+				ToBlock:   big.NewInt(2),
+				Topics:    [][]common.Hash{},
+			},
+			map[string]interface{}{
+				"address":   addresses,
+				"fromBlock": "0x1",
+				"toBlock":   "0x2",
+				"topics":    [][]common.Hash{},
+			},
+			nil,
+		},
+		{
+			"with nil fromBlock and nil toBlock",
+			ethereum.FilterQuery{
+				Addresses: addresses,
+				Topics:    [][]common.Hash{},
+			},
+			map[string]interface{}{
+				"address":   addresses,
+				"fromBlock": "0x0",
+				"toBlock":   "latest",
+				"topics":    [][]common.Hash{},
+			},
+			nil,
+		},
+		{
+			"with blockhash",
+			ethereum.FilterQuery{
+				Addresses: addresses,
+				BlockHash: &blockHash,
+				Topics:    [][]common.Hash{},
+			},
+			map[string]interface{}{
+				"address":   addresses,
+				"blockHash": blockHash,
+				"fromBlock": "0x0",
+				"topics":    [][]common.Hash{},
+			},
+			nil,
+		},
+		{
+			"with blockhash and from block",
+			ethereum.FilterQuery{
+				Addresses: addresses,
+				BlockHash: &blockHash,
+				FromBlock: big.NewInt(1),
+				Topics:    [][]common.Hash{},
+			},
+			nil,
+			blockHashErr,
+		},
+		{
+			"with blockhash and to block",
+			ethereum.FilterQuery{
+				Addresses: addresses,
+				BlockHash: &blockHash,
+				ToBlock:   big.NewInt(1),
+				Topics:    [][]common.Hash{},
+			},
+			nil,
+			blockHashErr,
+		},
+		{
+			"with blockhash and both from / to block",
+			ethereum.FilterQuery{
+				Addresses: addresses,
+				BlockHash: &blockHash,
+				FromBlock: big.NewInt(1),
+				ToBlock:   big.NewInt(2),
+				Topics:    [][]common.Hash{},
+			},
+			nil,
+			blockHashErr,
+		},
+	} {
+		t.Run(testCase.name, func(t *testing.T) {
+			output, err := toFilterArg(testCase.input)
+			if (testCase.err == nil) != (err == nil) {
+				t.Fatalf("expected error %v but got %v", testCase.err, err)
+			}
+			if testCase.err != nil {
+				if testCase.err.Error() != err.Error() {
+					t.Fatalf("expected error %v but got %v", testCase.err, err)
+				}
+			} else if !reflect.DeepEqual(testCase.output, output) {
+				t.Fatalf("expected filter arg %v but got %v", testCase.output, output)
+			}
+		})
+	}
+}