-
Notifications
You must be signed in to change notification settings - Fork 18
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
Use hivechain to generate test chain #27
Conversation
Damn, I based this on my outdated fork. This rebase is going to suck. |
Most tests work. Just 6 more to fix. |
This should be ready now. |
These were already removed in ethereum/execution-apis#294.
These tests were added to manually in ethereum/execution-apis#305, so they would be lost if tests are regenerated from scratch.
I've merged the cancun PR into this one in order to be able to regenerate all tests - execution-apis already had these tests included. |
This PR regenerates the tests with lightclient/rpctestgen#27.
Tests update with this PR: https://github.com/ethereum/execution-apis/pull/513/files |
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.
Great work overall, just a few questions / comments
Co-authored-by: lightclient <[email protected]>
Co-authored-by: lightclient <[email protected]>
Could you please take a look at this patch and apply if you agree? It should resolve all the diff --git a/cmd/speccheck/check.go b/cmd/speccheck/check.go
index 438cb28..586442f 100644
--- a/cmd/speccheck/check.go
+++ b/cmd/speccheck/check.go
@@ -37,14 +37,18 @@ func checkSpec(methods map[string]*methodSchema, rts []*roundTrip, re *regexp.Re
return fmt.Errorf("missing required parameter %s.param[%d]", rt.method, i)
}
if err := validate(&method.params[i].schema, rt.params[i], fmt.Sprintf("%s.param[%d]", rt.method, i)); err != nil {
- return fmt.Errorf("unable to validate parameter: %s", err)
+ return fmt.Errorf("unable to validate parameter in %s: %s", rt.name, err)
}
}
- if err := validate(&method.result.schema, rt.response, fmt.Sprintf("%s.result", rt.method)); err != nil {
+ if rt.response.Result == nil && rt.response.Error != nil {
+ // skip validation of errors, they haven't been standardized
+ continue
+ }
+ if err := validate(&method.result.schema, rt.response.Result, fmt.Sprintf("%s.result", rt.method)); err != nil {
// Print out the value and schema if there is an error to further debug.
buf, _ := json.Marshal(method.result.schema)
fmt.Println(string(buf))
- fmt.Println(string(rt.response))
+ fmt.Println(string(rt.response.Result))
fmt.Println()
return fmt.Errorf("invalid result %s\n%#v", rt.name, err)
}
diff --git a/cmd/speccheck/roundtrips.go b/cmd/speccheck/roundtrips.go
index 57f1d39..eb7b473 100644
--- a/cmd/speccheck/roundtrips.go
+++ b/cmd/speccheck/roundtrips.go
@@ -30,7 +30,7 @@ type roundTrip struct {
method string
name string
params [][]byte
- response []byte
+ response *jsonrpcMessage
}
// readRtts walks a root directory and parses round trip HTTP exchanges
@@ -99,7 +99,7 @@ func readTest(testname string, filename string) ([]*roundTrip, error) {
if err != nil {
return nil, fmt.Errorf("unable to parse params: %s %v", err, req.Params)
}
- rts = append(rts, &roundTrip{req.Method, testname, params, resp.Result})
+ rts = append(rts, &roundTrip{req.Method, testname, params, &resp})
req = nil
default:
return nil, fmt.Errorf("invalid line in test: %s", line)
diff --git a/testgen/generators.go b/testgen/generators.go
index 13df56e..374f6a7 100644
--- a/testgen/generators.go
+++ b/testgen/generators.go
@@ -60,8 +60,6 @@ var AllMethods = []MethodTests{
EthBlockNumber,
EthGetBlockByNumber,
EthGetBlockByHash,
- EthGetHeaderByNumber,
- EthGetHeaderByHash,
EthGetProof,
EthChainID,
EthGetBalance,
@@ -87,6 +85,10 @@ var AllMethods = []MethodTests{
DebugGetRawReceipts,
DebugGetRawTransaction,
+ // -- get header by xxx APIs are not currently in the standard spec
+ // EthGetHeaderByNumber,
+ // EthGetHeaderByHash,
+
// -- gas price tests are disabled because of non-determinism
// EthGasPrice,
// EthMaxPriorityFeePerGas,
@@ -688,7 +690,7 @@ var EthCreateAccessList = MethodTests{
"from": sender,
"to": emitContract,
"nonce": hexutil.Uint64(nonce),
- "gasLimit": hexutil.Uint64(60000),
+ "gas": hexutil.Uint64(60000),
"gasPrice": (*hexutil.Big)(gasprice),
"input": "0x010203040506",
}
@@ -722,7 +724,7 @@ This invocation uses EIP-1559 fields to specify the gas price.`,
"from": sender,
"to": emitContract,
"nonce": hexutil.Uint64(nonce),
- "gasLimit": hexutil.Uint64(60000),
+ "gas": hexutil.Uint64(60000),
"maxFeePerGas": (*hexutil.Big)(gasprice),
"maxPriorityFeePerGas": (*hexutil.Big)(big.NewInt(3)),
"input": "0x010203040506", |
I have applied the patch. |
This PR regenerates the tests with lightclient/rpctestgen#27.
Thanks so much! |
This PR regenerates the tests with lightclient/rpctestgen#27.
In this PR, I'm trying to remove the chain generator. We have a dedicated infrastructure to create an 'interesting' test chain with hivechain, and it's good to reuse. The chain generation code is also messy to maintain because it depends on go-ethereum core very deeply. Best to deal with this in one place only.