ref impl: Engine API client bindings #127
Conversation
| type ErrorCode int | ||
|
|
||
| const ( | ||
| UnavailablePayload ErrorCode = -32001 |
There was a problem hiding this comment.
Does this value come from somewhere?
There was a problem hiding this comment.
Yes, it's defined in the execution APIs standard. Not my idea to use large seemingly random negative numbers.
There was a problem hiding this comment.
Who comes up with these numbers? 😅 Also my bad, I realize I did a ctrl+F in the wrong document.
| return hexutil.Encode(b[:]) | ||
| } | ||
|
|
||
| type Uint64Quantity = hexutil.Uint64 |
There was a problem hiding this comment.
Why not use uint64 directly here?
I do understand that hexutil defines type Uint64 uint64 in order to be able to define methods on this new type (not allowed on primitive types), but shouldn't box this type inside the methods of this file, and expose the simpler uint64 in fields of ExecutionPayload?
Ditto for Data lower in this file.
There was a problem hiding this comment.
This was an attempt to follow spec definitions with "quantity"/"data" type separation more closely. Since payload ID was uint64 but encoded as data instead of quantity. Once the upstream PR with payload-id type lands I can clean this up.
|
|
||
| type Data = hexutil.Bytes | ||
|
|
||
| // TODO: implement neat 8 byte typed payload ID and upstream it to geth api definitions |
There was a problem hiding this comment.
Is this is a significant effort, could we do it now?
There was a problem hiding this comment.
This is part of what I already did in the PR to upstream geth (see PR status update in slack). Can merge this as-is and upgrade later.
Not yet, the execution apis repo has json schemas, but not for the engine API, yet. Maybe we can share testing with consensus-clients later on. |
protolambda
force-pushed
the
impl-engine-api
branch
from
005dae0
to
89a27f7
Compare
Codecov Report
@@ Coverage Diff @@
## main #127 +/- ##
=======================================
Coverage 77.20% 77.20%
=======================================
Files 1 1
Lines 136 136
=======================================
Hits 105 105
Misses 24 24
Partials 7 7 Continue to review full report at Codecov.
|
|
Rebased on |
Part of #119: staging -> main migration
This:
Bytes32andBytes256for stricter API decodingtransactionsfield in payload request attributes)el.Loginstead of global geth log)Depends on #125
Review: any team. No spec changes required, implements existing engine API spec.