feat: add devnet as a selectable network - #8
Conversation
Bump product-sdk ^0.19.1, chain-client ^0.9.1, contracts ^0.9.2, descriptors ^0.8.0, signer ^0.11.1, tx ^0.3.2; drop @novasamatech/host-api(-wrapper). The two remaining wrapper call sites (requestPermission, preimageManager) move to @parity/product-sdk-host. Result-API migration: contract .tx unwrapped in the getContract proxy, fromLiveClient and ensureContractAccountMapped branch on .ok. Override ws ^8.21.0 to clear GHSA-96hv-2xvq-fx4p pulled in via contracts' viem. Closes paritytech#5
94b8643 to
a0786fa
Compare
|
@ReinhardHatko Can you please review this? |
ReinhardHatko
left a comment
There was a problem hiding this comment.
Reviewed the devnet commit only, as asked. The wiring is right where it counts, and I verified the values rather than taking them on trust:
- Registry
0x59b0245778917af55224e5f8fb55f7f8d452619fmatchesDEVNET_REGISTRY_ADDRESSin CDM'ssrc/lib/env/src/registry.tsexactly. @parity/product-sdk-descriptors@0.8.0does export./devnet-asset-hub, and CDM'sconnection.tsimports the samedevnet_asset_hubbinding — the named export is correct.- Gateway ordering matches CDM's
DEVNET_IPFS_GATEWAY_URL(https://ipfs.io/ipfs) being first. - Passing
registryAddressexplicitly is necessary, not belt-and-braces: it defaults tocdmJson.registry, andcdm.json:133hardcodes the paseo-next registry. Without the override devnet would silently resolve against paseo-next. tsconfig.jsonalready hastypes: ["vite/client"], soimport.meta.env.VITE_NETWORKtypechecks.
The one I'd want resolved before merge. fromLiveClient is strict — it returns err(ContractLiveAddressResolutionError) if an address can't be resolved and never falls back to the cdm.json snapshot. So a VITE_NETWORK=devnet build hard-fails at contract init unless @example/feedback is actually registered in the devnet registry, and the PR body says devnet deploy/resolve is unexercised. Can you confirm the contract is deployed there? If it isn't yet, this merges a build flag that throws for anyone who sets it. Worth a note in DEPLOYMENT.md either way, since the failure surfaces as a resolution error rather than anything that points at "nothing deployed on this network".
Needs a version floor. cdm deploy -n devnet requires CDM v0.9.0+ (the preset landed in #61, released 2026-07-16). This repo pins no CDM version — it's a global binary, not a devDependency — and the new DEPLOYMENT.md note doesn't state a minimum. I get unknown preset on v0.8.26, which was current three weeks ago. Please add the floor to the note.
DEPLOYMENT.md is half-migrated. The new block covers deploy; the rest of the flow is still paseo-only — cdm account map -n paseo (line 100), cdm account bal -n paseo (109), the faucet link hardcoding ?parachain=1500 (87), and the AccountUnmapped troubleshooting row (293). Since this app requires a Revive mapping before any contract call, a devnet user hits AccountUnmapped and gets pointed at the wrong network to fix it. The devnet equivalents (mapping, balance, para-1000 faucet) would close the loop.
Type erasure worth avoiding. loadDescriptor(): Promise<unknown> plus as any drops the descriptor type at all three call sites, including the generic fromLiveClient<TDescriptor> — on main, paseo_asset_hub was fully typed there. A type-only import for the annotation keeps the runtime import dynamic (so the single-metadata-chunk property you verified still holds) while letting the generic bind.
Question on shared state. cdm.json holds one network's registry, address, version and ABI, and cdm deploy -n devnet rewrites them. After a devnet deploy the committed manifest describes devnet while the default build targets paseo-next. The frontend tolerates this (always passes registryAddress, re-resolves addresses live), but it'd help to state the intended workflow — two manifests, or accept the churn?
Nits: the title says "selectable" but selection is build-time only and NETWORK.label only reaches a console.log — surfacing it in the UI would match the wording. And const raw in resolveNetwork shadows the multiformats/codecs/raw import; I checked and raw.code at line 222 is in a different scope so it's not a bug, but a rename would save the next reader the same check.
Sequencing: this still carries #7's commit, so it wants a rebase once #7 lands.
a0786fa to
7a6fadd
Compare
Declare @parity/product-sdk-host directly and drop the unused @parity/product-sdk umbrella so host no longer resolves only by hoisting. Unwrap the contract .tx Result and wrap the account mapping in try/catch so the cause logging runs whether it returns an error or throws. Update the stale @novasamatech/host-api reference in the AI-config files.
Two-network config (paseo-next, devnet) selected at build time via VITE_NETWORK; default stays paseo-next. Descriptors load dynamically and the host-routed chain client takes its identity from the loaded descriptor. fromLiveClient now takes the selected network's registry address, so devnet resolves the contract against the devnet registry. Add the cdm devnet preset deploy script and a paseo-next vs devnet note in DEPLOYMENT.md. Closes paritytech#6
7a6fadd to
c9da8ce
Compare
|
Addressed the review:
The devnet contract deploy and the Desktop smoke test are still pending on my side. |
|
Went back through the full review; status:
Left the UI-label nit out for now (surfacing the network in a sticky-note board's UI is a bit out of scope). Still open on my side and gating merge: confirming |
Closes #6
Stacked on #7; review only the last commit until #7 merges.
Adds devnet as a build-time network (
VITE_NETWORK=devnet) alongside paseo-next, with the devnet registry, endpoint, and gateways. Contract addresses resolve live from the selected network's registry. Adds a devnet deploy script and a DEPLOYMENT.md note (CDM v0.9.0+, para-1000 faucet, and deploy-before-build guidance).Typecheck and both build variants pass. Devnet end-to-end needs the contract deployed there and a Desktop smoke test.