Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove standalone from_recovered functions and make part of TransactionCompat trait #13641

Closed
mattsse opened this issue Jan 3, 2025 · 6 comments · Fixed by #13653
Closed
Assignees
Labels
A-rpc Related to the RPC implementation C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started

Comments

@mattsse
Copy link
Collaborator

mattsse commented Jan 3, 2025

Describe the feature

pub fn from_recovered_with_block_context<Tx, T: TransactionCompat<Tx>>(
tx: RecoveredTx<Tx>,
tx_info: TransactionInfo,
resp_builder: &T,
) -> Result<T::Transaction, T::Error> {
resp_builder.fill(tx, tx_info)
}
/// Create a new rpc transaction result for a _pending_ signed transaction, setting block
/// environment related fields to `None`.
pub fn from_recovered<Tx, T: TransactionCompat<Tx>>(
tx: RecoveredTx<Tx>,
resp_builder: &T,
) -> Result<T::Transaction, T::Error> {
resp_builder.fill(tx, TransactionInfo::default())
}

from_recovered already just delegates to:

fn fill(
&self,
tx: RecoveredTx<T>,
tx_inf: TransactionInfo,
) -> Result<Self::Transaction, Self::Error>;

and can easily be removed.

the other one should be introduced as TransactionCompat::fill_pending as a helper

TODO

  • introduce TransactionCompat::fill_pending
  • remove standalone functions and update calls: flipping from_recovered(transaction, self.tx_resp_builder() to self.tx_resp_builder().fill_pending(transaction) for example

Additional context

No response

@mattsse mattsse added A-rpc Related to the RPC implementation C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started labels Jan 3, 2025
@tonypony220
Copy link
Contributor

hi! can i take this?

@mattsse
Copy link
Collaborator Author

mattsse commented Jan 3, 2025

assigned, ty

@mymiracle0118
Copy link

Could I take on this issue?

@aidenwong812
Copy link

I'd be happy to do this.

@PoulavBhowmick03
Copy link

Gm gm! I would love to work on this! I have some experience in Rust and have contributed to projects like Starknet Foundry and dojo. This would be my first time contributing to Reth!

@V15HNUPM
Copy link

V15HNUPM commented Jan 6, 2025

Can I try solving this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Related to the RPC implementation C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started
Projects
Archived in project
6 participants