From 058a4ac5f13f49a67d4b28111dd0ad3eea387c8c Mon Sep 17 00:00:00 2001
From: Martin Holst Swende <martin@swende.se>
Date: Tue, 4 Feb 2020 11:32:31 +0100
Subject: [PATCH] core/evm: less iteration in blockhash (#20589)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* core/vm/runtime: add test for blockhash

* core/evm: less iteration in blockhash

* core/vm/runtime: nitpickfix

Co-authored-by: Péter Szilágyi <peterke@gmail.com>
---
 core/evm.go                     |  34 ++++++----
 core/vm/runtime/env.go          |   4 +-
 core/vm/runtime/runtime_test.go | 113 ++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+), 16 deletions(-)

diff --git a/core/evm.go b/core/evm.go
index b654bbd47..8abe5a047 100644
--- a/core/evm.go
+++ b/core/evm.go
@@ -60,24 +60,32 @@ func NewEVMContext(msg Message, header *types.Header, chain ChainContext, author
 
 // GetHashFn returns a GetHashFunc which retrieves header hashes by number
 func GetHashFn(ref *types.Header, chain ChainContext) func(n uint64) common.Hash {
-	var cache map[uint64]common.Hash
+	// Cache will initially contain [refHash.parent],
+	// Then fill up with [refHash.p, refHash.pp, refHash.ppp, ...]
+	var cache []common.Hash
 
 	return func(n uint64) common.Hash {
 		// If there's no hash cache yet, make one
-		if cache == nil {
-			cache = map[uint64]common.Hash{
-				ref.Number.Uint64() - 1: ref.ParentHash,
-			}
+		if len(cache) == 0 {
+			cache = append(cache, ref.ParentHash)
 		}
-		// Try to fulfill the request from the cache
-		if hash, ok := cache[n]; ok {
-			return hash
+		if idx := ref.Number.Uint64() - n - 1; idx < uint64(len(cache)) {
+			return cache[idx]
 		}
-		// Not cached, iterate the blocks and cache the hashes
-		for header := chain.GetHeader(ref.ParentHash, ref.Number.Uint64()-1); header != nil; header = chain.GetHeader(header.ParentHash, header.Number.Uint64()-1) {
-			cache[header.Number.Uint64()-1] = header.ParentHash
-			if n == header.Number.Uint64()-1 {
-				return header.ParentHash
+		// No luck in the cache, but we can start iterating from the last element we already know
+		lastKnownHash := cache[len(cache)-1]
+		lastKnownNumber := ref.Number.Uint64() - uint64(len(cache))
+
+		for {
+			header := chain.GetHeader(lastKnownHash, lastKnownNumber)
+			if header == nil {
+				break
+			}
+			cache = append(cache, header.ParentHash)
+			lastKnownHash = header.ParentHash
+			lastKnownNumber = header.Number.Uint64() - 1
+			if n == lastKnownNumber {
+				return lastKnownHash
 			}
 		}
 		return common.Hash{}
diff --git a/core/vm/runtime/env.go b/core/vm/runtime/env.go
index 31c9b9cf9..38ee44890 100644
--- a/core/vm/runtime/env.go
+++ b/core/vm/runtime/env.go
@@ -17,7 +17,6 @@
 package runtime
 
 import (
-	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/vm"
 )
@@ -26,8 +25,7 @@ func NewEnv(cfg *Config) *vm.EVM {
 	context := vm.Context{
 		CanTransfer: core.CanTransfer,
 		Transfer:    core.Transfer,
-		GetHash:     func(uint64) common.Hash { return common.Hash{} },
-
+		GetHash:     cfg.GetHashFn,
 		Origin:      cfg.Origin,
 		Coinbase:    cfg.Coinbase,
 		BlockNumber: cfg.BlockNumber,
diff --git a/core/vm/runtime/runtime_test.go b/core/vm/runtime/runtime_test.go
index 15f545ddc..f2d05118c 100644
--- a/core/vm/runtime/runtime_test.go
+++ b/core/vm/runtime/runtime_test.go
@@ -23,8 +23,11 @@ import (
 
 	"github.com/ethereum/go-ethereum/accounts/abi"
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/consensus"
+	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/rawdb"
 	"github.com/ethereum/go-ethereum/core/state"
+	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/core/vm"
 	"github.com/ethereum/go-ethereum/params"
 )
@@ -203,3 +206,113 @@ func BenchmarkEVM_CREATE2_1200(bench *testing.B) {
 	// initcode size 1200K, repeatedly calls CREATE2 and then modifies the mem contents
 	benchmarkEVM_Create(bench, "5b5862124f80600080f5600152600056")
 }
+
+func fakeHeader(n uint64, parentHash common.Hash) *types.Header {
+	header := types.Header{
+		Coinbase:   common.HexToAddress("0x00000000000000000000000000000000deadbeef"),
+		Number:     big.NewInt(int64(n)),
+		ParentHash: parentHash,
+		Time:       1000,
+		Nonce:      types.BlockNonce{0x1},
+		Extra:      []byte{},
+		Difficulty: big.NewInt(0),
+		GasLimit:   100000,
+	}
+	return &header
+}
+
+type dummyChain struct {
+	counter int
+}
+
+// Engine retrieves the chain's consensus engine.
+func (d *dummyChain) Engine() consensus.Engine {
+	return nil
+}
+
+// GetHeader returns the hash corresponding to their hash.
+func (d *dummyChain) GetHeader(h common.Hash, n uint64) *types.Header {
+	d.counter++
+	parentHash := common.Hash{}
+	s := common.LeftPadBytes(big.NewInt(int64(n-1)).Bytes(), 32)
+	copy(parentHash[:], s)
+
+	//parentHash := common.Hash{byte(n - 1)}
+	//fmt.Printf("GetHeader(%x, %d) => header with parent %x\n", h, n, parentHash)
+	return fakeHeader(n, parentHash)
+}
+
+// TestBlockhash tests the blockhash operation. It's a bit special, since it internally
+// requires access to a chain reader.
+func TestBlockhash(t *testing.T) {
+	// Current head
+	n := uint64(1000)
+	parentHash := common.Hash{}
+	s := common.LeftPadBytes(big.NewInt(int64(n-1)).Bytes(), 32)
+	copy(parentHash[:], s)
+	header := fakeHeader(n, parentHash)
+
+	// This is the contract we're using. It requests the blockhash for current num (should be all zeroes),
+	// then iteratively fetches all blockhashes back to n-260.
+	// It returns
+	// 1. the first (should be zero)
+	// 2. the second (should be the parent hash)
+	// 3. the last non-zero hash
+	// By making the chain reader return hashes which correlate to the number, we can
+	// verify that it obtained the right hashes where it should
+
+	/*
+
+		pragma solidity ^0.5.3;
+		contract Hasher{
+
+			function test() public view returns (bytes32, bytes32, bytes32){
+				uint256 x = block.number;
+				bytes32 first;
+				bytes32 last;
+				bytes32 zero;
+				zero = blockhash(x); // Should be zeroes
+				first = blockhash(x-1);
+				for(uint256 i = 2 ; i < 260; i++){
+					bytes32 hash = blockhash(x - i);
+					if (uint256(hash) != 0){
+						last = hash;
+					}
+				}
+				return (zero, first, last);
+			}
+		}
+
+	*/
+	// The contract above
+	data := common.Hex2Bytes("6080604052348015600f57600080fd5b50600436106045576000357c010000000000000000000000000000000000000000000000000000000090048063f8a8fd6d14604a575b600080fd5b60506074565b60405180848152602001838152602001828152602001935050505060405180910390f35b600080600080439050600080600083409050600184034092506000600290505b61010481101560c35760008186034090506000816001900414151560b6578093505b5080806001019150506094565b508083839650965096505050505090919256fea165627a7a72305820462d71b510c1725ff35946c20b415b0d50b468ea157c8c77dff9466c9cb85f560029")
+	// The method call to 'test()'
+	input := common.Hex2Bytes("f8a8fd6d")
+	chain := &dummyChain{}
+	ret, _, err := Execute(data, input, &Config{
+		GetHashFn:   core.GetHashFn(header, chain),
+		BlockNumber: new(big.Int).Set(header.Number),
+	})
+	if err != nil {
+		t.Fatalf("expected no error, got %v", err)
+	}
+	if len(ret) != 96 {
+		t.Fatalf("expected returndata to be 96 bytes, got %d", len(ret))
+	}
+
+	zero := new(big.Int).SetBytes(ret[0:32])
+	first := new(big.Int).SetBytes(ret[32:64])
+	last := new(big.Int).SetBytes(ret[64:96])
+	if zero.BitLen() != 0 {
+		t.Fatalf("expected zeroes, got %x", ret[0:32])
+	}
+	if first.Uint64() != 999 {
+		t.Fatalf("second block should be 999, got %d (%x)", first, ret[32:64])
+	}
+	if last.Uint64() != 744 {
+		t.Fatalf("last block should be 744, got %d (%x)", last, ret[64:96])
+	}
+	if exp, got := 255, chain.counter; exp != got {
+		t.Errorf("suboptimal; too much chain iteration, expected %d, got %d", exp, got)
+	}
+}
-- 
GitLab