Skip to content

feat: Add Web3Transaction #3673

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tibor-reiss
Copy link
Contributor

What was wrong?

"There should be an easy way to take a transaction directly as a JSON-RPC dict..."

Related to Issue #3534

How was it fixed?

Add Web3Transaction thin wrapper for better UX.

Todo:

  • Clean up commit history
  • Add or update documentation related to these changes
  • Add entry to the release notes

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@tibor-reiss tibor-reiss force-pushed the i3534_transaction_class branch 3 times, most recently from 9c0f9fa to 8b5345a Compare April 20, 2025 07:41
@tibor-reiss
Copy link
Contributor Author

Hi @fselmo, could you please check if this is according to your vision?
I made a single class with ValueErrors. Other option would be to have a class hierarchy with Web3DynamicFeeTransaction, Web3AccessListTransaction, ...

@tibor-reiss tibor-reiss force-pushed the i3534_transaction_class branch from 8b5345a to 6ee9232 Compare April 20, 2025 07:50
@BobTheBuidler
Copy link
Contributor

might I recommend defining slots to make the wrapper even thinner?

@tibor-reiss
Copy link
Contributor Author

might I recommend defining slots to make the wrapper even thinner?

Hi @BobTheBuidler, could you please provide some example? E.g. should only typed_transaction be part of slots, or all the attributes from _dict? To me, this would make sense with a class hierarchy, because some of the attributes do not exist on specific types. Let me what you think.

@BobTheBuidler
Copy link
Contributor

I was just thinking about __slots__ = ("typed_transaction, "_dict"), not necessarily slots for each attr

Though that would be even thinner if you did it!

@fselmo
Copy link
Collaborator

fselmo commented Jun 4, 2025

Thanks for the brainstorming here @tibor-reiss 🙂 . I'd like to think on this some more and how we could maybe leverage this transaction API as more of a "first class citizen" of the library, maybe as a pydantic model for the next major version - possibly replacing TxParams altogether. That may require quite a bit of work though. This isn't a priority at the moment but I think it's good to get the conversation going on it at least, so thanks for getting it started 👍🏼

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

Successfully merging this pull request may close these issues.

3 participants