feat: discover native asset via BlockHeader.nativeAssetId() at cold start#190
feat: discover native asset via BlockHeader.nativeAssetId() at cold start#190WiktorStarczewski wants to merge 2 commits intomainfrom
Conversation
0xnullifier
left a comment
There was a problem hiding this comment.
This feels a little over engineered imho
I mean the native asset discovery for the network could just be done during onboarding (the first startup) because it will never change after genesis and whenever required it could just fetch from the zustand store
Also for mainnet we will not need any of this so it is just temporary code, I would actually prefer putting manual faucet id every time as it's not much of a hassle anyway I mean it is a one line change.
It's not temporary at all, we will always maintain testnet/devnet envs without persistence guarantees. Without this we will need to redeploy the wallet EVERY time we reset the network (if we do). And "one line change" is great but you need to know the faucet account id beforehand, so you can't build the package before network is actually deployed. It matters combined with long chrome review times and is overall correct. |
Okay I see the argument it's fine then but still I feel like the approach could be better. I don't understand the need to fetch native asset on every start isn't it better to just fetch it one time as it won't change if the network is deployed ? then the approach could just be simplified to doing the discovery one-time (on install) and putting it to storage and always fetch it from there cause if there is a network reset which makes the first fetched native asset different the wallet then has to be reinstalled anyway |
@miden-sdk/miden-sdk@0.14.4This PR uses
BlockHeader.nativeAssetId(), added in miden-client PR #2071.package.jsonpins@miden-sdk/miden-sdk@0.14.4but the version hasn't been published to npm yet —yarn install --frozen-lockfilewill fail until it is. Local testing was done against afile:../miden-client-nativeassetid/crates/web-clientlink.yarn.lockstill references0.14.2and needs regeneration once the real0.14.4is out.Summary
src/lib/miden-chain/native-asset.tsintroducesgetNativeAssetId()/getNativeAssetMetadata()— cache-hit-first, with a single-flight RPC that callsBlockHeader.nativeAssetId()on first miss. Results are keyed per-network (native_asset_id:<network>,native_asset_meta:<network>) in platform storage, so a cached install never re-hits the RPC.primeNativeAssetId()fires from both the extension SW (Actions.init) and the React root (MidenProvidermount). Mobile/desktop have no SW, so the root mount is load-bearing there.TOKEN_MAPPING/MidenTokensfromsrc/lib/miden-chain/constants.ts. The previous hardcodedmtst1aqmat9m63ctdsgz6xcyzpuprpulwk9vg_qruqqypuyphwas a testnet-format address used as the default on every network — meaning auto-consume was a permanent no-op on devnet / localnet. Now the real per-network value is used.getFaucetIdSetting()returnsstring | null. On a fresh install with RPC unreachable, discovery can't complete; rather than guess, we returnnull. All MIDEN-equality comparisons (balance.ts,updateBalancesFromSyncData.ts,useMidenFaucetId,useHistoryBadge, etc.) toleratenullby falling through to "not MIDEN" and rendering the default zero-balance row only once the ID is known.resetNativeAssetCache()called fromclearStorageandresetStorageDestructive).Two pre-existing race bugs surfaced + fixed in the same PR
With auto-consume now actually firing on devnet, two latent bugs showed up:
TOCTOU race in
initiateConsumeTransaction. Two concurrent callers for the samenoteId(theisBeingClaimedguard racing theNoteClaimStartedintercom round-trip) both saw[]from the dedup query and both.add()'d, producing two queued consume rows — the second of which fails on-chain with "note has already been consumed".Repo.db.transaction('rw', ...)so concurrent callers serialize at the Dexie layer. Regression test added (transactions.extended.test.ts).False-positive connectivity-issue banner.
withProverFallbackcalledaddConnectivityIssue()on any error thrown during a delegated transaction, including semantic WASM-client validation errors like "note has already been consumed" (which throws fromexecute_transactionbefore any RPC happens).isLikelyNetworkError()classifier. Banner only fires when the message matches fetch / transport / timeout / 5xx / abort / RPC patterns. Explicitly excludes"invalid transaction request"and"has already been consumed". 5 new tests inmiden-client-interface.test.ts.Verification reference
Previously hardcoded bech32 (testnet-format, used on all networks):
mtst1aqmat9m63ctdsgz6xcyzpuprpulwk9vg_qruqqypuyph.When testing against each network, the discovered
nativeAssetId()should match the table in~/.claude/projects/-Users-celrisen-miden-miden-wallet/memory/native-asset-verification.md. Devnet value is TBD — to be filled in on first discovery against devnet.Test plan
yarn jest— 1674/1674 passing, including newnative-asset.test.ts(11 cases), the atomic-dedup race regression, and the 5 connectivity-classification casesyarn tsc --noEmitclean (against localfile:link to 0.14.4 SDK build)yarn prettier --check+yarn eslintclean on all touched files@miden-sdk/miden-sdk@0.14.4publication, thenyarn installto regenerateyarn.lockOpen question for follow-up
The new
getNativeAssetMetadata()fetches and caches chain-truthsymbol/decimalsfor the native asset but current balance rendering still uses the hardcodedMIDEN_METADATAconstant for those fields. Consumers of the cache can be wired in a follow-up if we want decimals/symbol to be chain-sourced too. Left out of scope here because it's primarily a branding decision (thumbnail + display name stay hardcoded regardless).