-
Notifications
You must be signed in to change notification settings - Fork 91
test(jsonrpc): add jsonrpc compatibility test #419
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
Conversation
// Engine API namespace (not applicable for Cosmos chains) | ||
MethodNameEngineNewPayloadV1 types.RpcName = "engine_newPayloadV1" | ||
MethodNameEngineNewPayloadV2 types.RpcName = "engine_newPayloadV2" | ||
MethodNameEngineNewPayloadV3 types.RpcName = "engine_newPayloadV3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn’t find any existing tests for this API.
MethodNameEngineForkchoiceUpdatedV3 types.RpcName = "engine_forkchoiceUpdatedV3" | ||
MethodNameEngineGetPayloadV1 types.RpcName = "engine_getPayloadV1" | ||
MethodNameEngineGetPayloadV2 types.RpcName = "engine_getPayloadV2" | ||
MethodNameEngineGetPayloadV3 types.RpcName = "engine_getPayloadV3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn’t find any existing tests for this API.
MethodNameEngineNewPayloadV3 types.RpcName = "engine_newPayloadV3" | ||
MethodNameEngineForkchoiceUpdatedV1 types.RpcName = "engine_forkchoiceUpdatedV1" | ||
MethodNameEngineForkchoiceUpdatedV2 types.RpcName = "engine_forkchoiceUpdatedV2" | ||
MethodNameEngineForkchoiceUpdatedV3 types.RpcName = "engine_forkchoiceUpdatedV3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn’t find any existing tests for this API.
) | ||
|
||
const ( | ||
NamespaceDebug = "debug" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that for the debug namespace, this PR doesn’t compare results from Geth and from evmd.
To ensure JSON-RPC compatibility, every test should treat Geth’s result as the source of truth and verify whether evmd matches it or if anything is missing.
If that’s the case, could you create a follow-up issue for this? We should clarify the test methodology used in this PR and the exact coverage scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we have two start scripts serving similar purposes but in different contexts:
container-start-evmd.sh
: runs inside the container (no Docker CLI), handles init/genesis tweaks, then evmd start.start-evmd.sh
: runs on the host, manages Docker lifecycle, port exposure, and applies additional genesis/app.toml tweaks (prometheus, pruning, static precompiles, STRv2 pair, etc.).
To avoid drift and reduce maintenance, could we consolidate shared logic into a single container-side bootstrap (e.g., scripts/evmd/bootstrap.sh
) controlled via env vars (ENABLE_PROM
, ENABLE_STRV2
, RPC_BIND_ALL
, BLOCK_MAX_GAS
, …)?
The host script would then be a thin wrapper that only runs Docker and passes env/ports/volumes
.
If consolidation is out of scope here, can we at least document the intentional differences and extract the common init (keys/genesis/params) into a helper that both call?
MethodNameEthUnsubscribe types.RpcName = "eth_unsubscribe" | ||
) | ||
|
||
func EthCoinbase(rCtx *types.RPCContext) (*types.RpcResult, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be removed after #456 is merged, since even Geth no longer supports this API. Other legacy APIs should also be removed — something we should keep in mind going forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zsystm good to remove now then?
@cloudgray let's not commit a binary into the codebase. Can we just build this binary instead in the process? |
My bad. I removed. (a32dfe6) |
go 1.23.8 | ||
|
||
require ( | ||
github.com/ethereum/go-ethereum v1.15.10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to update the go-ethereum dep to the latest
@aljo242 Oh, this PR was merged just after added to merge queue, before I modify code from code-review comment. |
Description
Summary
This PR implements a comprehensive JSON-RPC compatibility test suite that validates API compatibility between our evmd implementation and standard Ethereum clients (geth). The test suite addresses issue #101 by providing automated testing infrastructure to ensure our JSON-RPC endpoints behave consistently with Ethereum standards.
Testing
Quick Start
Manual Testing
Start individual components
Configuration
Environment Variables
Test Accounts
Pre-configured test accounts with known private keys for deterministic testing:
Closes: #101
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
main
branch