QA Report

[howlbot-integration]

See the markdown file with the details of this report here.

[GalloDaSballo]

First 2 were good, went downhill from there

[c4-judge]

GalloDaSballo marked the issue as grade-b

[sathishpic22]

Hi @GalloDaSballo

Thank you for the grading. I acknowledge that some of my findings might appear minor or even easy to overlook.

I still believe that my report offers greater value to the protocol compared to those that were marked as Grade A. My findings provide meaningful insights and contribute to a deeper level of analysis, which I feel better serves the protocol's objectives. I provided explanations for some findings that clearly fall within the Low-risk category. These findings are intended to highlight potential minor issues while still adding value to the overall assessment.

[L-3] setGuardian() Implementation Does Not Enforce Paused State as Intended

The comment implies that granting a new guardian resets the pause timer (i.e., unpauses the contract).

What the Code Does:
setGuardian() can be called regardless of whether the contract is paused or not.
Even if the contract is already unpaused, calling setGuardian() will still reset the pause timer, allowing the operation to proceed without any constraint on the current state.

Implementation clearly breaks the actual comments .

[L-4] Automatic Unpause After pauseDuration elapsed Could Break Core Functionality

There is risk of unpausing in unexpected time even the problem resolved .

[L-5] Accidental ETH Transfers to Timelock Contract Risk Being permanently Locked

Yes, there is offchain tracking and emit events but there is no method implemented to recover for accidental eth transfers

[L-7] No Maximum Expiration Period Leads to Indefinite Timelocked Actions

Yes, There is mindelay check. But the expirationPeriod is mistakenly set so big values then Indefinite Timelocked Actions . Timelock actions not expired .expirationPeriod is uint256 this can store so big values.

[L-10] DoS Risk Due to Unbounded Enumerable Set Growth in getAllProposals()

EnumerableSet's values() function returns the entire set in a single call, which means the more proposals in the set, the higher the gas consumption.

Even this thing mentioned in DOCS

Also, using an Enumerable set can cause a Dos in the contract if the set grows large enough and it’s used in a function that modifies the state of the contract, this is commented in the openzeppelin documentation and it’s something to keep in mind for future iterations of the contracts.

[L-11] Misleading Comment in isOperation Function

This is clearly LOW findings. The comment is misleading to actual implementations.

The comment states the false only return when the operations cancelled but there is possibility that return false when contract is paused .

[L-12] Redundant Guardian Reassignment and Potential Governance Manipulation Risk

The current implementation not restrict to assign same old guardian

[L-13] Critical Immutable Variables Assigned Without Validation in Constructor

This is clearly the LOW risk

[L-14] Improper Memory Alignment in Assembly Block

This clearly violating the solidity memory assignments.

The findings I chose not to elaborate on are, in my view, ones that fall under the category of ignorable findings. I believe they have minimal impact and don’t warrant detailed explanation.

Thank you for your time and consideration.

[GalloDaSballo]

I have re-checked the report and I maintain my statement, I don't believe it's correct to comingle interesting observations with "padding findigns"

Reports with a higher signal to noise will be prioritized

| L-1 | Dynamic Expiration Period Changes Can Invalidate Ready Operations and Revive Expired Ones |
L
| L-2 | setGuardian() function is vulnerable to frontrunning attack |
L
| L-3 | setGuardian() Implementation Does Not Enforce Paused State as Intended |
NC
| L-4 | Automatic Unpause After pauseDuration Elapsed Could Break Core Functionality |
I
| L-5 | Accidental ETH Transfers to Timelock Contract Risk Being Permanently Locked |
I
| L-6 | Arbitrary Cancellation Risk: Unrestricted onlySafe Role May Disrupt Timelocked Operations |
I
| L-7 | No Maximum Expiration Period Leads to Indefinite Timelocked Actions |
I
| L-8 | Risk of DoS from Large Enumerable Sets in State-Modifying Functions |
TODO
| L-9 | Inconsistency between isOperationReady() and isOperationExpired() Checks |
TODO
| L-10 | DoS Risk Due to Unbounded Enumerable Set Growth in getAllProposals() |
TODO
| L-11 | Misleading Comment in isOperation() Function |
TODO
| L-12 | Redundant Guardian Reassignment and Potential Governance Manipulation Risk |
TODO
| L-13 | Critical Immutable Variables Assigned Without Validation in Constructor |
TODO
| L-14 | Improper Memory Alignment in Assembly Block |
TODO
| L-15 | Unsafe Assembly Blocks |
I
| L-16 | Lack of Access Control on Critical initialize() Function |
-3
| Issue No | Title |

L - 2
NC - 1
I - 5
TODO - 7
-3 - 1
| L-1 | Dynamic Expiration Period Changes Can Invalidate Ready Operations and Revive Expired Ones | - 1
| L-3 | setGuardian() Implementation Does Not Enforce Paused State as Intended | - 1
| L-4 | Automatic Unpause After pauseDuration Elapsed Could Break Core Functionality | - 1
| L-16 | Lack of Access Control on Critical initialize() Function | - 1
| L-15 | Unsafe Assembly Blocks | - 1
| Issue No | Title | - 1
2L 1NC - 3
Skipping rest

[thebrittfactor]

For awarding purposes, C4 staff have marked as tied for 2nd place.