Refactor handler

[Shourya742]

This PR aims to relax the strict constraints around handler implementations in the protocol crate, particularly concerning synchronicity and return types.

Handlers are no longer required to take Arc<Mutex<Self>>; instead, they now operate with a mutable reference (&mut self) for state mutation. This change simplifies the design and improves ergonomics.

We are also moving away from the SendTo_ abstraction in favor of returning Option<Vec<Message>>. The goal is to delegate message routing decisions to the upstream implementor rather than enforcing it within the library.

Despite these refactors, users still need to implement the relevant traits and invoke the appropriate handle_*_message methods.

We explored exposing a single handle_message method to unify the interface, but this approach would have forced implementors to implement all handler traits, which is not practical. You can find the associated code changes here: (Shourya742#20)

[plebhash]

an alternative approach (more aligned with the original feedback) would be to allow async methods with the following syntax for the trait definition:

fn handle_x(
   &mut self,
   ...,
) -> impl std::future::Future<Output = Result<(), Error>> + Send;

an implementation would look like this:

async fn handle_x(
  &mut self,
  ...,
) -> Result<(), Error> { ... }

this would allow self to carry async channels and avoid the need to rely on return types for message routing

alternatively, MSRV v1.75.0 would also allow using the async fn syntax on trait definition, as long as we also mark the traits with trait_variant::make proc macro

[Shourya742]

an alternative approach (more aligned with the original feedback) would be to allow async methods with the following syntax for the trait definition:

fn handle_x(
   &mut self,
   ...,
) -> impl std::future::Future<Output = Result<(), Error>> + Send;

an implementation would look like this:

async fn handle_x(
  &mut self,
  ...,
) -> Result<(), Error> { ... }

this would allow self to carry async channels and avoid the need to rely on return types for message routing

alternatively, MSRV v1.75.0 would also allow using the async fn syntax on trait definition, as long as we also mark the traits with trait_variant::make proc macro

The key consideration in both this and our current implementation is whether we want the trait implementor to send messages directly from within the handlers, or whether we expect handlers to return a list of client-specific messages to be sent. In the latter case, handle_message implementor would be responsible for sending those messages based on the handler's response.

[GitGab19]

@Shourya742 I think we should try to explore the possibility of sending messages inside the handlers, since it was the original request.

Maybe we could also consider to provide a double option, with one async trait which can be used to send messages directly there, and one trait which returns Option<SameMessageReceived> as we were discussing during yesterday's call.

[Shourya742]

@Shourya742 I think we should try to explore the possibility of sending messages inside the handlers, since it was the original request.

Maybe we could also consider to provide a double option, with one async trait which can be used to send messages directly there, and one trait which returns Option<SameMessageReceived> as we were discussing during yesterday's call.

I was thinking the same both approaches make sense, and we could let the implementors decide what works best for them. However, this would leave us with four possible combinations for the trait implementations:

1. Async + returns Vec
2. Sync + returns Vec
3. Async + returns ()
4. Sync + returns ()

OR

1. Async + returns Option<>
2. Sync + returns Option<>

[plebhash]

one trait which returns Option<SameMessageReceived> as we were discussing during yesterday's call.

maybe I'm missing something, but wouldn't the caller context already have SameMessageReceived?

if that's the case, what would be the value of returning the same message again, instead of a simple Ok that signals successful execution?

[Shourya742]

one trait which returns Option<SameMessageReceived> as we were discussing during yesterday's call.

maybe I'm missing something, but wouldn't the caller context already have SameMessageReceived?

if that's the case, what would be the value of returning the same message again, instead of a simple Ok that signals successful execution?

Yes, It should process the message and return list of new message to be shared or not shared with client.

[plebhash]

deleted previous message

sorry I'm on mobile... I'll try to follow up later when time allows

[GitGab19]

@Shourya742 I think we should have:

• async --> sends messages internally and returns Result<(), Error>
• sync --> returns Result<Vec<MessageToBeSent>, Error>

As pointed out by @plebhash , the caller already has the message in its context (in our current status the caller has the frame), so it makes no sense to return the same message in the sync case.

I don't know if returning the Vec would work well for every case we have, especially in NewTemplate or SetNewPrevHash ones. We need to think a bit more about these cases I guess.

[Shourya742]

We have now moved the handlers into their own crate within the protocol workspace. The handlers now return a unit Result and expect the implementor to manage the message lifecycle deciding how to process and dispatch messages within the handler itself. We now support both sync and async variants of handlers, and we're maintaining MSRV compatibility (thanks @plebhash!). The next step will be to migrate the new tproxy to use the new handlers module as an example implementation demonstrating how the handlers are intended to be used. I'm still exploring ways to improve the trait structure, if nothing significantly better emerges, the current design will be finalized.

[coleFD]

looks great!

[Shourya742]

Thanks for the review, @GitGab19. All suggestions have been addressed.