From 487367edf84f737d73bd20d296f0f7a83f7a4067 Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Mon, 1 Jun 2026 18:15:59 -0400 Subject: [PATCH 1/4] test(B20Security): add redeemWithMemo, updateMinimumRedeemable, updateShareRatio revertOrder tests BOP-221 --- .../redeem/redeemWithMemo_revertOrder.t.sol | 112 ++++++++++++++++++ .../updateMinimumRedeemable_revertOrder.t.sol | 19 +++ .../updateShareRatio_revertOrder.t.sol | 17 +++ 3 files changed, 148 insertions(+) create mode 100644 test/unit/B20Security/redeem/redeemWithMemo_revertOrder.t.sol create mode 100644 test/unit/B20Security/redeem/updateMinimumRedeemable_revertOrder.t.sol create mode 100644 test/unit/B20Security/shareRatio/updateShareRatio_revertOrder.t.sol diff --git a/test/unit/B20Security/redeem/redeemWithMemo_revertOrder.t.sol b/test/unit/B20Security/redeem/redeemWithMemo_revertOrder.t.sol new file mode 100644 index 00000000..6b99b2b1 --- /dev/null +++ b/test/unit/B20Security/redeem/redeemWithMemo_revertOrder.t.sol @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {IB20} from "src/interfaces/IB20.sol"; +import {IB20Security} from "src/interfaces/IB20Security.sol"; + +import {B20SecurityTest} from "test/lib/B20SecurityTest.sol"; +import {PolicyRegistryConstants} from "test/lib/mocks/MockPolicyRegistry.sol"; + +/// @title Differential check-order tests for `redeemWithMemo` (security variant). +/// +/// @notice **Canonical order (Solidity reference `_redeemBurn`, shared with `redeem`):** +/// 1. PAUSE (`_isPaused(REDEEM)`) → `ContractPaused` +/// 2. POLICY (`isAuthorized(REDEEMSenderPolicyId, msg.sender)`) → `PolicyForbids` +/// 3. BELOW-MIN (`shares == 0 || shares < minimum`) → `BelowMinimumRedeemable` +/// 4. BALANCE (`fromBalance < amount` in `_burnRaw`) → `InsufficientBalance` +/// +/// `redeemWithMemo` delegates to `_redeemBurn` identically to `redeem`; +/// only the subsequent `Memo` event emission differs. The ordering is +/// therefore the same as `redeem_revertOrder.t.sol`. +/// +/// C(4, 2) = 6 pairs. +contract B20SecurityRedeemWithMemoRevertOrderTest is B20SecurityTest { + // --- Pairs where PAUSE wins --- + + function test_redeemWithMemo_revertOrder_pause_beats_policy(uint256 amount, bytes32 memo) public { + amount = bound(amount, 1, type(uint128).max); + _pause(IB20.PausableFeature.REDEEM); + _setRedeemPolicy(PolicyRegistryConstants.ALWAYS_BLOCK_ID); + + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.REDEEM)); + security().redeemWithMemo(amount, memo); + } + + function test_redeemWithMemo_revertOrder_pause_beats_belowMinimum(uint256 amount, uint256 minimum, bytes32 memo) + public + { + amount = bound(amount, 1, type(uint64).max); + minimum = bound(minimum, amount + 1, type(uint256).max); + _pause(IB20.PausableFeature.REDEEM); + _updateMinimumRedeemable(minimum); + + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.REDEEM)); + security().redeemWithMemo(amount, memo); + } + + function test_redeemWithMemo_revertOrder_pause_beats_balance(uint256 amount, bytes32 memo) public { + amount = bound(amount, 1, type(uint128).max); + _pause(IB20.PausableFeature.REDEEM); + // alice has zero balance. + + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.REDEEM)); + security().redeemWithMemo(amount, memo); + } + + // --- Pairs where POLICY wins --- + + function test_redeemWithMemo_revertOrder_policy_beats_belowMinimum( + uint256 amount, + uint256 minimum, + bytes32 memo + ) public { + amount = bound(amount, 1, type(uint64).max); + minimum = bound(minimum, amount + 1, type(uint256).max); + _setRedeemPolicy(PolicyRegistryConstants.ALWAYS_BLOCK_ID); + _updateMinimumRedeemable(minimum); + + vm.prank(alice); + vm.expectRevert( + abi.encodeWithSelector( + IB20.PolicyForbids.selector, security().REDEEM_SENDER_POLICY(), PolicyRegistryConstants.ALWAYS_BLOCK_ID + ) + ); + security().redeemWithMemo(amount, memo); + } + + function test_redeemWithMemo_revertOrder_policy_beats_balance(uint256 amount, bytes32 memo) public { + amount = bound(amount, 1, type(uint128).max); + _setRedeemPolicy(PolicyRegistryConstants.ALWAYS_BLOCK_ID); + // alice has zero balance. + + vm.prank(alice); + vm.expectRevert( + abi.encodeWithSelector( + IB20.PolicyForbids.selector, security().REDEEM_SENDER_POLICY(), PolicyRegistryConstants.ALWAYS_BLOCK_ID + ) + ); + security().redeemWithMemo(amount, memo); + } + + // --- Pair where BELOW-MIN wins --- + + function test_redeemWithMemo_revertOrder_belowMinimum_beats_balance( + uint256 amount, + uint256 minimum, + bytes32 memo + ) public { + amount = bound(amount, 1, type(uint64).max); + minimum = bound(minimum, amount + 1, type(uint256).max); + _setRedeemPolicy(PolicyRegistryConstants.ALWAYS_ALLOW_ID); + _updateMinimumRedeemable(minimum); + // alice has zero balance → BALANCE would fire if BELOW-MIN didn't. + // Default WAD ratio: shares = amount * WAD / WAD = amount. amount < minimum → BelowMinimumRedeemable. + + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(IB20Security.BelowMinimumRedeemable.selector, amount, minimum)); + security().redeemWithMemo(amount, memo); + } +} diff --git a/test/unit/B20Security/redeem/updateMinimumRedeemable_revertOrder.t.sol b/test/unit/B20Security/redeem/updateMinimumRedeemable_revertOrder.t.sol new file mode 100644 index 00000000..c5421c08 --- /dev/null +++ b/test/unit/B20Security/redeem/updateMinimumRedeemable_revertOrder.t.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {IB20} from "src/interfaces/IB20.sol"; + +import {B20SecurityTest} from "test/lib/B20SecurityTest.sol"; + +import {B20Constants} from "src/lib/B20Constants.sol"; + +/// @title Differential check-order tests for `updateMinimumRedeemable`. +/// +/// @notice **Canonical order (Solidity reference):** +/// 1. ROLE (`onlyRole(DEFAULT_ADMIN_ROLE)` modifier) → `AccessControlUnauthorizedAccount` +/// +/// C(1, 2) = 0 pairs. `updateMinimumRedeemable` has a single revert condition +/// (role check via modifier) and accepts any `uint256` without further +/// validation, so there are no ordering pairs to pin. This file is present +/// for completeness and to document that the single guard is the only revert path. +contract B20SecurityUpdateMinimumRedeemableRevertOrderTest is B20SecurityTest {} diff --git a/test/unit/B20Security/shareRatio/updateShareRatio_revertOrder.t.sol b/test/unit/B20Security/shareRatio/updateShareRatio_revertOrder.t.sol new file mode 100644 index 00000000..db50aa4c --- /dev/null +++ b/test/unit/B20Security/shareRatio/updateShareRatio_revertOrder.t.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {IB20} from "src/interfaces/IB20.sol"; + +import {B20SecurityTest} from "test/lib/B20SecurityTest.sol"; + +/// @title Differential check-order tests for `updateShareRatio`. +/// +/// @notice **Canonical order (Solidity reference):** +/// 1. ROLE (`onlyRole(SECURITY_OPERATOR_ROLE)` modifier) → `AccessControlUnauthorizedAccount` +/// +/// C(1, 2) = 0 pairs. `updateShareRatio` has a single revert condition +/// (role check via modifier) and accepts any `uint256` without further +/// validation, so there are no ordering pairs to pin. This file is present +/// for completeness and to document that the single guard is the only revert path. +contract B20SecurityUpdateShareRatioRevertOrderTest is B20SecurityTest {} From 6de266a108a89a0e722282278cdfb5146487571a Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Mon, 1 Jun 2026 19:08:07 -0400 Subject: [PATCH 2/4] chore: forge fmt --- .../redeem/redeemWithMemo_revertOrder.t.sol | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/test/unit/B20Security/redeem/redeemWithMemo_revertOrder.t.sol b/test/unit/B20Security/redeem/redeemWithMemo_revertOrder.t.sol index 6b99b2b1..d02e1ef1 100644 --- a/test/unit/B20Security/redeem/redeemWithMemo_revertOrder.t.sol +++ b/test/unit/B20Security/redeem/redeemWithMemo_revertOrder.t.sol @@ -58,11 +58,9 @@ contract B20SecurityRedeemWithMemoRevertOrderTest is B20SecurityTest { // --- Pairs where POLICY wins --- - function test_redeemWithMemo_revertOrder_policy_beats_belowMinimum( - uint256 amount, - uint256 minimum, - bytes32 memo - ) public { + function test_redeemWithMemo_revertOrder_policy_beats_belowMinimum(uint256 amount, uint256 minimum, bytes32 memo) + public + { amount = bound(amount, 1, type(uint64).max); minimum = bound(minimum, amount + 1, type(uint256).max); _setRedeemPolicy(PolicyRegistryConstants.ALWAYS_BLOCK_ID); @@ -93,11 +91,9 @@ contract B20SecurityRedeemWithMemoRevertOrderTest is B20SecurityTest { // --- Pair where BELOW-MIN wins --- - function test_redeemWithMemo_revertOrder_belowMinimum_beats_balance( - uint256 amount, - uint256 minimum, - bytes32 memo - ) public { + function test_redeemWithMemo_revertOrder_belowMinimum_beats_balance(uint256 amount, uint256 minimum, bytes32 memo) + public + { amount = bound(amount, 1, type(uint64).max); minimum = bound(minimum, amount + 1, type(uint256).max); _setRedeemPolicy(PolicyRegistryConstants.ALWAYS_ALLOW_ID); From 13a201914b16b09d381f669e0775337c7d69e95f Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Mon, 1 Jun 2026 19:35:43 -0400 Subject: [PATCH 3/4] test(B20Security): implement updateMinimumRedeemable and updateShareRatio revertOrder assertions --- .../updateMinimumRedeemable_revertOrder.t.sol | 18 +++++++++++++----- .../updateShareRatio_revertOrder.t.sol | 18 +++++++++++++----- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/test/unit/B20Security/redeem/updateMinimumRedeemable_revertOrder.t.sol b/test/unit/B20Security/redeem/updateMinimumRedeemable_revertOrder.t.sol index c5421c08..fa324e9f 100644 --- a/test/unit/B20Security/redeem/updateMinimumRedeemable_revertOrder.t.sol +++ b/test/unit/B20Security/redeem/updateMinimumRedeemable_revertOrder.t.sol @@ -12,8 +12,16 @@ import {B20Constants} from "src/lib/B20Constants.sol"; /// @notice **Canonical order (Solidity reference):** /// 1. ROLE (`onlyRole(DEFAULT_ADMIN_ROLE)` modifier) → `AccessControlUnauthorizedAccount` /// -/// C(1, 2) = 0 pairs. `updateMinimumRedeemable` has a single revert condition -/// (role check via modifier) and accepts any `uint256` without further -/// validation, so there are no ordering pairs to pin. This file is present -/// for completeness and to document that the single guard is the only revert path. -contract B20SecurityUpdateMinimumRedeemableRevertOrderTest is B20SecurityTest {} +/// Single revert condition — no ordering pairs. One test pins the guard itself. +contract B20SecurityUpdateMinimumRedeemableRevertOrderTest is B20SecurityTest { + function test_updateMinimumRedeemable_revertOrder_unauthorized(address caller, uint256 newMinimum) public { + vm.assume(caller != admin); + vm.prank(caller); + vm.expectRevert( + abi.encodeWithSelector( + IB20.AccessControlUnauthorizedAccount.selector, caller, B20Constants.DEFAULT_ADMIN_ROLE + ) + ); + security().updateMinimumRedeemable(newMinimum); + } +} diff --git a/test/unit/B20Security/shareRatio/updateShareRatio_revertOrder.t.sol b/test/unit/B20Security/shareRatio/updateShareRatio_revertOrder.t.sol index db50aa4c..9033c7cf 100644 --- a/test/unit/B20Security/shareRatio/updateShareRatio_revertOrder.t.sol +++ b/test/unit/B20Security/shareRatio/updateShareRatio_revertOrder.t.sol @@ -10,8 +10,16 @@ import {B20SecurityTest} from "test/lib/B20SecurityTest.sol"; /// @notice **Canonical order (Solidity reference):** /// 1. ROLE (`onlyRole(SECURITY_OPERATOR_ROLE)` modifier) → `AccessControlUnauthorizedAccount` /// -/// C(1, 2) = 0 pairs. `updateShareRatio` has a single revert condition -/// (role check via modifier) and accepts any `uint256` without further -/// validation, so there are no ordering pairs to pin. This file is present -/// for completeness and to document that the single guard is the only revert path. -contract B20SecurityUpdateShareRatioRevertOrderTest is B20SecurityTest {} +/// Single revert condition — no ordering pairs. One test pins the guard itself. +contract B20SecurityUpdateShareRatioRevertOrderTest is B20SecurityTest { + function test_updateShareRatio_revertOrder_unauthorized(address caller, uint256 newRatio) public { + vm.assume(caller != admin); + vm.prank(caller); + vm.expectRevert( + abi.encodeWithSelector( + IB20.AccessControlUnauthorizedAccount.selector, caller, security().SECURITY_OPERATOR_ROLE() + ) + ); + security().updateShareRatio(newRatio); + } +} From d2024f2413d2d562fbdcd8d4d1dd67546d79962d Mon Sep 17 00:00:00 2001 From: Eric Shenghsiung Liu Date: Mon, 1 Jun 2026 19:57:20 -0400 Subject: [PATCH 4/4] test(B20Security): rewrite revertOrder tests to sequential walk-through style Replace 6 pairwise tests in redeemWithMemo_revertOrder with a single fuzz test that activates all conditions simultaneously and fixes them highest-to-lowest priority (PAUSE -> POLICY -> BELOW_MIN -> BALANCE -> success). Rewrite the single-condition tests for updateMinimumRedeemable and updateShareRatio in the same sequential style (unauthorized -> grant role -> success). Also adds _assumeValidCaller and pre-prank role resolution to guard against precompile interference and prank-consuming view calls. --- .../redeem/redeemWithMemo_revertOrder.t.sol | 100 +++++++----------- .../updateMinimumRedeemable_revertOrder.t.sol | 23 +++- .../updateShareRatio_revertOrder.t.sol | 30 ++++-- 3 files changed, 79 insertions(+), 74 deletions(-) diff --git a/test/unit/B20Security/redeem/redeemWithMemo_revertOrder.t.sol b/test/unit/B20Security/redeem/redeemWithMemo_revertOrder.t.sol index d02e1ef1..3005f696 100644 --- a/test/unit/B20Security/redeem/redeemWithMemo_revertOrder.t.sol +++ b/test/unit/B20Security/redeem/redeemWithMemo_revertOrder.t.sol @@ -4,10 +4,12 @@ pragma solidity ^0.8.20; import {IB20} from "src/interfaces/IB20.sol"; import {IB20Security} from "src/interfaces/IB20Security.sol"; +import {B20Constants} from "src/lib/B20Constants.sol"; + import {B20SecurityTest} from "test/lib/B20SecurityTest.sol"; import {PolicyRegistryConstants} from "test/lib/mocks/MockPolicyRegistry.sol"; -/// @title Differential check-order tests for `redeemWithMemo` (security variant). +/// @title Sequential revert-order test for `redeemWithMemo` (security variant). /// /// @notice **Canonical order (Solidity reference `_redeemBurn`, shared with `redeem`):** /// 1. PAUSE (`_isPaused(REDEEM)`) → `ContractPaused` @@ -15,57 +17,40 @@ import {PolicyRegistryConstants} from "test/lib/mocks/MockPolicyRegistry.sol"; /// 3. BELOW-MIN (`shares == 0 || shares < minimum`) → `BelowMinimumRedeemable` /// 4. BALANCE (`fromBalance < amount` in `_burnRaw`) → `InsufficientBalance` /// -/// `redeemWithMemo` delegates to `_redeemBurn` identically to `redeem`; -/// only the subsequent `Memo` event emission differs. The ordering is -/// therefore the same as `redeem_revertOrder.t.sol`. -/// -/// C(4, 2) = 6 pairs. +/// A single test activates all four conditions at once and fixes them from +/// highest to lowest priority, asserting each revert in turn then completing +/// a successful call once all conditions are resolved. contract B20SecurityRedeemWithMemoRevertOrderTest is B20SecurityTest { - // --- Pairs where PAUSE wins --- - - function test_redeemWithMemo_revertOrder_pause_beats_policy(uint256 amount, bytes32 memo) public { - amount = bound(amount, 1, type(uint128).max); - _pause(IB20.PausableFeature.REDEEM); - _setRedeemPolicy(PolicyRegistryConstants.ALWAYS_BLOCK_ID); - - vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.REDEEM)); - security().redeemWithMemo(amount, memo); + /// @dev Unpauses a single feature via the `unpauser` actor, lazily granting + /// `UNPAUSE_ROLE` on first call. Mirrors the inherited `_pause` helper. + function _unpause(IB20.PausableFeature feature) private { + if (!token.hasRole(B20Constants.UNPAUSE_ROLE, unpauser)) _grantRole(B20Constants.UNPAUSE_ROLE, unpauser); + vm.prank(unpauser); + token.unpause(_singleFeature(feature)); } - function test_redeemWithMemo_revertOrder_pause_beats_belowMinimum(uint256 amount, uint256 minimum, bytes32 memo) - public - { + function test_redeemWithMemo_revertOrder(uint256 amount, uint256 minimum, bytes32 memo) public { amount = bound(amount, 1, type(uint64).max); minimum = bound(minimum, amount + 1, type(uint256).max); - _pause(IB20.PausableFeature.REDEEM); - _updateMinimumRedeemable(minimum); - vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.REDEEM)); - security().redeemWithMemo(amount, memo); - } - - function test_redeemWithMemo_revertOrder_pause_beats_balance(uint256 amount, bytes32 memo) public { - amount = bound(amount, 1, type(uint128).max); + // Setup: activate ALL revert conditions simultaneously. + // REDEEM is paused, the sender policy blocks all callers, + // minimumRedeemable > amount (so shares == amount < minimum with the + // default 1:1 WAD ratio), and alice has zero balance. _pause(IB20.PausableFeature.REDEEM); - // alice has zero balance. + _setRedeemPolicy(PolicyRegistryConstants.ALWAYS_BLOCK_ID); + _updateMinimumRedeemable(minimum); + // alice balance is zero by default. + // 1. PAUSE fires first. vm.prank(alice); vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.REDEEM)); security().redeemWithMemo(amount, memo); - } - // --- Pairs where POLICY wins --- - - function test_redeemWithMemo_revertOrder_policy_beats_belowMinimum(uint256 amount, uint256 minimum, bytes32 memo) - public - { - amount = bound(amount, 1, type(uint64).max); - minimum = bound(minimum, amount + 1, type(uint256).max); - _setRedeemPolicy(PolicyRegistryConstants.ALWAYS_BLOCK_ID); - _updateMinimumRedeemable(minimum); + // Fix: unpause REDEEM. + _unpause(IB20.PausableFeature.REDEEM); + // 2. POLICY fires next (pause fixed). vm.prank(alice); vm.expectRevert( abi.encodeWithSelector( @@ -73,36 +58,29 @@ contract B20SecurityRedeemWithMemoRevertOrderTest is B20SecurityTest { ) ); security().redeemWithMemo(amount, memo); - } - function test_redeemWithMemo_revertOrder_policy_beats_balance(uint256 amount, bytes32 memo) public { - amount = bound(amount, 1, type(uint128).max); - _setRedeemPolicy(PolicyRegistryConstants.ALWAYS_BLOCK_ID); - // alice has zero balance. + // Fix: allow alice through the redeem-sender policy. + _setRedeemPolicy(PolicyRegistryConstants.ALWAYS_ALLOW_ID); + // 3. BELOW-MIN fires next (pause and policy fixed). + // With the default 1:1 WAD ratio shares == amount, and amount < minimum. vm.prank(alice); - vm.expectRevert( - abi.encodeWithSelector( - IB20.PolicyForbids.selector, security().REDEEM_SENDER_POLICY(), PolicyRegistryConstants.ALWAYS_BLOCK_ID - ) - ); + vm.expectRevert(abi.encodeWithSelector(IB20Security.BelowMinimumRedeemable.selector, amount, minimum)); security().redeemWithMemo(amount, memo); - } - // --- Pair where BELOW-MIN wins --- + // Fix: clear the minimum floor so shares == amount satisfies the check. + _updateMinimumRedeemable(0); - function test_redeemWithMemo_revertOrder_belowMinimum_beats_balance(uint256 amount, uint256 minimum, bytes32 memo) - public - { - amount = bound(amount, 1, type(uint64).max); - minimum = bound(minimum, amount + 1, type(uint256).max); - _setRedeemPolicy(PolicyRegistryConstants.ALWAYS_ALLOW_ID); - _updateMinimumRedeemable(minimum); - // alice has zero balance → BALANCE would fire if BELOW-MIN didn't. - // Default WAD ratio: shares = amount * WAD / WAD = amount. amount < minimum → BelowMinimumRedeemable. + // 4. BALANCE fires next (pause, policy, and minimum fixed). + vm.prank(alice); + vm.expectRevert(abi.encodeWithSelector(IB20.InsufficientBalance.selector, alice, 0, amount)); + security().redeemWithMemo(amount, memo); + // Fix: give alice exactly enough balance to cover the redemption. + _mint(alice, amount); + + // Success: all conditions resolved. vm.prank(alice); - vm.expectRevert(abi.encodeWithSelector(IB20Security.BelowMinimumRedeemable.selector, amount, minimum)); security().redeemWithMemo(amount, memo); } } diff --git a/test/unit/B20Security/redeem/updateMinimumRedeemable_revertOrder.t.sol b/test/unit/B20Security/redeem/updateMinimumRedeemable_revertOrder.t.sol index fa324e9f..6142b2d7 100644 --- a/test/unit/B20Security/redeem/updateMinimumRedeemable_revertOrder.t.sol +++ b/test/unit/B20Security/redeem/updateMinimumRedeemable_revertOrder.t.sol @@ -3,19 +3,25 @@ pragma solidity ^0.8.20; import {IB20} from "src/interfaces/IB20.sol"; -import {B20SecurityTest} from "test/lib/B20SecurityTest.sol"; - import {B20Constants} from "src/lib/B20Constants.sol"; -/// @title Differential check-order tests for `updateMinimumRedeemable`. +import {B20SecurityTest} from "test/lib/B20SecurityTest.sol"; + +/// @title Sequential revert-order test for `updateMinimumRedeemable`. /// /// @notice **Canonical order (Solidity reference):** /// 1. ROLE (`onlyRole(DEFAULT_ADMIN_ROLE)` modifier) → `AccessControlUnauthorizedAccount` /// -/// Single revert condition — no ordering pairs. One test pins the guard itself. +/// A single test verifies the guard fires for an unauthorized caller and then +/// confirms the call succeeds once the role is granted. contract B20SecurityUpdateMinimumRedeemableRevertOrderTest is B20SecurityTest { - function test_updateMinimumRedeemable_revertOrder_unauthorized(address caller, uint256 newMinimum) public { + function test_updateMinimumRedeemable_revertOrder(address caller, uint256 newMinimum) public { + // Exclude precompiles (which can distort msg.sender) and admin (who already + // holds DEFAULT_ADMIN_ROLE, which would make the initial call succeed). + _assumeValidCaller(caller); vm.assume(caller != admin); + + // 1. ROLE fires. vm.prank(caller); vm.expectRevert( abi.encodeWithSelector( @@ -23,5 +29,12 @@ contract B20SecurityUpdateMinimumRedeemableRevertOrderTest is B20SecurityTest { ) ); security().updateMinimumRedeemable(newMinimum); + + // Fix: grant DEFAULT_ADMIN_ROLE to caller. + _grantRole(B20Constants.DEFAULT_ADMIN_ROLE, caller); + + // Success: all conditions resolved. + vm.prank(caller); + security().updateMinimumRedeemable(newMinimum); } } diff --git a/test/unit/B20Security/shareRatio/updateShareRatio_revertOrder.t.sol b/test/unit/B20Security/shareRatio/updateShareRatio_revertOrder.t.sol index 9033c7cf..080ab0f2 100644 --- a/test/unit/B20Security/shareRatio/updateShareRatio_revertOrder.t.sol +++ b/test/unit/B20Security/shareRatio/updateShareRatio_revertOrder.t.sol @@ -5,21 +5,35 @@ import {IB20} from "src/interfaces/IB20.sol"; import {B20SecurityTest} from "test/lib/B20SecurityTest.sol"; -/// @title Differential check-order tests for `updateShareRatio`. +/// @title Sequential revert-order test for `updateShareRatio`. /// /// @notice **Canonical order (Solidity reference):** /// 1. ROLE (`onlyRole(SECURITY_OPERATOR_ROLE)` modifier) → `AccessControlUnauthorizedAccount` /// -/// Single revert condition — no ordering pairs. One test pins the guard itself. +/// A single test verifies the guard fires for an unauthorized caller and then +/// confirms the call succeeds once the role is granted. contract B20SecurityUpdateShareRatioRevertOrderTest is B20SecurityTest { - function test_updateShareRatio_revertOrder_unauthorized(address caller, uint256 newRatio) public { + function test_updateShareRatio_revertOrder(address caller, uint256 newRatio) public { + // Exclude precompiles (which can distort msg.sender) and admin (needed + // internally by _grantRole to approve the role grant). + _assumeValidCaller(caller); vm.assume(caller != admin); + + // Resolve the role identifier before setting the prank; an external view + // call inside abi.encodeWithSelector would otherwise consume vm.prank + // before the state-changing call, sending it as address(this) instead. + bytes32 operatorRole = security().SECURITY_OPERATOR_ROLE(); + + // 1. ROLE fires. + vm.prank(caller); + vm.expectRevert(abi.encodeWithSelector(IB20.AccessControlUnauthorizedAccount.selector, caller, operatorRole)); + security().updateShareRatio(newRatio); + + // Fix: grant SECURITY_OPERATOR_ROLE to caller. + _grantRole(operatorRole, caller); + + // Success: all conditions resolved. vm.prank(caller); - vm.expectRevert( - abi.encodeWithSelector( - IB20.AccessControlUnauthorizedAccount.selector, caller, security().SECURITY_OPERATOR_ROLE() - ) - ); security().updateShareRatio(newRatio); } }