Skip to content

Commit 874eefa

Browse files
authored
Merge pull request #108 from OpenZeppelin/feat/cairo-support-v3.0.0-audit-fixes-2
Cairo: support v3.0.0 audit fixes
2 parents 3641624 + 0819c2f commit 874eefa

File tree

4 files changed

+42
-3
lines changed

4 files changed

+42
-3
lines changed

content/contracts-cairo/3.x/api/access.mdx

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ Functions
241241
- [`change_default_admin_delay(new_delay)`](#IAccessControlDefaultAdminRules-change_default_admin_delay)
242242
- [`rollback_default_admin_delay()`](#IAccessControlDefaultAdminRules-rollback_default_admin_delay)
243243
- [`default_admin_delay_increase_wait()`](#IAccessControlDefaultAdminRules-default_admin_delay_increase_wait)
244+
- [`maximum_default_admin_transfer_delay()`](#IAccessControlDefaultAdminRules-maximum_default_admin_transfer_delay)
244245

245246
Events
246247

@@ -401,11 +402,25 @@ May emit a [DefaultAdminDelayChangeCanceled](#IAccessControlDefaultAdminRules-De
401402
id="IAccessControlDefaultAdminRules-default_admin_delay_increase_wait"
402403
kind="external"
403404
>
404-
Maximum time in seconds for an increase to [default\_admin\_delay](#IAccessControlDefaultAdminRules-default_admin_delay) (that is scheduled using [change\_default\_admin\_delay](#IAccessControlDefaultAdminRules-change_default_admin_delay)) to take effect. Defaults to 5 days.
405-
406-
When the [default\_admin\_delay](#IAccessControlDefaultAdminRules-default_admin_delay) is scheduled to be increased, it goes into effect after the new delay has passed with the purpose of giving enough time for reverting any accidental change (i.e. using milliseconds instead of seconds) that may lock the contract. However, to avoid excessive schedules, the wait is capped by this function and it can be overridden for a custom [default\_admin\_delay](#IAccessControlDefaultAdminRules-default_admin_delay) increase scheduling.
405+
Maximum time in seconds for an increase to [default\_admin\_delay](#IAccessControlDefaultAdminRules-default_admin_delay) (that is scheduled using [change\_default\_admin\_delay](#IAccessControlDefaultAdminRules-change_default_admin_delay)) to take effect.
407406

407+
<Callout type='warn'>
408408
Make sure to add a reasonable amount of time while overriding this value, otherwise, there’s a risk of setting a high new delay that goes into effect almost immediately without the possibility of human intervention in the case of an input error (e.g. set milliseconds instead of seconds).
409+
</Callout>
410+
411+
Consider carefully the value set for `MAXIMUM_DEFAULT_ADMIN_TRANSFER_DELAY` too, since it will affect how fast you can recover from an accidental delay increase.
412+
</APIItem>
413+
414+
<APIItem
415+
functionSignature="maximum_default_admin_transfer_delay() → u64"
416+
id="IAccessControlDefaultAdminRules-maximum_default_admin_transfer_delay"
417+
kind="external"
418+
>
419+
Maximum time in seconds for a `default_admin` transfer delay.
420+
421+
<Callout type='warn'>
422+
If `MAXIMUM_DEFAULT_ADMIN_TRANSFER_DELAY` is set too high, you might be unable to recover from an accidental delay increase for an extended period. Too low, and it unnecessarily restricts how much security delay you can impose for `default_admin` transfers. As a best practice, consider setting it in the 30-60 day range for a good balance between security and recoverability.
423+
</Callout>
409424
</APIItem>
410425

411426
#### Events [!toc] [#IAccessControlDefaultAdminRules-Events]
@@ -1148,6 +1163,7 @@ Embeddable Implementations
11481163
- [`change_default_admin_delay(self, new_delay)`](#IAccessControlDefaultAdminRules-change_default_admin_delay)
11491164
- [`rollback_default_admin_delay(self)`](#IAccessControlDefaultAdminRules-rollback_default_admin_delay)
11501165
- [`default_admin_delay_increase_wait(self)`](#IAccessControlDefaultAdminRules-default_admin_delay_increase_wait)
1166+
- [`maximum_default_admin_transfer_delay(self)`](#IAccessControlDefaultAdminRules-maximum_default_admin_transfer_delay)
11511167

11521168
#### AccessControlImpl [!toc] [#AccessControlDefaultAdminRulesComponent-Embeddable-Impls-AccessControlImpl]
11531169

@@ -1368,6 +1384,18 @@ Make sure to add a reasonable amount of time while overriding this value, otherw
13681384
</Callout>
13691385
</APIItem>
13701386

1387+
<APIItem
1388+
functionSignature="maximum_default_admin_transfer_delay(self: @ContractState) → u64"
1389+
id="AccessControlDefaultAdminRulesComponent-maximum_default_admin_transfer_delay"
1390+
kind="external"
1391+
>
1392+
Maximum time in seconds for a `default_admin` transfer delay.
1393+
1394+
<Callout type='warn'>
1395+
If `MAXIMUM_DEFAULT_ADMIN_TRANSFER_DELAY` is set too high, you might be unable to recover from an accidental delay increase for an extended period. Too low, and it unnecessarily restricts how much security delay you can impose for `default_admin` transfers. As a best practice, consider setting it in the 30-60 day range for a good balance between security and recoverability.
1396+
</Callout>
1397+
</APIItem>
1398+
13711399
<APIItem
13721400
functionSignature="has_role(self: @ContractState, role: felt252, account: ContractAddress) → bool"
13731401
id="AccessControlDefaultAdminRulesComponent-has_role"

content/contracts-cairo/3.x/api/erc20.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,6 +1139,8 @@ Allows contracts to hook logic into deposit and withdraw transactions. This is w
11391139

11401140
ERC4626 preview methods must be inclusive of any entry or exit fees. Fees are calculated using [FeeConfigTrait](#ERC4626Component-FeeConfigTrait) methods and automatically adjust the final asset and share amounts. Fee transfers are handled in `ERC4626HooksTrait` methods.
11411141

1142+
When a vault implements fees on deposits or withdrawals (either in shares or assets), fee transfers must be handled in these hooks by library clients. This creates a non-atomic operation flow consisting of multiple state-changing steps: transferring assets, minting or burning shares, and transferring (or minting) fees. Between these steps, the vault's state is temporarily inconsistent: the asset-to-share conversion rate does not accurately reflect the vault's final state until all steps have completed. Therefore, it is critical to avoid making any external calls (including to the vault contract itself) or querying conversion rates during hook execution.
1143+
11421144
Special care must be taken when calling external contracts in these hooks. In that case, consider implementing reentrancy protections. For example, in the `withdraw` flow, the `withdraw_limit` is checked **before** the `before_withdraw` hook is invoked. If this hook performs a reentrant call that invokes `withdraw` again, the subsequent check on `withdraw_limit` will be done before the first withdrawal's core logic (e.g., burning shares and transferring assets) is executed. This could lead to bypassing withdrawal constraints or draining funds.
11431145

11441146
See the [ERC4626AssetsFeesMock](https://github.com/OpenZeppelin/cairo-contracts/tree/main/packages/test_common/src/mocks/erc4626.cairo#L253) and [ERC4626SharesFeesMock](https://github.com/OpenZeppelin/cairo-contracts/tree/main/packages/test_common/src/mocks/erc4626.cairo#L426) examples.

content/contracts-cairo/3.x/api/governance.mdx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,8 @@ Whether a proposal needs to be queued before execution. This indicates if the pr
185185
Delay between when a proposal is created and when the vote starts. The unit this duration is expressed in depends on the clock (see [ERC-6372](https://eips.ethereum.org/EIPS/eip-6372)) this contract uses.
186186

187187
This can be increased to leave time for users to buy voting power, or delegate it, before the voting of a proposal starts.
188+
189+
While this function returns a u64 value, timepoints must fit into u48 according to the EIP-6372 specification. Consequently this value must fit in a u48 (when added to the current clock).
188190
</APIItem>
189191

190192
<APIItem
@@ -1674,6 +1676,7 @@ Cast a vote.
16741676
Requirements:
16751677

16761678
- The proposal must be active.
1679+
- The current timepoint must be greater than the proposal's snapshot timepoint.
16771680

16781681
Emits a [VoteCast](#GovernorComponent-VoteCast) event.
16791682
</APIItem>
@@ -1688,6 +1691,7 @@ Cast a vote with a `reason`.
16881691
Requirements:
16891692

16901693
- The proposal must be active.
1694+
- The current timepoint must be greater than the proposal's snapshot timepoint.
16911695

16921696
Emits a [VoteCast](#GovernorComponent-VoteCast) event.
16931697
</APIItem>
@@ -1702,6 +1706,7 @@ Cast a vote with a `reason` and additional serialized `params`.
17021706
Requirements:
17031707

17041708
- The proposal must be active.
1709+
- The current timepoint must be greater than the proposal's snapshot timepoint.
17051710

17061711
Emits either:
17071712

@@ -1719,6 +1724,7 @@ Cast a vote using the `voter`'s signature.
17191724
Requirements:
17201725

17211726
- The proposal must be active.
1727+
- The current timepoint must be greater than the proposal's snapshot timepoint.
17221728
- The nonce in the signed message must match the account's current nonce.
17231729
- `voter` must implement `SRC6::is_valid_signature`.
17241730
- `signature` must be valid for the message hash.
@@ -1736,6 +1742,7 @@ Cast a vote with a `reason` and additional serialized `params` using the `voter`
17361742
Requirements:
17371743

17381744
- The proposal must be active.
1745+
- The current timepoint must be greater than the proposal's snapshot timepoint.
17391746
- The nonce in the signed message must match the account's current nonce.
17401747
- `voter` must implement `SRC6::is_valid_signature`.
17411748
- `signature` must be valid for the message hash.

content/contracts-cairo/3.x/api/utilities.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,8 @@ Returns the current timepoint determined by the contract's operational mode, int
275275
Requirements:
276276

277277
- This function MUST always be non-decreasing.
278+
279+
While this function returns a u64 value, timepoints must fit into u48 according to the EIP-6372 specification.
278280
</APIItem>
279281

280282
<APIItem

0 commit comments

Comments
 (0)