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

chore: initial EVM Execution Client Implementation #10

Merged
merged 44 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
a28c3ca
feat: evm client implementation
jim380 Oct 29, 2024
d192f94
chore: add docker setup for reth
jim380 Oct 30, 2024
adc0ad0
chore: gitignore jwt token
jim380 Oct 30, 2024
8d69c2e
feat: support json-rpc proxy client
jim380 Oct 30, 2024
594f3f9
refactor: separate proxy client from engine api client
jim380 Oct 30, 2024
e845c33
chore: naming changes
jim380 Oct 30, 2024
c7cfbef
chore: address initial PR review comments
jim380 Nov 5, 2024
d74ca6e
chore: address initial PR review comments batch II
jim380 Nov 5, 2024
207cbc3
refactor: remove redundant abstraction in the proxy layer
jim380 Nov 6, 2024
2c06ad7
chore: rm rollkit as a dependency
jim380 Nov 6, 2024
50b32f0
chore: refine client implementation and tests
jim380 Nov 7, 2024
8020a03
chore: update execution api interface name
jim380 Nov 8, 2024
037d5ae
chore: renaming go-execution types import path
jim380 Nov 8, 2024
e739975
refactor: move mocks to its own pkg
jim380 Nov 8, 2024
d222190
test: SetFinal unit test passing
jim380 Nov 8, 2024
ee613fc
chore: add ctx in executor methods
jim380 Nov 12, 2024
40257d9
feat: upgrade to engine api cancun
jim380 Nov 12, 2024
e0957ca
spin up local network for tests
ProgramCpp Nov 7, 2024
764240e
init reth db for the local test network
ProgramCpp Nov 7, 2024
6fc3ea5
add integration tests
ProgramCpp Nov 7, 2024
60623e8
merge changes - upgrade to cancun api
ProgramCpp Nov 8, 2024
463b9c5
merge base: upgrade to cancun api
ProgramCpp Nov 8, 2024
cab390b
programatically setup docker dependencies for integration tests
ProgramCpp Nov 10, 2024
aae31e7
fix integration test cleanup
ProgramCpp Nov 11, 2024
3f848ec
add integration test for GetTxs
ProgramCpp Nov 12, 2024
d9ca6fd
fix getTxs integration test to assert the tx in mempool
ProgramCpp Nov 13, 2024
9d6a016
Add integration tests for evm api's
ProgramCpp Nov 13, 2024
4577d1f
fix mandatory field validation for payload creation
ProgramCpp Nov 14, 2024
a456a09
send signed transaction to execution layer client
ProgramCpp Nov 14, 2024
a64658d
feat: add proper jwt auth in engine api calls
jim380 Nov 14, 2024
fff67cf
refactor: use more concrete types for building engine api payloads
jim380 Nov 15, 2024
00a5fa7
fix blockhash for block proposal
ProgramCpp Nov 16, 2024
fe309e1
upgrade reth version for integration tests
ProgramCpp Nov 18, 2024
ca25040
Merge branch 'jay/execution-api' into jni/1802
ProgramCpp Nov 18, 2024
c2ab7be
merge jay/execution-api
ProgramCpp Nov 19, 2024
9dbd197
fix initChain unit tests
ProgramCpp Nov 19, 2024
2aef2ca
fix executeTxs api unit tests
ProgramCpp Nov 19, 2024
2f5442e
fix initChain api integration tests
ProgramCpp Nov 19, 2024
f0431cc
fix reth setup for integration tests
ProgramCpp Nov 19, 2024
a66d9c7
fix genproto module dependency
ProgramCpp Nov 19, 2024
73cc0eb
downgrade go-ethereum
ProgramCpp Nov 19, 2024
4f6afe6
fix: block hash mismatch when executing txs
jim380 Nov 20, 2024
ef272c4
test: fix ExecuteTxs
jim380 Nov 20, 2024
bd037f4
chore: remove redundant debug prints
jim380 Nov 20, 2024
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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
docker/jwttoken/*
!docker/jwttoken/.gitkeep
15 changes: 15 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
// Use IntelliSense to learn about possible attributes.
// Hover to view descriptions of existing attributes.
// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
"version": "0.2.0",
"configurations": [
{
"name": "Launch Package",
"type": "go",
"request": "launch",
"mode": "auto",
"program": "${fileDirname}"
}
]
}
70 changes: 70 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
## Architecture

```mermaid
jim380 marked this conversation as resolved.
Show resolved Hide resolved
graph LR
subgraph Test Environment
TestClient[Test Client]
MockExecutor[Mock Executor]
end

subgraph Execution Client
EngineAPIExecutionClient
subgraph Client Components
EthClient[Eth Client]
JsonRpcClient[JSON-RPC Client]
end
end

subgraph Execution Layer
Reth[Reth Node]
subgraph Reth APIs
EngineAPI[Engine API]
JsonRPC[JSON-RPC API]
end
end

%% Test Environment Connections
TestClient -->|uses| EngineAPIExecutionClient
JsonRpcClient -->|test mode| MockExecutor

%% Execution Client Connections
EngineAPIExecutionClient -->|eth calls| EthClient
EngineAPIExecutionClient -->|engine calls| JsonRpcClient
EthClient -->|eth/net/web3| JsonRPC
JsonRpcClient -->|engine api| EngineAPI

%% Reth Internal Connections
JsonRPC -->|internal| Reth
EngineAPI -->|internal| Reth

%% Styling
classDef primary fill:#f9f,stroke:#333,stroke-width:2px
classDef secondary fill:#bbf,stroke:#333,stroke-width:1px
class EngineAPIExecutionClient primary
class EthClient,JsonRpcClient,MockExecutor,EngineAPI,JsonRPC secondary
```

The architecture consists of several key components:

1. **Execution Client**

- `EngineAPIExecutionClient`: Main client interface that implements the Execute interface
- `EthClient`: Handles standard Ethereum JSON-RPC calls
- `JsonRpcClient`: Handles Engine API calls

2. **Execution Layer**

- `Reth Node`: Ethereum execution client
- Exposes Engine API and standard JSON-RPC endpoints

3. **Test Environment**
- `Test Client`: Integration tests
- `Mock Executor`: Simulates execution behavior for unit tests

## Development

```bash
$ cd docker
$ docker compose up -d
$ docker compose down
```
Comment on lines +64 to +70
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance development setup documentation.

The development instructions need more context and details:

  1. Add prerequisites (Docker, Docker Compose)
  2. Explain what each command does
  3. Include information about JWT token setup
  4. Remove $ prefix from commands as per markdown best practices
 ## Development

+### Prerequisites
+
+- Docker
+- Docker Compose
+
+### Setup
+
+The following commands will set up the development environment:
+
 ```bash
-$ cd docker
-$ docker compose up -d
-$ docker compose down
+# Navigate to the Docker configuration directory
+cd docker
+
+# Start the services (Reth node and JWT initialization)
+docker compose up -d
+
+# Stop the services when done
+docker compose down

+The setup process includes:
+- Initializing a JWT token for secure communication
+- Starting a Reth node as the execution client


<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 Markdownlint</summary>

67-67: null
Dollar signs used before commands without showing output

(MD014, commands-show-output)

---

68-68: null
Dollar signs used before commands without showing output

(MD014, commands-show-output)

---

69-69: null
Dollar signs used before commands without showing output

(MD014, commands-show-output)

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit -->

33 changes: 33 additions & 0 deletions docker/chain/genesis.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
{
"config": {
"chainId": 1234,
"homesteadBlock": 0,
"eip150Block": 0,
"eip155Block": 0,
"eip158Block": 0,
"byzantiumBlock": 0,
"constantinopleBlock": 0,
"petersburgBlock": 0,
"istanbulBlock": 0,
"berlinBlock": 0,
"londonBlock": 0,
"shanghaiTime": 1677557088,
"cancunTime": 1710338100
},
"alloc": {
"0xd143C405751162d0F96bEE2eB5eb9C61882a736E": {
"balance": "0x4a47e3c12448f4ad000000"
},
"0x944fDcD1c868E3cC566C78023CcB38A32cDA836E": {
"balance": "0x4a47e3c12448f4ad000000"
}
},
"coinbase": "0x0000000000000000000000000000000000000000",
"difficulty": "0x20000",
"extraData": "",
"gasLimit": "0xf4240",
"nonce": "0x0000000000000042",
"mixhash": "0x2c85bcbce56429100b2108254bb56906257582aeafcbd682bc9af67a9f5aee46",
"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"timestamp": "0x00"
}
66 changes: 66 additions & 0 deletions docker/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
name: "reth"

services:
jwt-init:
container_name: jwt-init
image: alpine:3.19
volumes:
- ./jwttoken:/jwt
healthcheck:
test: ["CMD", "test", "-f", "/jwt/jwt.hex"]
interval: 5s
timeout: 5s
retries: 3
command: >
/bin/sh -c "mkdir -p /jwt &&
if [ ! -f /jwt/jwt.hex ]; then
apk add --no-cache openssl &&
openssl rand -hex 32 | tr -d '\n' > /jwt/jwt.hex;
fi"
Comment on lines +14 to +19
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Set secure file permissions for JWT token.

The JWT token file needs restricted permissions since it's used for authentication. Add chmod 600 to ensure only the owner can read/write the file.

Apply this fix:

    command: >
      /bin/sh -c "mkdir -p /jwt && 
      if [ ! -f /jwt/jwt.hex ]; then 
        apk add --no-cache openssl &&
-        openssl rand -hex 32 | tr -d '\n' > /jwt/jwt.hex; 
+        openssl rand -hex 32 | tr -d '\n' > /jwt/jwt.hex &&
+        chmod 600 /jwt/jwt.hex;
      fi"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
command: >
/bin/sh -c "mkdir -p /jwt &&
if [ ! -f /jwt/jwt.hex ]; then
apk add --no-cache openssl &&
openssl rand -hex 32 | tr -d '\n' > /jwt/jwt.hex;
fi"
command: >
/bin/sh -c "mkdir -p /jwt &&
if [ ! -f /jwt/jwt.hex ]; then
apk add --no-cache openssl &&
openssl rand -hex 32 | tr -d '\n' > /jwt/jwt.hex &&
chmod 600 /jwt/jwt.hex;
fi"
🧰 Tools
🪛 yamllint

[error] 15-15: trailing spaces

(trailing-spaces)


[error] 16-16: trailing spaces

(trailing-spaces)


[error] 18-18: trailing spaces

(trailing-spaces)

Comment on lines +4 to +19
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance JWT token security and maintenance

While the basic JWT setup is good, there are additional security considerations:

  1. Set directory permissions for /jwt
  2. Add cleanup for old tokens
  3. Fix trailing spaces in the command

Apply these improvements:

    command: >
-      /bin/sh -c "mkdir -p /jwt && 
-      if [ ! -f /jwt/jwt.hex ]; then 
+      /bin/sh -c "mkdir -p /jwt &&
+      chmod 700 /jwt &&
+      if [ ! -f /jwt/jwt.hex ]; then
+        rm -f /jwt/*.hex &&
         apk add --no-cache openssl &&
-        openssl rand -hex 32 | tr -d '\n' > /jwt/jwt.hex; 
+        openssl rand -hex 32 | tr -d '\n' > /jwt/jwt.hex &&
+        chmod 600 /jwt/jwt.hex;
       fi"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
jwt-init:
container_name: jwt-init
image: alpine:3.19
volumes:
- ./jwttoken:/jwt
healthcheck:
test: ["CMD", "test", "-f", "/jwt/jwt.hex"]
interval: 5s
timeout: 5s
retries: 3
command: >
/bin/sh -c "mkdir -p /jwt &&
if [ ! -f /jwt/jwt.hex ]; then
apk add --no-cache openssl &&
openssl rand -hex 32 | tr -d '\n' > /jwt/jwt.hex;
fi"
jwt-init:
container_name: jwt-init
image: alpine:3.19
volumes:
- ./jwttoken:/jwt
healthcheck:
test: ["CMD", "test", "-f", "/jwt/jwt.hex"]
interval: 5s
timeout: 5s
retries: 3
command: >
/bin/sh -c "mkdir -p /jwt &&
chmod 700 /jwt &&
if [ ! -f /jwt/jwt.hex ]; then
rm -f /jwt/*.hex &&
apk add --no-cache openssl &&
openssl rand -hex 32 | tr -d '\n' > /jwt/jwt.hex &&
chmod 600 /jwt/jwt.hex;
fi"
🧰 Tools
🪛 yamllint

[error] 15-15: trailing spaces

(trailing-spaces)


[error] 16-16: trailing spaces

(trailing-spaces)


[error] 18-18: trailing spaces

(trailing-spaces)

reth:
container_name: reth
restart: unless-stopped
image: ghcr.io/paradigmxyz/reth:v1.1.1
depends_on:
jwt-init:
condition: service_completed_successfully
ports:
- "9001:9001" # metrics
- "30303:30303" # eth/66 peering
- "8545:8545" # rpc
- "8551:8551" # engine
Comment on lines +28 to +32
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance network security configuration.

The current port exposure configuration could be more secure:

  1. Consider restricting metrics port (9001) to internal networks only
  2. Document the purpose and security implications of each exposed port
  3. Consider using network segmentation for different types of traffic

Apply these improvements:

    ports:
-      - "9001:9001" # metrics
-      - "30303:30303" # eth/66 peering
-      - "8545:8545" # rpc
-      - "8551:8551" # engine
+      # Internal metrics - restrict to internal network
+      - "127.0.0.1:9001:9001"
+      # P2P networking port
+      - "30303:30303/tcp"
+      - "30303:30303/udp"
+      # JSON-RPC API
+      - "127.0.0.1:8545:8545"
+      # Engine API - only needed for validator connections
+      - "127.0.0.1:8551:8551"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ports:
- "9001:9001" # metrics
- "30303:30303" # eth/66 peering
- "8545:8545" # rpc
- "8551:8551" # engine
ports:
# Internal metrics - restrict to internal network
- "127.0.0.1:9001:9001"
# P2P networking port
- "30303:30303/tcp"
- "30303:30303/udp"
# JSON-RPC API
- "127.0.0.1:8545:8545"
# Engine API - only needed for validator connections
- "127.0.0.1:8551:8551"

volumes:
- mainnet_data:/root/.local/share/reth/mainnet
- sepolia_data:/root/.local/share/reth/sepolia
- holesky_data:/root/.local/share/reth/holesky
- logs:/root/logs
- ./jwttoken:/root/jwt:ro
- ./chain:/root/chain:ro
pid: host
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove unnecessary host PID namespace access

The pid: host configuration grants excessive privileges to the container and is not required for running a Reth node.

Remove this line:

-    pid: host
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pid: host

entrypoint: /bin/sh -c
command:
- |
reth init --chain /root/chain/genesis.json
reth node \
--chain /root/chain/genesis.json \
--metrics 0.0.0.0:9001 \
--log.file.directory /root/logs \
--authrpc.addr 0.0.0.0 \
--authrpc.port 8551 \
--authrpc.jwtsecret /root/jwt/jwt.hex \
--http --http.addr 0.0.0.0 --http.port 8545 \
--http.api "eth,net,web3,txpool" \
Comment on lines +47 to +53
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Secure network configuration and API exposure

The current configuration exposes services with minimal restrictions:

  1. Metrics endpoint is exposed on all interfaces
  2. HTTP API is exposed on all interfaces
  3. AuthRPC is exposed on all interfaces

Apply these security improvements:

-          --metrics 0.0.0.0:9001 \
+          --metrics 127.0.0.1:9001 \
           --log.file.directory /root/logs \
-          --authrpc.addr 0.0.0.0 \
+          --authrpc.addr 127.0.0.1 \
           --authrpc.port 8551 \
           --authrpc.jwtsecret /root/jwt/jwt.hex \
-          --http --http.addr 0.0.0.0 --http.port 8545 \
+          --http --http.addr 127.0.0.1 --http.port 8545 \
           --http.api "eth,net,web3,txpool" \
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
--metrics 0.0.0.0:9001 \
--log.file.directory /root/logs \
--authrpc.addr 0.0.0.0 \
--authrpc.port 8551 \
--authrpc.jwtsecret /root/jwt/jwt.hex \
--http --http.addr 0.0.0.0 --http.port 8545 \
--http.api "eth,net,web3,txpool" \
--metrics 127.0.0.1:9001 \
--log.file.directory /root/logs \
--authrpc.addr 127.0.0.1 \
--authrpc.port 8551 \
--authrpc.jwtsecret /root/jwt/jwt.hex \
--http --http.addr 127.0.0.1 --http.port 8545 \
--http.api "eth,net,web3,txpool" \

--disable-discovery \
--debug.tip 0x8bf225d50da44f60dee1c4ee6f810fe5b44723c76ac765654b6692d50459f216 \
-vvvvv
Comment on lines +54 to +56
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Debug settings should be removed for production deployment

Based on the documentation and docker-compose configuration, this setup appears to be intended for development and testing purposes:

  1. The README explicitly shows this docker-compose setup under the "Development" section
  2. The configuration contains multiple development-focused settings:
    • Discovery disabled (--disable-discovery)
    • Hardcoded debug tip
    • Maximum verbosity (-vvvvv)
  3. The architecture documentation confirms the presence of a test environment

These debug settings should not be used in production as they could impact performance and security.

🔗 Analysis chain

Review debug configuration for production readiness

The current configuration includes development settings that should be reviewed:

  1. Discovery is disabled (--disable-discovery)
  2. Debug tip is hardcoded
  3. Maximum verbosity is set (-vvvvv)

Let's verify if this is the intended configuration:


The previous search was too narrow. Let's broaden our search to understand the context better by:

  1. Looking for any documentation about production settings
  2. Checking other docker-compose files that might exist
  3. Looking for patterns related to production, debug, or discovery settings in config files

This will help us determine if these settings are intentionally different between environments.


Let's search for any documentation that might explain the purpose of this configuration, particularly if this is meant for development/testing purposes. This will help us determine if these debug settings are intentionally set this way.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if this configuration matches development guidelines
rg -l "disable-discovery|debug.tip" --type md

Length of output: 47


Script:

#!/bin/bash
# Let's check for any documentation or configuration files that might indicate intended production settings
fd -e md -e yaml -e yml -e env -e conf -e config | xargs rg -i "production|debug|discovery" -C2

# Also check for other docker-compose files that might show different configurations
fd "docker-compose" | xargs cat

Length of output: 2311


Script:

#!/bin/bash
# Let's check if there are any environment-specific configurations or documentation
# that might explain these debug settings
fd -e md -e txt | xargs rg -i "reth|ethereum node|development environment|test environment" -C2

# Also check for any README files that might contain setup instructions
fd "README" | xargs cat

Length of output: 2806

volumes:
mainnet_data:
driver: local
sepolia_data:
driver: local
holesky_data:
driver: local
logs:
driver: local
Loading