Improve RPC API compatibility for Ethers.js

Makefile

@@ -47,10 +47,10 @@ geth:
 
 prepare: prepare-system-contracts prepare-ethersjs-project
 
-prepare-ethersjs-project: ./ethersjs-api-check/node_modules
+prepare-ethersjs-project: ./e2e_test/ethersjs-api-check/node_modules
 
-./ethersjs-api-check/node_modules: ./ethersjs-api-check/package.json ./ethersjs-api-check/package-lock.json
-	@cd ./ethersjs-api-check && npm ci
+./e2e_test/ethersjs-api-check/node_modules: ./e2e_test/ethersjs-api-check/package.json ./e2e_test/ethersjs-api-check/package-lock.json
+	@cd ./e2e_test/ethersjs-api-check && npm ci
 
 # This rule checks out celo-monorepo under MONOREPO_PATH at the commit contained in
 # monorepo_commit and compiles the system solidity contracts. It then copies the

contracts/blockchain_parameters/blockchain_parameters.go

@@ -79,16 +79,16 @@ func getIntrinsicGasForAlternativeFeeCurrency(vmRunner vm.EVMRunner) (uint64, er
 // GetBlockGasLimitOrDefault retrieves the block max gas limit
 // In case of error, it returns the default value
 func GetBlockGasLimitOrDefault(vmRunner vm.EVMRunner) uint64 {
-	val, err := getBlockGasLimit(vmRunner)
+	val, err := GetBlockGasLimit(vmRunner)
 	if err != nil {
 		logError("blockGasLimit", err)
 		return params.DefaultGasLimit
 	}
 	return val
 }
 
-// getBlockGasLimit retrieves the block max gas limit
-func getBlockGasLimit(vmRunner vm.EVMRunner) (uint64, error) {
+// GetBlockGasLimit retrieves the block max gas limit
+func GetBlockGasLimit(vmRunner vm.EVMRunner) (uint64, error) {
 	var gasLimit *big.Int
 	err := blockGasLimitMethod.Query(vmRunner, &gasLimit)
 	if err != nil {

contracts/blockchain_parameters/blockchain_parameters_test.go

@@ -71,8 +71,8 @@ func TestGetIntrinsicGasForAlternativeFeeCurrencyOrDefault(t *testing.T) {
 }
 
 func TestGetBlockGasLimit(t *testing.T) {
-	testutil.TestFailOnFailingRunner(t, getBlockGasLimit)
-	testutil.TestFailsWhenContractNotDeployed(t, contracts.ErrSmartContractNotDeployed, getBlockGasLimit)
+	testutil.TestFailOnFailingRunner(t, GetBlockGasLimit)
+	testutil.TestFailsWhenContractNotDeployed(t, contracts.ErrSmartContractNotDeployed, GetBlockGasLimit)
 	t.Run("should return block gas limit", func(t *testing.T) {
 		g := NewGomegaWithT(t)
 
@@ -84,7 +84,7 @@ func TestGetBlockGasLimit(t *testing.T) {
 			},
 		)
 
-		gas, err := getBlockGasLimit(runner)
+		gas, err := GetBlockGasLimit(runner)
 		g.Expect(err).ToNot(HaveOccurred())
 		g.Expect(gas).To(Equal(uint64(50000)))
 	})

contracts/gasprice_minimum/gasprice_minimum.go

@@ -17,6 +17,7 @@
 package gasprice_minimum
 
 import (
+	"fmt"
 	"math/big"
 
 	"github.com/celo-org/celo-blockchain/common"
@@ -89,6 +90,32 @@ func GetGasPriceMinimum(vmRunner vm.EVMRunner, currency *common.Address) (*big.I
 	return gasPriceMinimum, err
 }
 
+// GetRealGasPriceMinimum is similar to GetRealGasPriceMinimum but if there is
+// a problem retrieving the gas price minimum it will return the error and a
+// nil gas price minimum.
+func GetRealGasPriceMinimum(vmRunner vm.EVMRunner, currency *common.Address) (*big.Int, error) {
+	var currencyAddress common.Address
+	var err error
+
+	if currency == nil {
+		currencyAddress, err = contracts.GetRegisteredAddress(vmRunner, params.GoldTokenRegistryId)
+
+		if err != nil {
+			return nil, fmt.Errorf("failed to retrieve gold token address: %w", err)
+		}
+	} else {
+		currencyAddress = *currency
+	}
+
+	var gasPriceMinimum *big.Int
+	err = getGasPriceMinimumMethod.Query(vmRunner, &gasPriceMinimum, currencyAddress)
+	if err != nil {
+		return nil, fmt.Errorf("failed to retrieve gas price minimum for currency %v, error: %w", currencyAddress.String(), err)
+	}
+
+	return gasPriceMinimum, nil
+}
+
 func GetGasPriceMinimumFloor(vmRunner vm.EVMRunner) (*big.Int, error) {
 	var err error
 

e2e_test/e2e_test.go

@@ -491,8 +491,8 @@ func TestEthersJSCompatibility(t *testing.T) {
 	// The tests don't seem to work on CI with IPV6 addresses so we convert to IPV4 here
 	addr := strings.Replace(network[0].Node.HTTPEndpoint(), "[::]", "127.0.0.1", 1)
 
-	cmd := exec.Command("npm", "run", "test", "--networkaddr="+addr, "--blocknum="+hexutil.Uint64(num).String(), "--", "--invert", "--grep", "block with pruned")
-	cmd.Dir = "../ethersjs-api-check/"
+	cmd := exec.Command("npm", "run", "test", "--networkaddr="+addr, "--blocknum="+hexutil.Uint64(num).String(), "--", "--grep", "ethers.js compatibility tests with state")
+	cmd.Dir = "./ethersjs-api-check/"
 	println("executing mocha test with", cmd.String())
 	output, err := cmd.CombinedOutput()
 	println(string(output))
@@ -507,8 +507,8 @@ func TestEthersJSCompatibility(t *testing.T) {
 	require.NoError(t, err)
 
 	// Execute typescript tests to check what happens with a pruned block.
-	cmd = exec.Command("npm", "run", "test", "--networkaddr="+addr, "--blocknum="+hexutil.Uint64(num).String(), "--", "--grep", "block with pruned")
-	cmd.Dir = "../ethersjs-api-check/"
+	cmd = exec.Command("npm", "run", "test", "--networkaddr="+addr, "--blocknum="+hexutil.Uint64(num).String(), "--", "--grep", "ethers.js compatibility tests with no state")
+	cmd.Dir = "./ethersjs-api-check/"
 	println("executing mocha test with", cmd.String())
 	output, err = cmd.CombinedOutput()
 	println(string(output))

e2e_test/ethersjs-api-check/test/test.ts

@@ -0,0 +1,94 @@
+/*
+ * Note these tests are intended to be invoked only by our e2e tests, they
+ * should not be executed in a standalone fashion.
+ * See e2e_test.TestEthersJSCompatibility
+ */
+import {ethers} from 'ethers';
+import {assert} from 'chai';
+import 'mocha';
+
+describe('ethers.js compatibility tests with state', () => {
+
+	it('provider.getBlock works (block has gasLimit set)', async () => {
+		let provider = new ethers.providers.JsonRpcProvider(process.env.npm_config_networkaddr);
+		let block = await provider.getBlock(process.env.npm_config_blocknum as string);
+
+		// These assertions trigger on undefined or null
+		assert.notEqual(block, null);
+		assert.notEqual(block.gasLimit, null);
+	});
+
+	it('EIP-1559 transactions supported (can get feeData)', async () => {
+		let provider = new ethers.providers.JsonRpcProvider(process.env.npm_config_networkaddr);
+
+		// The fee data is the construct used to determine if EIP-1559 transactions are supported, if it contains max
+		let feeData = await provider.getFeeData();
+
+		// These assertions trigger on undefined or null
+		assert.notEqual(feeData, null);
+		// If the following 2 fields are set then the network is assumed to support EIP-1559 transactions.
+		assert.notEqual(feeData.maxFeePerGas, null);
+		assert.notEqual(feeData.maxPriorityFeePerGas, null);
+		// We check the other 2 fields for completeness, they should also be set.
+		assert.notEqual(feeData.gasPrice, null);
+		assert.notEqual(feeData.lastBaseFeePerGas, null);
+	});
+
+	it('block has gasLimit', async () => {
+		let provider = new ethers.providers.JsonRpcProvider(process.env.npm_config_networkaddr);
+		const fullBlock = await provider.send(
+			'eth_getBlockByNumber',
+			[ethers.utils.hexValue(process.env.npm_config_blocknum as string), true]
+		)
+		assert.isTrue(fullBlock.hasOwnProperty('gasLimit'))
+	});
+
+	it('block has baseFeePerGas', async () => {
+		let provider = new ethers.providers.JsonRpcProvider(process.env.npm_config_networkaddr);
+		const fullBlock = await provider.send(
+			'eth_getBlockByNumber',
+			[ethers.utils.hexValue(process.env.npm_config_blocknum as string), true]
+		)
+		assert.isTrue(fullBlock.hasOwnProperty('baseFeePerGas'))
+	});
+
+});
+
+describe('ethers.js compatibility tests with no state', () => {
+
+	it('provider.getBlock throws exception (no gasLimit)', async () => {
+		let provider = new ethers.providers.JsonRpcProvider(process.env.npm_config_networkaddr);
+		try {
+			await provider.getBlock(process.env.npm_config_blocknum as string);
+		} catch (e) {
+			return
+		}
+		assert.fail("Expecting exception to be thrown when getting block")
+	});
+
+	it('block has no gasLimit', async () => {
+		let provider = new ethers.providers.JsonRpcProvider(process.env.npm_config_networkaddr);
+		const fullBlock = await provider.send(
+			'eth_getBlockByNumber',
+			[ethers.utils.hexValue(process.env.npm_config_blocknum as string), true]
+		)
+		assert.isFalse(fullBlock.hasOwnProperty('gasLimit'))
+	});
+
+	it('block has no baseFeePerGas', async () => {
+		let provider = new ethers.providers.JsonRpcProvider(process.env.npm_config_networkaddr);
+		const fullBlock = await provider.send(
+			'eth_getBlockByNumber',
+			[ethers.utils.hexValue(process.env.npm_config_blocknum as string), true]
+		)
+		assert.isFalse(fullBlock.hasOwnProperty('baseFeePerGas'))
+	});
+
+
+});
+
+
+// const fullBlock = await provider.send(
+// 	'eth_getBlockByNumber',
+// 	[ethers.utils.hexValue(blockNumber), true]
+// )

ethersjs-api-check/tsconfig.json → e2e_test/ethersjs-api-check/tsconfig.json

@@ -4,8 +4,9 @@
     "module": "commonjs",
     "esModuleInterop": true,
     "strict": true,
-    "rootDir": "src",
+    "rootDir": ".",
     "outDir": "dist",
-	"moduleResolution": "node"
+	"moduleResolution": "node",
+	"useUnknownInCatchVariables": true
   }
 }

eth/api_backend.go

@@ -19,6 +19,7 @@ package eth
 import (
 	"context"
 	"errors"
+	"fmt"
 	"math/big"
 
 	ethereum "github.com/celo-org/celo-blockchain"
@@ -315,6 +316,15 @@ func (b *EthAPIBackend) GasPriceMinimumForHeader(ctx context.Context, currencyAd
 	return gpm.GetGasPriceMinimum(vmRunner, currencyAddress)
 }
 
+func (b *EthAPIBackend) RealGasPriceMinimumForHeader(ctx context.Context, currencyAddress *common.Address, header *types.Header) (*big.Int, error) {
+	state, err := b.eth.blockchain.StateAt(header.Root)
+	if err != nil {
+		return nil, err
+	}
+	vmRunner := b.eth.BlockChain().NewEVMRunner(header, state)
+	return gpm.GetRealGasPriceMinimum(vmRunner, currencyAddress)
+}
+
 func (b *EthAPIBackend) SuggestPrice(ctx context.Context, currencyAddress *common.Address) (*big.Int, error) {
 	vmRunner, err := b.eth.BlockChain().NewEVMRunnerForCurrentBlock()
 	if err != nil {
@@ -334,6 +344,20 @@ func (b *EthAPIBackend) GetBlockGasLimit(ctx context.Context, blockNrOrHash rpc.
 	return blockchain_parameters.GetBlockGasLimitOrDefault(vmRunner)
 }
 
+func (b *EthAPIBackend) GetRealBlockGasLimit(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (uint64, error) {
+	statedb, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash)
+	if err != nil {
+		return 0, fmt.Errorf("LesApiBackend failed to retreive state for block gas limit for block %v: %w", blockNrOrHash, err)
+	}
+
+	caller := b.eth.BlockChain().NewEVMRunner(header, statedb)
+	limit, err := blockchain_parameters.GetBlockGasLimit(caller)
+	if err != nil {
+		return 0, fmt.Errorf("LesApiBackend failed to retreive block gas limit from blockchain parameters constract for block %v: %w", blockNrOrHash, err)
+	}
+	return limit, nil
+}
+
 func (b *EthAPIBackend) NewEVMRunner(header *types.Header, state vm.StateDB) vm.EVMRunner {
 	return b.eth.BlockChain().NewEVMRunner(header, state)
 }

internal/ethapi/api.go

@@ -749,21 +749,20 @@ func addEthCompatibilityFields(ctx context.Context, block map[string]interface{}
 	numhash := rpc.BlockNumberOrHash{
 		BlockHash: &hash,
 	}
-	gasLimit := b.GetBlockGasLimit(ctx, numhash)
-	block["gasLimit"] = hexutil.Uint64(gasLimit)
+	gasLimit, err := b.GetRealBlockGasLimit(ctx, numhash)
+	if err != nil {
+		log.Debug("Not adding gasLimit to RPC response, failed to retrieve it", "block", header.Number.Uint64(), "err", err)
+	} else {
+		block["gasLimit"] = hexutil.Uint64(gasLimit)
+	}
 
 	// Providing nil as the currency address gets the gas price minimum for the native celo asset.
-	baseFee, err := b.GasPriceMinimumForHeader(ctx, nil, header)
-
-	// GasPriceMinimumForHeader can return an error + non nil baseFee, an error
-	// and nil base fee or a base fee and a nil error. In all cases where a
-	// base fee is returned we want to add it as a field and if there was an
-	// error and nil base fee we log the failure.
-	if err != nil && baseFee == nil {
+	baseFee, err := b.RealGasPriceMinimumForHeader(ctx, nil, header)
+	if err != nil {
 		log.Debug("Not adding baseFeePerGas to RPC response, failed to retrieve gas price minimum", "block", header.Number.Uint64(), "err", err)
-		return
+	} else {
+		block["baseFeePerGas"] = (*hexutil.Big)(baseFee)
 	}
-	block["baseFeePerGas"] = (*hexutil.Big)(baseFee)
 }
 
 // GetUncleByBlockNumberAndIndex returns the uncle block for the given block hash and index. When fullTx is true

internal/ethapi/backend.go

@@ -44,6 +44,11 @@ type Backend interface {
 	SuggestGasTipCap(ctx context.Context, currencyAddress *common.Address) (*big.Int, error)
 	CurrentGasPriceMinimum(ctx context.Context, currencyAddress *common.Address) (*big.Int, error)
 	GasPriceMinimumForHeader(ctx context.Context, currencyAddress *common.Address, header *types.Header) (*big.Int, error)
+
+	// As opposed to GasPriceMinimumForHeader which returns a default value in
+	// some cases when it can't retrieve the gas price minimum, this function
+	// returns an error and no gas price minimum when it encounters a problem.
+	RealGasPriceMinimumForHeader(ctx context.Context, currencyAddress *common.Address, header *types.Header) (*big.Int, error)
 	SyncProgress() ethereum.SyncProgress
 
 	ChainDb() ethdb.Database
@@ -99,6 +104,11 @@ type Backend interface {
 	GatewayFee() *big.Int
 	GetIntrinsicGasForAlternativeFeeCurrency(ctx context.Context) uint64
 	GetBlockGasLimit(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) uint64
+
+	// As opposed to GetBlockGasLimit which returns a default value in the case
+	// that it can't retrieve the block gas limit, this function returns an
+	// error and no gas limit when it encounters a problem.
+	GetRealBlockGasLimit(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (uint64, error)
 	NewEVMRunner(*types.Header, vm.StateDB) vm.EVMRunner
 	Engine() consensus.Engine
 }