Improve spawning of supervisor worker tasks

[soareschen]

Description

This is a prerequisite for #1543 to make the spawning of supervisor worker tasks more manageable by parallelizing unrelated tasks.

A key challenge of the current design of the supervisor worker tasks is that we are relying on ad hoc multiplexing of multiple unrelated sub-tasks in a single thread. This result in various unintuitive hacks and bottlenecks:

• If one sub-task raise an unrecoverable error, the other sub-tasks are aborted as well.
• If one sub-task need to abort early, it have to disable itself through flags instead of returning.
• A sub-task may block another sub-task for too long, causing the other sub-task to not get executed.
• The shutdown handler is implemented as one of the sub-tasks, which may be blocked and cause the task to not get shutdown properly.
• The retry-on-error logic containing multiple sub-tasks interfere with the main task loop, causing it to not shutdown properly due to the sub-tasks returning error in the retry loop. Furthermore, the error from some sub-tasks cause the retry even when it should have abort, as there is no way to cancel only one sub-task inside the retry loop.
• There is no way to guarantee a worker task always get terminated at the end. Because it rely on sending an explicit shutdown signal, early returns caused by errors or forgetting to call .shutdown() will cause the task to dangle.
• There is no way to wait for all tasks to terminate before continuing. This is needed in the integration test, as otherwise at the end of the test, the worker tasks may raise various errors due to the full node being shutdown.

For the case of #1543, the main challenge is in multiplexing the client refresh and the detect misbehavior sub-tasks. When the client trusting period is reduced to tens of seconds, the misbehavior sub-task may interfere with the refresh sub-task to be able to refresh on time. Furthermore, in the case when a client is in fact frozen, it is difficult to modify the current code to abort that particular sub-task.

This PR introduce the ibc_relayer::task module, which encapsulate the logic of spawning background tasks and return a TaskHandle. The TaskHandle provides methods to shutdown a background task, or wait for the task to terminate. The TaskHandle also implements a Drop handler that always shutdown a background task when it is dropped, preventing dangling tasks.

The spawn_background_task function accepts a stepper closure that is executed repeatedly, until the shutdown signal is received or until the stepper closure return abort signal or fatal error. The stepper closure returns a TaskError<E>, which wraps around an error type E and contain 3 variants to indicate whether to abort, continue with an ignorable error, or abort with a fatal error. This design significantly simplifies how a step function should be implemented, without having to worry about the low level details of managing a background task.

The supervisor code and its workers are refactored to use the new task module. The various sub-tasks are also split into dedicated tasks to allow them to run in parallel without interfering with each others. Due to the use of shared state in the existing code, some mutexes are introduced so that the same shared state can be shared across multiple parallel tasks.

This PR does not introduce any change in logic aside from refactoring the supervisor code to use the task module. The work for fixing #1543 would instead be done in a separate PR that is based on this PR.

---

PR author checklist:

• Added changelog entry, using unclog.
• Added tests: integration (for Hermes) or unit/mock tests (for modules).
• Linked to GitHub issue.
• Updated code comments and documentation (e.g., docs/).

Reviewer checklist:

• Reviewed Files changed in the GitHub PR explorer.
• Manually tested (in case integration/unit/mock tests are absent).

[adizere]

What is our strategy for merging this?

• Should we have a sync session to discuss it, or
• Is just you Soares + @romac comfortable to merge this?

I think we have to make some progress here. Would like to get to the main dish (#1543) hehe.

[soareschen]

I have created #1664 to show the work in progress to handle the client expiry error in workers.

Unfortunately it seems like the refactoring in this PR would not be sufficient to handle the error, as the connection and channel workers are spawned with one per chain, rather than one per IBC client. Without further re-architecture we are face with the difficult choice of either keep looping to handle expired clients, or abort entirely and not able to handle any other new connections or channels on the chain. More details are discussed in #1664.

[romac]

🚀

[soareschen]

@romac I have added some tests to make sure that the connection and channel workers are working as expected. The packet worker is already verified to be working, since it is needed for the IBC transfer tests to pass.