Correctly encode non-legacy mempool transactions #415
Conversation
If a non-legacy transaction was fetched from the mempool and then RLP-encoded for inclusion in e.g. a flashbots bundle, then the bundle would fail because the encoding only worked for legacy transactions.
|
Not working yet. Please take code from here: @onbjerg might do a PR soon. |
Yeah, I have some tests locally and some fixes too, but I want to add one more before pushing. Thanks for the PR on my fork, I'll take a look today :) |
|
Should be good to go @gakonst |
| encoded.extend_from_slice(rlp_bytes.as_ref()); | ||
| encoded.into() | ||
| } | ||
| _ => rlp_bytes, |
There was a problem hiding this comment.
WDYT about matching Some(x) if x == U64::from(0) and encoding with the 0x0 prefix, and default to rlp_bytes only when self.transaction_type is not present OR if the legacy feature is turned on?
There was a problem hiding this comment.
AFAIK 0x0 is reserved for new tx types, not legacy: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-2718.md#backwards-compatibility
There was a problem hiding this comment.
NVM. You are right, forgot that we skip it for legacy txs:
ethers-rs/ethers-core/src/types/transaction/eip2718.rs
Lines 153 to 155 in 404e9c2
Motivation
If a non-legacy transaction was fetched from the mempool and then RLP-encoded for inclusion in e.g. a flashbots bundle, then the bundle would fail because the encoding only worked for legacy transactions. See onbjerg/ethers-flashbots#11 (comment)
Solution
RLP encode
Transactionresponse types based on theirtransaction_type. I'm not super confident about this piece of code since it is a lot of duplicate logic from other files, but I'm not super well known in the codebase yet so I'm not sure what the best way is to improve it. Please let me know if you have any suggestions!