Patch upstream changes for modular block handler #50
Conversation
Upstream PR: paritytech#14014 This reverts commit c9276cb.
| } | ||
|
|
||
| /// Run [`BlockRequestHandler`]. | ||
| pub async fn run(mut self) { | ||
| async fn process_requests(&mut self) { |
There was a problem hiding this comment.
Why did you change this from &mut self to mut self?
There was a problem hiding this comment.
You meant the other way I guess. This is for size determination with the trait, otherwise:
error[E0161]: cannot move a value of type `dyn BlockServer<TBl>`
--> client/service/src/builder.rs:793:3
|
793 | block_server.run().await;
| ^^^^^^^^^^^^^^^^^^ the size of `dyn BlockServer<TBl>` cannot be statically determined
Do you see an issue with this?
There was a problem hiding this comment.
I was just curious in Subspace PR why it was necessary and it lead me here. No significant concerns in this case, but changing mut self to &mut self in general does have implications because often async functions are actually not always ready for this. If you had some accumulator before while then there would be a problem.
There was a problem hiding this comment.
I see, can you give more details or example? Are you referring to capturing the reference in a closure?
There was a problem hiding this comment.
It is generally referred to as cancellation safety: https://smallcultfollowing.com/babysteps/blog/2022/06/13/async-cancellation-a-case-study-of-pub-sub-in-mini-redis/
Upstream PR: paritytech#14014
Note: this would break compilation on subspace repo. autonomys/subspace#1393 would need to be submitted to fix the build