feat(ledger): map Unichain, so its deployments are no longer skipped - #8
Open
venugopalanvip wants to merge 1 commit into
Open
venugopalanvip wants to merge 1 commit into
venugopalanvip wants to merge 1 commit into
Conversation
`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>
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.
The gap
ledger_network_idreturnsNonefor chain 130, soconvert_erc7730_v2_input_to_calldatadrops every deployment on it: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.0carries both: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-registrydeclare a deployment on one of the two and lose it today:registry/ekubo/calldata-MEVCaptureRouter.jsonregistry/kyberswap/calldata-MetaAggregationRouterV2.jsonregistry/layerswap/calldata-LayerswapDepository.jsonregistry/lido/calldata-stETH-L2.jsonregistry/lido/calldata-wstETH-L2.jsonregistry/okx/calldata-OkxDexRouterV1.0.7-multi-commission.jsonregistry/okx/calldata-OkxDexRouterV1.0.8-suffix-compat.jsonlido/calldata-stETH-L2.jsonis 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.pyis new — the file had none. It pins the two new ids, an existing one either side, theNonefor 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.