Skip to content

sdk: clean up interrupted reconciliation and fail rejected downloads - #1480

Closed
erskingardner wants to merge 4 commits into
nostrdevkit:masterfrom
erskingardner:codex/upstream-sdk-reconciliation
Closed

erskingardner wants to merge 4 commits into
nostrdevkit:masterfrom
erskingardner:codex/upstream-sdk-reconciliation

Conversation

@erskingardner

@erskingardner erskingardner commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Description

NIP-77 reconciliation can report success when a relay rejects a download REQ with CLOSED. Dropping or failing reconciliation can also leave its download subscription registered.

Add request-scoped cleanup that removes the download subscription and attempts CLOSE/NEG-CLOSE on interruption. Use a fresh subscription ID per download batch so late CLOSED replies cannot terminate the next batch. Track outstanding requested IDs within the current bounded batch and report rejection or premature closure as a sync failure.

Notes to the reviewers

This patch preserves the existing public result API. The separate partial-progress reporting API is deferred. Network cleanup is best effort after disconnection; unrelated subscriptions remain active. EOSE keeps its existing request-completion semantics.

The rejection and cancellation regression tests fail against the upstream sync implementation and pass with this patch. They cover rejection of the first and a later download batch, previously stored events surviving failure, and cancellation of a real pending download through the public API followed by delivery on an unrelated live subscription.

Validation: just fmt and the complete just precommit passed locally on the pinned toolchain, including SDK tests with default and local-relay features, the configured WASM checks, Clippy, and documentation checks.

Additional tests distinguish an empty CLOSED after all requested events arrived from one that omits requested events.

Checklist

  • I followed the contribution guidelines
  • I updated the relevant CHANGELOG.md (if applicable)
  • I understand and can explain all code in this PR

@codecov

codecov Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

yukibtc added a commit that referenced this pull request Sep 30, 2026
Replaces #1480

Co-authored-by: Yuki Kishimoto <yukikishimoto@protonmail.com>
Signed-off-by: Yuki Kishimoto <yukikishimoto@protonmail.com>
@yukibtc

yukibtc commented Sep 30, 2026

Copy link
Copy Markdown
Member

Thanks @erskingardner, I've cherry-picked, refined a couple of things, and merged at 381605f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants