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 6 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
docker/jwttoken/
80 changes: 80 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
## 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]
ProxyClient[JSON-RPC Proxy Client]
end
end

subgraph Proxy Layer
ProxyServer[JSON-RPC Proxy Server]
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
ProxyServer -->|delegates to| MockExecutor

%% Execution Client Connections
EngineAPIExecutionClient -->|eth calls| EthClient
EngineAPIExecutionClient -->|engine calls| ProxyClient
EthClient -->|eth/net/web3| JsonRPC
ProxyClient -->|forwards requests| ProxyServer

%% Proxy to Reth Connections
ProxyServer -->|authenticated requests| EngineAPI
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,ProxyServer primary
class EthClient,ProxyClient,MockExecutor,EngineAPI,JsonRPC secondary
```

The architecture consists of several key components:

1. **Execution Client**

- `EngineAPIExecutionClient`: Main client interface
- `EthClient`: Handles standard Ethereum JSON-RPC calls
- `ProxyClient`: Handles Engine API calls through the proxy

2. **Proxy Layer**

- `JSON-RPC Proxy Server`: Authenticates and forwards Engine API requests
- Handles JWT authentication with Reth

3. **Execution Layer**

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

4. **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 -->

54 changes: 54 additions & 0 deletions docker/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
name: "reth"

services:
jwt-init:
container_name: jwt-init
image: alpine
volumes:
- ./jwttoken:/jwt
command: >
/bin/sh -c "mkdir -p /jwt &&

Check failure on line 10 in docker/docker-compose.yml

View workflow job for this annotation

GitHub Actions / lint / yamllint

10:35 [trailing-spaces] trailing spaces
if [ ! -f /jwt/jwt.hex ]; then

Check failure on line 11 in docker/docker-compose.yml

View workflow job for this annotation

GitHub Actions / lint / yamllint

11:37 [trailing-spaces] trailing spaces
apk add --no-cache openssl &&
openssl rand -hex 32 | tr -d '\n' > /jwt/jwt.hex;

Check failure on line 13 in docker/docker-compose.yml

View workflow job for this annotation

GitHub Actions / lint / yamllint

13:58 [trailing-spaces] trailing spaces
fi"
jim380 marked this conversation as resolved.
Show resolved Hide resolved
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)


reth:
container_name: reth
restart: unless-stopped
image: ghcr.io/paradigmxyz/reth
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
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

command: >
node
--chain sepolia
--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"
jim380 marked this conversation as resolved.
Show resolved Hide resolved

volumes:
mainnet_data:
driver: local
sepolia_data:
driver: local
holesky_data:
driver: local
logs:
driver: local
50 changes: 50 additions & 0 deletions execution.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package execution

import (
"time"

execution "github.com/rollkit/go-execution"
rollkitTypes "github.com/rollkit/rollkit/types"
)

type EngineAPIExecutionClient struct {
execute execution.Execute
jim380 marked this conversation as resolved.
Show resolved Hide resolved
}

// NewEngineAPIExecutionClient creates a new instance of EngineAPIExecutionClient.
func NewEngineAPIExecutionClient(execute execution.Execute) (*EngineAPIExecutionClient, error) {
return &EngineAPIExecutionClient{
execute: execute,
}, nil
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider improving the constructor implementation.

The current implementation has a few areas for improvement:

  1. The constructor returns an error but never uses it
  2. The execute parameter should be validated for nil

Consider applying this change:

 func NewEngineAPIExecutionClient(execute execution.Execute) (*EngineAPIExecutionClient, error) {
+	if execute == nil {
+		return nil, fmt.Errorf("execute implementation is required")
+	}
 	return &EngineAPIExecutionClient{
 		execute: execute,
-	}, nil
+	}, nil
 }

Committable suggestion skipped: line range outside the PR's diff.


var _ execution.Execute = (*EngineAPIExecutionClient)(nil)

// InitChain initializes the blockchain with genesis information.
func (c *EngineAPIExecutionClient) InitChain(
genesisTime time.Time,
initialHeight uint64,
chainID string,
) (rollkitTypes.Hash, uint64, error) {
return c.execute.InitChain(genesisTime, initialHeight, chainID)
}

// GetTxs retrieves transactions from the transaction pool.
func (c *EngineAPIExecutionClient) GetTxs() ([]rollkitTypes.Tx, error) {
return c.execute.GetTxs()
}

// ExecuteTxs executes the given transactions and returns the new state root and gas used.
func (c *EngineAPIExecutionClient) ExecuteTxs(
txs []rollkitTypes.Tx,
blockHeight uint64,
timestamp time.Time,
prevStateRoot rollkitTypes.Hash,
) (rollkitTypes.Hash, uint64, error) {
return c.execute.ExecuteTxs(txs, blockHeight, timestamp, prevStateRoot)
}

// SetFinal marks a block at the given height as final.
func (c *EngineAPIExecutionClient) SetFinal(blockHeight uint64) error {
return c.execute.SetFinal(blockHeight)
}
162 changes: 162 additions & 0 deletions execution_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
package execution

import (
"net/http/httptest"
"os"
"testing"
"time"

"github.com/ethereum/go-ethereum/common"
"github.com/rollkit/go-execution-evm/proxy"
"github.com/rollkit/go-execution/mocks"
proxy_json_rpc "github.com/rollkit/go-execution/proxy/jsonrpc"
rollkitTypes "github.com/rollkit/rollkit/types"
"github.com/stretchr/testify/require"
)

const (
jwtSecretFile = "jwt.hex"
)
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Remove unused JWT constant

The jwtSecretFile constant is not being used in the codebase. While there is a jwtSecret field in the testEnv struct, it's only initialized as an empty string and the jwtSecretFile constant is never referenced. The constant can be safely removed.

🔗 Analysis chain

Verify if the JWT secret file constant is needed.

The constant jwtSecretFile is currently unused. If it's intended for future JWT authentication implementation, consider adding a TODO comment explaining its purpose or remove it if not needed.


Let me gather more context about the JWT usage in the codebase.


Let me check if this JWT-related code is used in any test cases or if there are any other files that might be using this constant.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for JWT-related code in the codebase
rg -l "jwt" --type go

Length of output: 39


Script:

#!/bin/bash
# Search for actual usage of JWT-related code
rg "jwt" -C 3 --type go

Length of output: 682


Script:

#!/bin/bash
# Look for test cases or other files that might use the JWT functionality
rg -B 5 -A 5 "jwtSecret|jwtSecretFile" --type go

# Also check for any Engine API related JWT authentication patterns
ast-grep --pattern 'func TestEngine$$$($$$) {
  $$$
}'

Length of output: 1119

🧰 Tools
🪛 golangci-lint

17-17: const jwtSecretFile is unused

(unused)


type testEnv struct {
server *httptest.Server
jwtSecret string
cleanup func()
client *EngineAPIExecutionClient
proxyConf *proxy_json_rpc.Config
mockExec *mocks.MockExecute
}

func setupTestEnv(t *testing.T) *testEnv {
t.Helper()

tmpDir, err := os.MkdirTemp("", "execution-test-*")
require.NoError(t, err)

cleanup := func() {
os.RemoveAll(tmpDir)
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle cleanup errors.

The cleanup function should handle potential errors from os.RemoveAll.

 cleanup := func() {
-    os.RemoveAll(tmpDir)
+    if err := os.RemoveAll(tmpDir); err != nil {
+        t.Errorf("failed to cleanup test directory: %v", err)
+    }
 }
📝 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
cleanup := func() {
os.RemoveAll(tmpDir)
}
cleanup := func() {
if err := os.RemoveAll(tmpDir); err != nil {
t.Errorf("failed to cleanup test directory: %v", err)
}
}
🧰 Tools
🪛 golangci-lint

36-36: Error return value of os.RemoveAll is not checked

(errcheck)


// setup a proxy config
proxyConf := &proxy_json_rpc.Config{
DefaultTimeout: 5 * time.Second,
MaxRequestSize: 1024 * 1024,
}

// create a mock execute from mocks package
mockExec := mocks.NewMockExecute(t)

// create a proxy server with mock execute
server := proxy_json_rpc.NewServer(mockExec, proxyConf)
testServer := httptest.NewServer(server)

// create a proxy client that implements the Execute interface
ethURL := "http://localhost:8545"
engineURL := "http://localhost:8551"
genesisHash := common.HexToHash("0x0")
feeRecipient := common.HexToAddress("0x0")

proxyClient, err := proxy.NewClient(proxyConf, ethURL, engineURL, genesisHash, feeRecipient)
require.NoError(t, err)

err = proxyClient.Start(testServer.URL)
require.NoError(t, err)

// create an execution client with the proxy client
client, err := NewEngineAPIExecutionClient(proxyClient)
require.NoError(t, err)

return &testEnv{
server: testServer,
jwtSecret: "",
cleanup: func() {
cleanup()
testServer.Close()
proxyClient.Stop()
},
client: client,
proxyConf: proxyConf,
mockExec: mockExec,
}
}

func TestEngineAPIExecutionClient_InitChain(t *testing.T) {
env := setupTestEnv(t)
defer env.cleanup()

genesisTime := time.Now().UTC().Truncate(time.Second)
initialHeight := uint64(0)
chainID := "11155111" // sepolia chain id

expectedStateRoot := rollkitTypes.Hash{}
copy(expectedStateRoot[:], []byte{1, 2, 3})
expectedGasLimit := uint64(1000000)

env.mockExec.On("InitChain", genesisTime, initialHeight, chainID).
Return(expectedStateRoot, expectedGasLimit, nil)

stateRoot, gasLimit, err := env.client.InitChain(genesisTime, initialHeight, chainID)
require.NoError(t, err)
require.Equal(t, expectedStateRoot, stateRoot)
require.Equal(t, expectedGasLimit, gasLimit)

env.mockExec.AssertExpectations(t)
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add negative test cases for InitChain.

The test only covers the happy path. Consider adding test cases for:

  • Invalid chain ID
  • Zero genesis time
  • Mock returning an error

Also, consider making the chain ID a named constant:

-	chainID := "11155111" // sepolia chain id
+	const (
+		sepoliaChainID = "11155111"
+	)
+	chainID := sepoliaChainID

Committable suggestion skipped: line range outside the PR's diff.


func TestEngineAPIExecutionClient_GetTxs(t *testing.T) {
env := setupTestEnv(t)
defer env.cleanup()

expectedTxs := []rollkitTypes.Tx{[]byte("tx1"), []byte("tx2")}
env.mockExec.On("GetTxs").Return(expectedTxs, nil)

txs, err := env.client.GetTxs()
require.NoError(t, err)
require.Equal(t, expectedTxs, txs)

env.mockExec.AssertExpectations(t)
}

func TestEngineAPIExecutionClient_ExecuteTxs(t *testing.T) {
env := setupTestEnv(t)
defer env.cleanup()

blockHeight := uint64(1)
timestamp := time.Now().UTC().Truncate(time.Second)
prevStateRoot := rollkitTypes.Hash{}
copy(prevStateRoot[:], []byte{1, 2, 3})
testTx := rollkitTypes.Tx("test transaction")

expectedStateRoot := rollkitTypes.Hash{}
copy(expectedStateRoot[:], []byte{4, 5, 6})
expectedGasUsed := uint64(21000)

env.mockExec.On("ExecuteTxs", []rollkitTypes.Tx{testTx}, blockHeight, timestamp, prevStateRoot).
Return(expectedStateRoot, expectedGasUsed, nil)

stateRoot, gasUsed, err := env.client.ExecuteTxs(
[]rollkitTypes.Tx{testTx},
blockHeight,
timestamp,
prevStateRoot,
)
require.NoError(t, err)
require.Equal(t, expectedStateRoot, stateRoot)
require.Equal(t, expectedGasUsed, gasUsed)

env.mockExec.AssertExpectations(t)
}
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 ExecuteTxs test coverage.

The test should include additional scenarios:

  • Empty transaction list
  • Invalid previous state root
  • Multiple transactions
  • Large transactions that might hit size limits
  • Transactions that would exceed block gas limit

Also, consider making the gas values constants:

+	const (
+		standardTxGas = uint64(21000)
+	)
-	expectedGasUsed := uint64(21000)
+	expectedGasUsed := standardTxGas

Committable suggestion skipped: line range outside the PR's diff.


func TestEngineAPIExecutionClient_SetFinal(t *testing.T) {
env := setupTestEnv(t)
defer env.cleanup()

blockHeight := uint64(1)

env.mockExec.On("SetFinal", blockHeight).Return(nil)

err := env.client.SetFinal(blockHeight)
require.NoError(t, err)

env.mockExec.AssertExpectations(t)
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error scenarios to SetFinal test.

The test only covers the happy path. Consider adding test cases for:

  • Mock returning an error
  • Invalid block height (e.g., 0 or max uint64)

Loading
Loading