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] Receipt trait in alloy-consensus #466

Closed
prestwich opened this issue Apr 5, 2024 · 5 comments · Fixed by #477
Closed

[Feature] Receipt trait in alloy-consensus #466

prestwich opened this issue Apr 5, 2024 · 5 comments · Fixed by #477
Labels
enhancement New feature or request

Comments

@prestwich
Copy link
Member

prestwich commented Apr 5, 2024

  • Update trait TxReceipt in alloy-consensus
  • fn status in trait Receipt
  • Receipt bound on type Recept in trait Network
  • impl Receipt for ReceiptEnvelope and related types
  • change status function on rpc receipt type to delegate to Receipt implementation of inner
  • impl<T: Receipt> Receipt for ReceiptResponse

Originally posted by @prestwich in #456 (comment)

@prestwich prestwich added the enhancement New feature or request label Apr 5, 2024
@prestwich prestwich changed the title [Feature] Receipt trait in alloy-network [Feature] Receipt trait in alloy-consensus Apr 6, 2024
@developeruche
Copy link
Contributor

@prestwich can you give more context to;

Receipt bound on type Recept in trait Network

and

impl<T: Receipt> Receipt for ReceiptResponse

@zilayo
Copy link
Contributor

zilayo commented Apr 6, 2024

seems related so leaving here instead of a new issue.

Current UX for accessing logs from a WithOtherFields<TransactionReceipt<AnyReceiptEnvelope<Log>>> receipt is:

receipt.inner.inner.inner.receipt.logs

Currently there are only implementations for AnyReceiptEnvelope<()> - these need to be exposed for AnyReceiptEnvelope<T> as well.

pub struct AnyReceiptEnvelope<T = Log> {
/// The receipt envelope.
#[cfg_attr(feature = "serde", serde(flatten))]
pub inner: ReceiptWithBloom<T>,
/// The transaction type.
#[cfg_attr(feature = "serde", serde(with = "alloy_serde::num::u8_hex"))]
pub r#type: u8,
}
impl AnyReceiptEnvelope {

Note - there's already a TxReceipt trait that has fn success which is equivalent to what fn status would be.

Perhaps makes sense to remove this trait altogether in favor of the Receipt trait mentioned above?

@prestwich
Copy link
Member Author

Note - there's already a TxReceipt trait that has fn success which is equivalent to what fn status would be.

Yeah, i think my issue here isn't "create a Receipt trait" it's "ensure that the existing Receipt trait is properly implemented for relevant types further up the abstraction stack, like ReceiptResponse and AnyReceiptEnvelope, and used in abstracted code like the Provider and PendingTransaction

@prestwich
Copy link
Member Author

Current UX for accessing logs from a WithOtherFields<TransactionReceipt<AnyReceiptEnvelope<Log>>> receipt is:

receipt.inner.inner.inner.receipt.logs

Currently there are only implementations for AnyReceiptEnvelope<()> - these need to be exposed for AnyReceiptEnvelope<T> as well.

This is the sort of UX problem that a consistenyl applied and used Receipt trait would solve 👍

@developeruche
Copy link
Contributor

Note - there's already a TxReceipt trait that has fn success which is equivalent to what fn status would be.

Yeah, i think my issue here isn't "create a Receipt trait" it's "ensure that the existing Receipt trait is properly implemented for relevant types further up the abstraction stack, like ReceiptResponse and AnyReceiptEnvelope, and used in abstracted code like the Provider and PendingTransaction

Oh yeah... got it

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

Successfully merging a pull request may close this issue.

3 participants