-
-
Notifications
You must be signed in to change notification settings - Fork 155
feat!: support decoding multiple ABIs at the same time, including ds-note library logs #757
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
Conversation
857e3de
to
af3407e
Compare
bb50a2b
to
41347d3
Compare
ready for review |
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.
My only real concern is the potential AttributeError
, but looks good!
[Doing QA with @dtdang and wearing that hat right now] Tried to follow the
and it hangs for 2 minutes before timing out. I realize now that I am supposed to use mainnet. However - I think the default timeout should be smaller for local networks. Thoughts?
|
Yes, I think this makes sense |
Done! https://github.com/ApeWorX/ape/pull/911/files |
@@ -305,13 +305,13 @@ def encode_transaction( | |||
""" | |||
|
|||
@abstractmethod | |||
def decode_logs(self, abi: EventABI, raw_logs: List[Dict]) -> Iterator[ContractLog]: | |||
def decode_logs(self, logs: List[Dict], *events: EventABI) -> Iterator["ContractLog"]: |
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.
the variadic argument feels a bit weird here
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.
@fubuloubu wanted it and I was hesitant at first, but I kind of like how it feels by not having to wrap single ABIs in a list first.
But I can go either way still, let's reach a consensus
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.
It's a breaking change anyways, live life on the edge 🤘
wen merge |
When using Hardhat, it makes infinite requests to Edit: Ah okay, I am literally checking the entire blockchain because I am using mainnet-fork. But literally this command will check the entire blockchain for logs of a single event: |
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.
What's with the use of snake casing everywhere?
What I did
Receipt.decode_logs
, it would fetch all contract types that have emitted logs and decode all events. undecoded logs are returned as is.How I did it
added ds-note decoding to ecosystem api and added a fallback to it in the receipt api. this is very cheap, it can deduce whether it's a note from byte padding in the topic before fetching any contract types. then it finds a function which matches the selector and decodes the data based on the function abi.
How to verify it
See also my similar pull request in brownie.
Checklist