[wip] feat: embed TxEnvelope into rpc_types::Transaction
#1169
Conversation
| @@ -205,7 +204,7 @@ mod tests { | |||
| "type": "0x2", | |||
| "accessList": [], | |||
| "chainId": "0x1", | |||
| "v": "0x0", | |||
| "yParity": "0x0", | |||
There was a problem hiding this comment.
v on non-legacy transactions is deprecated, so that's a serialization change, deser should be fine
though we've decided to test this before merging
There was a problem hiding this comment.
bc7b4d5
to
3fb8b53
Compare
|
The only breaking change this PR adds is for serde roundrip for This might only result in incorrect payload for legacy transactions as for them
Which likely means that it is safe to expect I've changed test fixtures because they contained |
Motivation
Closes #1165
Solution
Envelopegeneric for rpc transaction which isconsensus::TxEnvelopeby default. ForAnyNetworkwe need a catch-all envelope, so I've extracted currentrpc_types_eth::Transactionwithout block-related fields intoAnyTxEnvelopetype.SerializeandDeserializeforTxType. It deserializes asOption<U8>withNonerepresenting legacy and serializes asU8.TxEnvelopeto use custom deserialize impl to handle case of missingtypefield as legacy tx.After this PR,
Transactionwill only implement arbitrary ifk256feature is enabled because of similar requirement onSignedAuthorization. I think this makes sense and believe we also want to add similar restriction forArbitraryimpl onSigned<T>which will only produce valid transactions.At this point we are maintaining 6 very similar serde schemes + rlp encoding implementations for different tx holding types (5 tx types +
AnyTxEnvelope). Wondering if we should add a macro for generating those so that serde attrs for e.g.gasare only defined once?PR Checklist