Skip to content

Commit f461f70

Browse files
authored
fix: hourglass part 1 and 2 audit fixes (#1609)
**Hourglass part 1 and 2 audit fixes** * TaskMailbox fixes: #1604 * ReleaseManager fixes: #1608 **High:** * `H-03: Aggregator TOCTOU Issues Regarding Stake Weights and Operator Set` : Added a `MAX_TASK_SLA` immutable that will be set as `DEALLOCATION_DELAY / 2` so that AVSs have half the Deallocation Delay to do any operator slashing in case of misbehavior : #1604 **Low:** * `L-01: TaskMailbox::createTask() may create tasks that cannot be completed` : Checking that `block.timestamp + taskConfig.taskSLA <= operatorTableReferenceTimestamp + maxStaleness` during taskCreation so that a task cannot be created if its max response time breaches the staleness period of the certificate. : #1604 * `L-03: Restrictive check in_validateBN254Certificate()` : Updated the check to only be for the (0,0) coordinate. : #1604 **Info:** * `I-01. isValidRelease() and getLatestUpgradeByTime() may panic when no releases exist` : Added `NoReleases()` custom error to those 2 functions in the case of no error. : #1608 * `I-02: Incorrect NatSpec in registerExecutorOperatorSet()` : Clearer natspec. : #1604 * `I-04. Unused imports can be removed`: Removed unused imports : #1608 **Additional Features:** * Support for signaling instant upgrades by setting `upgradeByTime` to 0 in the `ReleaseManager` contract: #1608
2 parents dba4a18 + 5e25244 commit f461f70

25 files changed

+617
-93
lines changed

docs/avs/task/TaskMailbox.md

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,23 @@ The `TaskMailbox`'s responsibilities are broken down into the following concepts
3636
* [Fee Management](#fee-management)
3737
* [Task Hooks and AVS Integration](#task-hooks-and-avs-integration)
3838

39+
### Immutable Configuration
40+
41+
The `TaskMailbox` contract has the following immutable parameters set at deployment:
42+
43+
* **BN254_CERTIFICATE_VERIFIER**: Address of the BN254 certificate verifier contract
44+
* **ECDSA_CERTIFICATE_VERIFIER**: Address of the ECDSA certificate verifier contract
45+
* **MAX_TASK_SLA**: Maximum allowed task SLA duration (typically set to `DEALLOCATION_DELAY / 2`)
46+
47+
The `MAX_TASK_SLA` parameter is particularly important as it ensures AVSs can still slash operators in case of stake deallocation during inflight tasks. By limiting task SLAs to half the deallocation delay, the system guarantees that operators cannot avoid slashing by deallocating their stake while a task is still pending.
48+
3949
## Parameterization
4050

4151
The `TaskMailbox` uses the following key parameters:
4252

4353
* **Fee Split**: Configurable percentage (0-10000 basis points) that determines how task fees are split between the protocol fee collector and the AVS fee collector
4454
* **Task SLA**: Service Level Agreement duration (in seconds) set per executor operator set, defining how long operators have to complete a task
55+
* **MAX_TASK_SLA**: Maximum allowed task SLA duration (immutable, set at deployment)
4556
* **Consensus Thresholds**: Configurable per operator set, defining the required stake proportion for result verification
4657

4758
---
@@ -50,6 +61,16 @@ The `TaskMailbox` uses the following key parameters:
5061

5162
Tasks in the TaskMailbox system follow a well-defined lifecycle with specific states and transitions. Each task is uniquely identified by a hash computed from the task parameters, block context, and a global counter.
5263

64+
### Certificate Staleness Protection
65+
66+
When creating a task, the system checks that the operator table reference timestamp is fresh enough to remain valid for the entire task SLA duration. This prevents tasks from being created with stale operator sets that might change before the task completes. The check ensures:
67+
68+
```
69+
block.timestamp + taskSLA <= operatorTableReferenceTimestamp + maxStaleness
70+
```
71+
72+
If `maxStaleness` is 0 (unconfigured), this check is skipped. Otherwise, if the certificate would become stale before the task SLA expires, the transaction reverts with `CertificateStale()`.
73+
5374
**State Variables:**
5475
* `_globalTaskCount`: Counter ensuring unique task hashes
5576
* `_tasks`: Mapping from task hash to task details
@@ -81,12 +102,14 @@ function createTask(TaskParams memory taskParams) external nonReentrant returns
81102
Creates a new task in the system. The method performs several validations and operations:
82103

83104
1. Validates that the executor operator set is registered and has a valid configuration
84-
2. Calls the AVS task hook for pre-creation validation
85-
3. Calculates the task fee using the AVS task hook
86-
4. Generates a unique task hash
87-
5. Transfers the fee from the caller to the TaskMailbox
88-
6. Stores the task with its current configuration snapshot
89-
7. Calls the AVS task hook for post-creation handling
105+
2. Validates that the task SLA does not exceed MAX_TASK_SLA
106+
3. Checks certificate staleness to ensure the operator table reference timestamp is fresh enough for the task SLA
107+
4. Calls the AVS task hook for pre-creation validation
108+
5. Calculates the task fee using the AVS task hook
109+
6. Generates a unique task hash
110+
7. Transfers the fee from the caller to the TaskMailbox
111+
8. Stores the task with its current configuration snapshot
112+
9. Calls the AVS task hook for post-creation handling
90113

91114
*Effects*:
92115
* Increments `_globalTaskCount`
@@ -98,6 +121,8 @@ Creates a new task in the system. The method performs several validations and op
98121
*Requirements*:
99122
* Executor operator set must be registered
100123
* Executor operator set must have valid task configuration
124+
* Task SLA must not exceed MAX_TASK_SLA (reverts with `TaskSLAExceedsMaximum`)
125+
* Certificate must not be stale (reverts with `CertificateStale` if maxStaleness is exceeded)
101126
* Task payload must not be empty
102127
* Fee transfer must succeed
103128
* AVS validation must pass
@@ -205,7 +230,7 @@ function setExecutorOperatorSetTaskConfig(
205230

206231
Configures how tasks should be executed by a specific operator set. The configuration includes:
207232
- **Task Hook**: AVS-specific contract for custom validation and handling
208-
- **Task SLA**: Time limit for task completion
233+
- **Task SLA**: Time limit for task completion (must not exceed MAX_TASK_SLA)
209234
- **Fee Token**: Token used for task fees (can be zero address for no fees). **Fees will not be collected if this is the zero address.**
210235
- **Fee Collector**: Address to receive AVS portion of fees
211236
- **Curve Type**: Cryptographic curve used by operators (BN254 or ECDSA)
@@ -221,6 +246,7 @@ Configures how tasks should be executed by a specific operator set. The configur
221246
* Caller must be the operator set owner (verified via certificate verifier)
222247
* Task hook must not be zero address
223248
* Task SLA must be greater than zero
249+
* Task SLA must not exceed MAX_TASK_SLA (reverts with `TaskSLAExceedsMaximum`)
224250
* Consensus type and curve type must be valid
225251
* Consensus value must be properly formatted
226252

@@ -500,4 +526,6 @@ The TaskMailbox implements several security measures:
500526
2. **Certificate Verification**: Results must include valid consensus proofs verified by the appropriate certificate verifier
501527
3. **Fee Safety**: Uses OpenZeppelin's `SafeERC20` for all token transfers
502528
4. **Access Control**: Operator set owners control their configurations; contract owner controls global parameters
503-
5. **Timestamp Validation**: Tasks cannot be verified at their creation timestamp to prevent same-block manipulation
529+
5. **Timestamp Validation**: Tasks cannot be verified at their creation timestamp to prevent same-block manipulation
530+
6. **Task SLA Limits**: MAX_TASK_SLA immutable constant prevents excessively long task durations
531+
7. **Certificate Staleness Protection**: Ensures operator table reference timestamps are fresh enough for the task SLA duration

docs/core/ReleaseManager.md

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ An AVS in the context of `ReleaseManager` is defined as the `address` of the con
3131

3232
* **Latest Release Validity**: Only the latest release for an operator set is considered valid. Previous releases become obsolete as soon as a new release is published.
3333
* **Upgrade Deadlines**: The `upgradeByTime` timestamp (in Unix time) is a suggested deadline and is not enforced on-chain or off-chain. It serves as a communication mechanism for AVSs to indicate when operators should complete their upgrades.
34+
* **Instant Upgrades**: When `upgradeByTime` is set to 0, this signals an instant upgrade requirement.
3435
* **Multiple Releases in Same Block**: If multiple releases are published in the same block with the same `upgradeByTime`, the last transaction processed in that block will determine the latest valid release.
3536

3637
---
@@ -58,7 +59,8 @@ struct Artifact {
5859
/**
5960
* @notice Represents a release containing multiple artifacts and an upgrade deadline.
6061
* @param artifacts Array of artifacts included in this release.
61-
* @param upgradeByTime Timestamp by which operators must upgrade to this release.
62+
* @param upgradeByTime Timestamp by which operators must upgrade to this release.
63+
* A value of 0 signals an instant upgrade requirement.
6264
*/
6365
struct Release {
6466
Artifact[] artifacts;
@@ -90,6 +92,7 @@ mapping(bytes32 operatorSetKey => Release[]) internal _operatorSetReleases;
9092
```solidity
9193
/**
9294
* @notice Publishes a new release for an operator set.
95+
* @dev If the upgradeByTime is 0, the release is meant to signal an instant upgrade.
9396
* @param operatorSet The operator set this release is for.
9497
* @param release The release that was published.
9598
* @return releaseId The index of the newly published release.
@@ -107,6 +110,8 @@ _Note: this method can be called directly by an AVS, or by a caller authorized b
107110

108111
AVSs use this method to publish new software releases for their operator sets. Each release contains one or more artifacts that represent the software components operators must run (e.g., validator clients, network monitors, etc.). The AVS specifies a deadline (`upgradeByTime`) by which all operators in the operator set must upgrade to the new release.
109112

113+
**Special Case - Instant Upgrades**: Setting `upgradeByTime` to 0 signals that this is an instant upgrade that operators should apply immediately. This is typically used for critical security patches or emergency updates.
114+
110115
The `releaseId` returned is the zero-based index of the release in the operator set's release array. This ID can be used to query the release later using [`getRelease`](#getrelease).
111116

112117
*Effects*:
@@ -117,7 +122,8 @@ The `releaseId` returned is the zero-based index of the release in the operator
117122

118123
*Requirements*:
119124
* Caller MUST be authorized, either as the AVS itself or an admin/appointee (see [`PermissionController.md`](../permissions/PermissionController.md))
120-
* `release.upgradeByTime` MUST be greater than or equal to the current block timestamp
125+
* Operator set MUST have published metadata URI via `updateOperatorSetMetadataURI`
126+
* `release.upgradeByTime` MUST be either 0 (instant upgrade) or greater than or equal to the current block timestamp
121127
---
122128

123129
### View Functions
@@ -149,6 +155,7 @@ Returns the total number of releases that have been published for the specified
149155
```solidity
150156
/**
151157
* @notice Returns a specific release by index.
158+
* @dev If the upgradeByTime is 0, the release is meant to signal an instant upgrade.
152159
* @param operatorSet The operator set to query.
153160
* @param releaseId The id of the release to get.
154161
* @return The release at the specified index.
@@ -166,13 +173,15 @@ Retrieves a specific release by its ID for a given operator set. The `releaseId`
166173

167174
*Returns*:
168175
* The complete `Release` struct including all artifacts and the upgrade deadline
176+
* If `upgradeByTime` is 0, this indicates an instant upgrade requirement
169177
* Reverts if `releaseId` is out of bounds
170178

171179
#### `getLatestRelease`
172180

173181
```solidity
174182
/**
175183
* @notice Returns the latest release for an operator set.
184+
* @dev If the upgradeByTime is 0, the release is meant to signal an instant upgrade.
176185
* @param operatorSet The operator set to query.
177186
* @return The id of the latest release.
178187
* @return The latest release.
@@ -188,14 +197,16 @@ function getLatestRelease(
188197
Retrieves the most recently published release for an operator set. This is typically the release that operators should be running or upgrading to.
189198

190199
*Returns*:
191-
* The latest `Release` struct from the operator set's release array
192-
* Reverts if no releases have been published for the operator set
200+
* The release ID and the latest `Release` struct from the operator set's release array
201+
* If `upgradeByTime` is 0, this indicates an instant upgrade requirement
202+
* Reverts with `NoReleases()` if no releases have been published for the operator set
193203

194204
#### `getLatestUpgradeByTime`
195205

196206
```solidity
197207
/**
198208
* @notice Returns the upgrade by time for the latest release.
209+
* @dev If the upgradeByTime is 0, the release is meant to signal an instant upgrade.
199210
* @param operatorSet The operator set to query.
200211
* @return The upgrade by time for the latest release.
201212
*/
@@ -204,14 +215,15 @@ function getLatestUpgradeByTime(
204215
)
205216
external
206217
view
207-
returns (uint256)
218+
returns (uint32)
208219
```
209220

210221
A convenience function that returns just the upgrade deadline from the latest release. This can be useful for quickly checking when operators must complete their upgrades.
211222

212223
*Returns*:
213224
* The `upgradeByTime` timestamp from the latest release
214-
* Reverts if no releases have been published for the operator set
225+
* A value of 0 indicates an instant upgrade requirement
226+
* Reverts with `NoReleases()` if no releases have been published for the operator set
215227

216228
#### `isValidRelease`
217229

@@ -238,6 +250,17 @@ Checks whether a given release ID corresponds to the latest release for an opera
238250
*Returns*:
239251
* `true` if the `releaseId` matches the latest release index
240252
* `false` if the `releaseId` refers to an older release
241-
* Reverts if the operator set has no releases
253+
* Reverts with `NoReleases()` if the operator set has no releases
254+
255+
---
256+
257+
## Error Definitions
258+
259+
The `ReleaseManager` defines the following custom errors:
260+
261+
* `MustPublishMetadataURI()`: Thrown when attempting to publish a release for an operator set that hasn't published its metadata URI via `updateOperatorSetMetadataURI`.
262+
* `InvalidUpgradeByTime()`: Thrown when the `upgradeByTime` is in the past (not including 0, which is valid for instant upgrades).
263+
* `InvalidMetadataURI()`: Thrown when the metadata URI is empty.
264+
* `NoReleases()`: Thrown when querying release information for an operator set that has no published releases.
242265

243266
---

pkg/bindings/ITaskMailbox/binding.go

Lines changed: 32 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)