refactor: use Recovered<Tx> when setting up TxEnv#14580
refactor: use Recovered<Tx> when setting up TxEnv#14580klkvr merged 1 commit intoklkvr/wip-exec-transactionfrom
Conversation
klkvr
requested review from
Rjected,
fgimenez,
gakonst,
mattsse,
rakita and
rkrasiuk
as code owners
| } | ||
|
|
||
| let tx_env = self.evm_config.tx_env(tx.tx(), tx.signer()); | ||
| let tx_env = self.evm_config.tx_env(tx.clone()); |
There was a problem hiding this comment.
this should be okay because we convert the tx anyway
There was a problem hiding this comment.
yeah it would get fixed with alloy-rs/alloy#2082
| fn tx_env(&self, transaction: &TransactionSigned, sender: Address) -> Self::TxEnv { | ||
| fn tx_env<T: Borrow<Self::Transaction>>( | ||
| &self, | ||
| transaction: impl Borrow<Recovered<T>>, |
There was a problem hiding this comment.
I wonder if we should just operate on Recovered<&T> instead
because this Borrow forces &Recovered<Tx> which should be fine as long as the input here wraps a tx: Recovered<Tx>
but the additional Borrow on T should also cover use cases where the sender is stored separately
but effectively this is just Recovered<&T>
so this seems fine.
but I think we should also consider the consuming use case Recovered<T>
which should probably be a separate function?
There was a problem hiding this comment.
yeah, the optimization of consuming vs borrowing would only really exist if those would be two separate code paths
2 participants
Based on #14480
Towards #14551
Changes all
Tx -> TxEnvconversions to operate onRecovered<Tx>instead of passing sender separately. This should be better for the API because we can now hide this behind aFillTxEnv/IntoTxEnvtrait and havefn transact(&self, tx: impl IntoTxEnv<Self::Tx>)directly on Evm. This would not have been possible withtransact(tx, sender)API