[utils] Introduce Acknowledgement

[patrick-ogrady]

Replaces: #2202
Replaces: #2212

Instead of adding a custom Reporter impl, I think the most maintainable approach is to treat Acknowledgement as a first-party action.

[cloudflare-workers-and-pages]

Deploying monorepo with    Cloudflare Pages

Latest commit:	3897706
Status:	✅  Deploy successful!
Preview URL:	https://464645fc.monorepo-eu0.pages.dev
Branch Preview URL:	https://acknowledgement.monorepo-eu0.pages.dev

View logs

[patrick-ogrady]

I originally considered migrating to channels::tracked (so we could enqueue multiple blocks at once) but opted to take a simpler approach at first to unblock.

[Copilot]

Pull Request Overview

This PR introduces a new Acknowledgement trait as a first-party abstraction for handling asynchronous task completion acknowledgements. The implementation replaces the previous oneshot::channel() approach with a more flexible and maintainable acknowledgement system.

Key changes:

• Added a new Acknowledgement trait with an Exact<N> implementation that supports waiting for exactly N acknowledgements
• Replaced oneshot::Sender<()> with the new Acknowledgement trait throughout the marshal module
• Made Update clonable to support multiple consumers acknowledging the same block

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
utils/src/lib.rs	Exports the new acknowledgement module and Acknowledgement trait
utils/src/acknowledgement.rs	Defines the Acknowledgement trait and Exact<N> implementation with atomic state management
examples/reshare/src/dkg/ingress.rs	Updates DKG message types to use Acknowledgement instead of oneshot::Sender<()>
examples/reshare/src/dkg/actor.rs	Changes block finalization to call acknowledge() instead of send(())
consensus/src/marshal/mod.rs	Updates Update enum to use Acknowledgement and makes it clonable
consensus/src/marshal/mocks/application.rs	Updates mock application to use acknowledge() method
consensus/src/marshal/actor.rs	Refactors marshal actor to use Acknowledgement trait and its associated types

[SuperFluffy]

Bit worried about the usability of putting the number of required acks into the type signature instead of handling it inline.

EDIT: I also think this might run into troubles with the Reporters forking two one or more optional reporters.

[patrick-ogrady]

EDIT: I also think this might run into troubles with the Reporters forking two one or more optional reporters.

Correct. You need to know exactly how many participants you want to acknowledge delivery a prioi (see race condition point above).

We could alternatively accept some sort of arg into handle() for listeners but the reporter logic (and what may or may not be listening) is opaque as you know.

[SuperFluffy]

Correct. You need to know exactly how many participants you want to acknowledge delivery a prioi (see race condition point above).
We could alternatively accept some sort of arg into handle() for listeners but the reporter logic (and what may or may not be listening) is opaque as you know.

That's right, thanks for pointing that out.

I have pushed a possible solution to #2218:

Instead of requiring Activity: Clone, we can require Activity: Splittable, which is just:

trait Splittable: Send {
    fn split(self) -> (Self, Self);
}

Modulo me overlooking some other race conditions, I think this might be an elegant way of a) hiding the cardinality of how many drops are expected from the consumer (some mailbox should not need to know how many other mailboxes need to ack the update), b) it solves the problem with the Reporters splitter and Option<R>, and c) it solves the race condition because cloning is banned and every consumer gets their "own" increment.

[patrick-ogrady]

Correct. You need to know exactly how many participants you want to acknowledge delivery a prioi (see race condition point above).
We could alternatively accept some sort of arg into handle() for listeners but the reporter logic (and what may or may not be listening) is opaque as you know.

That's right, thanks for pointing that out.

I have pushed a possible solution to #2218:

Instead of requiring Activity: Clone, we can require Activity: Splittable, which is just:

trait Splittable: Send {
    fn split(self) -> (Self, Self);
}

Modulo me overlooking some other race conditions, I think this might be an elegant way of a) hiding the cardinality of how many drops are expected from the consumer (some mailbox should not need to know how many other mailboxes need to ack the update), b) it solves the problem with the Reporters splitter and Option<R>, and c) it solves the race condition because cloning is banned and every consumer gets their "own" increment.

Figured out a way to avoid the race condition (and we can just use Clone 🚀 )

[clabby]

Neat :)

[clabby]

👍

[codecov]

Codecov Report

❌ Patch coverage is 93.16239% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.26%. Comparing base (f395c9e) to head (3897706).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
utils/src/acknowledgement.rs	92.30%	7 Missing ⚠️
consensus/src/reporter.rs	92.30%	1 Missing ⚠️

@@            Coverage Diff             @@
##             main    #2216      +/-   ##
==========================================
+ Coverage   92.21%   92.26%   +0.04%     
==========================================
  Files         314      317       +3     
  Lines       86380    86951     +571     
==========================================
+ Hits        79657    80222     +565     
- Misses       6723     6729       +6     

Files with missing lines	Coverage Δ
consensus/src/marshal/actor.rs	87.58% <100.00%> (+0.06%)	⬆️
consensus/src/marshal/mocks/application.rs	100.00% <100.00%> (ø)
consensus/src/marshal/mod.rs	99.21% <ø> (ø)
utils/src/lib.rs	100.00% <ø> (ø)
consensus/src/reporter.rs	43.39% <92.30%> (+43.39%)	⬆️
utils/src/acknowledgement.rs	92.30% <92.30%> (ø)

... and 12 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f395c9e...3897706. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.