fix(generate): pass owner and legal name in the right order

[venugopalanvip]

Closes #338.

The bug

generate_descriptor calls _generate_metadata with its first two arguments transposed:

def _generate_metadata(owner: str | None, legal_name: str | None, url: HttpUrl | None) -> InputMetadata:
...
metadata = _generate_metadata(legal_name, owner, url)   # swapped

So --owner lands in metadata.info.legalName and --legal-name in metadata.owner:

$ erc7730 generate --chain-id 1 --address 0xae78…6393 \
    --owner "Rocket Pool" --legal-name "Rocket Pool Ltd" --url "https://rocketpool.net"

"metadata": {
  "owner": "Rocket Pool Ltd",
  "info": { "legalName": "Rocket Pool", "url": "https://rocketpool.net" }
}

And because info was only built when a legal name and a url were both present, generating with --owner alone dropped the value completely and silently:

$ erc7730 generate --chain-id 1 --address 0xae78…6393 --owner "Rocket Pool"

"metadata": {}

That is the flag a first-time contributor is most likely to reach for, and it fails without any error.

The fix

Two lines, one consequential.

The swap. Arguments now match the signature.

The and. Correcting the swap alone would introduce a regression: --owner X --url Y currently produces info (because the owner stood in for the legal name), and after the swap it would produce none — silently dropping the url in the common case. So info is now built when either is given. OwnerInfo.legalName is required, so the display name stands in when only a url is supplied.

invocation	before	after
--owner X	{}	{"owner": "X"}
--owner X --url Y	{"info": {"legalName": "X", "url": "Y"}}	{"owner": "X", "info": {"legalName": "X", "url": "Y"}}
--owner X --legal-name Z --url Y	{"owner": "Z", "info": {"legalName": "X", …}}	{"owner": "X", "info": {"legalName": "Z", …}}

Tests

Three cases added to tests/generate/test_generate.py, using the existing local ABI fixture so they need no network. The existing suite passes owner=label but never asserts where it lands, which is how this survived.

They fail on the previous behaviour exactly as reported:

assert 'Legal Name Ltd' == 'Display Name'
assert None == 'Display Name'
assert None == 'Display Name'
3 failed

and pass with the fix. pytest tests/generate --run-v1 is green: 19 passed.

One note

These tests inherit the file's pytestmark = pytest.mark.v1, so they are skipped unless --run-v1 is passed. generate_descriptor builds erc7730.model.input v1 models, which is also why its output carries metadata.info.legalName — a field the registry's published v2 schema rejects with additionalProperties: false. That mismatch is the other half of #338 and is a larger change than this one, so I have left it alone; happy to look at it separately if that would be useful.

🤖 Generated with Claude Code

[fsamier]

This is also a v1 only fix, which will be deprecated soon

[venugopalanvip]

Understood on v1 — and thanks, that prompted a re-read that found a real problem in my own patch.

The patch as it stood could crash. OwnerInfo.url is a required field, so relaxing the guard to legal_name is not None or url is not None created a path where --legal-name with no --url raised a ValidationError instead of generating a descriptor. 2e21077 turns the guard on the url, which is what OwnerInfo actually needs, and keeps the part that was the point of the change: --owner with --url and no legal name now fills legalName from the owner rather than dropping the url.

My three tests had all supplied a url, so none of them reached it. There is now a case that does, and it fails against aa85e44. The ruff SIM102 is gone with the same edit — the condition is a single expression again.

On v1: happy to close this if you would rather not carry it. One thing to weigh first — generate.py is v1-only, but it has no v2 counterpart at all (src/erc7730/generate/ is a single module, importing model.input.context and model.input.metadata), so there is nowhere to move the fix to. Until a v2 generator exists, erc7730 generate is what users run, and today --owner X --legal-name Y writes them into each other's fields.

So: merge as a stopgap, or close and let it go with v1? Your call — I'm not attached to it, and I'll keep to v2 paths for anything new.

[fsamier]

On v1: happy to close this if you would rather not carry it. One thing to weigh first — generate.py is v1-only, but it has no v2 counterpart at all (src/erc7730/generate/ is a single module, importing model.input.context and model.input.metadata), so there is nowhere to move the fix to. Until a v2 generator exists, erc7730 generate is what users run, and today --owner X --legal-name Y writes them into each other's fields.

So: merge as a stopgap, or close and let it go with v1? Your call — I'm not attached to it, and I'll keep to v2 paths for anything new.

Indeed, generate had more sense at the kick off of the project and is now lower priority. Therefore I would prefer to not invest in v1, and consider porting generate to v2 as a backlog task

[venugopalanvip]

Makes sense — closing.

#338 stays open as the record, so whoever picks up the v2 port of generate has the detail without re-deriving it. Summarised there: the argument-order swap at the call site, and that OwnerInfo requires a url, which is the part that makes the "url with no legal name" case need a decision rather than just a reorder.

Thanks for the quick reviews on both of these.