mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
publish: all-timeout content publishes are unconfirmed soft successes (#49)
signAndPublishEvent (video publish/edit/repost/mirror), rebroadcastEvent, and
revertVideo now use assertAnyRelayAcceptedOrUnconfirmed: when every relay
times out without rejecting, the event was sent and almost always persisted,
so it no longer surfaces as "Failed to share video". Explicit rejections
still throw.
Messaging stays honest at every consumer:
- relay tally carries `unconfirmed` (now attached on edits too);
describePublishOutcome maps 0-accepted+unconfirmed to a WARNING ("sent,
no relay confirmed yet") — never a fake relay count, never a hard error
- edit/revert controllers say "sent, not confirmed yet" when unconfirmed
- deletes stay CONSERVATIVE: "deleted successfully" only when >=1 relay
ACKed a tombstone or kind-5 (anyRelayAcceptedInSummaries); an unconfirmed
revert no longer aborts deleteAllVersions midway, so the kind-5 always runs
Tests: tests/relay-ack-unconfirmed-content.test.mjs (6).
Co-Authored-By: Claude Fable 5 <[email protected]>
This commit is contained in:
1 parent
e32298cc89
commit
1934f9386a
7 files changed
+286
-14
No files matched your search
@@ -92,6 +92,7 @@ import {
|
||||
assertAnyRelayAcceptedOrUnconfirmed,
|
||||
describePublishOutcome,
|
||||
readRelayPublishSummary,
|
||||
anyRelayAcceptedInSummaries,
|
||||
} from "./nostrPublish.js";
|
||||
import {
|
||||
getActiveSigner,
|
||||
@@ -5073,7 +5074,7 @@ class Application {
|
||||
this.showStatus("Deleting. Please wait.", { showSpinner: true });
|
||||
|
||||
try {
|
||||
await this.nostrService.handleFullDeleteVideo({
|
||||
const deleteDetail = await this.nostrService.handleFullDeleteVideo({
|
||||
videoRootId: rootId,
|
||||
video: targetVideo,
|
||||
pubkey: this.pubkey,
|
||||
@@ -5110,7 +5111,21 @@ class Application {
|
||||
}
|
||||
|
||||
await this.loadVideos();
|
||||
this.showSuccess("All versions deleted successfully!");
|
||||
// #49 conservative delete semantics: only claim "deleted" when at least
|
||||
// one relay ACKed something. An all-timeout run (tombstones + kind-5 sent
|
||||
// but unconfirmed) must not read as a confirmed delete — the video may
|
||||
// linger on relays until they catch up.
|
||||
const deleteSummaries = [
|
||||
...(deleteDetail?.result?.reverts || []),
|
||||
...(deleteDetail?.result?.deletes || []),
|
||||
].map((entry) => entry?.summary);
|
||||
if (anyRelayAcceptedInSummaries(deleteSummaries)) {
|
||||
this.showSuccess("All versions deleted successfully!");
|
||||
} else {
|
||||
this.showSuccess(
|
||||
"Delete request sent, but no relay has confirmed it yet — the video may reappear until relays catch up. Check again in a bit.",
|
||||
);
|
||||
}
|
||||
if (storageLockedRemnant) {
|
||||
this.showStatus(
|
||||
"Heads up: the hosted file is still in your storage bucket because storage is locked. Unlock storage and delete again, or remove it from your bucket manually.",
|
||||
|
||||
+16
-3
@@ -111,7 +111,7 @@ import "./maxListenerDiagnostics.js";
|
||||
import {
|
||||
publishEventToRelay,
|
||||
publishEventToRelays,
|
||||
assertAnyRelayAccepted,
|
||||
assertAnyRelayAcceptedOrUnconfirmed,
|
||||
attachRelayPublishSummary,
|
||||
} from "../nostrPublish.js";
|
||||
import {
|
||||
@@ -2960,13 +2960,17 @@ export class NostrClient {
|
||||
await this.ensureActiveSignerForPubkey(userPubkeyLower);
|
||||
|
||||
try {
|
||||
const { signedEvent } = await this.signAndPublishEvent(context.event, {
|
||||
const { signedEvent, summary } = await this.signAndPublishEvent(context.event, {
|
||||
context: "edited video note",
|
||||
logName: "Edited video note",
|
||||
devLogLabel: "edited video note",
|
||||
resolveActiveSigner: (p) => this.signerManager.resolveActiveSigner(p),
|
||||
});
|
||||
|
||||
// Same tally as publishVideo so the edit UI can tell an unconfirmed
|
||||
// (all-timeout, #49) update apart from a confirmed one.
|
||||
attachRelayPublishSummary(signedEvent, summary);
|
||||
|
||||
await handlePublishNip94(this, signedEvent, context);
|
||||
await handlePublishNip71(this, signedEvent, context);
|
||||
|
||||
@@ -3216,7 +3220,10 @@ export class NostrClient {
|
||||
|
||||
let publishSummary;
|
||||
try {
|
||||
publishSummary = assertAnyRelayAccepted(publishResults, {
|
||||
// #49: an all-timeout tombstone publish is an unconfirmed soft success
|
||||
// (summary.unconfirmed=true) so a slow-ack revert/delete doesn't abort
|
||||
// midway; explicit relay rejections still throw below.
|
||||
publishSummary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, {
|
||||
context: "video revert",
|
||||
});
|
||||
} catch (publishError) {
|
||||
@@ -3237,6 +3244,12 @@ export class NostrClient {
|
||||
devLogger.log(`Revert event published to ${url}`)
|
||||
);
|
||||
|
||||
if (publishSummary.unconfirmed) {
|
||||
userLogger.warn(
|
||||
"[nostr] Video revert was sent but no relay confirmed it within the timeout — proceeding as unconfirmed.",
|
||||
);
|
||||
}
|
||||
|
||||
if (publishSummary.failed.length) {
|
||||
publishSummary.failed.forEach(({ url, error: relayError }) => {
|
||||
const reason =
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { ENSURE_PRESENCE_REBROADCAST_COOLDOWN_SECONDS } from "../config.js";
|
||||
import {
|
||||
publishEventToRelays,
|
||||
assertAnyRelayAccepted,
|
||||
assertAnyRelayAcceptedOrUnconfirmed,
|
||||
summarizePublishResults,
|
||||
} from "../nostrPublish.js";
|
||||
import {
|
||||
@@ -469,7 +469,10 @@ export async function signAndPublishEvent({
|
||||
|
||||
let publishSummary;
|
||||
try {
|
||||
publishSummary = assertAnyRelayAccepted(publishResults, { context });
|
||||
// #49: an all-timeout publish (sent, nothing rejected) is treated as an
|
||||
// unconfirmed soft success — summary.unconfirmed=true — instead of a false
|
||||
// "failed to publish". Explicit relay rejections still throw below.
|
||||
publishSummary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, { context });
|
||||
} catch (publishError) {
|
||||
if (publishError?.relayFailures?.length) {
|
||||
const logLevel = rejectionLogLevel === "warn" ? "warn" : "error";
|
||||
@@ -485,6 +488,12 @@ export async function signAndPublishEvent({
|
||||
devLogger.log(`${logName} published to ${url}`);
|
||||
});
|
||||
|
||||
if (publishSummary.unconfirmed) {
|
||||
userLogger.warn(
|
||||
`[nostr] ${logName} was sent but no relay confirmed it within the timeout — treating as an unconfirmed publish.`,
|
||||
);
|
||||
}
|
||||
|
||||
if (publishSummary.failed.length) {
|
||||
publishSummary.failed.forEach(({ url, error: relayError }) => {
|
||||
const reason =
|
||||
@@ -1331,7 +1340,11 @@ export async function rebroadcastEvent({ client, eventId, options = {} }) {
|
||||
const publishResults = await publishEventToRelays(client?.pool, relays, rawEvent);
|
||||
|
||||
try {
|
||||
const summary = assertAnyRelayAccepted(publishResults, { context: "rebroadcast" });
|
||||
// #49: rebroadcast re-publishes an existing event, so an all-timeout is a
|
||||
// safe unconfirmed success; only explicit rejections fall to the catch.
|
||||
const summary = assertAnyRelayAcceptedOrUnconfirmed(publishResults, {
|
||||
context: "rebroadcast",
|
||||
});
|
||||
return {
|
||||
ok: true,
|
||||
rebroadcast: true,
|
||||
|
||||
+41
-2
@@ -278,9 +278,16 @@ export function attachRelayPublishSummary(event, summary) {
|
||||
}
|
||||
const accepted = Array.isArray(summary?.accepted) ? summary.accepted.length : 0;
|
||||
const failed = Array.isArray(summary?.failed) ? summary.failed.length : 0;
|
||||
// #49: carry the unconfirmed flag (all relays timed out, nothing rejected)
|
||||
// so the outcome describer can say "sent, unconfirmed" instead of lying in
|
||||
// either direction ("published to N relays" / "failed").
|
||||
const tally =
|
||||
summary?.unconfirmed === true
|
||||
? { accepted, total: accepted + failed, unconfirmed: true }
|
||||
: { accepted, total: accepted + failed };
|
||||
try {
|
||||
Object.defineProperty(event, RELAY_PUBLISH_SUMMARY_KEY, {
|
||||
value: { accepted, total: accepted + failed },
|
||||
value: tally,
|
||||
enumerable: false,
|
||||
configurable: true,
|
||||
writable: true,
|
||||
@@ -308,7 +315,11 @@ export function readRelayPublishSummary(event) {
|
||||
* @param {{ accepted?: number, total?: number }} tally
|
||||
* @returns {{ tone: "success"|"warning"|"error", message: string }}
|
||||
*/
|
||||
export function describePublishOutcome({ accepted = 0, total = 0 } = {}) {
|
||||
export function describePublishOutcome({
|
||||
accepted = 0,
|
||||
total = 0,
|
||||
unconfirmed = false,
|
||||
} = {}) {
|
||||
const a = Number.isFinite(accepted) ? Math.max(0, Math.floor(accepted)) : 0;
|
||||
const t = Number.isFinite(total) ? Math.max(0, Math.floor(total)) : 0;
|
||||
|
||||
@@ -317,6 +328,16 @@ export function describePublishOutcome({ accepted = 0, total = 0 } = {}) {
|
||||
return { tone: "success", message: "Video shared successfully!" };
|
||||
}
|
||||
if (a <= 0) {
|
||||
// #49: every relay timed out without rejecting — the event was sent and
|
||||
// almost always persisted (it shows up after a refresh). Say exactly that:
|
||||
// neither a hard failure nor a fake "published to N relays".
|
||||
if (unconfirmed === true) {
|
||||
return {
|
||||
tone: "warning",
|
||||
message:
|
||||
"Video sent, but no relay has confirmed receiving it yet. It should appear shortly — refresh in a moment to double-check.",
|
||||
};
|
||||
}
|
||||
return {
|
||||
tone: "error",
|
||||
message:
|
||||
@@ -335,6 +356,24 @@ export function describePublishOutcome({ accepted = 0, total = 0 } = {}) {
|
||||
};
|
||||
}
|
||||
|
||||
// #49 delete semantics: a delete flow publishes several events (tombstone
|
||||
// reverts + kind-5 chunks) and the UI must NOT claim "deleted" unless at least
|
||||
// one relay actually ACKed something — a false "deleted" is worse than a false
|
||||
// "failed". An empty list (nothing needed publishing) counts as confirmed.
|
||||
export function anyRelayAcceptedInSummaries(summaries = []) {
|
||||
const list = Array.isArray(summaries) ? summaries.filter(Boolean) : [];
|
||||
if (!list.length) {
|
||||
return true;
|
||||
}
|
||||
return list.some((summary) => {
|
||||
const accepted = summary?.accepted;
|
||||
if (Array.isArray(accepted)) {
|
||||
return accepted.length > 0;
|
||||
}
|
||||
return Number.isFinite(accepted) && accepted > 0;
|
||||
});
|
||||
}
|
||||
|
||||
// A RelayPublishError whose per-relay failures are ALL "publish timeout" means the
|
||||
// event was sent but not ACKed in the window — NOT that any relay rejected it. For
|
||||
// an idempotent, replaceable event this almost always persisted (users see it
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { devLogger } from "../utils/logger.js";
|
||||
import r2Service from "../services/r2Service.js";
|
||||
import { readRelayPublishSummary } from "../nostrPublish.js";
|
||||
|
||||
export default class EditModalController {
|
||||
constructor({ services, state, ui, callbacks, helpers }) {
|
||||
@@ -86,7 +87,7 @@ export default class EditModalController {
|
||||
}
|
||||
|
||||
try {
|
||||
await this.services.nostrService.handleEditVideoSubmit({
|
||||
const editedEvent = await this.services.nostrService.handleEditVideoSubmit({
|
||||
originalEvent,
|
||||
updatedData,
|
||||
pubkey,
|
||||
@@ -139,7 +140,16 @@ export default class EditModalController {
|
||||
videosMap.clear();
|
||||
}
|
||||
|
||||
this.ui.showSuccess("Video updated successfully!");
|
||||
// #49: an all-timeout publish reaches here flagged unconfirmed — say so
|
||||
// honestly instead of claiming a confirmed update.
|
||||
const tally = readRelayPublishSummary(editedEvent);
|
||||
if (tally?.unconfirmed === true && !(tally.accepted > 0)) {
|
||||
this.ui.showSuccess(
|
||||
"Video update sent, but no relay has confirmed it yet — it may take a moment to propagate.",
|
||||
);
|
||||
} else {
|
||||
this.ui.showSuccess("Video updated successfully!");
|
||||
}
|
||||
|
||||
if (editModal?.setSubmitState) {
|
||||
editModal.setSubmitState({ pending: false });
|
||||
|
||||
@@ -91,8 +91,12 @@ export default class RevertModalController {
|
||||
this.revertModal.setBusy(true, "Reverting…");
|
||||
|
||||
try {
|
||||
// #49: revertVideo no longer hard-fails when every relay merely times
|
||||
// out (summary.unconfirmed) — track whether ANY relay actually ACKed so
|
||||
// the success message stays honest.
|
||||
let anyAccepted = false;
|
||||
for (const entry of entries) {
|
||||
await this.services.nostrClient.revertVideo(
|
||||
const revertResult = await this.services.nostrClient.revertVideo(
|
||||
{
|
||||
id: entry.id,
|
||||
pubkey: entry.pubkey,
|
||||
@@ -100,12 +104,22 @@ export default class RevertModalController {
|
||||
},
|
||||
pubkey,
|
||||
);
|
||||
const accepted = revertResult?.summary?.accepted;
|
||||
if (Array.isArray(accepted) && accepted.length > 0) {
|
||||
anyAccepted = true;
|
||||
}
|
||||
}
|
||||
|
||||
await this.callbacks.loadVideos();
|
||||
|
||||
const timestampLabel = this.helpers.formatAbsoluteTimestamp(target.created_at);
|
||||
this.ui.showSuccess(`Reverted to revision from ${timestampLabel}.`);
|
||||
if (anyAccepted) {
|
||||
this.ui.showSuccess(`Reverted to revision from ${timestampLabel}.`);
|
||||
} else {
|
||||
this.ui.showSuccess(
|
||||
`Revert to the revision from ${timestampLabel} was sent, but no relay has confirmed it yet — check again in a moment.`,
|
||||
);
|
||||
}
|
||||
this.revertModal.close();
|
||||
this.callbacks.forceRefreshAllProfiles();
|
||||
} catch (err) {
|
||||
|
||||
@@ -0,0 +1,168 @@
|
||||
// #49: relay-ack false-negatives on the FLAGSHIP content flows. When no relay
|
||||
// ACKs within the publish window but nothing rejected (all-timeout), the event
|
||||
// was sent and almost always persisted — it must surface as an UNCONFIRMED soft
|
||||
// success, not "Failed to share video". Equally important: the outcome messaging
|
||||
// must stay honest — unconfirmed is never reported as "published to N relays",
|
||||
// and a delete is only "deleted" when at least one relay ACKed something.
|
||||
//
|
||||
// test_integrity_note:
|
||||
// change_type: ["new_tests"]
|
||||
// scenarios:
|
||||
// - id: SCN-content-publish-all-timeout
|
||||
// given: "a relay pool where every relay times out (reason 'publish timeout') vs one that rejects"
|
||||
// when: "signAndPublishEvent (video publish/edit path) runs"
|
||||
// then: "all-timeout resolves with summary.unconfirmed=true; an explicit rejection still throws RelayPublishError"
|
||||
// - id: SCN-honest-outcome-messaging
|
||||
// given: "relay tallies including the new unconfirmed flag"
|
||||
// when: "describePublishOutcome / attachRelayPublishSummary / anyRelayAcceptedInSummaries run"
|
||||
// then: "unconfirmed -> warning ('sent, not confirmed'), never a relay-count success and never a hard error; deletes only 'confirmed' with >=1 ACK"
|
||||
// observable_outcomes:
|
||||
// - "signAndPublishEvent resolves (no throw) on all-timeout with unconfirmed summary"
|
||||
// - "signAndPublishEvent still throws on explicit rejection"
|
||||
// - "unconfirmed tally round-trips through attach/readRelayPublishSummary"
|
||||
// - "describePublishOutcome(unconfirmed) is a warning with honest wording"
|
||||
// determinism_controls:
|
||||
// - "fake pool fires ok/failed handlers synchronously; no real sockets or timers race the 10s window"
|
||||
// anti_cheat_rationale:
|
||||
// prevents: ["hard-coded return value", "over-mocking internal logic", "snapshot rubber-stamping"]
|
||||
// relaxation:
|
||||
// did_relax_any_assertion: false
|
||||
|
||||
import test from "node:test";
|
||||
import { strict as assert } from "node:assert";
|
||||
import {
|
||||
RelayPublishError,
|
||||
describePublishOutcome,
|
||||
attachRelayPublishSummary,
|
||||
readRelayPublishSummary,
|
||||
anyRelayAcceptedInSummaries,
|
||||
} from "../js/nostrPublish.js";
|
||||
import { signAndPublishEvent } from "../js/nostr/publishHelpers.js";
|
||||
|
||||
const PUB = "a".repeat(64);
|
||||
|
||||
// Fake nostr-tools pool: publish([url]) returns a handle whose "failed" (or
|
||||
// "ok") handler fires immediately with the scripted outcome for that relay.
|
||||
function makePool(outcomes) {
|
||||
let call = 0;
|
||||
return {
|
||||
publish(urls, _event) {
|
||||
const outcome = outcomes[call % outcomes.length];
|
||||
call += 1;
|
||||
return {
|
||||
on(eventName, handler) {
|
||||
if (outcome === "ok" && eventName === "ok") {
|
||||
handler();
|
||||
} else if (outcome !== "ok" && eventName === "failed") {
|
||||
handler(new Error(outcome));
|
||||
}
|
||||
return this;
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const signer = {
|
||||
pubkey: PUB,
|
||||
signEvent: async (event) => ({ ...event, id: "signed-1", sig: "sig" }),
|
||||
};
|
||||
|
||||
function publishWith(pool) {
|
||||
return signAndPublishEvent({
|
||||
client: {
|
||||
pubkey: PUB,
|
||||
pool,
|
||||
relays: ["wss://a.example", "wss://b.example"],
|
||||
writeRelays: ["wss://a.example", "wss://b.example"],
|
||||
},
|
||||
event: { kind: 30078, pubkey: PUB, created_at: 100, tags: [], content: "{}" },
|
||||
resolveActiveSigner: () => signer,
|
||||
shouldRequestExtensionPermissions: () => false,
|
||||
options: { context: "video note", logName: "Video note" },
|
||||
});
|
||||
}
|
||||
|
||||
test("all-timeout video publish resolves as an unconfirmed soft success", async () => {
|
||||
const result = await publishWith(makePool(["publish timeout", "publish timeout"]));
|
||||
assert.equal(result.signedEvent.id, "signed-1");
|
||||
assert.equal(result.summary.unconfirmed, true, "flagged unconfirmed");
|
||||
assert.equal(result.summary.accepted.length, 0, "no fake acceptances");
|
||||
assert.equal(result.summary.failed.length, 2, "both timeouts recorded");
|
||||
});
|
||||
|
||||
test("an explicit relay rejection still hard-fails the publish", async () => {
|
||||
await assert.rejects(
|
||||
publishWith(makePool(["publish timeout", "blocked: spam"])),
|
||||
RelayPublishError,
|
||||
);
|
||||
});
|
||||
|
||||
test("a confirmed ack stays a confirmed (not unconfirmed) success", async () => {
|
||||
const result = await publishWith(makePool(["ok", "publish timeout"]));
|
||||
assert.equal(result.summary.unconfirmed, false);
|
||||
assert.equal(result.summary.accepted.length, 1);
|
||||
});
|
||||
|
||||
test("the attached tally carries the unconfirmed flag to the UI", () => {
|
||||
const event = { id: "e1", kind: 30078 };
|
||||
attachRelayPublishSummary(event, {
|
||||
accepted: [],
|
||||
failed: [{ url: "wss://a" }, { url: "wss://b" }],
|
||||
unconfirmed: true,
|
||||
});
|
||||
assert.deepEqual(readRelayPublishSummary(event), {
|
||||
accepted: 0,
|
||||
total: 2,
|
||||
unconfirmed: true,
|
||||
});
|
||||
|
||||
// Confirmed publishes keep the exact legacy tally shape (no stray flag).
|
||||
const confirmed = { id: "e2" };
|
||||
attachRelayPublishSummary(confirmed, {
|
||||
accepted: [{ url: "wss://a" }],
|
||||
failed: [],
|
||||
});
|
||||
assert.deepEqual(readRelayPublishSummary(confirmed), { accepted: 1, total: 1 });
|
||||
});
|
||||
|
||||
test("describePublishOutcome: unconfirmed is an honest warning, not success or error", () => {
|
||||
const out = describePublishOutcome({ accepted: 0, total: 3, unconfirmed: true });
|
||||
assert.equal(out.tone, "warning");
|
||||
assert.match(out.message, /no relay has confirmed/i);
|
||||
assert.doesNotMatch(out.message, /\d+ relays?/, "must not fake a relay count");
|
||||
assert.doesNotMatch(out.message, /could not be published/i);
|
||||
|
||||
// Without the flag, zero acceptance is still the hard error.
|
||||
assert.equal(describePublishOutcome({ accepted: 0, total: 3 }).tone, "error");
|
||||
// A real acceptance is unaffected by a stray flag.
|
||||
assert.equal(
|
||||
describePublishOutcome({ accepted: 2, total: 3, unconfirmed: false }).tone,
|
||||
"warning",
|
||||
);
|
||||
assert.equal(describePublishOutcome({ accepted: 3, total: 3 }).tone, "success");
|
||||
});
|
||||
|
||||
test("delete confirmation: only >=1 relay ACK counts as a confirmed delete", () => {
|
||||
const unconfirmedSummary = { accepted: [], failed: [{ url: "wss://a" }], unconfirmed: true };
|
||||
const confirmedSummary = { accepted: [{ url: "wss://b" }], failed: [] };
|
||||
|
||||
assert.equal(
|
||||
anyRelayAcceptedInSummaries([unconfirmedSummary, unconfirmedSummary]),
|
||||
false,
|
||||
"all-timeout delete must NOT be reported as deleted",
|
||||
);
|
||||
assert.equal(
|
||||
anyRelayAcceptedInSummaries([unconfirmedSummary, confirmedSummary]),
|
||||
true,
|
||||
);
|
||||
assert.equal(
|
||||
anyRelayAcceptedInSummaries([]),
|
||||
true,
|
||||
"nothing needed publishing counts as confirmed",
|
||||
);
|
||||
assert.equal(anyRelayAcceptedInSummaries([null, undefined]), true);
|
||||
// Numeric tally shape (from readRelayPublishSummary) also works.
|
||||
assert.equal(anyRelayAcceptedInSummaries([{ accepted: 0, total: 2 }]), false);
|
||||
assert.equal(anyRelayAcceptedInSummaries([{ accepted: 1, total: 2 }]), true);
|
||||
});
|
||||
Reference in new issue
Block a user