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

Improve RPC API compatibility for Ethers.js #1957

Merged
merged 31 commits into from
Sep 27, 2022
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
9e3fc5d
Add minimal ts project
piersy Sep 9, 2022
9ebdd19
Add ethers.js dependency
piersy Sep 9, 2022
11fa86e
Add etherejs api test (currently failing)
piersy Sep 9, 2022
70232a0
Add dummy data for gas limit field
piersy Sep 9, 2022
33de2a5
Build ethersjs project before executing test
piersy Sep 9, 2022
8e2c1c9
Return real gas limit in place of dummy data
piersy Sep 12, 2022
aec0159
Return baseFee if it is acessible
piersy Sep 12, 2022
84bd94c
Use framework for ethers.js compatibility tests
piersy Sep 13, 2022
c624df4
Store package lock
piersy Sep 14, 2022
9d175c1
Add make rule to install ethersjs project deps
piersy Sep 14, 2022
cb1479c
Update ci to prepare ehtersjs project
piersy Sep 14, 2022
bd71fd1
Add dockerfile for a node/golang CI executor
piersy Sep 14, 2022
9895733
Run e2e coverage CI tests verbosely
piersy Sep 14, 2022
45e386b
Add debug
piersy Sep 14, 2022
dd17619
Add more debug
piersy Sep 14, 2022
a0174f4
Change address format
piersy Sep 15, 2022
2bdc31e
Update log message for clarity
piersy Sep 21, 2022
b354bc4
Update node-golang dockerfile
piersy Sep 21, 2022
ccd6e59
Merge branch 'master' into piersy/ethersjs-compatibility
piersy Sep 21, 2022
5fd28ff
Update readme to specify a minimum go version
piersy Sep 22, 2022
e4e0216
Default to single quotes in typescript
piersy Sep 22, 2022
80d55aa
Merge branch 'master' into piersy/ethersjs-compatibility
piersy Sep 22, 2022
0195bec
Flag to disable RPC compatibilty fields
piersy Sep 26, 2022
16dc1a7
Merge remote-tracking branch 'origin/master' into piersy/ethersjs-com…
piersy Sep 27, 2022
99b9750
Split the tests under 2 describe headings
piersy Sep 27, 2022
0fa5309
Document that js tests shoudn't be run standalone
piersy Sep 27, 2022
af4224f
Move ethersjs test project under e2e_test
piersy Sep 27, 2022
419c504
Do not return block fields that cant be retreived
piersy Sep 27, 2022
986b06c
Fix typos
hbandura Sep 27, 2022
f8dc1ed
Remove unused code
hbandura Sep 27, 2022
8f3e4f6
Merge branch 'master' into piersy/ethersjs-compatibility
gastonponti Sep 27, 2022
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
7 changes: 5 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ parameters:
executors:
golang:
docker:
- image: circleci/golang:1.16
- image: "us.gcr.io/celo-testnet/circleci-node12-golang1.17.5"
working_directory: ~/repos/geth
environment:
# Change the go modules to be cached under ~/repos so that we can add
Expand Down Expand Up @@ -309,6 +309,7 @@ jobs:
- attach_workspace:
at: ~/repos
- *restore-go-mod-cache
- run: make prepare-ethersjs-project
- run: go get github.com/jstemmer/go-junit-report
- run:
name: Run tests
Expand All @@ -330,6 +331,7 @@ jobs:
- attach_workspace:
at: ~/repos
- *restore-go-mod-cache
- run: make prepare-ethersjs-project
- run: go get github.com/jstemmer/go-junit-report
- run:
name: Run tests
Expand All @@ -351,11 +353,12 @@ jobs:
- attach_workspace:
at: ~/repos
- *restore-go-mod-cache
- run: make prepare-ethersjs-project
# Run the tests with coverage parse the coverage and output the summary
- run:
name: Run tests and print coverage summary
command: |
go test -coverprofile cov.out -coverpkg ./consensus/istanbul/... ./e2e_test
go test -v -coverprofile cov.out -coverpkg ./consensus/istanbul/... ./e2e_test
palango marked this conversation as resolved.
Show resolved Hide resolved
go run tools/parsecov/main.go -packagePrefix github.com/celo-org/celo-blockchain/ cov.out > summary
cat summary

