Skip to content

fix: preserve signature notifications across transaction retries - #149

Merged
bmuddha merged 1 commit into
devfrom
fix/request-owned-completion
Sep 18, 2026
Merged

bmuddha merged 1 commit into
devfrom
fix/request-owned-completion

Conversation

@bmuddha

@bmuddha bmuddha commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

Resending a transaction can currently give a signature subscriber AlreadyProcessed instead of the transaction's execution result. This change keeps submission errors separate from signature notifications, so retries cannot consume observers.

The common schedule() path still allocates no completion channel. Publishing updates when nobody is subscribed now skips the subscription map entirely, for signatures as well as persistent streams.

Closes #148

Impact

execute() receives submission errors directly; schedule() remains queue-only. Duplicate and invalid-blockhash rejections no longer appear as execution status. Rejected signatures still retain their deduplication reservation until expiry.

MBV #1710 tracks the consumer-side work. Stored and replicated formats are unchanged.

Reviewer notes

A subscriber joining during commit must not fall between notification and status lookup. Registration happens before the lookup, and commit makes the result readable before notifying subscribers.

@bmuddha bmuddha added the bug Something isn't working label Sep 18, 2026
@bmuddha bmuddha self-assigned this Sep 18, 2026
@coderabbitai

coderabbitai Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 5b286f69-231b-41c2-b1ef-b19be4ec4372

📥 Commits

Reviewing files that changed from the base of the PR and between 39aacda and 4ace04b.

📒 Files selected for processing (17)
  • README.md
  • engine/README.md
  • engine/src/accessor.rs
  • engine/tests/transactions.rs
  • keeper/README.md
  • keeper/src/accessor.rs
  • keeper/src/subscriptions.rs
  • keeper/src/tests/recovery.rs
  • keeper/src/tests/subscriptions.rs
  • nucleus/README.md
  • nucleus/src/runtime.rs
  • processor/README.md
  • processor/src/executor.rs
  • processor/src/sequencer/mod.rs
  • processor/src/sequencer/order.rs
  • processor/src/sequencer/tests.rs
  • processor/src/tests.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change adds optional request-owned completion channels to transaction requests. execute receives admission or committed execution results, while schedule acknowledges queueing without a completion channel. Keeper separates admission rejection from signature-result fanout, caches committed results before notification, and updates subscription storage. Processor components propagate replies through ordering and execution. Tests and documentation cover retries, retained results, rejected transactions, and lease retention.

Priority: ➖ Normal

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 4ace0

The transaction reply and subscription changes appear internally consistent and ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: preserving signature notifications when transactions are retried.
Description check ✅ Passed The description directly explains the changes to retries, submission errors, signature notifications, schedule behavior, and subscription handling.
Linked Issues check ✅ Passed The changes satisfy the coding requirements in [#148]. execute() uses a request-owned oneshot reply for admission errors or the committed execution result. schedule() sends no reply channel and do…
Out of Scope Changes check ✅ Passed The changes remain within [#148]. Runtime request plumbing, sequencer handling, executor replies, subscription storage, deduplication tests, recovery updates, and documentation directly support reques…
Docstring Coverage ✅ Passed Docstring coverage is 96.15% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 52 functions across 12 files. (5 skipped: 5…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@bmuddha bmuddha changed the title fix: separate request completion from signature subscriptions fix: preserve signature notifications across transaction retries Sep 18, 2026
@bmuddha
bmuddha marked this pull request as ready for review September 18, 2026 11:43
@bmuddha
bmuddha merged commit 09cfada into dev Sep 18, 2026
15 checks passed
@bmuddha
bmuddha deleted the fix/request-owned-completion branch September 18, 2026 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Transaction retries can prevent subscribers from receiving the execution result

2 participants