FIX: Accept 'input' or 'data', (but not both), in transaction request #2697
Conversation
| skip_serializing_if = "Option::is_none", | ||
| deserialize_with = "deserialize_input_or_data", | ||
| serialize_with = "serialize_input_or_data", | ||
| flatten |
There was a problem hiding this comment.
I think this is just alias = "input"?
There was a problem hiding this comment.
That's what I tried to do as well, however it fails on the example where both fields are present:
---- types::transaction::eip1559::tests::test_tx_with_input_or_data stdout ----
thread 'types::transaction::eip1559::tests::test_tx_with_input_or_data' panicked at 'failed to parse request { "gas": "0x186a0",
"maxFeePerGas": "0x77359400",
"maxPriorityFeePerGas": "0x77359400",
"nonce": "0x2",
"to": "0x96216849c49358B10257cb55b28eA603c874b05E",
"value": "0x5af3107a4000",
"chainId": "0x539",
"accessList": [],
"data":"0x02", "input":"0x01"
}: duplicate field `data` at line 9 column 43', ethers-core/src/types/transaction/eip1559.rs:340:37
I'm not sure how important that is, whether there are actual clients that are so defensive that they send both fields, not knowing which one the server might expect; I only got it as a hint from someone who debugged this problem that certain implementation have a preference for input, which presumes that there is a choice to made.
There was a problem hiding this comment.
then I guess the only way is the serialize functions
There was a problem hiding this comment.
Just saw in web3/web3.js#1340 that Parity rejects the request if both input and data is present, so maybe it's okay to require one or the other with a simple alias.
There was a problem hiding this comment.
Duplication used to be an issue with web3 but got fixed apparently: web3/web3.js#6301
There was a problem hiding this comment.
Switched to alias, and added it to the legacy transaction type as well, just in case.
| fn serialize_input_or_data<S: Serializer>(data: &Option<Bytes>, s: S) -> Result<S::Ok, S::Error> { | ||
| #[derive(Serialize)] | ||
| struct InputOrData<'a> { | ||
| data: &'a Option<Bytes>, | ||
| } | ||
|
|
||
| InputOrData { data }.serialize(s) | ||
| } |
There was a problem hiding this comment.
this we don't need, right?
There was a problem hiding this comment.
or is this to make flatten work?
There was a problem hiding this comment.
Yeah, I thought so too, but unfortunately the flatten trick which makes both input and data available for deserialize_input_or_data means that during serialization it also wants to flatten the results, which it can't because it's a String. This returns a {"data": "0x..."} object which can be flattened 😞
There was a problem hiding this comment.
Removed it to keep it simple.
aakoshh
changed the title
Motivation
ethereum/go-ethereum#28078 changed the
go-ethereumclient to send the transaction payload in aninputfield instead ofdata, which is how it's defined in the API. Apparently most server implementations accept both fields, preferringinputwhere both are present at the same time.The
Eip1559TransactionRequesthas adatafield, and thus it could not be used to deserialize such requests.Solution
The PR annotates
Eip1559TransactionRequest::datafield so that it accepts eitherinputordata. Initially I added a special deserializer to accept both fields at the same time and preferinput, but it's a bit of an overkill, and the trend seems to be that clients likeweb3.jsare fixed not to send both unconditionally.PR Checklist