Skip to content

Commit 5e133a1

Browse files
authored
fix: multichain pt2 audit fixes (#1592)
**Multichain pt2 Audit Fixes:** **High:** - #1575 - #1569 **Medium:** - #1580 **Low:** - #1589 **Misc** - #1561 - #1587 - #1584
2 parents c6be96a + 53a12d7 commit 5e133a1

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+2122
-466
lines changed

docs/multichain/destination/CertificateVerifier.md

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,11 @@ struct ECDSAOperatorInfo {
7373
* @dev This function can only be called by the `OperatorTableUpdater` contract, which is itself permissionless to call
7474
* @dev The `referenceTimestamp` must correspond to a reference timestamp for a globalTableRoot stored in the `OperatorTableUpdater`
7575
* In addition, it must be greater than the latest reference timestamp for the given operatorSet
76+
* @dev Reverts for:
77+
* - OnlyTableUpdater: Caller is not the operatorTableUpdater
78+
* - TableUpdateStale: The referenceTimestamp is not greater than the latest reference timestamp
79+
* @dev Emits the following events:
80+
* - TableUpdated: When the operator table is successfully updated
7681
*/
7782
function updateOperatorTable(
7883
OperatorSet calldata operatorSet,
@@ -138,7 +143,16 @@ struct ECDSACertificate {
138143
* a. An in-flight certificate for a past reference timestamp and an operator table update for a newer reference timestamp. The AVS should decide whether it
139144
* wants to only confirm tasks against the *latest* certificate
140145
* b. An in-flight certificate against a stake table with a majority-stake operator that has been slashed or removed from the operatorSet
141-
* @dev Reverts if the certificate's `referenceTimestamp` is too stale with respect to the `maxStalenessPeriod` of the operatorSet
146+
* @dev Reverts for:
147+
* - CertificateStale: The certificate's referenceTimestamp is too stale with respect to the maxStalenessPeriod of the operatorSet
148+
* - ReferenceTimestampDoesNotExist: The root at referenceTimestamp does not exist
149+
* - RootDisabled: The root at referenceTimestamp is not valid
150+
* - InvalidSignatureLength: Signatures are not proper length
151+
* - InvalidSignature: Each signature is not valid
152+
* - SignersNotOrdered: Signatures are not ordered by signer address ascending
153+
* - ReferenceTimestampDoesNotExist: The operatorSet has not been updated for the referenceTimestamp
154+
* - OperatorCountZero: There are zero operators for the referenceTimestamp
155+
* - VerificationFailed: Any signer is not a registered operator
142156
*/
143157
function verifyCertificate(
144158
OperatorSet calldata operatorSet,
@@ -187,7 +201,9 @@ Verifies an ECDSA certificate by checking individual signatures from operators.
187201
* a. An in-flight certificate for a past reference timestamp and an operator table update for a newer reference timestamp. The AVS should decide whether it
188202
* wants to only confirm tasks against the *latest* certificate
189203
* b. An in-flight certificate against a stake table with a majority-stake operator that has been slashed or removed from the operatorSet
190-
* @dev Reverts if the certificate's `referenceTimestamp` is too stale with respect to the `maxStalenessPeriod` of the operatorSet
204+
* @dev Reverts for:
205+
* - All requirements from verifyCertificate
206+
* - ArrayLengthMismatch: signedStakes.length does not equal totalStakeProportionThresholds.length
191207
*/
192208
function verifyCertificateProportion(
193209
OperatorSet calldata operatorSet,
@@ -207,7 +223,6 @@ Verifies that a certificate meets specified proportion thresholds as a percentag
207223
*Requirements*:
208224
* All requirements from `verifyCertificate`
209225
* `signedStakes.length` MUST equal `totalStakeProportionThresholds.length`
210-
* For each stake type: `signedStakes[i] >= (totalStakes[i] * totalStakeProportionThresholds[i]) / 10_000`
211226

212227
#### `verifyCertificateNominal`
213228

@@ -228,7 +243,9 @@ Verifies that a certificate meets specified proportion thresholds as a percentag
228243
* a. An in-flight certificate for a past reference timestamp and an operator table update for a newer reference timestamp. The AVS should decide whether it
229244
* wants to only confirm tasks against the *latest* certificate
230245
* b. An in-flight certificate against a stake table with a majority-stake operator that has been slashed or removed from the operatorSet
231-
* @dev Reverts if the certificate's `referenceTimestamp` is too stale with respect to the `maxStalenessPeriod` of the operatorSet
246+
* @dev Reverts for:
247+
* - All requirements from verifyCertificate
248+
* - ArrayLengthMismatch: signedStakes.length does not equal totalStakeNominalThresholds.length
232249
*/
233250
function verifyCertificateNominal(
234251
OperatorSet calldata operatorSet,
@@ -247,7 +264,6 @@ Verifies that a certificate meets specified nominal (absolute) stake thresholds
247264
*Requirements*:
248265
* All requirements from `verifyCertificate`
249266
* `signedStakes.length` MUST equal `totalStakeNominalThresholds.length`
250-
* For each stake type: `signedStakes[i] >= totalStakeNominalThresholds[i]`
251267

252268
### Utility Functions
253269

@@ -270,10 +286,11 @@ The ECDSA Certificate Verifier uses a modified domain separator that intentional
270286

271287
```solidity
272288
/**
273-
* @notice Calculate the EIP-712 digest for a certificate
289+
* @notice Calculate the EIP-712 digest for a certificate, returning the hash of the digest
274290
* @param referenceTimestamp The reference timestamp
275-
* @param messageHash The message hash
291+
* @param messageHash The message hash of the task
276292
* @return The EIP-712 digest
293+
* @dev EIP-712 is a standard ECDSA signature verification framework. See https://eips.ethereum.org/EIPS/eip-712 for more details
277294
* @dev This function is public to allow offchain tools to calculate the same digest
278295
* @dev Note: This does not support smart contract based signatures for multichain
279296
* @dev This is a chain-agnostic digest, so it can be used to verify certificates across

docs/multichain/destination/OperatorTableUpdater.md

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@ Global table roots must be confirmed by the `generator` before operator tables c
5353
* @dev The `msgHash` in the `globalOperatorTableRootCert` is the hash of the `globalTableRoot`, `referenceTimestamp`, and `referenceBlockNumber`
5454
* @dev The `referenceTimestamp` nested in the `globalTableRootCert` should be `getGeneratorReferenceTimestamp`, whereas
5555
* the `referenceTimestamp` passed directly in the calldata is the block timestamp at which the global table root was calculated
56+
* @dev Reverts for:
57+
* - GlobalTableRootInFuture: referenceTimestamp is in the future
58+
* - GlobalTableRootStale: referenceTimestamp is not greater than latest reference timestamp
59+
* - InvalidMessageHash: certificate messageHash does not match expected EIP-712 hash
60+
* - CertificateInvalid: certificate verification failed against confirmation threshold
61+
* @dev Emits the following events:
62+
* - NewGlobalTableRoot: When global table root is successfully confirmed
5663
*/
5764
function confirmGlobalTableRoot(
5865
BN254Certificate calldata globalTableRootCert,
@@ -90,12 +97,21 @@ Once a global root is confirmed, individual operator tables can be updated by pr
9097
```solidity
9198
/**
9299
* @notice Updates an operator table
93-
* @param referenceTimestamp the reference block number of the globalTableRoot
100+
* @param referenceTimestamp the reference timestamp of the globalTableRoot
94101
* @param globalTableRoot the new globalTableRoot
95102
* @param operatorSetIndex the index of the given operatorSet being updated
96103
* @param proof the proof of the leaf at index against the globalTableRoot
97104
* @param operatorTableBytes the bytes of the operator table
98-
* @dev Depending on the decoded KeyType, the tableInfo will be decoded
105+
* @dev This function calls `updateOperatorTable` on the `ECDSACertificateVerifier` or `BN254CertificateVerifier`
106+
* depending on the `KeyType` of the operatorSet, which is encoded in the `operatorTableBytes`
107+
* @dev Function silently returns if the `referenceTimestamp` has already been updated for the `operatorSet`
108+
* @dev Reverts for:
109+
* - InvalidRoot: globalTableRoot is disabled or invalid
110+
* - InvalidOperatorSet: operatorSet is the generator (not allowed for regular updates)
111+
* - TableUpdateForPastTimestamp: referenceTimestamp is not greater than latest for the operatorSet
112+
* - InvalidGlobalTableRoot: provided globalTableRoot does not match stored root for referenceTimestamp
113+
* - InvalidOperatorSetProof: merkle proof verification failed
114+
* - InvalidCurveType: unsupported curve type in operatorTableBytes
99115
*/
100116
function updateOperatorTable(
101117
uint32 referenceTimestamp,
@@ -135,14 +151,19 @@ The `owner` can configure the `generator` and confirmation parameters.
135151
/**
136152
* @notice Updates the `Generator` to a new operatorSet
137153
* @param generator The operatorSet which certifies against global roots
138-
* @param GeneratorInfo The operatorSetInfo for the generator
139-
* @param GeneratorConfig The operatorSetConfig for the generator
154+
* @param generatorInfo The operatorSetInfo for the generator
140155
* @dev We have a separate function for updating this operatorSet since it's not transported and updated
141156
* in the same way as the other operatorSets
142157
* @dev Only callable by the owner of the contract
143158
* @dev Uses GENERATOR_GLOBAL_TABLE_ROOT constant to break circular dependency for certificate verification
144159
* @dev We ensure that there are no collisions with other reference timestamps because we expect the generator to have an initial reference timestamp of 0
145160
* @dev The `_latestReferenceTimestamp` is not updated since this root is ONLY used for the `Generator`
161+
* @dev The `_referenceBlockNumber` and `_referenceTimestamps` mappings are not updated since they are only used for introspection for official operatorSets
162+
* @dev Reverts for:
163+
* - "Ownable: caller is not the owner": caller is not the owner
164+
* - InvalidGenerator: generator has a non-zero reference timestamp
165+
* @dev Emits the following events:
166+
* - GeneratorUpdated: When generator is successfully updated
146167
*/
147168
function updateGenerator(
148169
OperatorSet calldata generator,
@@ -169,8 +190,12 @@ Updates the operator set responsible for confirming global table roots. This fun
169190
```solidity
170191
/**
171192
* @notice The threshold, in bps, for a global root to be signed off on and updated
172-
* @param bps The threshold in basis points
173193
* @dev Only callable by the owner of the contract
194+
* @dev Reverts for:
195+
* - "Ownable: caller is not the owner": caller is not the owner
196+
* - InvalidConfirmationThreshold: bps is greater than MAX_BPS (10000)
197+
* @dev Emits the following events:
198+
* - GlobalRootConfirmationThresholdUpdated: When threshold is successfully updated
174199
*/
175200
function setGlobalRootConfirmationThreshold(
176201
uint16 bps
@@ -194,6 +219,13 @@ Sets the stake proportion threshold required for confirming global table roots.
194219
* @notice Disables a global table root
195220
* @param globalTableRoot the global table root to disable
196221
* @dev Only callable by the pauser
222+
* @dev Cannot disable the GENERATOR_GLOBAL_TABLE_ROOT
223+
* @dev Reverts for:
224+
* - OnlyPauser: caller is not the pauser
225+
* - InvalidRoot: globalTableRoot is already disabled or does not exist
226+
* - CannotDisableGeneratorRoot: attempting to disable the generator's global table root
227+
* @dev Emits the following events:
228+
* - GlobalRootDisabled: When global table root is successfully disabled
197229
*/
198230
function disableRoot(
199231
bytes32 globalTableRoot

0 commit comments

Comments
 (0)