Try to reproduce and fix eth_getBlockByNumber returning nil (ledgerwa…

…tch#4608) * Small optimisation for eth_getBlockByNumber * Option to not retrieve transactions * fixes * Check hash * Fixes * Avoid shadowing of err in BlockWithSenders * Fix test Co-authored-by: Alexey Sharp <alexeysharp@Alexeys-iMac.local> Co-authored-by: Alex Sharp <alexsharp@Alexs-MacBook-Pro.local>
maticnetwork · Jul 6, 2022 · 329d586 · 329d586
1 parent 71525fa
commit 329d586
Show file tree

Hide file tree

Showing 14 changed files with 94 additions and 19 deletions.
diff --git a/cmd/rpctest/main.go b/cmd/rpctest/main.go
@@ -242,6 +242,16 @@ func main() {
 	}
 	with(benchTraceReplayTransactionCmd, withGethUrl, withErigonUrl, withNeedCompare, withBlockNum, withRecord, withErrorFile)
 
+	var benchEthBlockByNumberCmd = &cobra.Command{
+		Use:   "benchBlockByNumber",
+		Short: "",
+		Long:  ``,
+		Run: func(cmd *cobra.Command, args []string) {
+			rpctest.BenchEthGetBlockByNumber(erigonURL)
+		},
+	}
+	with(benchEthBlockByNumberCmd, withErigonUrl)
+
 	var replayCmd = &cobra.Command{
 		Use:   "replay",
 		Short: "",
@@ -293,6 +303,7 @@ func main() {
 		benchTxReceiptCmd,
 		compareAccountRange,
 		benchTraceReplayTransactionCmd,
+		benchEthBlockByNumberCmd,
 		replayCmd,
 	)
 	if err := rootCmd.ExecuteContext(rootContext()); err != nil {

diff --git a/cmd/rpctest/rpctest/bench1.go b/cmd/rpctest/rpctest/bench1.go
@@ -55,7 +55,7 @@ func Bench1(erigonURL, gethURL string, needCompare bool, fullTest bool, blockFro
 	for bn := blockFrom; bn <= blockTo; bn++ {
 		reqGen.reqID++
 		var b EthBlockByNumber
-		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn), &b)
+		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, true /* withTxs */), &b)
 		resultsCh <- res
 		if res.Err != nil {
 			fmt.Printf("Could not retrieve block (Erigon) %d: %v\n", bn, res.Err)
@@ -68,7 +68,7 @@ func Bench1(erigonURL, gethURL string, needCompare bool, fullTest bool, blockFro
 
 		if needCompare {
 			var bg EthBlockByNumber
-			res = reqGen.Geth("eth_getBlockByNumber", reqGen.getBlockByNumber(bn), &bg)
+			res = reqGen.Geth("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, true /* withTxs */), &bg)
 			if res.Err != nil {
 				fmt.Printf("Could not retrieve block (geth) %d: %v\n", bn, res.Err)
 				return

diff --git a/cmd/rpctest/rpctest/bench_blockbynumber.go b/cmd/rpctest/rpctest/bench_blockbynumber.go
@@ -0,0 +1,63 @@
+package rpctest
+
+import (
+	"fmt"
+	"net/http"
+	"time"
+)
+
+// BenchEthGetBlockByNumber generates lots of requests for eth_getBlockByNumber to attempt to reproduce issue where empty results are being returned
+func BenchEthGetBlockByNumber(erigonURL string) {
+	setRoutes(erigonURL, erigonURL)
+	var client = &http.Client{
+		Timeout: time.Second * 600,
+	}
+	var res CallResult
+	reqGen := &RequestGenerator{
+		client: client,
+	}
+	reqGen.reqID++
+	var blockNumber EthBlockNumber
+	res = reqGen.Erigon("eth_blockNumber", reqGen.blockNumber(), &blockNumber)
+	if res.Err != nil {
+		fmt.Printf("Could not get block number: %v\n", res.Err)
+		return
+	}
+	if blockNumber.Error != nil {
+		fmt.Printf("Error getting block number: %d %s\n", blockNumber.Error.Code, blockNumber.Error.Message)
+		return
+	}
+	fmt.Printf("Last block: %d\n", blockNumber.Number)
+	for bn := uint64(0); bn <= uint64(blockNumber.Number)/2; bn++ {
+		reqGen.reqID++
+		res = reqGen.Erigon2("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, false /* withTxs */))
+		if res.Err != nil {
+			fmt.Printf("Could not retrieve block (Erigon) %d: %v\n", bn, res.Err)
+			return
+		}
+		if errVal := res.Result.Get("error"); errVal != nil {
+			fmt.Printf("error: %d %s", errVal.GetInt("code"), errVal.GetStringBytes("message"))
+			return
+		}
+		if res.Result.Get("result") == nil || res.Result.Get("result").Get("number") == nil {
+			fmt.Printf("empty result: %s\n", res.Response)
+			return
+		}
+
+		reqGen.reqID++
+		bn1 := uint64(blockNumber.Number) - bn
+		res = reqGen.Erigon2("eth_getBlockByNumber", reqGen.getBlockByNumber(bn1, false /* withTxs */))
+		if res.Err != nil {
+			fmt.Printf("Could not retrieve block (Erigon) %d: %v\n", bn1, res.Err)
+			return
+		}
+		if errVal := res.Result.Get("error"); errVal != nil {
+			fmt.Printf("error: %d %s", errVal.GetInt("code"), errVal.GetStringBytes("message"))
+			return
+		}
+		if res.Result.Get("result") == nil || res.Result.Get("result").Get("number") == nil {
+			fmt.Printf("empty result: %s\n", res.Response)
+			return
+		}
+	}
+}
diff --git a/cmd/rpctest/rpctest/bench_debugtracecall.go b/cmd/rpctest/rpctest/bench_debugtracecall.go
@@ -60,7 +60,7 @@ func BenchDebugTraceCall(erigonURL, gethURL string, needCompare bool, blockFrom
 	for bn := blockFrom; bn <= blockTo; bn++ {
 		reqGen.reqID++
 		var b EthBlockByNumber
-		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn), &b)
+		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, true /* withTxs */), &b)
 		if res.Err != nil {
 			fmt.Printf("Could not retrieve block (Erigon) %d: %v\n", bn, res.Err)
 			return
@@ -73,7 +73,7 @@ func BenchDebugTraceCall(erigonURL, gethURL string, needCompare bool, blockFrom
 
 		if needCompare {
 			var bg EthBlockByNumber
-			res = reqGen.Geth("eth_getBlockByNumber", reqGen.getBlockByNumber(bn), &bg)
+			res = reqGen.Geth("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, true /* withTxs */), &bg)
 			if res.Err != nil {
 				fmt.Printf("Could not retrieve block (geth) %d: %v\n", bn, res.Err)
 				return

diff --git a/cmd/rpctest/rpctest/bench_ethcall.go b/cmd/rpctest/rpctest/bench_ethcall.go
@@ -64,7 +64,7 @@ func BenchEthCall(erigonURL, gethURL string, needCompare, latest bool, blockFrom
 	for bn := blockFrom; bn <= blockTo; bn++ {
 		reqGen.reqID++
 		var b EthBlockByNumber
-		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn), &b)
+		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, true /* withTxs */), &b)
 		if res.Err != nil {
 			fmt.Printf("Could not retrieve block (Erigon) %d: %v\n", bn, res.Err)
 			return
@@ -77,7 +77,7 @@ func BenchEthCall(erigonURL, gethURL string, needCompare, latest bool, blockFrom
 
 		if needCompare {
 			var bg EthBlockByNumber
-			res = reqGen.Geth("eth_getBlockByNumber", reqGen.getBlockByNumber(bn), &bg)
+			res = reqGen.Geth("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, true /* withTxs */), &bg)
 			if res.Err != nil {
 				fmt.Printf("Could not retrieve block (geth) %d: %v\n", bn, res.Err)
 				return

diff --git a/cmd/rpctest/rpctest/bench_traceblock.go b/cmd/rpctest/rpctest/bench_traceblock.go
@@ -60,7 +60,7 @@ func BenchTraceBlock(erigonURL, oeURL string, needCompare bool, blockFrom uint64
 	for bn := blockFrom; bn <= blockTo; bn++ {
 		reqGen.reqID++
 		var b EthBlockByNumber
-		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn), &b)
+		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, true /* withTxs */), &b)
 		if res.Err != nil {
 			fmt.Printf("Could not retrieve block (Erigon) %d: %v\n", bn, res.Err)
 			return

diff --git a/cmd/rpctest/rpctest/bench_tracecall.go b/cmd/rpctest/rpctest/bench_tracecall.go
@@ -60,7 +60,7 @@ func BenchTraceCall(erigonURL, oeURL string, needCompare bool, blockFrom uint64,
 	for bn := blockFrom; bn <= blockTo; bn++ {
 		reqGen.reqID++
 		var b EthBlockByNumber
-		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn), &b)
+		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, true /* withTxs */), &b)
 		if res.Err != nil {
 			fmt.Printf("Could not retrieve block (Erigon) %d: %v\n", bn, res.Err)
 			return

diff --git a/cmd/rpctest/rpctest/bench_tracecallmany.go b/cmd/rpctest/rpctest/bench_tracecallmany.go
@@ -63,7 +63,7 @@ func BenchTraceCallMany(erigonURL, oeURL string, needCompare bool, blockFrom uin
 	for bn := blockFrom; bn <= blockTo; bn++ {
 		reqGen.reqID++
 		var b EthBlockByNumber
-		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn), &b)
+		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, true /* withTxs */), &b)
 		if res.Err != nil {
 			fmt.Printf("Could not retrieve block (Erigon) %d: %v\n", bn, res.Err)
 			return

diff --git a/cmd/rpctest/rpctest/bench_tracereplaytransaction.go b/cmd/rpctest/rpctest/bench_tracereplaytransaction.go
@@ -46,7 +46,7 @@ func BenchTraceReplayTransaction(erigonUrl, gethUrl string, needCompare bool, bl
 
 	for bn := blockFrom; bn < blockTo; bn++ {
 		var b EthBlockByNumber
-		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn), &b)
+		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, true /* withTxs */), &b)
 		if res.Err != nil {
 			fmt.Printf("retrieve block (Erigon) %d: %v", blockFrom, res.Err)
 			return

diff --git a/cmd/rpctest/rpctest/bench_tracetransaction.go b/cmd/rpctest/rpctest/bench_tracetransaction.go
@@ -46,7 +46,7 @@ func BenchTraceBlockByHash(erigonUrl, gethUrl string, needCompare bool, blockFro
 
 	for bn := blockFrom; bn < blockTo; bn++ {
 		var b EthBlockByNumber
-		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn), &b)
+		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, true /* withTxs */), &b)
 		if res.Err != nil {
 			fmt.Printf("retrieve block (Erigon) %d: %v", blockFrom, res.Err)
 			return
@@ -103,7 +103,7 @@ func BenchTraceTransaction(erigonUrl, gethUrl string, needCompare bool, blockFro
 
 	for bn := blockFrom; bn < blockTo; bn++ {
 		var b EthBlockByNumber
-		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn), &b)
+		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, true /* withTxs */), &b)
 		if res.Err != nil {
 			fmt.Printf("retrieve block (Erigon) %d: %v", blockFrom, res.Err)
 			return

diff --git a/cmd/rpctest/rpctest/bench_txreceipts.go b/cmd/rpctest/rpctest/bench_txreceipts.go
@@ -61,7 +61,7 @@ func BenchTxReceipt(erigonURL, gethURL string, needCompare bool, blockFrom uint6
 	for bn := blockFrom; bn <= blockTo; bn++ {
 		reqGen.reqID++
 		var b EthBlockByNumber
-		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn), &b)
+		res = reqGen.Erigon("eth_getBlockByNumber", reqGen.getBlockByNumber(bn, false /* withTxs */), &b)
 		if res.Err != nil {
 			fmt.Printf("Could not retrieve block (Erigon) %d: %v\n", bn, res.Err)
 			return

diff --git a/cmd/rpctest/rpctest/request_generator.go b/cmd/rpctest/rpctest/request_generator.go
@@ -31,9 +31,9 @@ func (g *RequestGenerator) blockNumber() string {
 	const template = `{"jsonrpc":"2.0","method":"eth_blockNumber","params":[],"id":%d}`
 	return fmt.Sprintf(template, g.reqID)
 }
-func (g *RequestGenerator) getBlockByNumber(blockNum uint64) string {
-	const template = `{"jsonrpc":"2.0","method":"eth_getBlockByNumber","params":["0x%x",true],"id":%d}`
-	return fmt.Sprintf(template, blockNum, g.reqID)
+func (g *RequestGenerator) getBlockByNumber(blockNum uint64, withTxs bool) string {
+	const template = `{"jsonrpc":"2.0","method":"eth_getBlockByNumber","params":["0x%x",%t],"id":%d}`
+	return fmt.Sprintf(template, blockNum, withTxs, g.reqID)
 }
 
 func (g *RequestGenerator) storageRangeAt(hash common.Hash, i int, to *common.Address, nextKey common.Hash) string {

diff --git a/cmd/rpctest/rpctest/type.go b/cmd/rpctest/rpctest/type.go
@@ -2,6 +2,7 @@ package rpctest
 
 import (
 	"fmt"
+
 	"github.com/ledgerwatch/erigon/common"
 	"github.com/ledgerwatch/erigon/common/hexutil"
 	"github.com/ledgerwatch/erigon/core/state"
@@ -61,7 +62,7 @@ type EthBlockByNumberResult struct {
 
 type EthBlockByNumber struct {
 	CommonResponse
-	Result EthBlockByNumberResult `json:"result"`
+	Result *EthBlockByNumberResult `json:"result"`
 }
 
 type StructLog struct {

diff --git a/turbo/snapshotsync/block_reader.go b/turbo/snapshotsync/block_reader.go
@@ -427,7 +427,7 @@ func (back *BlockReaderWithSnapshots) BlockWithSenders(ctx context.Context, tx k
 		return nil
 	})
 	if err != nil {
-		return
+		return nil, nil, err
 	}
 	if ok && h != nil {
 		var b *types.Body
@@ -441,7 +441,7 @@ func (back *BlockReaderWithSnapshots) BlockWithSenders(ctx context.Context, tx k
 			return nil
 		})
 		if err != nil {
-			return
+			return nil, nil, err
 		}
 		if ok && b != nil {
 			if txsAmount == 0 {