From fcebd219ed434139d6e48f32b6f7bba1473ada61 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 2 Sep 2026 00:34:22 +0200 Subject: [PATCH 1/2] fix: resolve usernames by document id, not by display name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A transfer of any username containing l, i or o failed with 'was not found'. The name list is built from the raw label property, but dpns.getUsernameByName matches on normalizedLabel, which is homograph-folded (l and i to 1, o to 0). So "testfjdksla234123.dash" listed correctly and then resolved to nothing. listOwnedUsernames now queries the DPNS domain documents directly and returns the document id alongside the display name, and transferUsername takes that id instead of looking the name up again. The round trip that could not round-trip is gone rather than patched. Discovery also no longer stops at the first identity a seed matches. One seed commonly controls several, and the first is not necessarily the one holding the name; it now scans all of them and prefers one that owns a username. The empty case is a clear callout instead of a grey line under a permanently disabled button. Verified live on testnet: registered xferlive7test.dash (normalizedLabel xfer11ve7test, which reproduces the failure on main) and transferred it — ownerId and records.identity both moved, revision 1 to 2. The mock usernames now contain an l so this class stays covered. Co-Authored-By: Claude Opus 5 (1M context) --- src/main.ts | 89 ++++++++++++++++++++++++----- src/platform/dpns-utils.test.ts | 11 ++++ src/platform/username-transfer.ts | 93 ++++++++++++++++++++----------- src/types.ts | 15 ++++- src/ui/components.ts | 13 ++++- src/ui/state.ts | 9 ++- 6 files changed, 176 insertions(+), 54 deletions(-) diff --git a/src/main.ts b/src/main.ts index c8b84bb..b90469e 100644 --- a/src/main.ts +++ b/src/main.ts @@ -189,6 +189,7 @@ import type { DpnsUsernameEntry, DpnsRegistrationResult, IdentityPublicKeyInfo, + OwnedUsername, E2EMockWindow, AssetLockProofData, } from './types.js'; @@ -430,8 +431,14 @@ function createE2EMockIdentityKeys(): IdentityPublicKeyInfo[] { ]; } -function createE2EMockOwnedUsernames(): string[] { - return ['mockname.dash', 'second-mockname.dash']; +function createE2EMockOwnedUsernames(): OwnedUsername[] { + // Both labels contain "l", which homograph-folds to "1" in normalizedLabel. + // Real names like these cannot be resolved from their display form, so the + // mock keeps the flow honest about carrying document ids around. + return [ + { username: 'mocklabel.dash', documentId: E2E_MOCK_IDENTITY_ID, ownerId: E2E_MOCK_IDENTITY_ID }, + { username: 'second-mocklabel.dash', documentId: E2E_MOCK_XFER_RECIPIENT_ID, ownerId: E2E_MOCK_IDENTITY_ID }, + ]; } function createE2EMockDpnsAvailability(entries: DpnsUsernameEntry[]): DpnsUsernameEntry[] { @@ -3416,7 +3423,7 @@ async function startManageUpdate() { * known: the names it owns, and the network's protocol version. */ async function loadTransferContext(identityId: string): Promise<{ - usernames: string[]; + usernames: OwnedUsername[]; protocolVersion?: number; }> { const { listOwnedUsernames, getProtocolVersion } = await loadUsernameTransferModule(); @@ -3456,16 +3463,16 @@ async function startXferUnlockFromSeed(mnemonic: string) { const candidates = deriveCandidateKeys(mnemonic, state.network); - const { discoverIdentityFromCandidates } = await loadUsernameTransferModule(); - const discovered = await discoverIdentityFromCandidates( + const { discoverIdentitiesFromCandidates } = await loadUsernameTransferModule(); + const discovered = await discoverIdentitiesFromCandidates( candidates, state.network, - (checked, total) => { - updateState(setXferDiscoveryStatus(state, `Searching for your identity (${checked}/${total})...`)); + (checked: number, total: number) => { + updateState(setXferDiscoveryStatus(state, `Searching for your identities (${checked}/${total})...`)); } ); - if (!discovered) { + if (discovered.length === 0) { updateState(setXferCredentialError( state, 'No identity on this network uses a key from that seed phrase. Check you picked the right network, or use the Private Key tab to enter an identity ID directly.' @@ -3473,14 +3480,59 @@ async function startXferUnlockFromSeed(mnemonic: string) { return; } - updateState(setXferDiscoveryStatus(state, 'Identity found. Checking your keys...')); + updateState(setXferDiscoveryStatus( + state, + discovered.length === 1 + ? 'Identity found. Checking your keys...' + : `Found ${discovered.length} identities. Checking which own usernames...` + )); + + // A seed commonly controls several identities, and the first one found is not + // necessarily the one holding the name. Load each, then prefer one that + // actually owns a username. + const loaded: { + identityId: string; + selection: ReturnType; + usernames: OwnedUsername[]; + protocolVersion?: number; + }[] = []; + + for (const { identityId } of discovered) { + const keys = await getIdentityPublicKeys(identityId, state.network); + const selection = selectTransferSigningKey(candidates, keys, state.network); + const context = selection.status === 'ok' + ? await loadTransferContext(identityId) + : { usernames: [] as OwnedUsername[], protocolVersion: undefined }; + loaded.push({ identityId, selection, ...context }); + } + + const usable = loaded.filter((entry) => entry.selection.status === 'ok'); + if (usable.length === 0) { + // Report the most informative failure we saw rather than a generic one. + const ineligible = loaded.find((entry) => entry.selection.status === 'ineligible'); + await applyXferKeySelection( + loaded[0].identityId, + ineligible?.selection ?? loaded[0].selection, + 'That seed phrase does not control any active key on the identities it points to.' + ); + return; + } + + const chosen = usable.find((entry) => entry.usernames.length > 0) ?? usable[0]; + const selection = chosen.selection; + if (selection.status !== 'ok') return; // narrowed by `usable` above - const keys = await getIdentityPublicKeys(discovered.identityId, state.network); - await applyXferKeySelection( - discovered.identityId, - selectTransferSigningKey(candidates, keys, state.network), - 'That seed phrase does not control any active key on the identity it points to.' - ); + updateState(setXferIdentityUnlocked(state, { + identityId: chosen.identityId, + privateKeyWif: selection.candidate.privateKeyWif, + keyId: selection.keyId, + securityLevel: selection.securityLevel, + usernames: chosen.usernames, + protocolVersion: chosen.protocolVersion, + otherIdentities: usable + .filter((entry) => entry.identityId !== chosen.identityId) + .map((entry) => entry.identityId), + })); } /** @@ -3700,9 +3752,16 @@ async function startUsernameTransfer() { return; } + const selected = (state.xferOwnedUsernames || []).find((u) => u.username === username); + if (!selected) { + updateState(setXferResult(state, { success: false, error: `"${username}" is no longer in the list of names this identity owns` })); + return; + } + const { transferUsername } = await loadUsernameTransferModule(); const result = await transferUsername({ username, + documentId: selected.documentId, identityId, publicKeyId: keyId, privateKeyWif, diff --git a/src/platform/dpns-utils.test.ts b/src/platform/dpns-utils.test.ts index fd1319e..7c6814f 100644 --- a/src/platform/dpns-utils.test.ts +++ b/src/platform/dpns-utils.test.ts @@ -22,6 +22,17 @@ describe('DPNS username helpers', () => { expect(validateDpnsLabel('dash--user')).toEqual({ isValid: false, error: 'No consecutive hyphens allowed' }); }); + it('folds l, i and o, so a display label is not its normalizedLabel', () => { + // This is why a username cannot be resolved back to its document from the + // name shown in the UI: the index is on normalizedLabel, but the display + // form uses the raw label. Any name containing l, i or o diverges. + expect(convertToHomographSafe('testfjdksla234123')).toBe('testfjdks1a234123'); + expect(convertToHomographSafe('pastafaucettesting1234')).toBe('pastafaucettest1ng1234'); + // A name with none of those characters round-trips, which is how a live + // transfer of "xfertest7pasta" passed while the bug was present. + expect(convertToHomographSafe('xfertest7pasta')).toBe('xfertest7pasta'); + }); + it('normalizes labels for homograph-safe contested-name checks', () => { expect(convertToHomographSafe('Oil-Loom')).toBe('011-100m'); expect(isContestedUsername('dash')).toBe(true); diff --git a/src/platform/username-transfer.ts b/src/platform/username-transfer.ts index fad0498..6457d77 100644 --- a/src/platform/username-transfer.ts +++ b/src/platform/username-transfer.ts @@ -11,7 +11,7 @@ import { import { extractErrorMessage } from '../utils/errors.js'; import { loadSdkModule } from './sdkModule.js'; import type { DerivedCandidateKey } from './username-transfer-utils.js'; -import type { UsernameTransferOutcome } from '../types.js'; +import type { OwnedUsername, UsernameTransferOutcome } from '../types.js'; /** * DPNS is a system data contract with the same ID on every network. @@ -67,22 +67,26 @@ async function findIdentityIdByPublicKeyHash( } /** - * Walk derived candidates in order and return the first one that resolves to a - * registered identity. Candidates are pre-ordered cheapest-first, so this - * usually resolves on the first or second lookup. + * Find every identity the seed controls, in candidate order. + * + * This deliberately scans all candidates rather than stopping at the first hit: + * one seed routinely controls several identities, and the first one found is + * not necessarily the one holding the username the user wants to move. * * Throws if no lookup ever completed, so an unreachable network is not reported * to the user as an unrecognised seed phrase. */ -export async function discoverIdentityFromCandidates( +export async function discoverIdentitiesFromCandidates( candidates: DerivedCandidateKey[], network: string, onProgress?: (checked: number, total: number) => void, retryOptions?: RetryOptions -): Promise { +): Promise { return withConnectedPlatformSdk( network, async (sdk) => { + const found: DiscoveredIdentity[] = []; + const seen = new Set(); let anyAnswered = false; let lastError: unknown; @@ -92,7 +96,14 @@ export async function discoverIdentityFromCandidates( const result = await findIdentityIdByPublicKeyHash(sdk, candidate.publicKeyHash); if (result.identityId) { - return { identityId: result.identityId, candidate }; + anyAnswered = true; + // Several derived keys belong to the same identity; keep the first + // candidate that reached it. + if (!seen.has(result.identityId)) { + seen.add(result.identityId); + found.push({ identityId: result.identityId, candidate }); + } + continue; } if (result.error === undefined) { anyAnswered = true; @@ -101,10 +112,10 @@ export async function discoverIdentityFromCandidates( } } - if (!anyAnswered && lastError !== undefined) { + if (found.length === 0 && !anyAnswered && lastError !== undefined) { throw lastError; } - return undefined; + return found; }, retryOptions ); @@ -113,22 +124,43 @@ export async function discoverIdentityFromCandidates( /** * List the usernames an identity owns. * - * Note this queries by `records.identity`, not `$ownerId` — the two are kept in - * sync by Platform on transfer, but the caller still re-checks `ownerId` on the - * document before signing rather than trusting this listing. + * Queries by `records.identity` because that is the indexed field — `$ownerId` + * is not indexed on the DPNS `domain` type, so Drive rejects a where clause on + * it. Platform keeps the two in sync on transfer, and the caller re-checks + * `ownerId` on the document before signing regardless. */ export async function listOwnedUsernames( identityId: string, network: string, retryOptions?: RetryOptions -): Promise { +): Promise { return withConnectedPlatformSdk( network, - (sdk) => - withRetry( - () => sdk.dpns.usernames({ identityId, limit: USERNAME_LIST_LIMIT }), + async (sdk) => { + const documents = await withRetry( + () => sdk.documents.query({ + dataContractId: DPNS_CONTRACT_ID, + documentTypeName: DPNS_DOCUMENT_TYPE, + where: [['records.identity', '==', identityId]], + limit: USERNAME_LIST_LIMIT, + }), retryOptions - ), + ); + + const owned: OwnedUsername[] = []; + for (const document of documents.values()) { + if (!document) continue; + const label = document.properties?.label; + const parent = document.properties?.normalizedParentDomainName; + if (typeof label !== 'string' || typeof parent !== 'string') continue; + owned.push({ + username: `${label}.${parent}`, + documentId: document.id.toString(), + ownerId: document.ownerId.toString(), + }); + } + return owned; + }, retryOptions ); } @@ -212,7 +244,10 @@ async function readDomainOwnership( } export interface TransferUsernameParams { + /** Display name, used only for messages. */ username: string; + /** The DPNS domain document that backs the name. */ + documentId: string; identityId: string; publicKeyId: number; privateKeyWif: string; @@ -230,7 +265,7 @@ export async function transferUsername( params: TransferUsernameParams, retryOptions?: RetryOptions ): Promise { - const { username, identityId, publicKeyId, privateKeyWif, recipientId, network } = params; + const { username, documentId, identityId, publicKeyId, privateKeyWif, recipientId, network } = params; if (identityId === recipientId) { return { success: false, error: 'Cannot transfer a username to the identity that already owns it' }; @@ -239,29 +274,19 @@ export async function transferUsername( return withConnectedPlatformSdk( network, async (sdk) => { - const info = await withRetry(() => sdk.dpns.getUsernameByName(username), retryOptions); - if (!info) { - return { success: false, error: `Username "${username}" was not found on ${network}` }; - } - - const documentId = info.documentId.toString(); - if (info.identityId.toString() !== identityId) { - return { - success: false, - error: `"${username}" is owned by ${info.identityId.toString()}, not ${identityId}`, - }; - } - + // The document id comes from the listing rather than a name lookup: + // dpns.getUsernameByName matches on the homograph-folded normalizedLabel, + // so any name containing l, i or o would not resolve from its display form. const document = await withRetry( () => sdk.documents.get(DPNS_CONTRACT_ID, DPNS_DOCUMENT_TYPE, documentId), retryOptions ); if (!document) { - return { success: false, error: `Domain document ${documentId} could not be fetched` }; + return { success: false, error: `The document behind "${username}" could not be fetched` }; } - // Re-check ownership on the document itself. `getUsernameByName` reports - // the owner, but this is the object we are about to sign over. + // Re-check ownership on the document we are about to sign over, rather + // than trusting the listing it came from. if (document.ownerId.toString() !== identityId) { return { success: false, diff --git a/src/types.ts b/src/types.ts index 18ab1b8..bbcb7bc 100644 --- a/src/types.ts +++ b/src/types.ts @@ -130,6 +130,17 @@ export interface IdentityPublicKeyInfo { */ export type UsernameTransferCredentialSource = 'seed' | 'key'; +/** + * A username an identity owns, paired with the DPNS document that backs it. + * The document id is carried so the transfer never has to resolve a display + * name back to a document. + */ +export interface OwnedUsername { + username: string; + documentId: string; + ownerId: string; +} + /** * Outcome of a username transfer attempt. */ @@ -413,7 +424,9 @@ export interface BridgeState { /** Transfer: validated signing key */ xferSigningKeyInfo?: { keyId: number; securityLevel: number }; /** Transfer: usernames owned by the source identity */ - xferOwnedUsernames?: string[]; + xferOwnedUsernames?: OwnedUsername[]; + /** Transfer: other identities the seed controls, if it found more than one */ + xferOtherIdentities?: string[]; /** Transfer: the username selected for transfer */ xferSelectedUsername?: string; /** Transfer: destination identity ID */ diff --git a/src/ui/components.ts b/src/ui/components.ts index 25f6404..8a79add 100644 --- a/src/ui/components.ts +++ b/src/ui/components.ts @@ -2891,10 +2891,19 @@ function renderXferSelectUsernameStep(state: BridgeState): HTMLElement { list.className = 'xfer-username-list'; if (usernames.length === 0) { - list.innerHTML = `

This identity does not own any usernames.

`; + const others = state.xferOtherIdentities?.length ?? 0; + list.innerHTML = ` +
+

This identity does not own any usernames.

+

There is nothing to transfer from it.${ + others > 0 + ? ` Your seed also controls ${others} other ${others === 1 ? 'identity' : 'identities'}, none of which own a name either.` + : '' + } Go back to pick a different identity, or register a username for this one first.

+
`; } else { list.innerHTML = usernames - .map((username) => ` + .map(({ username }) => `