Expand Down
7 changes: 7 additions & 0 deletions .circleci/node-golang-dockerfile/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This has been pushed to us.gcr.io/celo-testnet/circleci-node12-golang1.17.5
FROM circleci/node:12

COPY --from=circleci/golang:1.17.5 /usr/local/go/ /usr/local/go/

ENV PATH="/home/circleci/go/bin:${PATH}"
ENV PATH="/usr/local/go/bin:${PATH}"
9 changes: 8 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
.PHONY: geth-linux-arm geth-linux-arm-5 geth-linux-arm-6 geth-linux-arm-7 geth-linux-arm64
.PHONY: geth-darwin geth-darwin-amd64
.PHONY: geth-windows geth-windows-386 geth-windows-amd64
.PHONY: prepare-system-contracts
.PHONY: prepare prepare-system-contracts prepare-ethersjs-project

GOBIN = ./build/bin
GO ?= latest
Expand Down Expand Up @@ -45,6 +45,13 @@ geth:
@echo "Done building."
@echo "Run \"$(GOBIN)/geth\" to launch geth."

prepare: prepare-system-contracts prepare-ethersjs-project

prepare-ethersjs-project: ./e2e_test/ethersjs-api-check/node_modules

./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
# compiled contracts from the monorepo to the compiled-system-contracts, so
Expand Down
13 changes: 9 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Most functionality of this client is similar to `go-ethereum`, also known as `ge

## Building the source

Building `geth` requires both a Go (version 1.16) and a C compiler.
Building `geth` requires both Go (min version 1.15) and a C compiler.
You can install them using your favourite package manager. Once the dependencies are installed, run

```shell
Expand Down Expand Up @@ -58,7 +58,12 @@ The Celo blockchain client comes with several wrappers/executables found in the

## Running tests

Prior to running tests you will need to run `make prepare-system-contracts`.
Prior to running tests you will need to run `make prepare`, this will run two sub rules.

Without first running this `make prepare`, certain tests will fail.

### prepare-system-contracts

This will shallow checkout the
[celo-monorepo](https://github.com/celo-org/celo-monorepo) under
`../.celo-blockchain-monorepo-checkout` relative to this project's root at the
Expand Down Expand Up @@ -86,8 +91,8 @@ those checked out by the `prepare-system-contracts` rule, and the checkouts
created by the `prepare-system-contracts` rule should not be manually modifed,
aside from changing the contract source.


Without first running this make rule, certain tests will fail.
### prepare-ethersjs-project
This will install dependencies for the `ethersjs-api-check` typescript project.

## Running Celo

Expand Down
1 change: 1 addition & 0 deletions cmd/geth/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ var (
}

rpcFlags = []cli.Flag{
utils.DisableRPCETHCompatibility,
utils.HTTPEnabledFlag,
utils.HTTPListenAddrFlag,
utils.HTTPPortFlag,
Expand Down
9 changes: 9 additions & 0 deletions cmd/utils/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,10 @@ var (
Usage: "Disables db compaction after import",
}
// RPC settings
DisableRPCETHCompatibility = cli.BoolFlag{
Name: "disablerpcethcompatibility",
Usage: "If set, blocks returned from the RPC api will not contain the 'gasLimit' and 'baseFeePerGas' fields, which were added to the RPC block representation in order to improve compatibility with ethereum tooling. Note these fields do not currently exist on the internal block representation so they should be disregarded for the purposes of hashing or signing",
}

IPCDisabledFlag = cli.BoolFlag{
Name: "ipcdisable",
Expand Down Expand Up @@ -1768,6 +1772,11 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
cfg.RPCTxFeeCap = ctx.GlobalFloat64(RPCGlobalTxFeeCapFlag.Name)
}

cfg.RPCEthCompatibility = true
if ctx.GlobalIsSet(DisableRPCETHCompatibility.Name) {
cfg.RPCEthCompatibility = false
}

// Disable DNS discovery by default (by using the flag's value even if it hasn't been set and so
// has the default value ""), since we don't have DNS discovery set up for Celo.
// Note that passing --discovery.dns "" is the way the Geth docs specify for disabling DNS discovery,
Expand Down
6 changes: 3 additions & 3 deletions contracts/blockchain_parameters/blockchain_parameters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions contracts/blockchain_parameters/blockchain_parameters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)))
})
Expand Down
27 changes: 27 additions & 0 deletions contracts/gasprice_minimum/gasprice_minimum.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package gasprice_minimum

