Implement TryFrom<Transaction> for TransactionInfo
#662
Conversation
tcoratger
requested review from
DaniPopes,
gakonst,
mattsse,
onbjerg,
prestwich and
Evalir
as code owners
| block_number: tx.block_number, | ||
| base_fee: match tx.transaction_type { | ||
| Some(tx_type) => match tx_type.try_into() { | ||
| Ok(TxType::Legacy) | Ok(TxType::Eip2930) => tx.gas_price, |
There was a problem hiding this comment.
is this behavior correct? wouldn't it be the block base fee regardless of tx content?
There was a problem hiding this comment.
I would say yes because it is precisely in the situation where we do not have information on the block but only on the tx. So we try to reconstruct the base fee as a function of the transaction type with the available information.
There was a problem hiding this comment.
yeah, but it produces the wrong output. Like whatever else we know, we know that this output is not the basefee. I'm reluctant to merge code that we know to be incorrect
There was a problem hiding this comment.
Maybe we can just put a None to say that we don't know precisely what is the base fee as we just have information about the transaction, wdyt?
There was a problem hiding this comment.
i would prefer that, as it guarantees we give no bad output. However, i don't know what this type is for so i can't say that the incorrect output is not expected 😅 What is the motivating use case of this PR?
There was a problem hiding this comment.
In fact I realized that in tracing to trace a transaction, we quite often do the conversion Transaction to TransactionInfo and so I said to myself that it was useful to have a try_from here but it's true that we can set to None and then fill with the base fee coming from the env block.
Just pushed that, tell me if it's ok for you :)
There was a problem hiding this comment.
In fact I realized that in tracing to trace a transaction, we quite often do the conversion Transaction to TransactionInfo and so I said to myself that it was useful to have a try_from here but it's true that we can set to None and then fill with the base fee coming from the env block.
Gotcha. makes sense. I think that now we have no errors tho, so we can make this From<&Transaction> for infallible conversion :)
Co-authored-by: evalir <hi@enriqueortiz.dev>
* impl TryFrom<Transaction> for TransactionInfo * Update crates/rpc-types/src/eth/transaction/common.rs Co-authored-by: evalir <hi@enriqueortiz.dev> * fix base fee * fix comment * From<&Transaction> --------- Co-authored-by: evalir <hi@enriqueortiz.dev>
No description provided.