Json rpc eip1898 correctness (#2736)

erigontech · Sep 28, 2021 · d027d71 · d027d71
1 parent 0d34a5b
commit d027d71
Show file tree

Hide file tree

Showing 5 changed files with 191 additions and 5 deletions.
diff --git a/cmd/rpcdaemon/commands/eth_api_test.go b/cmd/rpcdaemon/commands/eth_api_test.go
@@ -2,6 +2,11 @@ package commands
 
 import (
 	"context"
+	"fmt"
+	"github.com/ledgerwatch/erigon/core"
+	"github.com/ledgerwatch/erigon/internal/ethapi"
+	"github.com/ledgerwatch/erigon/rpc"
+	"github.com/stretchr/testify/assert"
 	"testing"
 
 	"github.com/ledgerwatch/erigon/cmd/rpcdaemon/rpcdaemontest"
@@ -25,3 +30,172 @@ func TestGetTransactionReceiptUnprotected(t *testing.T) {
 		t.Errorf("calling GetTransactionReceipt for unprotected tx: %v", err)
 	}
 }
+
+// EIP-1898 test cases
+
+func TestGetStorageAt_ByBlockNumber_WithRequireCanonicalDefault(t *testing.T) {
+	assert := assert.New(t)
+	db := rpcdaemontest.CreateTestKV(t)
+	api := NewEthAPI(NewBaseApi(nil), db, nil, nil, nil, 5000000)
+	addr := common.HexToAddress("0x71562b71999873db5b286df957af199ec94617f7")
+
+	result, err := api.GetStorageAt(context.Background(), addr, "0x0", rpc.BlockNumberOrHashWithNumber(0))
+	if err != nil {
+		t.Errorf("calling GetStorageAt: %v", err)
+	}
+
+	assert.Equal(common.HexToHash("0x0").String(), result)
+}
+
+func TestGetStorageAt_ByBlockHash_WithRequireCanonicalDefault(t *testing.T) {
+	assert := assert.New(t)
+	m, _, _ := rpcdaemontest.CreateTestSentry(t)
+	db := m.DB
+	api := NewEthAPI(NewBaseApi(nil), db, nil, nil, nil, 5000000)
+	addr := common.HexToAddress("0x71562b71999873db5b286df957af199ec94617f7")
+
+	result, err := api.GetStorageAt(context.Background(), addr, "0x0", rpc.BlockNumberOrHashWithHash(m.Genesis.Hash(), false))
+	if err != nil {
+		t.Errorf("calling GetStorageAt: %v", err)
+	}
+
+	assert.Equal(common.HexToHash("0x0").String(), result)
+}
+
+func TestGetStorageAt_ByBlockHash_WithRequireCanonicalTrue(t *testing.T) {
+	assert := assert.New(t)
+	m, _, _ := rpcdaemontest.CreateTestSentry(t)
+	db := m.DB
+	api := NewEthAPI(NewBaseApi(nil), db, nil, nil, nil, 5000000)
+	addr := common.HexToAddress("0x71562b71999873db5b286df957af199ec94617f7")
+
+	result, err := api.GetStorageAt(context.Background(), addr, "0x0", rpc.BlockNumberOrHashWithHash(m.Genesis.Hash(), true))
+	if err != nil {
+		t.Errorf("calling GetStorageAt: %v", err)
+	}
+
+	assert.Equal(common.HexToHash("0x0").String(), result)
+}
+
+func TestGetStorageAt_ByBlockHash_WithRequireCanonicalDefault_BlockNotFoundError(t *testing.T) {
+	m, _, _ := rpcdaemontest.CreateTestSentry(t)
+	db := m.DB
+	api := NewEthAPI(NewBaseApi(nil), db, nil, nil, nil, 5000000)
+	addr := common.HexToAddress("0x71562b71999873db5b286df957af199ec94617f7")
+
+	offChain, err := core.GenerateChain(m.ChainConfig, m.Genesis, m.Engine, m.DB, 1, func(i int, block *core.BlockGen) {
+	}, true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	offChainBlock := offChain.Blocks[0]
+
+	if _, err := api.GetStorageAt(context.Background(), addr, "0x0", rpc.BlockNumberOrHashWithHash(offChainBlock.Hash(), false)); err != nil {
+		if fmt.Sprintf("%v", err) != fmt.Sprintf("block %s not found", offChainBlock.Hash().String()[2:]) {
+			t.Errorf("wrong error: %v", err)
+		}
+	} else {
+		t.Error("error expected")
+	}
+}
+
+func TestGetStorageAt_ByBlockHash_WithRequireCanonicalTrue_BlockNotFoundError(t *testing.T) {
+	m, _, _ := rpcdaemontest.CreateTestSentry(t)
+	db := m.DB
+	api := NewEthAPI(NewBaseApi(nil), db, nil, nil, nil, 5000000)
+	addr := common.HexToAddress("0x71562b71999873db5b286df957af199ec94617f7")
+
+	offChain, err := core.GenerateChain(m.ChainConfig, m.Genesis, m.Engine, m.DB, 1, func(i int, block *core.BlockGen) {
+	}, true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	offChainBlock := offChain.Blocks[0]
+
+	if _, err := api.GetStorageAt(context.Background(), addr, "0x0", rpc.BlockNumberOrHashWithHash(offChainBlock.Hash(), true)); err != nil {
+		if fmt.Sprintf("%v", err) != fmt.Sprintf("block %s not found", offChainBlock.Hash().String()[2:]) {
+			t.Errorf("wrong error: %v", err)
+		}
+	} else {
+		t.Error("error expected")
+	}
+}
+
+func TestGetStorageAt_ByBlockHash_WithRequireCanonicalDefault_NonCanonicalBlock(t *testing.T) {
+	assert := assert.New(t)
+	m, _, orphanedChain := rpcdaemontest.CreateTestSentry(t)
+	db := m.DB
+	api := NewEthAPI(NewBaseApi(nil), db, nil, nil, nil, 5000000)
+	addr := common.HexToAddress("0x71562b71999873db5b286df957af199ec94617f7")
+
+	orphanedBlock := orphanedChain[0].Blocks[0]
+
+	result, err := api.GetStorageAt(context.Background(), addr, "0x0", rpc.BlockNumberOrHashWithHash(orphanedBlock.Hash(), false))
+	if err != nil {
+		t.Errorf("calling GetStorageAt: %v", err)
+	}
+
+	assert.Equal(common.HexToHash("0x0").String(), result)
+}
+
+func TestGetStorageAt_ByBlockHash_WithRequireCanonicalTrue_NonCanonicalBlock(t *testing.T) {
+	m, _, orphanedChain := rpcdaemontest.CreateTestSentry(t)
+	db := m.DB
+	api := NewEthAPI(NewBaseApi(nil), db, nil, nil, nil, 5000000)
+	addr := common.HexToAddress("0x71562b71999873db5b286df957af199ec94617f7")
+
+	orphanedBlock := orphanedChain[0].Blocks[0]
+
+	if _, err := api.GetStorageAt(context.Background(), addr, "0x0", rpc.BlockNumberOrHashWithHash(orphanedBlock.Hash(), true)); err != nil {
+		if fmt.Sprintf("%v", err) != fmt.Sprintf("hash %s is not currently canonical", orphanedBlock.Hash().String()[2:]) {
+			t.Errorf("wrong error: %v", err)
+		}
+	} else {
+		t.Error("error expected")
+	}
+}
+
+func TestCall_ByBlockHash_WithRequireCanonicalDefault_NonCanonicalBlock(t *testing.T) {
+	m, _, orphanedChain := rpcdaemontest.CreateTestSentry(t)
+	db := m.DB
+	api := NewEthAPI(NewBaseApi(nil), db, nil, nil, nil, 5000000)
+	from := common.HexToAddress("0x71562b71999873db5b286df957af199ec94617f7")
+	to := common.HexToAddress("0x0d3ab14bbad3d99f4203bd7a11acb94882050e7e")
+
+	orphanedBlock := orphanedChain[0].Blocks[0]
+
+	if _, err := api.Call(context.Background(), ethapi.CallArgs{
+		From: &from,
+		To:   &to,
+	}, rpc.BlockNumberOrHashWithHash(orphanedBlock.Hash(), false), nil); err != nil {
+		if fmt.Sprintf("%v", err) != fmt.Sprintf("hash %s is not currently canonical", orphanedBlock.Hash().String()[2:]) {
+			/* Not sure. Here https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1898.md it is not explicitly said that
+			   eth_call should only work with canonical blocks.
+			   But since there is no point in changing the state of non-canonical block, it ignores RequireCanonical. */
+			t.Errorf("wrong error: %v", err)
+		}
+	} else {
+		t.Error("error expected")
+	}
+}
+
+func TestCall_ByBlockHash_WithRequireCanonicalTrue_NonCanonicalBlock(t *testing.T) {
+	m, _, orphanedChain := rpcdaemontest.CreateTestSentry(t)
+	db := m.DB
+	api := NewEthAPI(NewBaseApi(nil), db, nil, nil, nil, 5000000)
+	from := common.HexToAddress("0x71562b71999873db5b286df957af199ec94617f7")
+	to := common.HexToAddress("0x0d3ab14bbad3d99f4203bd7a11acb94882050e7e")
+
+	orphanedBlock := orphanedChain[0].Blocks[0]
+
+	if _, err := api.Call(context.Background(), ethapi.CallArgs{
+		From: &from,
+		To:   &to,
+	}, rpc.BlockNumberOrHashWithHash(orphanedBlock.Hash(), true), nil); err != nil {
+		if fmt.Sprintf("%v", err) != fmt.Sprintf("hash %s is not currently canonical", orphanedBlock.Hash().String()[2:]) {
+			t.Errorf("wrong error: %v", err)
+		}
+	} else {
+		t.Error("error expected")
+	}
+}
diff --git a/cmd/rpcdaemon/rpcdaemontest/test_util.go b/cmd/rpcdaemon/rpcdaemontest/test_util.go
@@ -26,6 +26,11 @@ import (
 )
 
 func CreateTestKV(t *testing.T) kv.RwDB {
+	s, _, _ := CreateTestSentry(t)
+	return s.DB
+}
+
+func CreateTestSentry(t *testing.T) (*stages.MockSentry, *core.ChainPack, []*core.ChainPack) {
 	// Configure and generate a sample block chain
 	var (
 		key, _   = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")
@@ -196,7 +201,7 @@ func CreateTestKV(t *testing.T) kv.RwDB {
 		t.Fatal(err)
 	}
 
-	return m.DB
+	return m, chain, []*core.ChainPack{orphanedChain}
 }
 
 type IsMiningMock struct{}

diff --git a/libmdbx b/libmdbx
diff --git a/turbo/rpchelper/helper.go b/turbo/rpchelper/helper.go
@@ -23,6 +23,14 @@ func (e nonCanonocalHashError) Error() string {
 }
 
 func GetBlockNumber(blockNrOrHash rpc.BlockNumberOrHash, tx kv.Tx, filters *filters.Filters) (uint64, common.Hash, error) {
+	return _GetBlockNumber(blockNrOrHash.RequireCanonical, blockNrOrHash, tx, filters)
+}
+
+func GetCanonicalBlockNumber(blockNrOrHash rpc.BlockNumberOrHash, tx kv.Tx, filters *filters.Filters) (uint64, common.Hash, error) {
+	return _GetBlockNumber(true, blockNrOrHash, tx, filters)
+}
+
+func _GetBlockNumber(requireCanonical bool, blockNrOrHash rpc.BlockNumberOrHash, tx kv.Tx, filters *filters.Filters) (uint64, common.Hash, error) {
 	var blockNumber uint64
 	var err error
 	hash, ok := blockNrOrHash.Hash()
@@ -63,7 +71,7 @@ func GetBlockNumber(blockNrOrHash rpc.BlockNumberOrHash, tx kv.Tx, filters *filt
 		if err != nil {
 			return 0, common.Hash{}, err
 		}
-		if blockNrOrHash.RequireCanonical && ch != hash {
+		if requireCanonical && ch != hash {
 			return 0, common.Hash{}, nonCanonocalHashError{hash}
 		}
 	}

diff --git a/turbo/transactions/call.go b/turbo/transactions/call.go
@@ -32,8 +32,7 @@ func DoCall(ctx context.Context, args ethapi.CallArgs, tx kv.Tx, blockNrOrHash r
 			return state, block.Header(), nil
 		}
 	*/
-	blockNrOrHash.RequireCanonical = true // DoCall cannot be executed on non-canonical blocks
-	blockNumber, hash, err := rpchelper.GetBlockNumber(blockNrOrHash, tx, filters)
+	blockNumber, hash, err := rpchelper.GetCanonicalBlockNumber(blockNrOrHash, tx, filters) // DoCall cannot be executed on non-canonical blocks
 	if err != nil {
 		return nil, err
 	}
+1 −1		.github/FUNDING.yml
+0 −1		.github/actions/spelling/excludes.txt
+2 −28		.github/actions/spelling/expect.txt
+2 −2		.github/workflows/release-assets.yml
+0 −5		.gitignore
+10 −32		CMakeLists.txt
+0 −26		CMakeSettings.json
+5 −62		ChangeLog.md
+95 −133		GNUmakefile
+2 −2		Makefile
+6 −68		README.md
+3 −3		appveyor.yml
+27 −42		cmake/compiler.cmake
+2 −7		cmake/profile.cmake
+3 −11		cmake/utils.cmake
+26 −76		mdbx.h
+733 −1,866		mdbx.h++
+219 −0		packages/buildroot/v5-0000-cover-letter-package-libmdbx.patch
+47 −25		packages/buildroot/v5-0001-package-libmdbx-new-package-library-database.patch
+389 −526		src/core.c
+7 −2		src/defs.h
+4 −16		src/internals.h
+8 −8		src/lck-posix.c
+128 −132		src/mdbx.c++
+23 −67		src/mdbx_chk.c
+14 −14		src/options.h
+39 −110		src/osal.c
+1 −1		src/osal.h
+1 −1		src/version.c.in
+1 −7		test/CMakeLists.txt
+1 −1		test/base.h
+0 −1		test/config.cc
+5 −7		test/keygen.cc
+3 −3		test/keygen.h
+88 −206		test/long_stochastic.sh
+6 −61		test/main.cc
+6 −14		test/nested.cc
+6 −16		test/test.cc
+9 −9		test/test.h
+169 −9		test/utils.cc
+2 −0		test/utils.h
+9 −16		test/valgrind_suppress.txt