import (
"fmt"
"math/big"

"github.com/celo-org/celo-blockchain/common"
Expand Down Expand Up @@ -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

Expand Down
94 changes: 94 additions & 0 deletions e2e_test/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"fmt"
"math/big"
"os"
"os/exec"
"strings"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -463,3 +465,95 @@ func pruneStateOfBlock(ctx context.Context, node *test.Node, blockHash common.Ha

return nil
}

func TestEthersJSCompatibility(t *testing.T) {
ac := test.AccountConfig(1, 1)
gc, ec, err := test.BuildConfig(ac)
require.NoError(t, err)
network, shutdown, err := test.NewNetwork(ac, gc, ec)
require.NoError(t, err)
defer shutdown()

ctx, cancel := context.WithTimeout(context.Background(), time.Second*20)
defer cancel()

num, err := network[0].WsClient.BlockNumber(ctx)
require.NoError(t, err)

// Execute typescript tests to check ethers.js compatibility.
//
// The '--networkaddr' and '--blocknum' flags are npm config variables, the
// values become available under 'process.env.npm_config_networkaddr' and
// 'process.env.npm_config_blocknum' in typescript test. Everything after
// '--' are flags that are passed to mocha and these flags are controlling
// which tests to run.

// 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(), "--", "--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))
require.NoError(t, err)

err = network[0].Tracker.AwaitBlock(ctx, num+1)
require.NoError(t, err)
block := network[0].Tracker.GetProcessedBlock(num)

// Prune state
err = pruneStateOfBlock(ctx, network[0], block.Hash())
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", "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))
require.NoError(t, err)
}

// This test checks the functionality of the configuration to enable/disable
// returning the 'gasLimit' and 'baseFeePerGas' fields on RPC blocks.
func TestEthersJSCompatibilityDisable(t *testing.T) {
ac := test.AccountConfig(1, 1)
gc, ec, err := test.BuildConfig(ac)
require.NoError(t, err)

// Check fields present (compatibility set by default)
network, shutdown, err := test.NewNetwork(ac, gc, ec)
require.NoError(t, err)
defer shutdown()

ctx, cancel := context.WithTimeout(context.Background(), time.Second*20)
defer cancel()

result := make(map[string]interface{})
err = network[0].WsClient.GetRPCClient().CallContext(ctx, &result, "eth_getBlockByNumber", "latest", true)
require.NoError(t, err)

_, ok := result["gasLimit"]
assert.True(t, ok, "gasLimit field should be present on RPC block")
_, ok = result["baseFeePerGas"]
assert.True(t, ok, "baseFeePerGas field should be present on RPC block")

// Turn of compatibility and check fields are not present
ec.RPCEthCompatibility = false
network, shutdown, err = test.NewNetwork(ac, gc, ec)
require.NoError(t, err)
defer shutdown()

ctx, cancel = context.WithTimeout(context.Background(), time.Second*20)
defer cancel()

result = make(map[string]interface{})
err = network[0].WsClient.GetRPCClient().CallContext(ctx, &result, "eth_getBlockByNumber", "latest", true)
require.NoError(t, err)

_, ok = result["gasLimit"]
assert.False(t, ok, "gasLimit field should not be present on RPC block")
_, ok = result["baseFeePerGas"]
assert.False(t, ok, "baseFeePerGas field should not be present on RPC block")
}
3 changes: 3 additions & 0 deletions e2e_test/ethersjs-api-check/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
node_modules
dist

Loading