[consensus::marshal] Fix: Responses from broadcast::buffered are not immediately processed
#1381
Conversation
Makes sense to me (after doing a quick scan here) |
f339dc1 to
6717430
Compare
| /// | ||
| /// If the pool is empty, the future will never resolve. | ||
| /// Returns `Ok(T)` for successful completion or `Err(Aborted)` for aborted futures. | ||
| pub fn next_completed( |
There was a problem hiding this comment.
next_completed for consistency with futures::Pool above. I did originally consider next() for this, but decided not to for some reason (maybe too close to stream next?)
There was a problem hiding this comment.
I think next() in Rust for this is pretty generic (at least I interpret as the correct method name for something like this...we can change in the future but 🤷 ).
consensus/src/marshal/actor.rs
Outdated
| let (tx, rx) = oneshot::channel(); | ||
| buffer.subscribe_prepared(None, commitment, None, tx).await; | ||
| entry.insert(Waiter::new(rx, response)); | ||
| let hook = waiters.push(async move { |
There was a problem hiding this comment.
I wonder if it makes sense to make a "wrapper" struct that handles the waiters and listeners?
The logic you introduced here looks solid but seems it could be simplified if we introduced one additional util (unless the generics get too bad)?
There was a problem hiding this comment.
I agree there could be (?) room for a util here but not yet confident that I have an idea for one that's general enough yet to be useful in multiple places. In the future I think
| (schemes, peers, identity, shares) | ||
| } | ||
|
|
||
| async fn setup_network_links(oracle: &mut Oracle<P>, peers: &[P], link: Link) { |
There was a problem hiding this comment.
JFYI I asked Denis to make some of these helpers exported functions in p2p::simulated to avoid further duplication of these utils: #1344 (comment)
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #1381 +/- ##
==========================================
+ Coverage 91.34% 91.50% +0.16%
==========================================
Files 265 265
Lines 66706 67023 +317
==========================================
+ Hits 60930 61330 +400
+ Misses 5776 5693 -83
... and 27 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Fixes #1380