fix(calldata): name the descriptor when conversion fails - #348
Open
venugopalanvip wants to merge 1 commit into
Open
venugopalanvip wants to merge 1 commit into
venugopalanvip wants to merge 1 commit into
Conversation
The catch-all around v2 conversion reported:
Error processing v2 ERC-7730 file None, skipping it
`source` is an optional URL and is None for a local file, which is what the CLI
passes, so the message named nothing. It is the only line that says which
descriptor was abandoned -- the exception detail that follows it carries no file
or name -- so a run over more than one descriptor gave no way to tell which one
had failed.
It now falls back to what the descriptor calls itself, using the same identifier
the converted output carries as its contract name:
Error processing v2 ERC-7730 descriptor pFT NFT, skipping it
Error processing v2 ERC-7730 descriptor Ekubo Swap Router, skipping it
A source is kept beside the name when both are known, since a batch run wants
the path as well. With neither, it says so rather than printing None.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Small one, found while diagnosing #345.
The message named nothing
The catch-all around v2 conversion reported:
sourceis an optionalHttpUrland isNonefor a local file — which is whatcommand_calldatapasses unless--sourceis given, so the normal CLI path always printedfile None.That line is the only one that says which descriptor was abandoned. The
exception_to_outputdetail underneath it carries no file, no name and no position — just the raw exception. So converting more than one descriptor gave no way to tell which had failed; I found the two descriptors behind #345 by bisecting the registry by hand, which is the sort of thing a message is supposed to save you.Now
The fallback is what the descriptor calls itself, using the idiom already in this file for naming a contract in the converted output:
A
sourceis kept beside the name when both are known —MyContract (https://…)— because a batch run wants the path as well as the name. With neither, it says<unidentified>rather than printingNone.getattrguards the context id: the exception may be raised before the contract-context check at the top of the function, so the handler must not assume which context type it has and must not raise on its own.Tests
tests/v2/convert/calldata/test_describe.py:contractNamepreferred over$id,$idas fallback, name and source together, source alone, neither, and a parametrised check that the result never contains the textNone— which is the defect being fixed.Scope
v2 only.
convert_erc7730_input_to_calldata.pyhas the same line for v1, left alone since v1 is frozen.