-
Notifications
You must be signed in to change notification settings - Fork 2.4k
draft(optimism): sendRawTransactionConditional #13926
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,13 @@ pub struct OpPooledTransaction { | |
| inner: EthPooledTransaction<OpTransactionSigned>, | ||
| /// The estimated size of this transaction, lazily computed. | ||
| estimated_tx_compressed_size: OnceLock<u64>, | ||
|
|
||
| /// Optional conditional attached to this transaction. Is this | ||
| /// needed if this field is on OpTransactionSigned? | ||
| conditional: Option<TransactionConditional>, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes we need this field here. this option is also fine because this tx type is always wrapped in an Arc so it's always on the heap anyway |
||
|
|
||
| /// Indiciator if this transaction has been marked as rejected | ||
| rejected: AtomicBool // (is AtomicBool appropriate here?) | ||
|
Comment on lines
+54
to
+55
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can remove this for now, because we'll handle eviction differently |
||
| } | ||
|
|
||
| impl OpPooledTransaction { | ||
|
|
@@ -54,6 +61,7 @@ impl OpPooledTransaction { | |
| Self { | ||
| inner: EthPooledTransaction::new(transaction, encoded_length), | ||
| estimated_tx_compressed_size: Default::default(), | ||
| conditional: None, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -65,6 +73,21 @@ impl OpPooledTransaction { | |
| .estimated_tx_compressed_size | ||
| .get_or_init(|| tx_estimated_size_fjord(&self.inner.transaction().encoded_2718())) | ||
| } | ||
|
|
||
| // TODO: Setter with the conditional | ||
| pub fn conditional(&self) -> Option<&TransactionConditional> { | ||
| self.conditional.as_ref() | ||
| } | ||
|
|
||
| /// Mark this transaction as rejected | ||
| pub fn reject(&self) { | ||
| self.rejected.store(true, Ordering::Relaxed); | ||
| } | ||
|
|
||
| /// Returns true if this transaction has been marked as rejected | ||
| pub fn rejected(&self) -> bool { | ||
| self.rejected.load(Ordering::Relaxed) | ||
| } | ||
| } | ||
|
|
||
| /// Calculate the estimated compressed transaction size in bytes, scaled by 1e6. | ||
|
|
@@ -343,6 +366,19 @@ where | |
| ) | ||
| } | ||
|
|
||
| // If validated at the RPC layer pre-submission, this is not needed. The pool simply | ||
| // needs handle the rejected status on the pooled transaction set by the builder | ||
| if let Some(conditional) = transaction.conditional() { | ||
| //let client = self.client(); | ||
| //let header = client.latest_header()?.header(); | ||
| //if !conditional.matches_block_number(header.number()) { | ||
| // return TransactionValidationOutcome::Invalid( | ||
| // transaction, | ||
| // InvalidTransactionError::TxTypeNotSupported.into(), | ||
| // ) | ||
| //} | ||
| } | ||
|
|
||
| let outcome = self.inner.validate_one(origin, transaction); | ||
|
|
||
| if !self.requires_l1_data_gas_fee() { | ||
|
|
@@ -388,6 +424,13 @@ where | |
| ) | ||
| } | ||
|
|
||
| // Conditional transactions should not be propagated | ||
| // let propagate = if transaction.transaction_conditional().is_some() { | ||
| // false | ||
| // } else { | ||
| // propagate | ||
| // }; | ||
|
|
||
| return TransactionValidationOutcome::Valid { | ||
| balance, | ||
| state_nonce, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ use alloy_primitives::{ | |
| keccak256, Address, Bytes, PrimitiveSignature as Signature, TxHash, TxKind, Uint, B256, | ||
| }; | ||
| use alloy_rlp::Header; | ||
| use alloy_rpc_types_eth::erc4337::TransactionConditional; | ||
| use core::{ | ||
| hash::{Hash, Hasher}, | ||
| mem, | ||
|
|
@@ -33,7 +34,7 @@ use reth_primitives_traits::{ | |
| /// Signed transaction. | ||
| #[cfg_attr(any(test, feature = "reth-codec"), reth_codecs::add_arbitrary_tests(rlp))] | ||
| #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] | ||
| #[derive(Debug, Clone, Eq, AsRef, Deref)] | ||
| #[derive(Debug, Clone, AsRef, Deref)] | ||
| pub struct OpTransactionSigned { | ||
| /// Transaction hash | ||
| #[cfg_attr(feature = "serde", serde(skip))] | ||
|
|
@@ -44,8 +45,14 @@ pub struct OpTransactionSigned { | |
| #[deref] | ||
| #[as_ref] | ||
| pub transaction: OpTypedTransaction, | ||
|
|
||
| /// Can we attach a conditional the moment a transaction is deserialized? | ||
| pub conditional: Option<TransactionConditional> | ||
|
Comment on lines
+48
to
+50
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this we shouldn't do, because this is a representation fo the actual tx type and shouldn't be annotated with out of protocol data |
||
| } | ||
|
|
||
| // TEMPORARY since TransactionConditional does not impl eq | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mind submitting a pr on alloy for this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes definitely
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looping back to these changes this week. Saw alloy-rs/alloy#1953. Thanks for adding that in there |
||
| impl Eq for OpTransactionSigned {} | ||
|
|
||
| impl OpTransactionSigned { | ||
| /// Calculates hash of given transaction and signature and returns new instance. | ||
| pub fn new(transaction: OpTypedTransaction, signature: Signature) -> Self { | ||
|
|
@@ -61,9 +68,10 @@ impl OpTransactionSigned { | |
| /// | ||
| /// Note: this only calculates the hash on the first [`OpTransactionSigned::hash`] call. | ||
| pub fn new_unhashed(transaction: OpTypedTransaction, signature: Signature) -> Self { | ||
| Self { hash: Default::default(), signature, transaction } | ||
| Self { hash: Default::default(), signature, transaction, conditional: None } | ||
| } | ||
|
|
||
|
|
||
| /// Returns whether this transaction is a deposit. | ||
| pub const fn is_deposit(&self) -> bool { | ||
| matches!(self.transaction, OpTypedTransaction::Deposit(_)) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's rename this to enable-transaction-conditional