Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/eth_getBlockByNumber and eth_getUncle... nullable fields #341

Merged
merged 22 commits into from
Mar 29, 2021
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
418e29f
ethclient,internal/ethapi: add Nonce (null) to pending block JSON
meowsbits Mar 15, 2021
be15ecb
ethclient: add test case for expected non-null values ('number')
meowsbits Mar 15, 2021
b446973
ethclient: refactor test, unifying to map of expected value (null, ze…
meowsbits Mar 15, 2021
dc4e75c
ethclient: add missing fields to test and compare diff
meowsbits Mar 15, 2021
f0cd460
ethclient,internal/ethapi: add Hash (null) to pending block JSON
meowsbits Mar 15, 2021
b3140ce
ethclient: assert positive generic match on special case timestamp
meowsbits Mar 15, 2021
0e6d656
ethclient,internal/ethapi: jsonrpc header marshaling assign totalDiff…
meowsbits Mar 15, 2021
f11d477
internal/ethapi: add Miner (null) to pending block JSON
meowsbits Mar 15, 2021
6b3f223
ethclient,internal/ethapi: wip: totalDifficulty is fucked up; either …
meowsbits Mar 15, 2021
a6b943e
ethclient: move test to 'cg' namespaced file, allows testing @ethereu…
meowsbits Mar 15, 2021
0b3cf0d
ethclient: refactor test to be generic using pattern matching
meowsbits Mar 15, 2021
97e24db
ethclient: add Latest suffix for a more descriptive test name
meowsbits Mar 15, 2021
e52bcad
ethclient: extrapolate regex-based test to earliest,latest,pending
meowsbits Mar 15, 2021
cfcf223
ethclient: rename test to be more conventional and descriptive
meowsbits Mar 15, 2021
14ec0c0
internal/ethapi: remove dead wip-commented code
meowsbits Mar 15, 2021
4fd659c
.github/workflows,ethclient: init Github Actions CI workflow to test …
meowsbits Mar 15, 2021
3b7de79
ethclient: use go subtest syntax for better descriptiveness and logging
meowsbits Mar 15, 2021
904066f
ethclient: additional commentary
meowsbits Mar 15, 2021
119b604
.github/workflows,ethclient: remove go-ethereum 1:1 comparative testing
meowsbits Mar 16, 2021
78e602b
ethclient: assert hash len for block hash field
meowsbits Mar 16, 2021
67ca5fc
internal/ethapi: omit the transactions field when marshaling uncle bl…
meowsbits Mar 17, 2021
136e50f
ethclient: test that the transactions field is omitted for uncles res…
meowsbits Mar 17, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 117 additions & 0 deletions ethclient/ethclient_cg_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
package ethclient

import (
"context"
"encoding/json"
"fmt"
"regexp"
"testing"
"time"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/types"
)

// 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))
}
}
})
}
}
}
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
13 changes: 4 additions & 9 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))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now matches the ethereum/go-ethereum implementation.

https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L1187-L1199

}

// setAsPending sets fields that must be nil for pending headers and blocks.
Expand Down