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

TypeTransaction conversion trait impls #472

Merged
merged 5 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions crates/consensus/src/signed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ impl<T, Sig> Signed<T, Sig> {
pub fn into_parts(self) -> (T, Sig, B256) {
(self.tx, self.signature, self.hash)
}

/// Returns the transaction without signature.
pub fn strip_signature(self) -> T {
self.tx
}
}

impl<T: SignableTransaction<Sig>, Sig> Signed<T, Sig> {
Expand Down
13 changes: 12 additions & 1 deletion crates/consensus/src/transaction/typed.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{Transaction, TxEip1559, TxEip2930, TxEip4844Variant, TxLegacy, TxType};
use crate::{Transaction, TxEip1559, TxEip2930, TxEip4844Variant, TxEnvelope, TxLegacy, TxType};
use alloy_primitives::TxKind;

/// The TypedTransaction enum represents all Ethereum transaction request types.
Expand Down Expand Up @@ -50,6 +50,17 @@ impl From<TxEip4844Variant> for TypedTransaction {
}
}

impl From<TxEnvelope> for TypedTransaction {
fn from(envelope: TxEnvelope) -> Self {
match envelope {
TxEnvelope::Legacy(tx) => Self::Legacy(tx.strip_signature()),
TxEnvelope::Eip2930(tx) => Self::Eip2930(tx.strip_signature()),
TxEnvelope::Eip1559(tx) => Self::Eip1559(tx.strip_signature()),
TxEnvelope::Eip4844(tx) => Self::Eip4844(tx.strip_signature()),
}
}
}

impl TypedTransaction {
/// Return the [`TxType`] of the inner txn.
pub const fn tx_type(&self) -> TxType {
Expand Down
8 changes: 6 additions & 2 deletions crates/network/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub trait Network: Clone + Copy + Sized + Send + Sync + 'static {
type TxEnvelope: Eip2718Envelope;

/// An enum over the various transaction types.
type UnsignedTx;
type UnsignedTx: From<Self::TxEnvelope>;

/// The network receipt envelope type.
type ReceiptEnvelope: Eip2718Envelope;
Expand All @@ -65,7 +65,11 @@ pub trait Network: Clone + Copy + Sized + Send + Sync + 'static {
// -- JSON RPC types --

/// The JSON body of a transaction request.
type TransactionRequest: RpcObject + TransactionBuilder<Self> + std::fmt::Debug;
type TransactionRequest: RpcObject
+ TransactionBuilder<Self>
+ std::fmt::Debug
+ From<Self::TxEnvelope>
+ From<Self::UnsignedTx>;
/// The JSON body of a transaction response.
type TransactionResponse: RpcObject;
/// The JSON body of a transaction receipt.
Expand Down
14 changes: 10 additions & 4 deletions crates/rpc-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ exclude.workspace = true
alloy-rlp = { workspace = true, features = ["arrayvec", "derive"] }
alloy-primitives = { workspace = true, features = ["rlp", "serde", "std"] }
alloy-serde.workspace = true
alloy-genesis.workspace=true
alloy-genesis.workspace = true

alloy-consensus = { workspace = true, features = ["std", "serde"] }
alloy-eips = {workspace = true, features = ["std", "serde"]}
alloy-eips = { workspace = true, features = ["std", "serde"] }

itertools.workspace = true
serde = { workspace = true, features = ["derive"] }
Expand All @@ -42,13 +42,19 @@ arbitrary = [
"dep:proptest-derive",
"dep:proptest",
"alloy-primitives/arbitrary",
"alloy-eips/arbitrary"
"alloy-eips/arbitrary",
]
jsonrpsee-types = ["dep:jsonrpsee-types"]
ssz = ["alloy-primitives/ssz", "alloy-eips/ssz"]
k256 = ["alloy-consensus/k256"]

[dev-dependencies]
alloy-primitives = { workspace = true, features = ["rand", "rlp", "serde", "arbitrary"] }
alloy-primitives = { workspace = true, features = [
"rand",
"rlp",
"serde",
"arbitrary",
] }
alloy-consensus = { workspace = true, features = ["std", "arbitrary"] }

arbitrary = { workspace = true, features = ["derive"] }
Expand Down
201 changes: 200 additions & 1 deletion crates/rpc-types/src/eth/transaction/request.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
//! Alloy basic Transaction Request type.

use crate::{eth::transaction::AccessList, BlobTransactionSidecar, Transaction};
use alloy_primitives::{Address, Bytes, ChainId, B256, U256};
use alloy_consensus::{
TxEip1559, TxEip2930, TxEip4844, TxEip4844Variant, TxEip4844WithSidecar, TxEnvelope, TxLegacy,
TypedTransaction,
};
use alloy_primitives::{Address, Bytes, ChainId, TxKind, B256, U256};
use serde::{Deserialize, Serialize};
use std::hash::Hash;

Expand Down Expand Up @@ -238,6 +242,201 @@ impl From<Transaction> for TransactionRequest {
}
}

impl From<TxLegacy> for TransactionRequest {
fn from(tx: TxLegacy) -> TransactionRequest {
TransactionRequest {
from: None,
to: if let TxKind::Call(to) = tx.to { Some(to) } else { None },
gas_price: Some(tx.gas_price),
gas: Some(tx.gas_limit),
value: Some(tx.value),
input: TransactionInput::from(tx.input),
nonce: Some(tx.nonce),
chain_id: tx.chain_id,
transaction_type: Some(0),
..Default::default()
}
}
}

impl From<TxEip2930> for TransactionRequest {
fn from(tx: TxEip2930) -> TransactionRequest {
TransactionRequest {
from: None,
to: if let TxKind::Call(to) = tx.to { Some(to) } else { None },
gas_price: Some(tx.gas_price),
gas: Some(tx.gas_limit),
value: Some(tx.value),
input: TransactionInput::from(tx.input),
nonce: Some(tx.nonce),
chain_id: Some(tx.chain_id),
transaction_type: Some(1),
access_list: Some(tx.access_list),
..Default::default()
}
}
}

impl From<TxEip1559> for TransactionRequest {
fn from(tx: TxEip1559) -> TransactionRequest {
TransactionRequest {
from: None,
to: if let TxKind::Call(to) = tx.to { Some(to) } else { None },
max_fee_per_gas: Some(tx.max_fee_per_gas),
max_priority_fee_per_gas: Some(tx.max_priority_fee_per_gas),
gas: Some(tx.gas_limit),
value: Some(tx.value),
input: TransactionInput::from(tx.input),
nonce: Some(tx.nonce),
chain_id: Some(tx.chain_id),
transaction_type: Some(2),
access_list: Some(tx.access_list),
..Default::default()
}
}
}

impl From<TxEip4844> for TransactionRequest {
fn from(tx: TxEip4844) -> TransactionRequest {
TransactionRequest {
from: None,
to: Some(tx.to),
max_fee_per_blob_gas: Some(tx.max_fee_per_blob_gas),
gas: Some(tx.gas_limit),
max_fee_per_gas: Some(tx.max_fee_per_gas),
max_priority_fee_per_gas: Some(tx.max_priority_fee_per_gas),
value: Some(tx.value),
input: TransactionInput::from(tx.input),
nonce: Some(tx.nonce),
chain_id: Some(tx.chain_id),
transaction_type: Some(3),
access_list: Some(tx.access_list),
blob_versioned_hashes: Some(tx.blob_versioned_hashes),
..Default::default()
}
}
}

impl From<TxEip4844WithSidecar> for TransactionRequest {
fn from(tx: TxEip4844WithSidecar) -> TransactionRequest {
let sidecar = tx.sidecar;
let tx = tx.tx;
TransactionRequest {
from: None,
to: Some(tx.to),
max_fee_per_blob_gas: Some(tx.max_fee_per_blob_gas),
gas: Some(tx.gas_limit),
max_fee_per_gas: Some(tx.max_fee_per_gas),
max_priority_fee_per_gas: Some(tx.max_priority_fee_per_gas),
value: Some(tx.value),
input: TransactionInput::from(tx.input),
nonce: Some(tx.nonce),
chain_id: Some(tx.chain_id),
transaction_type: Some(3),
access_list: Some(tx.access_list),
blob_versioned_hashes: Some(tx.blob_versioned_hashes),
sidecar: Some(sidecar),
..Default::default()
}
}
}

impl From<TxEip4844Variant> for TransactionRequest {
fn from(tx: TxEip4844Variant) -> TransactionRequest {
match tx {
TxEip4844Variant::TxEip4844(tx) => tx.into(),
TxEip4844Variant::TxEip4844WithSidecar(tx) => tx.into(),
}
}
}

impl From<TypedTransaction> for TransactionRequest {
fn from(tx: TypedTransaction) -> TransactionRequest {
match tx {
TypedTransaction::Legacy(tx) => tx.into(),
TypedTransaction::Eip2930(tx) => tx.into(),
TypedTransaction::Eip1559(tx) => tx.into(),
TypedTransaction::Eip4844(tx) => tx.into(),
}
}
}

impl From<TxEnvelope> for TransactionRequest {
fn from(envelope: TxEnvelope) -> TransactionRequest {
match envelope {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could attempt to extract from from the signature here (ignoring any error on the ecdsa recovery)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recover_signer is gated behind the k256 feature here. We can make k256 default in consensus or add cfg gate here. But I do not like adding so many cfg gates with different features. It makes the UX cumbersome. Curious to hear your view?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does make the UX cumbersome, but I also don't like dropping information that we definitely have and the user is likely to want if that makes sense

I wonder if it would make sense to extend the signed struct with signed_by: OnceLock<Address> which can be computed if K256 enabled

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regardless, this decision shouldn't block the PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would make sense to extend the signed struct with signed_by: OnceLock

which can be computed if K256 enabled

If I'm not wrong, OnceLock enables lazy initialization, so this wouldn't affect performance even if the signer is recovered implicitly.

TxEnvelope::Legacy(tx) => {
#[cfg(feature = "k256")]
{
let from = tx.recover_signer().ok();
let tx: TransactionRequest = tx.strip_signature().into();
if let Some(from) = from {
tx.from(from)
} else {
tx
}
}

#[cfg(not(feature = "k256"))]
{
tx.strip_signature().into()
}
}
TxEnvelope::Eip2930(tx) => {
#[cfg(feature = "k256")]
{
let from = tx.recover_signer().ok();
let tx: TransactionRequest = tx.strip_signature().into();
if let Some(from) = from {
tx.from(from)
} else {
tx
}
}

#[cfg(not(feature = "k256"))]
{
tx.strip_signature().into()
}
}
TxEnvelope::Eip1559(tx) => {
#[cfg(feature = "k256")]
{
let from = tx.recover_signer().ok();
let tx: TransactionRequest = tx.strip_signature().into();
if let Some(from) = from {
tx.from(from)
} else {
tx
}
}

#[cfg(not(feature = "k256"))]
{
tx.strip_signature().into()
}
}
TxEnvelope::Eip4844(tx) => {
#[cfg(feature = "k256")]
{
let from = tx.recover_signer().ok();
let tx: TransactionRequest = tx.strip_signature().into();
if let Some(from) = from {
tx.from(from)
} else {
tx
}
}

#[cfg(not(feature = "k256"))]
{
tx.strip_signature().into()
}
}
_ => Default::default(),
}
}
}

/// Error thrown when both `data` and `input` fields are set and not equal.
#[derive(Debug, Default, thiserror::Error)]
#[error("both \"data\" and \"input\" are set and not equal. Please use \"input\" to pass transaction call data")]
Expand Down
15 changes: 14 additions & 1 deletion crates/rpc-types/src/with_other.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::other::OtherFields;
use crate::{other::OtherFields, TransactionRequest};
use alloy_consensus::{TxEnvelope, TypedTransaction};
use serde::{Deserialize, Serialize};
use std::ops::{Deref, DerefMut};

Expand All @@ -21,6 +22,18 @@ impl<T> WithOtherFields<T> {
}
}

impl From<TypedTransaction> for WithOtherFields<TransactionRequest> {
fn from(tx: TypedTransaction) -> Self {
Self::new(tx.into())
}
}

impl From<TxEnvelope> for WithOtherFields<TransactionRequest> {
fn from(envelope: TxEnvelope) -> Self {
Self::new(envelope.into())
}
}

impl<T> Deref for WithOtherFields<T> {
type Target = T;

Expand Down
Loading