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

[Feature] encodable/decodable for Signed transaction #398

Closed
allnil opened this issue Mar 26, 2024 · 4 comments
Closed

[Feature] encodable/decodable for Signed transaction #398

allnil opened this issue Mar 26, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@allnil
Copy link
Contributor

allnil commented Mar 26, 2024

Component

consensus, eips, genesis

Describe the feature you would like

alloy-consensus:
e.g: usual TxEip4844 implements Encodable/Decodable
But Signed doesn't
we should have the ability to encode/decode Signed transactions directly

Additional context

e.g at this moment we can't do this:

let mut encoded = Vec::new();
my_tx.encode_for_signing(&mut encoded);
let signature = local_wallet.sign_signable(&mut my_tx.clone()).unwrap();
let mut signed_tx = my_tx.clone().into_signed(signature);
let encoded = &alloy_rlp::encode(signed_tx); // trait `Encodable` is not implemented for `Signed<TxEip4844>` 

pub struct Signed<T, Sig = Signature> {

we want the Encodable2718 trait, impl in TxEnvelope:

impl Encodable2718 for TxEnvelope {

@allnil allnil added the enhancement New feature or request label Mar 26, 2024
@mattsse
Copy link
Member

mattsse commented Mar 26, 2024

@prestwich I think what we want is

pub trait Encodable2718: Sized + Send + Sync + 'static {

for Signed<Tx>

we could either do this manually for all Signed<Tx*> types or make this

fn encode_2718(&self, out: &mut dyn alloy_rlp::BufMut) {
match self {
// Legacy transactions have no difference between network and 2718
TxEnvelope::Legacy(tx) => tx.tx().encode_with_signature_fields(tx.signature(), out),
TxEnvelope::Eip2930(tx) => {
tx.tx().encode_with_signature(tx.signature(), out, false);
}
TxEnvelope::Eip1559(tx) => {
tx.tx().encode_with_signature(tx.signature(), out, false);
}
TxEnvelope::Eip4844(tx) => {
tx.tx().encode_with_signature(tx.signature(), out, false);
}
}

part of a trait, so we can do:

impl<T: SignableTransaction<Sig>, Sig> Signed<T, Sig> {

@onbjerg
Copy link
Member

onbjerg commented Mar 27, 2024

You can do this by wrapping it in an envelope (TxEnvelope) which IIRC was intended, I vaguely remember the blanket impl for Signed<Tx> being removed by @Rjected in a recent PR

@prestwich
Copy link
Member

prestwich commented Mar 27, 2024

It was removed, because it is often incorrect to use it, in a way that is not obvious to users. If people are asking for it, we should push them towards TxEnvelope more aggressively

@allnil
Copy link
Contributor Author

allnil commented Mar 27, 2024

Hmmm, okay! I'll take a look into the TxEnvelope
Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants