-
Notifications
You must be signed in to change notification settings - Fork 29
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
feat(l1): pooled transactions #1444
base: main
Are you sure you want to change the base?
Conversation
LegacyTransaction(LegacyTransaction), | ||
EIP2930Transaction(EIP2930Transaction), | ||
EIP1559Transaction(EIP1559Transaction), | ||
WrappedEIP4844Transaction(WrappedEIP4844Transaction), |
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.
maybe EIP4844TransactionWithBlobs
would be clearer?
/// The same as a Transaction enum, only that blob transactions are in wrapped format, including | ||
/// the blobs bundle. | ||
#[derive(Clone, Debug, PartialEq, Eq)] | ||
pub enum P2PTransaction { |
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.
If this is a structure specifically made to fit the format expected by p2p can we move it there? Placing it in the same module as core types can lead to mix ups in the future
@@ -390,6 +390,14 @@ impl<S: AsyncWrite + AsyncRead + std::marker::Unpin> RLPxConnection<S> { | |||
let request = GetPooledTransactions::new(random(), hashes); | |||
self.send(Message::GetPooledTransactions(request)).await?; | |||
} | |||
// TODO: Also add handler for get pooled transactions. |
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.
We can remove this TODO
.flatten() | ||
.collect(); | ||
|
||
// TODO: add getting of the blob bundle, as we'll implement this as a p2p transaction. |
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.
This TODO can be removed
This PR adds handling for:
Closes #385
Closes #384