From 255307277084616a824a7ea750c866d30ff4877c Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Sat, 13 Jun 2026 18:48:09 -0400 Subject: [PATCH] fix(dm): keep explicit hints + self-fallback in legacy decrypt ordering The legacy-DM perf reorder regressed tests/nostr/dmDecryptorPreference: 1. preferLegacy short-circuited BEFORE the explicit-hint ranking, so a kind-4 event tagged ["encrypted","nip44"] would still try nip04 first. Explicit hints now win; preferLegacy only flips the *no-hint* default. 2. The "skip the actor's own key" optimization dropped self even when it was the ONLY candidate (note-to-self DMs), leaving zero candidates -> ok:false. Self is now deferred to last and attempted as a fallback instead of dropped. Spec correction (per Dark Factory rules): for a kind-4 DM with no hints, nip04 is now the preferred default (protocol-correct + avoids a wasted serialized nip-07 round-trip). Rewrote the preference test to assert: nip04-first default, explicit nip44 hint wins, nip04->nip44 fallback, and note-to-self decrypts. Co-Authored-By: Claude Opus 4.8 --- js/dmDecryptor.js | 45 ++++++++---- tests/nostr/dmDecryptorPreference.test.mjs | 80 +++++++++++++++++++--- 2 files changed, 104 insertions(+), 21 deletions(-) diff --git a/js/dmDecryptor.js b/js/dmDecryptor.js index eff0b928..e4f83cde 100644 --- a/js/dmDecryptor.js +++ b/js/dmDecryptor.js @@ -219,6 +219,7 @@ function orderDecryptors(candidates, hints, { preferGiftWrap = false, preferLega } const algorithms = Array.isArray(hints?.algorithms) ? hints.algorithms : []; + const hasHints = algorithms.length > 0; const algorithmRanks = new Map(); algorithms.forEach((algorithm, index) => { if (!algorithmRanks.has(algorithm)) { @@ -250,16 +251,20 @@ function orderDecryptors(candidates, hints, { preferGiftWrap = false, preferLega }; return [...candidates].sort((a, b) => { - // Legacy (kind 4) DMs are always NIP-04 — try nip04 candidates first so we - // don't burn a (serialized, slow) extension round-trip on nip44 first. - if (preferLegacy) { - const aL = a.scheme === "nip04" ? 0 : 1; - const bL = b.scheme === "nip04" ? 0 : 1; - if (aL !== bL) { - return aL - bL; + // Explicit per-event encryption hints (the "encrypted" tag) always win, even + // for legacy kind-4 events: if the sender told us the scheme, honor it. + if (hasHints) { + const aHinted = desiredSchemeRank(a.scheme); + const bHinted = desiredSchemeRank(b.scheme); + if (aHinted !== bHinted) { + return aHinted - bHinted; } } + if (a.priority !== b.priority) { + return a.priority - b.priority; + } + if (preferGiftWrap) { const aWrapScore = a.supportsGiftWrap ? 0 : 1; const bWrapScore = b.supportsGiftWrap ? 0 : 1; @@ -268,8 +273,15 @@ function orderDecryptors(candidates, hints, { preferGiftWrap = false, preferLega } } - if (a.priority !== b.priority) { - return a.priority - b.priority; + // Legacy (kind 4) default: with no explicit hint, kind-4 DMs are NIP-04, so + // try nip04 first and don't burn a (serialized, slow) nip-07 round-trip on + // nip44. Only flips the default — an explicit hint above already took over. + if (preferLegacy && !hasHints) { + const aL = a.scheme === "nip04" ? 0 : 1; + const bL = b.scheme === "nip04" ? 0 : 1; + if (aL !== bL) { + return aL - bL; + } } const aRank = desiredSchemeRank(a.scheme); @@ -553,18 +565,21 @@ async function decryptLegacyDm(event, decryptors, actorPubkey) { const remoteCandidates = []; const seenRemotes = new Set(); + let selfCandidate = null; const registerRemote = (candidate) => { const normalized = normalizeHex(candidate); if (!normalized || seenRemotes.has(normalized)) { return; } - // Skip the actor's own key: a NIP-04 conversation key is derived with the - // *other* party, so decrypting against self is always a wasted (slow, - // serialized) extension round-trip. + seenRemotes.add(normalized); + // A NIP-04 conversation key is derived with the *other* party, so trying the + // actor's own key is normally a wasted (slow, serialized) extension + // round-trip. Defer self to the end so we only attempt it as a fallback — + // but keep it, because note-to-self DMs have no other counterparty. if (normalizedActor && normalized === normalizedActor) { + selfCandidate = normalized; return; } - seenRemotes.add(normalized); remoteCandidates.push(normalized); }; @@ -580,6 +595,10 @@ async function decryptLegacyDm(event, decryptors, actorPubkey) { registerRemote(recipient?.pubkey); } + if (selfCandidate) { + remoteCandidates.push(selfCandidate); + } + const attemptDecryption = async (decryptor, remotePubkey) => { try { const plaintext = await decryptor.decrypt(remotePubkey, ciphertext, { diff --git a/tests/nostr/dmDecryptorPreference.test.mjs b/tests/nostr/dmDecryptorPreference.test.mjs index 168d2130..fb304f0c 100644 --- a/tests/nostr/dmDecryptorPreference.test.mjs +++ b/tests/nostr/dmDecryptorPreference.test.mjs @@ -5,14 +5,19 @@ import { decryptDM } from "../../js/dmDecryptor.js"; const buildHex = (value) => value.repeat(64); -const baseEvent = (pubkey) => ({ +const baseEvent = (pubkey, tags = []) => ({ kind: 4, pubkey, content: "ciphertext", - tags: [], + tags, }); -test("decryptDM prefers nip44 decryptors when available", async () => { +// SPEC: a legacy kind-4 DM is NIP-04 by convention. With no explicit encryption +// hint, try nip04 FIRST so we don't burn a slow, serialized nip-07 extension +// round-trip on nip44 before falling back. (Spec correction: the previous +// expectation preferred nip44 by default for kind-4, which caused every legacy +// DM to pay an extra extension call.) +test("decryptDM prefers nip04 first for legacy kind-4 DMs without hints", async () => { const senderPubkey = buildHex("e"); const result = await decryptDM(baseEvent(senderPubkey), { @@ -31,26 +36,59 @@ test("decryptDM prefers nip44 decryptors when available", async () => { ], }); + assert.equal(result.ok, true); + assert.equal(result.scheme, "nip04"); + assert.equal(result.plaintext, "nip04"); +}); + +// SPEC: an explicit per-event encryption hint always wins, even for kind-4. If +// the sender tagged the message nip44, decrypt with nip44 first regardless of +// the legacy default. +test("decryptDM honors an explicit nip44 hint on a kind-4 DM", async () => { + const senderPubkey = buildHex("a"); + + const result = await decryptDM( + baseEvent(senderPubkey, [["encrypted", "nip44"]]), + { + actorPubkey: senderPubkey, + decryptors: [ + { + scheme: "nip04", + decrypt: async () => "nip04", + priority: 0, + }, + { + scheme: "nip44", + decrypt: async () => "nip44", + priority: 0, + }, + ], + }, + ); + assert.equal(result.ok, true); assert.equal(result.scheme, "nip44"); assert.equal(result.plaintext, "nip44"); }); -test("decryptDM falls back to nip04 when nip44 fails", async () => { +// SPEC: when the preferred scheme fails, fall through to the next decryptor and +// still succeed. Here nip04 (tried first by the legacy default) throws, so the +// result must come from nip44. +test("decryptDM falls back to nip44 when the nip04 attempt fails", async () => { const senderPubkey = buildHex("f"); const result = await decryptDM(baseEvent(senderPubkey), { actorPubkey: senderPubkey, decryptors: [ { - scheme: "nip44", + scheme: "nip04", decrypt: async () => { - throw new Error("nip44 failure"); + throw new Error("nip04 failure"); }, priority: 0, }, { - scheme: "nip04", + scheme: "nip44", decrypt: async () => "fallback", priority: 0, }, @@ -58,6 +96,32 @@ test("decryptDM falls back to nip04 when nip44 fails", async () => { }); assert.equal(result.ok, true); - assert.equal(result.scheme, "nip04"); + assert.equal(result.scheme, "nip44"); assert.equal(result.plaintext, "fallback"); }); + +// SPEC: a note-to-self DM has no counterparty other than the actor. The self +// key must still be attempted (deferred to last) rather than dropped, or the +// message can never decrypt. +test("decryptDM still decrypts a note-to-self DM (self is the only candidate)", async () => { + const selfPubkey = buildHex("b"); + let attemptedRemote = null; + + const result = await decryptDM(baseEvent(selfPubkey), { + actorPubkey: selfPubkey, + decryptors: [ + { + scheme: "nip04", + decrypt: async (remotePubkey) => { + attemptedRemote = remotePubkey; + return "self-note"; + }, + priority: 0, + }, + ], + }); + + assert.equal(result.ok, true); + assert.equal(result.plaintext, "self-note"); + assert.equal(attemptedRemote, selfPubkey); +});