-
Notifications
You must be signed in to change notification settings - Fork 9
refactor: rework multicall handling #1014
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
base: master
Are you sure you want to change the base?
Conversation
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0624a23e67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| # NOTE: value comes from eth_utils.function_signature_to_4byte_selector("tryBlockAndAggregate(bool,(address,bytes)[])") | ||
| fourbyte: Final = b"9\x95B\xe9" | ||
| class Multicall(_Batch[bytes, eth_call]): |
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.
Multicall still subclasses _Batch, but this commit removed the _Batch definition from _requests.py, and I can't find any class _Batch anywhere in the repo (rg "class _Batch"). That means importing this module will raise NameError at class definition time and all batching/multicall functionality fails immediately. Either reintroduce _Batch or import it from a new location.
Useful? React with 👍 / 👎.
| if response.exception: | ||
| # NOTE: If the response is an error, we need to raise it so it can be handled by the caller | ||
| raise response.exception |
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.
Decode RawResponse before checking exception
post() returns a RawResponse, which only exposes decode(); it does not have .exception or .error attributes. Accessing response.exception here raises AttributeError for every multicall response (even successful ones), so the new multicall path fails immediately. You likely need to call response.decode(partial=True) and inspect the decoded object instead.
Useful? React with 👍 / 👎.
| response = await super().make_request(num_previous_timeouts) | ||
| response.bytes = bytes(self.calldata) # type: ignore [attr-defined] |
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.
Stop assigning attributes onto RawResponse
RawResponse uses __slots__ = ("_raw",) (see dank_mids/helpers/_codec.py), so it cannot accept new attributes. Assigning response.bytes will raise AttributeError on every eth_call request after the await, turning successful RPC responses into failures. If you need to retain calldata, store it outside the RawResponse or extend the class’ slots.
Useful? React with 👍 / 👎.
Summary
Notes
Testing