Skip to content

Commit 2cae40e

Browse files
committed
fix: taskMailbox audit fixes (#1604)
**Motivation:** Certora TaskMailbox audit fixes for Hourglass Part 1 and Part 2. **Modifications:** * `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 * `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. * `L-03: Restrictive check in_validateBN254Certificate()` : Updated the check to only be for the (0,0) coordinate. * `I-02: Incorrect NatSpec in registerExecutorOperatorSet()` : Clearer natspec. * Updated release scripts * Updated Bindings * Updated Docs * Updated Unit tests **Result:** Bug free code.
1 parent dba4a18 commit 2cae40e

19 files changed

+548
-74
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

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.

pkg/bindings/TaskMailbox/binding.go

Lines changed: 35 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/bindings/TaskMailboxStorage/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.

script/releases/Env.sol

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ library Env {
8080
return _string("ZEUS_ENV");
8181
}
8282

83+
function envVersion() internal view returns (string memory) {
84+
return _string("ZEUS_ENV_VERSION");
85+
}
86+
8387
function deployVersion() internal view returns (string memory) {
8488
return _string("ZEUS_DEPLOY_TO_VERSION");
8589
}
@@ -184,6 +188,11 @@ library Env {
184188
return _envU32("TABLE_UPDATE_CADENCE");
185189
}
186190

191+
function MAX_TASK_SLA() internal view returns (uint96) {
192+
// MAX_TASK_SLA is stored as uint256 in zeus env, cast to uint96
193+
return uint96(_envU256("MAX_TASK_SLA"));
194+
}
195+
187196
function isSourceChain() internal view returns (bool) {
188197
return _envBool("SOURCE_CHAIN");
189198
}

script/releases/v1.7.0-multichain/1-deploySourceChain.s.sol

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ contract DeploySourceChain is EOADeployer {
1212

1313
/// forgefmt: disable-next-item
1414
function _runAsEOA() internal override {
15-
// If we're not on a source chain, we don't need to deploy any contracts
16-
if (!Env.isSourceChain()) {
15+
// If we're not on a source chain or we're on a version that already has these contracts deployed, we don't need to deploy any contracts
16+
if (!Env.isSourceChain() || _isAlreadyDeployedToSourceChain()) {
1717
return;
1818
}
1919

@@ -100,7 +100,7 @@ contract DeploySourceChain is EOADeployer {
100100
}
101101

102102
function testScript() public virtual {
103-
if (!Env.isSourceChain()) {
103+
if (!Env.isSourceChain() || _isAlreadyDeployedToSourceChain()) {
104104
return;
105105
}
106106

@@ -259,4 +259,18 @@ contract DeploySourceChain is EOADeployer {
259259
function _assertFalse(bool b, string memory err) private pure {
260260
assertFalse(b, err);
261261
}
262+
263+
/// @dev Check if the version is already deployed
264+
function _isAlreadyDeployedToSourceChain() private view returns (bool) {
265+
if (keccak256(bytes(Env.envVersion())) == keccak256(bytes("1.7.0-rc.0"))) {
266+
return true;
267+
} else if (keccak256(bytes(Env.envVersion())) == keccak256(bytes("1.7.0-rc.1"))) {
268+
return true;
269+
} else if (keccak256(bytes(Env.envVersion())) == keccak256(bytes("1.7.0-rc.2"))) {
270+
return true;
271+
} else if (keccak256(bytes(Env.envVersion())) == keccak256(bytes("1.8.0-rc.0"))) {
272+
return true;
273+
}
274+
return false;
275+
}
262276
}

script/releases/v1.7.0-multichain/2-deployDestinationChainProxies.s.sol

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ contract DeployDestinationChainProxies is MultisigBuilder {
1414

1515
/// forgefmt: disable-next-item
1616
function _runAsMultisig() internal virtual override {
17-
// If we're not on a destination chain, we don't need to deploy any contracts
18-
if (!Env.isDestinationChain()) {
17+
// If we're not on a destination chain or we're on a version that already has these contracts deployed, we don't need to deploy any contracts
18+
if (!Env.isDestinationChain() || _isAlreadyDeployed()) {
1919
return;
2020
}
2121

@@ -55,7 +55,7 @@ contract DeployDestinationChainProxies is MultisigBuilder {
5555
}
5656

5757
function testScript() public virtual {
58-
if (!Env.isDestinationChain()) {
58+
if (!Env.isDestinationChain() || _isAlreadyDeployed()) {
5959
return;
6060
}
6161

@@ -135,4 +135,18 @@ contract DeployDestinationChainProxies is MultisigBuilder {
135135
name: name
136136
});
137137
}
138+
139+
/// @dev Check if the version is already deployed
140+
function _isAlreadyDeployed() internal view returns (bool) {
141+
if (keccak256(bytes(Env.envVersion())) == keccak256(bytes("1.7.0-rc.0"))) {
142+
return true;
143+
} else if (keccak256(bytes(Env.envVersion())) == keccak256(bytes("1.7.0-rc.1"))) {
144+
return true;
145+
} else if (keccak256(bytes(Env.envVersion())) == keccak256(bytes("1.7.0-rc.2"))) {
146+
return true;
147+
} else if (keccak256(bytes(Env.envVersion())) == keccak256(bytes("1.8.0-rc.0"))) {
148+
return true;
149+
}
150+
return false;
151+
}
138152
}

script/releases/v1.7.0-multichain/3-deployDestinationChainImpls.s.sol

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ contract DeployDestinationChainImpls is EOADeployer, DeployDestinationChainProxi
1414
using Env for *;
1515

1616
function _runAsEOA() internal override {
17-
// If we're not on a destination chain, we don't need to deploy any contracts
18-
if (!Env.isDestinationChain()) {
17+
// If we're not on a destination chain or we're on a version that already has these contracts deployed, we don't need to deploy any contracts
18+
if (!Env.isDestinationChain() || _isAlreadyDeployed()) {
1919
return;
2020
}
2121

@@ -58,7 +58,7 @@ contract DeployDestinationChainImpls is EOADeployer, DeployDestinationChainProxi
5858
}
5959

6060
function testScript() public virtual override {
61-
if (!Env.isDestinationChain()) {
61+
if (!Env.isDestinationChain() || _isAlreadyDeployed()) {
6262
return;
6363
}
6464

script/releases/v1.7.0-multichain/4-instantiateDestinationChainProxies.s.sol

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ contract InstantiateDestinationChainProxies is DeployDestinationChainImpls {
2525

2626
/// forgefmt: disable-next-item
2727
function _runAsMultisig() internal override prank(Env.multichainDeployerMultisig()) {
28-
// If we're not on a destination chain, we don't need to do anything
29-
if (!Env.isDestinationChain()) {
28+
// If we're not on a destination chain or we're on a version that already has these contracts deployed, we don't need to do anything
29+
if (!Env.isDestinationChain() || _isAlreadyDeployed()) {
3030
return;
3131
}
3232

@@ -66,7 +66,7 @@ contract InstantiateDestinationChainProxies is DeployDestinationChainImpls {
6666
}
6767

6868
function testScript() public virtual override {
69-
if (!Env.isDestinationChain()) {
69+
if (!Env.isDestinationChain() || _isAlreadyDeployed()) {
7070
return;
7171
}
7272

script/releases/v1.7.0-multichain/5-configureCrossChainRegistry.s.sol

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ contract ConfigureCrossChainRegistry is DeploySourceChain, DeployDestinationChai
1313
using Env for *;
1414

1515
function _runAsMultisig() internal override prank(Env.opsMultisig()) {
16-
// If we're not on a source chain, we don't need to run this
17-
if (!Env.isSourceChain()) {
16+
// If we're not on a source chain or we're on a version that already has these contracts deployed, we don't need to run this
17+
if (!Env.isSourceChain() || _isAlreadyDeployed()) {
1818
return;
1919
}
2020

@@ -30,7 +30,7 @@ contract ConfigureCrossChainRegistry is DeploySourceChain, DeployDestinationChai
3030
}
3131

3232
function testScript() public virtual override(DeploySourceChain, DeployDestinationChainProxies) {
33-
if (!Env.isSourceChain()) {
33+
if (!Env.isSourceChain() || _isAlreadyDeployed()) {
3434
return;
3535
}
3636

0 commit comments

Comments
 (0)