-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix: ethrpc: Add missing fields to EthTx #10223
Conversation
@@ -37,7 +37,8 @@ type EthTx struct { | |||
Gas EthUint64 `json:"gas"` | |||
MaxFeePerGas EthBigInt `json:"maxFeePerGas"` | |||
MaxPriorityFeePerGas EthBigInt `json:"maxPriorityFeePerGas"` | |||
V EthBigInt `json:"v"` | |||
AccessList []EthHash `json:"accessList"` | |||
V EthBigInt `json:"yParity"` |
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.
Afaict what we put here is in fact YParity according to https://github.com/ethereum/EIPs/blob/master/EIPS/eip-2098.md; V is only used for legacy transactions
This value seems to be set to 0/1; not 27/28 which are V values
Not sure if it makes sense to rename the V field to YParity
as well, no strong opinions.
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'd prefer to rename it to YParity to keep them consistent.
@@ -37,7 +37,8 @@ type EthTx struct { | |||
Gas EthUint64 `json:"gas"` | |||
MaxFeePerGas EthBigInt `json:"maxFeePerGas"` | |||
MaxPriorityFeePerGas EthBigInt `json:"maxPriorityFeePerGas"` | |||
V EthBigInt `json:"v"` | |||
AccessList []EthHash `json:"accessList"` |
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.
Those come from https://eips.ethereum.org/EIPS/eip-2930, which I don't think are supported in FEVM
Given that those aren't possible to specify (?), and optional, I just add the field and keep it empty, which is enough to keep openrpc happy.
0efd461
to
e321761
Compare
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.
Changes themselves look good to me, but review from @ychiaoli18 / @raulk / Aadi would be good
Related Issues
Closes #10106
Proposed Changes
Additional Info
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>
fix: mempool: Introduce a cache for valid signatures
PR type
: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, testarea
, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps