[OIP] Solve TaskScheduled and TaskRescheduled duplicate problem

[chrisli30]

OAK Improvement Proposal

Motivation

Looking at the task history on Turing Network, specifically the task with the hash 0xcae095bc2b84249e4ca65671759808e0bb10680fb4fedd1ad9e358e4e084de39 shown in the screenshot below, it is evident that after each execution, both TaskScheduled and TaskRescheduled events contain identical data.

Another issue is that the event of task triggering is intertwined with execution and has no indication of whether to reschedule for the next execution. To address this, I propose the following standard for task events.

For all tasks,
After the creation extrinsic, a TaskScheduled needs to be fired. ✅

During task execution,

1. A TaskTriggered event should be fired to indicate the successful triggering of a task.
   Event Name: TaskTriggered
   Attributes:

   • taskId:
   • who:
   • condition: { type:time, value: 1689903366 }
   • CancelUponError: whether to cancel a task upon execution error. A map of errorCode and true of false, by default all erros are true, meaning the task will be cancelled upon error.
     { "errorCode": true or false}, for example, {"InsufficientBalance": false}

2. Next, the actual logic should run and generate an event for its result, such as , parachainStake.delegationIncreased. This event should resemble the ones triggered by manual signing and should not be related to task events. The event can indicate success or failure, marking the execution result of the task. The execution event index should always be equal to the event index of the above TaskTrigger + 1. Currently, events have different names like XcmpTaskSucceeded and SuccessfullyAutoCompoundedDelegatorStake. We do not need to create special events like these; instead, we can rely on the existing events in the underlying pallets.

3. If the task is a recurring schedule, a TaskRescheduled event should subsequently be fired, indicating that a new task has been paid for and scheduled for the next run. If the user's wallet does not have sufficient fees to pay for the next execution, an error will occur at this step, but it will not stop the execution mentioned above.

These changes offer several benefits:

1. The firing of a task execution is clearly indicated through TaskTriggered, serving as a notification that the triggering function is functioning properly.
2. The success or failure of the execution is independent of the triggering notification mentioned above.
3. The execution logic is separated from the rescheduling process. For a fixed schedule, TaskRescheduled will not fire, while for recurring tasks, TaskRescheduled will fire upon successful events and may or may not fire upon errors. For example, if the AutoCompoundDelegationStake fails due to insufficient balance to pay for the next execution fee, the TaskRescheduled event should still be fired.

Implementing these changes will enhance the clarity and separation of task-related events, providing a more structured and meaningful representation of task execution and rescheduling.

[chrisli30]

With this design, we ensure that TaskScheduled and TaskRescheduled events are distinctly different:

1. TaskScheduled events can only occur when a user signs an extrinsic.
2. TaskRescheduled events can only be fired by automation tasks with a Recurring schedule.
3. TaskNotRescheduled events can only be fired by automation tasks with a Recurring schedule when the conditions in the abortErrors parameter are met.

[v9n]

Yeah, I agree with this proposal.

Also I just check the even we emit and we already emit these 2 events:

Example:
TaskNotRescheduled https://turing.subscan.io/block/3040396?tab=event
TaskFailedToReschedule https://turing.subscan.io/block/2561245?tab=event

Should we consolidate and sunset one?

[chrisli30]

Hmm, I see your point. After reviewing the code below, I understand that TaskNotRescheduled is a regular occurrence in our code logic when a decision is made not to reschedule a task. On the other hand, TaskFailedToReschedule is an exception, arising when the reschedule_existing_task process encounters an error.
https://github.com/OAK-Foundation/OAK-blockchain/blob/d15fe711a38d53ed12f3221865524d801119f08b/pallets/automation-time/src/lib.rs#L1406

To maintain consistency with other event names, I propose renaming TaskFailedToReschedule to TaskRescheduleFailed, so we have the following:

• TaskRescheduled: Represents a successful rescheduling of a task.
• TaskRescheduleFailed: Indicates an exception where the attempt to reschedule the task was unsuccessful.

[chrisli30]

Two new types:

TaskExecuted if a task is successfully executed.
TaskExecutionFailed if a task fails to execute.

Now the full flow will be,

During task creation,

1. TaskScheduled

After task creation,

1. Triggering - TaskTriggered
2. Execution - TaskExecuted or TaskExecutionFailed
3. Reschedule, if needed, TaskRescheduled, TaskNotRescheduled or TaskRescheduleFailed

During task cancellation,

1. TaskCancelled