Skip to content

Commit

Permalink
Merge pull request ethereum#341 from etclabscore/fix/eth_getBlockByNu…
Browse files Browse the repository at this point in the history
…mber-nullable-fields

Fix/eth_getBlockByNumber and eth_getUncle... nullable fields
  • Loading branch information
meowsbits authored Mar 29, 2021
2 parents 9a25f4f + 136e50f commit f0dd598
Show file tree
Hide file tree
Showing 3 changed files with 249 additions and 50 deletions.
218 changes: 218 additions & 0 deletions ethclient/ethclient_cg_test.go
Original file line number Diff line number Diff line change
@@ -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))
}
}
}
38 changes: 0 additions & 38 deletions ethclient/ethclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"errors"
"fmt"
"log"
"math/big"
"reflect"
"testing"
Expand Down Expand Up @@ -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()
Expand Down
43 changes: 31 additions & 12 deletions internal/ethapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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.
Expand All @@ -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"`
Expand All @@ -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,
Expand All @@ -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) {
Expand Down

0 comments on commit f0dd598

Please sign in to comment.