Skip to content

fix(calldata): name the descriptor when conversion fails - #348

Open
venugopalanvip wants to merge 1 commit into
LedgerHQ:mainfrom
venugopalanvip:name-the-descriptor-on-failure
Open

venugopalanvip wants to merge 1 commit into
LedgerHQ:mainfrom
venugopalanvip:name-the-descriptor-on-failure

Conversation

@venugopalanvip

Copy link
Copy Markdown
Contributor

Small one, found while diagnosing #345.

The message named nothing

The catch-all around v2 conversion reported:

🟠 warning: Error processing v2 ERC-7730 file None, skipping it
🔴 error: Failed processing descriptor: invalid literal for int() with base 10: 'true'

source is an optional HttpUrl and is None for a local file — which is what command_calldata passes unless --source is given, so the normal CLI path always printed file None.

That line is the only one that says which descriptor was abandoned. The exception_to_output detail 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

🟠 warning: Error processing v2 ERC-7730 descriptor pFT NFT, skipping it
🟠 warning: Error processing v2 ERC-7730 descriptor Ekubo Swap Router, skipping it

The fallback is what the descriptor calls itself, using the idiom already in this file for naming a contract in the converted output:

contract_name=descriptor.metadata.contractName or descriptor.context.id,

A source is 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 printing None.

getattr guards 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: contractName preferred over $id, $id as fallback, name and source together, source alone, neither, and a parametrised check that the result never contains the text None — which is the defect being fixed.

Scope

v2 only. convert_erc7730_input_to_calldata.py has the same line for v1, left alone since v1 is frozen.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant