Fix/eth_getBlockByNumber and eth_getUncle... nullable fields

ethclient/ethclient_cg_test.go

@@ -0,0 +1,218 @@
+package ethclient
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"regexp"
+	"testing"
+	"time"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/ethereum/go-ethereum/consensus/ethash"
+	"github.com/ethereum/go-ethereum/core"
+	"github.com/ethereum/go-ethereum/core/rawdb"
+	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/eth"
+	"github.com/ethereum/go-ethereum/node"
+	"github.com/ethereum/go-ethereum/params"
+	"github.com/ethereum/go-ethereum/params/types/genesisT"
+)
+
+// TestEthGetBlockByNumber_ValidJSONResponse tests that
+// JSON RPC API responses to eth_getBlockByNumber meet pattern-based expectations.
+// These validations include the null-ness of certain fields for the 'pending' block
+// as well existence of all expected keys and values.
+func TestEthGetBlockByNumber_ValidJSONResponse(t *testing.T) {
+	backend, _ := newTestBackend(t)
+	client, _ := backend.Attach()
+	defer backend.Close()
+	defer client.Close()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
+	defer cancel()
+
+	// Have to sleep a little to make sure miner has time to set pending.
+	time.Sleep(time.Millisecond * 100)
+
+	// Get a reference block.
+	parent, err := NewClient(client).HeaderByNumber(ctx, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if parent == nil {
+		t.Fatal("bad test")
+	}
+
+	reNull := regexp.MustCompile(`^null$`)
+	reHexAnyLen := regexp.MustCompile(`^"0x[a-zA-Z0-9]+"$`)
+	reHexHashLen := regexp.MustCompile(fmt.Sprintf(`^"0x[a-zA-Z0-9]{%d}"$`, common.HashLength*2))
+
+	// completeBlockExpectations define expectations for 'earliest' and 'latest' blocks.
+	completeBlockExpectations := map[string]*regexp.Regexp{
+		"nonce": reHexAnyLen,
+		"hash":  reHexHashLen,
+		"miner": regexp.MustCompile(fmt.Sprintf(`^"0x[a-zA-Z0-9]{%d}"$`, common.AddressLength*2)),
+
+		"totalDifficulty": reHexAnyLen,
+
+		"mixHash":   regexp.MustCompile(fmt.Sprintf(`^"0x[0]{%d}"$`, common.HashLength*2)),
+		"logsBloom": regexp.MustCompile(fmt.Sprintf(`^"0x[0]{%d}"$`, types.BloomByteLength*2)),
+
+		"number":     reHexAnyLen,
+		"difficulty": reHexAnyLen,
+		"gasLimit":   reHexAnyLen,
+		"gasUsed":    reHexAnyLen,
+		"size":       reHexAnyLen,
+		"timestamp":  reHexAnyLen,
+		"extraData":  reHexAnyLen,
+
+		"parentHash":       reHexHashLen,
+		"transactionsRoot": reHexHashLen,
+		"stateRoot":        reHexHashLen,
+		"receiptsRoot":     reHexHashLen,
+		"sha3Uncles":       reHexHashLen,
+
+		"uncles":       regexp.MustCompile(`^\[\]$`),
+		"transactions": regexp.MustCompile(`^\[\]$`),
+	}
+
+	// Construct the 'pending' block expectations as a copy of the concrete block
+	// expectations.
+	pendingBlockExpectations := map[string]*regexp.Regexp{}
+	for k, v := range completeBlockExpectations {
+		pendingBlockExpectations[k] = v
+	}
+
+	// Make 'pending' specific adjustments.
+	pendingBlockExpectations["nonce"] = reNull
+	pendingBlockExpectations["hash"] = reNull
+	pendingBlockExpectations["miner"] = reNull
+	pendingBlockExpectations["totalDifficulty"] = reNull
+
+	for blockHeight, cases := range map[string]map[string]*regexp.Regexp{
+		"earliest": completeBlockExpectations,
+		"latest":   completeBlockExpectations,
+		"pending":  pendingBlockExpectations,
+	} {
+		for _, fullTxes := range []bool{true, false} {
+			t.Run(fmt.Sprintf("eth_getBlockByNumber-%s-%v", blockHeight, fullTxes), func(t *testing.T) {
+				gotPending := make(map[string]json.RawMessage)
+				err := client.CallContext(ctx, &gotPending, "eth_getBlockByNumber", blockHeight, fullTxes)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				for key, re := range cases {
+					gotVal, ok := gotPending[key]
+					if !ok {
+						t.Errorf("%s: missing key", key)
+					}
+					if !re.Match(gotVal) {
+						t.Errorf("%s want: %v, got: %v", key, re, string(gotVal))
+					}
+				}
+
+				for k, v := range gotPending {
+					if _, ok := cases[k]; !ok {
+						t.Errorf("%s: missing key (value: %v)", k, string(v))
+					}
+				}
+			})
+		}
+	}
+}
+
+// newTestBackendWithUncles duplicates the logic of newTestBackend, except that it generates a backend
+// on top of a chain that has uncles.
+func newTestBackendWithUncles(t *testing.T) (*node.Node, []*types.Block) {
+	// Generate test chain.
+	genesis, blocks := generateTestChainWithUncles()
+	// Create node
+	n, err := node.New(&node.Config{})
+	if err != nil {
+		t.Fatalf("can't create new node: %v", err)
+	}
+	// Create Ethereum Service
+	config := &eth.Config{Genesis: genesis}
+	config.Ethash.PowMode = ethash.ModeFake
+	ethservice, err := eth.New(n, config)
+	if err != nil {
+		t.Fatalf("can't create new ethereum service: %v", err)
+	}
+	// Import the test chain.
+	if err := n.Start(); err != nil {
+		t.Fatalf("can't start test node: %v", err)
+	}
+	if _, err := ethservice.BlockChain().InsertChain(blocks[1:]); err != nil {
+		t.Fatalf("can't import test blocks: %v", err)
+	}
+	return n, blocks
+}
+
+// generateTestChainWithUncles is a test helper function that essentially duplicates generateTestChain,
+// except that the chain generated includes block with uncles.
+func generateTestChainWithUncles() (*genesisT.Genesis, []*types.Block) {
+	db := rawdb.NewMemoryDatabase()
+	config := params.AllEthashProtocolChanges
+	genesis := &genesisT.Genesis{
+		Config:    config,
+		Alloc:     genesisT.GenesisAlloc{testAddr: {Balance: testBalance}},
+		ExtraData: []byte("test genesis"),
+		Timestamp: 9000,
+	}
+	generate := func(i int, g *core.BlockGen) {
+		g.OffsetTime(5)
+		g.SetExtra([]byte("test"))
+		if i >= 6 {
+			b2 := g.PrevBlock(i - 3).Header()
+			b2.Extra = []byte("foo")
+			g.AddUncle(b2)
+		}
+	}
+	gblock := core.GenesisToBlock(genesis, db)
+	engine := ethash.NewFaker()
+	blocks, _ := core.GenerateChain(config, gblock, engine, db, 8, generate)
+	blocks = append([]*types.Block{gblock}, blocks...)
+
+	return genesis, blocks
+}
+
+// TestUncleResponseEncoding tests the correctness of the JSON encoding of uncle-type responses.
+// These, different from canonical or side blocks, should NOT include the transactions field.
+func TestUncleResponseEncoding(t *testing.T) {
+	backend, chain := newTestBackendWithUncles(t)
+	client, _ := backend.Attach()
+	defer backend.Close()
+	defer client.Close()
+
+	res := make(map[string]json.RawMessage)
+	err := client.Call(&res, "eth_getUncleByBlockNumberAndIndex", hexutil.EncodeUint64(uint64(len(chain)-1)), "0x0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(res) == 0 {
+		t.Fatal("empty response")
+	}
+
+	if v, ok := res["uncles"]; !ok {
+		t.Fatal("uncles: missing field")
+	} else if !regexp.MustCompile(`^\[\]$`).Match(v) {
+		t.Fatal("uncles: should be empty array")
+	}
+
+	if _, ok := res["transactions"]; ok {
+		t.Fatal("transactions: field not omitted")
+	}
+
+	// Sanity check a few other fields
+	reHexAnyLen := regexp.MustCompile(`^"0x[a-zA-Z0-9]+"$`)
+	for _, field := range []string{"number", "hash", "nonce", "parentHash", "transactionsRoot"} {
+		if v, ok := res[field]; !ok {
+			t.Fatalf("%s: missing field", field)
+		} else if !reHexAnyLen.Match(v) {
+			t.Fatalf("%s: unexpected value: %s", field, string(v))
+		}
+	}
+}

ethclient/ethclient_test.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"log"
 	"math/big"
 	"reflect"
 	"testing"
@@ -278,43 +277,6 @@ func TestHeader(t *testing.T) {
 	}
 }
 
