Core Contracts

TimelockController.sol

https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/governance/proposals/TimelockController.sol#L211

Overview

Within cancel(...), the contract checks:

Observe that:

Logically, if executed == true, the correct revert message should indicate that the operation has already been executed, not that it was “already scheduled.” The revert message and its label “OperationAlreadyScheduled” do not match the condition being checked.

Impact

1. Misleading Error Messages
   A user or developer who sees “OperationAlreadyScheduled” upon attempting cancellation might believe the operation was never executed and only scheduled. In reality, the operation was fully executed and thus unstoppable.

2. Potential Developer Confusion
   During debugging or integration, receiving an “OperationAlreadyScheduled” revert for an executed operation can cause wasted time trying to interpret the mismatch. It also breaks the standard pattern of “OperationAlreadyExecuted” reverts typically used in timelock contracts.

3. Confidence / Clarity
   Clear, consistent error reporting is crucial in governance and timelock flows. A mismatch can lead to incorrect assumptions or hamper urgent troubleshooting if a cancellation attempt fails with an unrelated message.

Recommended Fix

Replace the revert statement with a more accurate error label. For example:

And define an error or revert reason that matches the condition:

This ensures that when someone tries to cancel an operation that has already been executed, the revert explicitly tells them it’s “Already Executed,” eliminating confusion.