Skip to content

feat(ledger): map Unichain, so its deployments are no longer skipped - #8

Open
venugopalanvip wants to merge 1 commit into
sourcifyeth:mainfrom
venugopalanvip:sourcify-add-unichain
Open

venugopalanvip wants to merge 1 commit into
sourcifyeth:mainfrom
venugopalanvip:sourcify-add-unichain

Conversation

@venugopalanvip

Copy link
Copy Markdown

The gap

ledger_network_id returns None for chain 130, so convert_erc7730_v2_input_to_calldata drops every deployment on it:

if ledger_network_id(deployment.chainId) is None:
    out.warning(f"Chain id {deployment.chainId} is not known, skipping it")
    continue

No device descriptor is produced for that chain, and the only signal is a warning on a converter run that still exits successfully.

Ledger does support Unichain. @ledgerhq/cryptoassets@13.56.0 carries both:

unichain          { type: "CryptoCurrency", family: "evm", name: "Unichain",         chainId: 130  }
unichain_sepolia  { type: "CryptoCurrency", family: "evm", name: "Unichain Sepolia", chainId: 1301 }

Both ids are taken verbatim from that package rather than guessed. As a check on the source: of the 123 chain ids this file already maps, 61 also appear in the package, and all 61 agree exactly — so it is the right list, and the ids are in the right form.

What it recovers

Seven descriptors in clear-signing-erc7730-registry declare a deployment on one of the two and lose it today:

descriptor recovered
registry/ekubo/calldata-MEVCaptureRouter.json +2 of 18
registry/kyberswap/calldata-MetaAggregationRouterV2.json +1 of 18
registry/layerswap/calldata-LayerswapDepository.json +1 of 56
registry/lido/calldata-stETH-L2.json +1 of 2
registry/lido/calldata-wstETH-L2.json +1 of 6
registry/okx/calldata-OkxDexRouterV1.0.7-multi-commission.json +1 of 25
registry/okx/calldata-OkxDexRouterV1.0.8-suffix-compat.json +1 of 25

lido/calldata-stETH-L2.json is the sharpest: it declares two deployments, and one of them is Unichain.

Why this stops at two

I swept every calldata descriptor in the registry. It uses 95 distinct chain ids, of which this file knows 48; the other 47 are dropped, covering 87 deployments across 16 descriptors.

But of those 47, only these two are in the cryptoassets package. The rest — Plasma (9745, 12 deployments), Soneium, TAC, MegaETH, Manta Pacific, opBNB, Fraxtal, Ronin, Taiko, Zora and others — have no Ledger currency id at all, so there is nothing correct to map them to. Dropping them is the right behaviour, and inventing ids would be worse than the warning.

That is why this adds two lines rather than forty-seven. If Ledger adds currencies for any of those chains, the same one-line-per-chain change applies; happy to file the full list as an issue if it is useful for tracking.

Tests

tests/common/test_ledger_network_id.py is new — the file had none. It pins the two new ids, an existing one either side, the None for an unsupported chain, and that the match arms stay in ascending chain-id order, which is what makes the insertion point for the next chain unambiguous.

Also raised upstream as LedgerHQ/python-erc7730#346, unreviewed.

`ledger_network_id` returns None for chain 130, so the calldata converter drops
every deployment on it with a warning and emits no device descriptor for that
chain. Ledger does support it: `@ledgerhq/cryptoassets@13.56.0` carries

    unichain         { type: CryptoCurrency, family: evm, chainId: 130 }
    unichain_sepolia { type: CryptoCurrency, family: evm, chainId: 1301 }

Both ids are taken verbatim from that package rather than guessed, and every
one of the 61 chain ids this file shares with it already agrees exactly.

Seven descriptors in clear-signing-erc7730-registry declare a deployment on one
of the two and lose it today, among them Lido's L2 stETH and wstETH, OKX's
router on both versions, and Kyberswap's aggregator.

The registry uses 95 distinct chain ids across its calldata descriptors, of
which this file knew 48. The other 46 are not in the cryptoassets package
either, so they have no Ledger currency id to map to and are left alone.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant