Build 34: harden the nostr ingest path (audit Medium items)

- N-F1: commit the wrap/rumor/slate dedup markers immediately after the
  durable receive/finalize, before the reply+sync tail, so a crash there
  can't re-trigger the action on catch-up. (grin + decide() already
  backstopped it; this closes the window cleanly.)
- N-F3: prune the processed-dedup store hourly in the heartbeat, not only
  at startup — a long-lived session could otherwise grow it unbounded
  under fresh-keypair spam since the 30-day TTL never re-applied.
- N-F2: kept Created/SendFailed in the finalize allow-set (removing them
  would strand a real send whose S1 reached the peer before we persisted
  AwaitingS2) and documented why it is not a forgery vector; added a test.
- Update replay_check e2e: a same-pubkey second register is now blocked by
  the name-change cooldown (fires before the one-name rule); accept either.

Validated: 35 lib tests + live nip17_slatepack_roundtrip + replay_check green.
This commit is contained in:
2ro
2026-06-12 12:48:34 -04:00
parent 60e4e8b5a9
commit 878f7728eb
3 changed files with 56 additions and 3 deletions
+22
View File
@@ -450,6 +450,10 @@ async fn run_service(svc: Arc<NostrService>, wallet: Wallet) {
svc.store.prune_processed();
let mut notifications = client.notifications();
// Re-run TTL pruning periodically, not just at startup: a session that
// never restarts would otherwise let the processed-dedup store grow
// unbounded under fresh-keypair spam (the 30-day TTL never applied).
let mut last_prune = unix_time();
loop {
if svc.shutdown.load(Ordering::SeqCst) || !wallet.is_open() {
break;
@@ -473,6 +477,10 @@ async fn run_service(svc: Arc<NostrService>, wallet: Wallet) {
let connected = client.relays().await.values()
.any(|r| r.status() == RelayStatus::Connected);
svc.connected.store(connected, Ordering::Relaxed);
if unix_time() - last_prune >= 3600 {
svc.store.prune_processed();
last_prune = unix_time();
}
}
}
}
@@ -704,6 +712,13 @@ async fn handle_wrap(svc: &Arc<NostrService>, wallet: &Wallet, client: &Client,
created_at: now,
updated_at: now,
});
// Commit dedup markers now the receive is durable, BEFORE
// the reply + sync tail. A crash there must not let this
// wrap re-trigger a second receive on catch-up (decide()
// and grin's TransactionAlreadyReceived also backstop it).
svc.store.mark_processed(&wrap_id);
svc.store.mark_processed(&rumor_id);
svc.store.mark_processed(&slate_marker);
match svc
.send_payment_dm(&sender_hex, &reply_text, None, &[])
.await
@@ -744,6 +759,13 @@ async fn handle_wrap(svc: &Arc<NostrService>, wallet: &Wallet, client: &Client,
Ok(()) => {
svc.store
.update_tx_status(&slate.id.to_string(), NostrSendStatus::Finalized);
// Finalize+post committed; mark dedup before the sync tail so a
// crash can't re-finalize on catch-up (grin rejects a second
// finalize and the meta is now Finalized, which decide() drops —
// this just avoids the redundant attempt).
svc.store.mark_processed(&wrap_id);
svc.store.mark_processed(&rumor_id);
svc.store.mark_processed(&slate_marker);
if let Some(mut contact) = svc.store.contact(&sender_hex) {
contact.last_paid_at = Some(unix_time());
svc.store.save_contact(&contact);
+24
View File
@@ -80,6 +80,16 @@ pub fn decide(ctx: &IngestContext) -> IngestDecision {
AcceptPolicy::Ask => IngestDecision::SurfaceIncoming,
}
}
// Standard2 is the counterparty's reply to a send WE initiated. The
// status allow-set below includes Created/SendFailed (not just
// AwaitingS2) on purpose: our send records intent as Created BEFORE
// dispatch and only flips to AwaitingS2 after a relay accepts S1, so
// a crash in that gap leaves a legitimate pending send marked
// Created/SendFailed even though the counterparty did receive S1.
// This is NOT a forgery vector: a finalizing S2 must carry our S1
// partial signature over our locked outputs, which the counterparty
// can only have if we actually sent it. Sender + tx_exists are still
// required, and grin rejects finalizing an already-finalized tx.
SlateState::Standard2 => match ctx.meta {
Some(meta)
if meta.direction == NostrTxDirection::Sent
@@ -251,6 +261,20 @@ mod tests {
assert!(matches!(decide(&c), IngestDecision::Drop(_)));
}
#[test]
fn s2_finalizes_from_pre_dispatch_states() {
// Created/SendFailed are deliberately accepted: a crash between
// relay-accept and the AwaitingS2 write must not strand a real send.
for status in [NostrSendStatus::Created, NostrSendStatus::SendFailed] {
let m = meta(NostrTxDirection::Sent, status, ALICE);
let c = ctx(SlateState::Standard2, 100, ALICE, Some(&m), true);
assert_eq!(decide(&c), IngestDecision::FinalizePost);
// Still bound to the counterparty: a stranger's S2 drops.
let c2 = ctx(SlateState::Standard2, 100, MALLORY, Some(&m), true);
assert!(matches!(decide(&c2), IngestDecision::Drop(_)));
}
}
#[test]
fn i1_never_pays_automatically() {
// Even from a contact under the most permissive policy.
+10 -3
View File
@@ -56,7 +56,12 @@ async fn replay_and_double_name_rejected() {
"replay should be rejected, got: {r2}"
);
// Second DISTINCT name with a FRESH signature but same pubkey -> one-name rule.
// Second DISTINCT name with a FRESH signature but same pubkey -> blocked.
// Two protections can fire here: the per-pubkey name-change cooldown (one
// change per 10 min, which the just-completed register of name_a triggers)
// and the one-active-name-per-pubkey rule. The cooldown is checked first, so
// within 10 min of a successful register a same-pubkey second register is
// rejected with name_change_cooldown; either rejection is a valid block.
let name_b = format!("s{}", &pk[..7]);
let body_b = serde_json::json!({"name": name_b, "pubkey": pk}).to_string();
let ph_b = hex::encode(Sha256::digest(body_b.as_bytes()));
@@ -73,8 +78,10 @@ async fn replay_and_double_name_rejected() {
let r3 = post(&auth_b, &body_b);
println!("2nd name: {r3}");
assert!(
r3.contains("already has a name") || r3.contains("pubkey already"),
"one-name rule should block, got: {r3}"
r3.contains("already has a name")
|| r3.contains("pubkey already")
|| r3.contains("name_change_cooldown"),
"a same-pubkey second name should be blocked (one-name rule or cooldown), got: {r3}"
);
// Cleanup name A.