-func TestHeader_TxesUnclesNotEmpty(t *testing.T) {
-	backend, _ := newTestBackend(t)
-	client, _ := backend.Attach()
-	defer backend.Close()
-	defer client.Close()
-
-	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
-	defer cancel()
-
-	res := make(map[string]interface{})
-	err := client.CallContext(ctx, &res, "eth_getBlockByNumber", "latest", false)
-	if err != nil {
-		log.Fatalln(err)
-	}
-
-	// Sanity check response
-	if v, ok := res["number"]; !ok {
-		t.Fatal("missing 'number' field")
-	} else if n, err := hexutil.DecodeBig(v.(string)); err != nil || n == nil {
-		t.Fatal(err)
-	} else if n.Cmp(big.NewInt(1)) != 0 {
-		t.Fatalf("unexpected 'latest' block number: %v", n)
-	}
-	// 'transactions' key should exist as []
-	if v, ok := res["transactions"]; !ok {
-		t.Fatal("missing transactions field")
-	} else if len(v.([]interface{})) != 0 {
-		t.Fatal("'transactions' value not []")
-	}
-	// 'uncles' key should exist as []
-	if v, ok := res["uncles"]; !ok {
-		t.Fatal("missing uncles field")
-	} else if len(v.([]interface{})) != 0 {
-		t.Fatal("'uncles' value not []'")
-	}
-}
-
 func TestBalanceAt(t *testing.T) {
 	backend, _ := newTestBackend(t)
 	client, _ := backend.Attach()

internal/ethapi/api.go

@@ -1147,14 +1147,14 @@ func RPCMarshalHeader(head *types.Header) map[string]interface{} {
 // RPCMarshalHeaderT defines the RPC marshaling type for block headers.
 type RPCMarshalHeaderT struct {
 	Number           *hexutil.Big      `json:"number"`
-	Hash             *common.Hash      `json:"hash,omitempty"` // Pending will be nil
+	Hash             *common.Hash      `json:"hash"` // -- Pending will be nil --
 	ParentHash       common.Hash       `json:"parentHash"`
-	Nonce            *types.BlockNonce `json:"nonce,omitempty"` // Pending will be nil
+	Nonce            *types.BlockNonce `json:"nonce"` // -- Pending will be nil --
 	MixHash          common.Hash       `json:"mixHash"`
 	Sha3Uncles       common.Hash       `json:"sha3Uncles"`
 	LogsBloom        types.Bloom       `json:"logsBloom"`
 	StateRoot        common.Hash       `json:"stateRoot"`
-	Miner            *common.Address   `json:"miner,omitempty"` // Pending will be nil
+	Miner            *common.Address   `json:"miner"` // -- Pending will be nil --
 	Difficulty       *hexutil.Big      `json:"difficulty"`
 	TotalDifficulty  *hexutil.Big      `json:"totalDifficulty"`
 	ExtraData        hexutil.Bytes     `json:"extraData"`
@@ -1198,12 +1198,7 @@ func NewRPCMarshalHeaderTFromHeader(header *types.Header) *RPCMarshalHeaderT {
 // rpcMarshalHeaderTSetTotalDifficulty sets the total difficulty field for RPC response headers.
 // If the hash is unavailable (ie in Pending state), the value will be 0.
 func (s *PublicBlockChainAPI) rpcMarshalHeaderTSetTotalDifficulty(ctx context.Context, header *RPCMarshalHeaderT) {
-	hash := header.Hash
-	if hash == nil {
-		c := common.Hash{}
-		hash = &c
-	}
-	header.TotalDifficulty = (*hexutil.Big)(s.b.GetTd(ctx, *hash))
+	header.TotalDifficulty = (*hexutil.Big)(s.b.GetTd(ctx, *header.Hash))
 }
 
 // setAsPending sets fields that must be nil for pending headers and blocks.
@@ -1227,7 +1222,6 @@ type RPCMarshalBlockT struct {
 
 // RPCMarshalBlockTIR is the intermediate representation of RPCMarshalBlockT.
 // This exists to avoid a circular reference when overriding the json marshaling interface.
-// RPCMarshalHeaderT is an embedded struct.
 type RPCMarshalBlockTIR struct {
 	*RPCMarshalHeaderT
 	Transactions []interface{} `json:"transactions"`
@@ -1239,16 +1233,39 @@ type RPCMarshalBlockTIR struct {
 	fullTx bool
 }
 
+// RPCMarshalUncleTIR is the intermediate representation of RPCMarshalBlockT which excludes the transactions field.
+// Transactions are excluded when the API returns block information for uncle blocks.
+// This exists to avoid a circular reference when overriding the json marshaling interface.
+type RPCMarshalUncleTIR struct {
+	*RPCMarshalHeaderT
+	Uncles []common.Hash `json:"uncles"`
+
+	Error string `json:"error,omitempty"`
+
+	inclTx bool
+	fullTx bool
+}
+
 // MarshalJSON marshals JSON for RPCMarshalBlockT.
 // If an error is present on the struct, an object with only the error is returned.
 // This logic follows the established logic at eth/api.go's handling of bad blocks.
 func (b *RPCMarshalBlockT) MarshalJSON() ([]byte, error) {
 	if b.Error != "" {
 		return json.Marshal(map[string]interface{}{"error": b.Error})
 	}
-	ir := &RPCMarshalBlockTIR{
+	if b.inclTx {
+		ir := &RPCMarshalBlockTIR{
+			RPCMarshalHeaderT: b.RPCMarshalHeaderT,
+			Transactions:      b.Transactions,
+			Uncles:            b.Uncles,
+			Error:             "",
+			inclTx:            b.inclTx,
+			fullTx:            b.fullTx,
+		}
+		return json.Marshal(ir)
+	}
+	ir := &RPCMarshalUncleTIR{
 		RPCMarshalHeaderT: b.RPCMarshalHeaderT,
-		Transactions:      b.Transactions,
 		Uncles:            b.Uncles,
 		Error:             "",
 		inclTx:            b.inclTx,
@@ -1263,6 +1280,8 @@ func (b *RPCMarshalBlockT) MarshalJSON() ([]byte, error) {
 func RPCMarshalBlock(block *types.Block, inclTx bool, fullTx bool) (*RPCMarshalBlockT, error) {
 	fields := &RPCMarshalBlockT{RPCMarshalHeaderT: NewRPCMarshalHeaderTFromHeader(block.Header())}
 	fields.Size = hexutil.Uint64(block.Size())
+	fields.inclTx = inclTx
+	fields.fullTx = fullTx
 
 	if inclTx {
 		formatTx := func(tx *types.Transaction) (interface{}, error) {