diff --git a/docs/B20/Security.md b/docs/B20/Security.md index ae743f5..023b5e5 100644 --- a/docs/B20/Security.md +++ b/docs/B20/Security.md @@ -37,18 +37,17 @@ IB20Security(token).announce({ }); ``` -The four corporate-actions setters should be wrapped in `announce()`: +The three corporate-actions setters should be wrapped in `announce()`: - `updateShareRatio(...)` - `batchMint(...)` -- `batchBurn(...)` - `updateSecurityIdentifier(...)` Direct invocation by a role holder is permitted as an **emergency override** — it succeeds but produces no bracket events. Suitable only for break-glass scenarios where the inability to emit an announcement is itself part of the response. -## Batch Mint/Burn +## Batch Mint -`batchMint(recipients, amounts)` mints to many accounts in one call, gated by `MINT_ROLE`. `batchBurn(holders, amounts)` burns from many accounts in one call, gated by `BURN_FROM_ROLE`. Both should be wrapped in `announce()`, which additionally requires the operator to hold `SECURITY_OPERATOR_ROLE` (typically granted as a single bundle). +`batchMint(recipients, amounts)` mints to many accounts in one call, gated by `MINT_ROLE`. It should be wrapped in `announce()`, which additionally requires the operator to hold `SECURITY_OPERATOR_ROLE` (typically granted as a single bundle). ## Redemptions @@ -71,11 +70,7 @@ Each Security token can carry one or more standardized identifiers (ISIN, CUSIP, ### `SECURITY_OPERATOR_ROLE` -Gates the four corporate-actions setters (`updateShareRatio`, `batchMint`, `batchBurn`, `updateSecurityIdentifier`) and the `announce` wrapper itself. Held separately from `DEFAULT_ADMIN_ROLE` so corporate-actions operators don't need full admin authority. Operationally paired with `METADATA_ROLE` — when granting one, you typically grant the other to the same address. - -### `BURN_FROM_ROLE` - -Gates `batchBurn`, which burns balances held by other accounts as part of an announced corporate action. Distinct from `BURN_ROLE` (caller burns their own balance) and `BURN_BLOCKED_ROLE` (sanctions-seizure against policy-blocked addresses) — three burn primitives serve three different operational scenarios. +Gates the three corporate-actions setters (`updateShareRatio`, `batchMint`, `updateSecurityIdentifier`) and the `announce` wrapper itself. Held separately from `DEFAULT_ADMIN_ROLE` so corporate-actions operators don't need full admin authority. Operationally paired with `METADATA_ROLE` — when granting one, you typically grant the other to the same address. ## Fixed Decimals (6) diff --git a/src/interfaces/IB20Security.sol b/src/interfaces/IB20Security.sol index 31327f6..4f1628b 100644 --- a/src/interfaces/IB20Security.sol +++ b/src/interfaces/IB20Security.sol @@ -7,7 +7,7 @@ import {IB20} from "./IB20.sol"; /// @author Coinbase /// /// @notice A B-20 token variant for tokenized securities. Extends `IB20` with announcements, -/// share-ratio accounting, batched mint/burn for corporate actions, holder-initiated +/// share-ratio accounting, batched mint for corporate actions, holder-initiated /// redemption, and security-identifier metadata. interface IB20Security is IB20 { /*////////////////////////////////////////////////////////////// @@ -80,10 +80,6 @@ interface IB20Security is IB20 { /// @return Role identifier. function SECURITY_OPERATOR_ROLE() external view returns (bytes32); - /// @notice Required to call `batchBurn`. - /// @return Role identifier. - function BURN_FROM_ROLE() external view returns (bytes32); - /*////////////////////////////////////////////////////////////// PRECISION //////////////////////////////////////////////////////////////*/ @@ -164,7 +160,7 @@ interface IB20Security is IB20 { function updateShareRatio(uint256 newSharesToTokensRatio) external; /*////////////////////////////////////////////////////////////// - BATCHED ISSUANCE AND CORP-ACTION CLAWBACK + BATCHED ISSUANCE //////////////////////////////////////////////////////////////*/ /// @notice Mints `amounts[i]` to `recipients[i]` in one call. All-or-nothing: any element @@ -183,21 +179,6 @@ interface IB20Security is IB20 { /// @param amounts Per-recipient amounts, parallel to `recipients`. function batchMint(address[] calldata recipients, uint256[] calldata amounts) external; - /// @notice Burns `amounts[i]` from `accounts[i]` in one call, unconditionally on the supplied - /// accounts (no policy gate). All-or-nothing: any element revert unwinds the whole - /// transaction. Emits `Transfer(accounts[i], address(0), amounts[i])` per element; - /// does NOT emit `BurnedBlocked`. - /// - /// @dev Reverts with `ContractPaused(BURN)` when `BURN` is paused. - /// @dev Reverts with `AccessControlUnauthorizedAccount` when the caller does not hold `BURN_FROM_ROLE`. Strict — no factory-bootstrap bypass. - /// @dev Reverts with `LengthMismatch` when `accounts.length != amounts.length`. - /// @dev Reverts with `EmptyBatch` when either array is empty. - /// @dev Reverts with `InsufficientBalance` when any `accounts[i]`'s balance is below `amounts[i]`. - /// - /// @param accounts Accounts whose balances will be debited. - /// @param amounts Per-account amounts, parallel to `accounts`. - function batchBurn(address[] calldata accounts, uint256[] calldata amounts) external; - /*////////////////////////////////////////////////////////////// REDEMPTION //////////////////////////////////////////////////////////////*/ diff --git a/src/lib/B20Constants.sol b/src/lib/B20Constants.sol index f52cbdd..527b295 100644 --- a/src/lib/B20Constants.sol +++ b/src/lib/B20Constants.sol @@ -8,7 +8,6 @@ library B20Constants { bytes32 internal constant MINT_ROLE = keccak256("MINT_ROLE"); bytes32 internal constant BURN_ROLE = keccak256("BURN_ROLE"); bytes32 internal constant BURN_BLOCKED_ROLE = keccak256("BURN_BLOCKED_ROLE"); - bytes32 internal constant BURN_FROM_ROLE = keccak256("BURN_FROM_ROLE"); bytes32 internal constant PAUSE_ROLE = keccak256("PAUSE_ROLE"); bytes32 internal constant UNPAUSE_ROLE = keccak256("UNPAUSE_ROLE"); bytes32 internal constant METADATA_ROLE = keccak256("METADATA_ROLE"); diff --git a/src/lib/B20FactoryLib.sol b/src/lib/B20FactoryLib.sol index 6d68cd3..d107d16 100644 --- a/src/lib/B20FactoryLib.sol +++ b/src/lib/B20FactoryLib.sol @@ -50,7 +50,7 @@ library B20FactoryLib { } /// @notice Bootstrap role-grant bundle for `B20Variant.SECURITY`. Superset of `B20RoleHolders` - /// with `BURN_FROM_ROLE` and `SECURITY_OPERATOR_ROLE` slots. + /// with a `SECURITY_OPERATOR_ROLE` slot. /// /// @dev `DEFAULT_ADMIN_ROLE` is assigned via `B20SecurityCreateParams.initialAdmin`, not this struct. struct B20SecurityRoleHolders { @@ -60,8 +60,6 @@ library B20FactoryLib { address burner; /// @dev Account granted `BURN_BLOCKED_ROLE`. address burnBlocker; - /// @dev Account granted `BURN_FROM_ROLE`. - address burnFromOperator; /// @dev Account granted `PAUSE_ROLE`. address pauser; /// @dev Account granted `UNPAUSE_ROLE`. @@ -253,25 +251,23 @@ library B20FactoryLib { /// @param holders Security role-holder bundle. /// @return initCalls ABI-encoded `grantRole` initCalls. function buildRoleGrants(B20SecurityRoleHolders memory holders) internal pure returns (bytes[] memory initCalls) { - bytes32[] memory roles = new bytes32[](8); + bytes32[] memory roles = new bytes32[](7); roles[0] = B20Constants.MINT_ROLE; roles[1] = B20Constants.BURN_ROLE; roles[2] = B20Constants.BURN_BLOCKED_ROLE; - roles[3] = B20Constants.BURN_FROM_ROLE; - roles[4] = B20Constants.PAUSE_ROLE; - roles[5] = B20Constants.UNPAUSE_ROLE; - roles[6] = B20Constants.METADATA_ROLE; - roles[7] = B20Constants.SECURITY_OPERATOR_ROLE; + roles[3] = B20Constants.PAUSE_ROLE; + roles[4] = B20Constants.UNPAUSE_ROLE; + roles[5] = B20Constants.METADATA_ROLE; + roles[6] = B20Constants.SECURITY_OPERATOR_ROLE; - address[] memory accounts = new address[](8); + address[] memory accounts = new address[](7); accounts[0] = holders.minter; accounts[1] = holders.burner; accounts[2] = holders.burnBlocker; - accounts[3] = holders.burnFromOperator; - accounts[4] = holders.pauser; - accounts[5] = holders.unpauser; - accounts[6] = holders.metadataAdmin; - accounts[7] = holders.securityOperator; + accounts[3] = holders.pauser; + accounts[4] = holders.unpauser; + accounts[5] = holders.metadataAdmin; + accounts[6] = holders.securityOperator; return buildRoleGrants(roles, accounts); } diff --git a/test/lib/B20SecurityTest.sol b/test/lib/B20SecurityTest.sol index 492731a..cd50fec 100644 --- a/test/lib/B20SecurityTest.sol +++ b/test/lib/B20SecurityTest.sol @@ -10,9 +10,9 @@ import {IB20Security} from "src/interfaces/IB20Security.sol"; /// Extends `B20Test` for the inherited test surface (actors, labels, /// setUp wiring, the `_singleFeature` helper, the `_grantRole` / /// `_mint` / `_pause` action wrappers, and the security-variant token -/// deployed by `_deployToken`). Adds the variant-specific role holders -/// (`operator`, `burnFromActor`) plus helpers for the announcement, -/// share-ratio, redemption, and identifier surfaces. +/// deployed by `_deployToken`). Adds the variant-specific role holder +/// (`operator`) plus helpers for the announcement, share-ratio, +/// redemption, and identifier surfaces. /// /// The inherited `token` member is typed `IB20`. Tests that need the /// variant-only surface (`announce`, `redeem`, etc.) cast inline via @@ -20,7 +20,6 @@ import {IB20Security} from "src/interfaces/IB20Security.sol"; contract B20SecurityTest is B20Test { // -- Security-variant role-holder actors -- address internal operator = makeAddr("operator"); - address internal burnFromActor = makeAddr("burnFromActor"); // ============================================================ // SECURITY-VARIANT IDENTIFIER FIXTURES @@ -43,7 +42,6 @@ contract B20SecurityTest is B20Test { function setUp() public virtual override { super.setUp(); vm.label(operator, "operator"); - vm.label(burnFromActor, "burnFromActor"); } // ============================================================ @@ -67,13 +65,6 @@ contract B20SecurityTest is B20Test { if (!token.hasRole(role, operator)) _grantRole(role, operator); } - /// @notice Grants `BURN_FROM_ROLE` to the `burnFromActor` actor as - /// the admin, idempotent. - function _grantBurnFrom() internal { - bytes32 role = security().BURN_FROM_ROLE(); - if (!token.hasRole(role, burnFromActor)) _grantRole(role, burnFromActor); - } - // ============================================================ // SHARE-RATIO HELPERS // ============================================================ @@ -188,6 +179,5 @@ contract B20SecurityTest is B20Test { // `test/unit/B20Security/constants/` pins that down. bytes32 internal constant SECURITY_OPERATOR_ROLE = keccak256("SECURITY_OPERATOR_ROLE"); - bytes32 internal constant BURN_FROM_ROLE = keccak256("BURN_FROM_ROLE"); bytes32 internal constant REDEEM_SENDER_POLICY = keccak256("REDEEM_SENDER_POLICY"); } diff --git a/test/lib/B20Test.sol b/test/lib/B20Test.sol index 4e8c654..f22eda2 100644 --- a/test/lib/B20Test.sol +++ b/test/lib/B20Test.sol @@ -141,13 +141,19 @@ contract B20Test is B20FactoryTest { return B20Constants.MINT_RECEIVER_POLICY; } - /// @notice True iff `policyType` is one of the four base-token - /// supported policy types. Used by tests that fuzz arbitrary - /// `bytes32` and need to filter to the supported / unsupported - /// partition. + /// @notice True iff `policyType` is supported by the deployed token. + /// Used by tests that fuzz arbitrary `bytes32` and need to filter + /// to the supported / unsupported partition. Includes the four + /// base-token policy types AND the security variant's own + /// `REDEEM_SENDER_POLICY`, because `_deployToken()` returns a + /// security-variant token (see contract-level natspec) — fuzzing + /// a `bytes32` that happens to equal `keccak256("REDEEM_SENDER_POLICY")` + /// would otherwise fall through to the variant's `_readPolicyId` + /// override and not revert as `UnsupportedPolicyType`. function _isKnownPolicyType(bytes32 policyType) internal pure returns (bool) { return policyType == B20Constants.TRANSFER_SENDER_POLICY || policyType == B20Constants.TRANSFER_RECEIVER_POLICY - || policyType == B20Constants.TRANSFER_EXECUTOR_POLICY || policyType == B20Constants.MINT_RECEIVER_POLICY; + || policyType == B20Constants.TRANSFER_EXECUTOR_POLICY || policyType == B20Constants.MINT_RECEIVER_POLICY + || policyType == keccak256("REDEEM_SENDER_POLICY"); } /// @notice Pauses a single `PausableFeature`, lazily granting `PAUSE_ROLE` diff --git a/test/lib/mocks/MockB20.sol b/test/lib/mocks/MockB20.sol index a213f41..207b41c 100644 --- a/test/lib/mocks/MockB20.sol +++ b/test/lib/mocks/MockB20.sol @@ -755,10 +755,10 @@ abstract contract MockB20 is IB20 { /// @dev Pure mechanics: balance + effects. Pause and role gates are /// enforced by entrypoint modifiers (`whenNotPaused`, /// `onlyRole`) on every caller (`burn`, `burnWithMemo`, - /// `burnBlocked`, and `MockB20Security`'s `batchBurn` / - /// `_redeemBurn`); see those functions for the per-caller - /// authorization surface. This helper never authorizes the - /// destruction on its own. + /// `burnBlocked`, and `MockB20Security`'s `_redeemBurn`); + /// see those functions for the per-caller authorization + /// surface. This helper never authorizes the destruction on + /// its own. function _burnRaw(address from, uint256 amount) internal { MockB20Storage.Layout storage $ = MockB20Storage.layout(); uint256 fromBalance = $.balances[from]; diff --git a/test/lib/mocks/MockB20Security.sol b/test/lib/mocks/MockB20Security.sol index 81ef02c..1056f2c 100644 --- a/test/lib/mocks/MockB20Security.sol +++ b/test/lib/mocks/MockB20Security.sol @@ -59,13 +59,12 @@ import {MockB20RedeemStorage} from "test/lib/mocks/MockB20Storage.sol"; /// `_isPrivileged()` so the factory can stage initial /// announcements, batched issuance, ratios, identifiers, and /// minimum-redeemable values during the bootstrap window -/// without first granting itself roles. The two paths that -/// the factory will never legitimately call during bootstrap -/// (`redeem` / `redeemWithMemo`, which are holder-initiated, -/// and `batchBurn`, which is a corporate-actions clawback -/// against existing balances) deliberately do NOT bypass -/// their authorization checks: there is no init-time use case -/// for them, so the bypass would be dead code that widens the +/// without first granting itself roles. `redeem` / +/// `redeemWithMemo` are the only paths the factory will never +/// legitimately call during bootstrap (they are +/// holder-initiated) and deliberately do NOT bypass their +/// authorization checks: there is no init-time use case for +/// them, so the bypass would be dead code that widens the /// attack surface without buying anything. Token invariants /// (supply-cap math, balance accounting, share-amount floor /// on `redeem`) are NOT bypassed anywhere. @@ -75,7 +74,6 @@ contract MockB20Security is MockB20, IB20Security { // ============================================================ bytes32 public constant SECURITY_OPERATOR_ROLE = keccak256("SECURITY_OPERATOR_ROLE"); - bytes32 public constant BURN_FROM_ROLE = keccak256("BURN_FROM_ROLE"); bytes32 public constant REDEEM_SENDER_POLICY = keccak256("REDEEM_SENDER_POLICY"); @@ -96,25 +94,6 @@ contract MockB20Security is MockB20, IB20Security { return 6; } - // ============================================================ - // MODIFIERS - // ============================================================ - - /// @dev Like the base `onlyRole` but without the factory bootstrap - /// bypass: the role check is unconditional, even for the - /// factory in the init window. Reserved for variant paths - /// that deliberately reject the bypass — currently just - /// `batchBurn`, the corporate-actions clawback that has no - /// init-time use case (see this contract's natspec). Lives - /// here, not on the base, because no base function needs the - /// stricter gate. - modifier onlyRoleStrict(bytes32 role) { - if (!hasRole(role, msg.sender)) { - revert AccessControlUnauthorizedAccount(msg.sender, role); - } - _; - } - // ============================================================ // ANNOUNCEMENTS // ============================================================ @@ -170,7 +149,7 @@ contract MockB20Security is MockB20, IB20Security { } // ============================================================ - // BATCHED ISSUANCE / CLAWBACK + // BATCHED ISSUANCE // ============================================================ /// @dev Pause + role enforced ONCE for the entire batch via the @@ -190,28 +169,6 @@ contract MockB20Security is MockB20, IB20Security { } } - /// @dev `onlyRoleStrict` (not `onlyRole`): the factory bootstrap - /// bypass is deliberately NOT honored here, per the contract - /// natspec — clawback against existing balances has no init-time - /// use case, so granting the factory a bypass would only widen - /// the attack surface. - function batchBurn(address[] calldata accounts, uint256[] calldata amounts) - external - whenNotPaused(PausableFeature.BURN) - onlyRoleStrict(BURN_FROM_ROLE) - { - if (accounts.length != amounts.length) { - revert LengthMismatch(accounts.length, amounts.length); - } - if (accounts.length == 0) revert EmptyBatch(); - for (uint256 i = 0; i < accounts.length; i++) { - // Zero amounts are allowed per ERC-20 conventions (`transfer(0)` is valid); - // `_burnRaw` is a no-op for amount == 0 and emits `Transfer(account, 0, 0)`. - // Callers that want all-non-zero semantics can validate upstream. - _burnRaw(accounts[i], amounts[i]); - } - } - // ============================================================ // REDEMPTION // ============================================================ diff --git a/test/unit/B20FactoryLib/buildRoleGrants.t.sol b/test/unit/B20FactoryLib/buildRoleGrants.t.sol index 823a532..dd143db 100644 --- a/test/unit/B20FactoryLib/buildRoleGrants.t.sol +++ b/test/unit/B20FactoryLib/buildRoleGrants.t.sol @@ -225,15 +225,14 @@ contract B20FactoryLibBuildRoleGrantsTest is B20FactoryLibTest { } /// @notice A fully-populated `B20SecurityRoleHolders` bundle emits the - /// eight grants in struct-field order. + /// seven grants in struct-field order. /// @dev Same field-to-role pinning as the IB20 bundle, extended to - /// the security-only `burnFromOperator` and `securityOperator` - /// slots. Catches any swap among the eight role IDs. - function test_buildRoleGrants_securityBundle_success_emitsAllEightInStructOrder( + /// the security-only `securityOperator` slot. Catches any + /// swap among the seven role IDs. + function test_buildRoleGrants_securityBundle_success_emitsAllSevenInStructOrder( address minter_, address burner_, address burnBlocker_, - address burnFromOperator_, address pauser_, address unpauser_, address metadataAdmin_, @@ -242,7 +241,6 @@ contract B20FactoryLibBuildRoleGrantsTest is B20FactoryLibTest { vm.assume(minter_ != address(0)); vm.assume(burner_ != address(0)); vm.assume(burnBlocker_ != address(0)); - vm.assume(burnFromOperator_ != address(0)); vm.assume(pauser_ != address(0)); vm.assume(unpauser_ != address(0)); vm.assume(metadataAdmin_ != address(0)); @@ -252,7 +250,6 @@ contract B20FactoryLibBuildRoleGrantsTest is B20FactoryLibTest { minter: minter_, burner: burner_, burnBlocker: burnBlocker_, - burnFromOperator: burnFromOperator_, pauser: pauser_, unpauser: unpauser_, metadataAdmin: metadataAdmin_, @@ -261,7 +258,7 @@ contract B20FactoryLibBuildRoleGrantsTest is B20FactoryLibTest { bytes[] memory result = B20FactoryLib.buildRoleGrants(holders); - assertEq(result.length, 8, "fully-populated security bundle must emit eight grants"); + assertEq(result.length, 7, "fully-populated security bundle must emit seven grants"); assertEq(result[0], abi.encodeCall(IB20.grantRole, (B20Constants.MINT_ROLE, minter_)), "0: MINT_ROLE"); assertEq(result[1], abi.encodeCall(IB20.grantRole, (B20Constants.BURN_ROLE, burner_)), "1: BURN_ROLE"); assertEq( @@ -269,41 +266,33 @@ contract B20FactoryLibBuildRoleGrantsTest is B20FactoryLibTest { abi.encodeCall(IB20.grantRole, (B20Constants.BURN_BLOCKED_ROLE, burnBlocker_)), "2: BURN_BLOCKED_ROLE" ); + assertEq(result[3], abi.encodeCall(IB20.grantRole, (B20Constants.PAUSE_ROLE, pauser_)), "3: PAUSE_ROLE"); + assertEq(result[4], abi.encodeCall(IB20.grantRole, (B20Constants.UNPAUSE_ROLE, unpauser_)), "4: UNPAUSE_ROLE"); assertEq( - result[3], - abi.encodeCall(IB20.grantRole, (B20Constants.BURN_FROM_ROLE, burnFromOperator_)), - "3: BURN_FROM_ROLE" - ); - assertEq(result[4], abi.encodeCall(IB20.grantRole, (B20Constants.PAUSE_ROLE, pauser_)), "4: PAUSE_ROLE"); - assertEq(result[5], abi.encodeCall(IB20.grantRole, (B20Constants.UNPAUSE_ROLE, unpauser_)), "5: UNPAUSE_ROLE"); - assertEq( - result[6], abi.encodeCall(IB20.grantRole, (B20Constants.METADATA_ROLE, metadataAdmin_)), "6: METADATA_ROLE" + result[5], abi.encodeCall(IB20.grantRole, (B20Constants.METADATA_ROLE, metadataAdmin_)), "5: METADATA_ROLE" ); assertEq( - result[7], + result[6], abi.encodeCall(IB20.grantRole, (B20Constants.SECURITY_OPERATOR_ROLE, securityOperator_)), - "7: SECURITY_OPERATOR_ROLE" + "6: SECURITY_OPERATOR_ROLE" ); } /// @notice A `B20SecurityRoleHolders` bundle with only the - /// security-only slots populated emits exactly those two - /// grants in struct-field order. - /// @dev Pins that `BURN_FROM_ROLE` and `SECURITY_OPERATOR_ROLE` - /// pick up the right slot positions and the IB20 slots are - /// skipped when zero. - function test_buildRoleGrants_securityBundle_success_emitsOnlySecurityOnlySlots( - address burnFromOperator_, - address securityOperator_ - ) public pure { - vm.assume(burnFromOperator_ != address(0)); + /// security-only slot populated emits exactly that one + /// grant. + /// @dev Pins that `SECURITY_OPERATOR_ROLE` picks up its slot + /// position and the IB20 slots are skipped when zero. + function test_buildRoleGrants_securityBundle_success_emitsOnlySecurityOnlySlot(address securityOperator_) + public + pure + { vm.assume(securityOperator_ != address(0)); B20FactoryLib.B20SecurityRoleHolders memory holders = B20FactoryLib.B20SecurityRoleHolders({ minter: address(0), burner: address(0), burnBlocker: address(0), - burnFromOperator: burnFromOperator_, pauser: address(0), unpauser: address(0), metadataAdmin: address(0), @@ -312,16 +301,11 @@ contract B20FactoryLibBuildRoleGrantsTest is B20FactoryLibTest { bytes[] memory result = B20FactoryLib.buildRoleGrants(holders); - assertEq(result.length, 2, "exactly the two security-only entries must survive"); + assertEq(result.length, 1, "exactly the security-only entry must survive"); assertEq( result[0], - abi.encodeCall(IB20.grantRole, (B20Constants.BURN_FROM_ROLE, burnFromOperator_)), - "burnFromOperator first" - ); - assertEq( - result[1], abi.encodeCall(IB20.grantRole, (B20Constants.SECURITY_OPERATOR_ROLE, securityOperator_)), - "securityOperator second" + "securityOperator only" ); } } diff --git a/test/unit/B20Security/batch/batchBurn.t.sol b/test/unit/B20Security/batch/batchBurn.t.sol deleted file mode 100644 index ede3840..0000000 --- a/test/unit/B20Security/batch/batchBurn.t.sol +++ /dev/null @@ -1,190 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; - -import {B20SecurityTest} from "test/lib/B20SecurityTest.sol"; - -import {IB20} from "src/interfaces/IB20.sol"; -import {IB20Security} from "src/interfaces/IB20Security.sol"; - -contract B20SecurityBatchBurnTest is B20SecurityTest { - /// @notice Verifies batchBurn reverts when caller lacks BURN_FROM_ROLE - /// @dev Access control via `onlyRoleStrict(BURN_FROM_ROLE)`: NOT the inherited `onlyRole`, - /// because this path deliberately rejects the factory bootstrap bypass (see contract - /// natspec). Non-role-holder caller hits the standard AccessControlUnauthorizedAccount. - function test_batchBurn_revert_unauthorized(address caller) public { - _assumeValidCaller(caller); - vm.assume(caller != admin); - vm.assume(caller != burnFromActor); - - vm.prank(caller); - vm.expectRevert(abi.encodeWithSelector(IB20.AccessControlUnauthorizedAccount.selector, caller, BURN_FROM_ROLE)); - security().batchBurn(_singletonAddresses(alice), _singletonUints(1)); - } - - /// @notice Verifies the factory bootstrap bypass is deliberately NOT honored for batchBurn - /// @dev `onlyRoleStrict` rejects even calls from the factory during the bootstrap window - /// (unlike `onlyRole`, which honors `_isPrivileged()`). Triggered by passing a - /// `batchBurn(...)` initCall: the call arrives at the token with `msg.sender == factory` - /// and `initialized == false`, but `onlyRoleStrict` requires the factory to actually - /// hold `BURN_FROM_ROLE` (which it never does), so the init-call reverts and the - /// factory bubbles the inner AccessControlUnauthorizedAccount per IB20Factory.InitCallFailed. - function test_batchBurn_revert_factoryBootstrapBypassRejected(bytes32 salt) public { - bytes[] memory initCalls = new bytes[](1); - initCalls[0] = abi.encodeWithSelector( - IB20Security.batchBurn.selector, _singletonAddresses(alice), _singletonUints(uint256(1)) - ); - - vm.expectRevert( - abi.encodeWithSelector(IB20.AccessControlUnauthorizedAccount.selector, address(factory), BURN_FROM_ROLE) - ); - _createSecurity(address(this), salt, _securityParams(), initCalls); - } - - /// @notice Verifies batchBurn reverts when accounts.length != amounts.length - /// @dev Length-mismatch guard fires after the role check; checks - /// LengthMismatch(accounts.length, amounts.length). - function test_batchBurn_revert_lengthMismatch() public { - _grantBurnFrom(); - address[] memory accounts = _singletonAddresses(alice); - uint256[] memory amounts = new uint256[](2); - amounts[0] = 1; - amounts[1] = 2; - - vm.prank(burnFromActor); - vm.expectRevert(abi.encodeWithSelector(IB20Security.LengthMismatch.selector, uint256(1), uint256(2))); - security().batchBurn(accounts, amounts); - } - - /// @notice Verifies batchBurn reverts when both arrays are empty - /// @dev EmptyBatch guard: same rationale as batchMint. - function test_batchBurn_revert_emptyBatch() public { - _grantBurnFrom(); - vm.prank(burnFromActor); - vm.expectRevert(IB20Security.EmptyBatch.selector); - security().batchBurn(new address[](0), new uint256[](0)); - } - - /// @notice Verifies batchBurn reverts when BURN feature is paused - /// @dev Pause guard fires AFTER the length / empty checks but BEFORE per-element burn. - function test_batchBurn_revert_whenBurnPaused(uint256 amount) public { - _grantBurnFrom(); - _pause(IB20.PausableFeature.BURN); - - vm.prank(burnFromActor); - vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.BURN)); - security().batchBurn(_singletonAddresses(alice), _singletonUints(amount)); - } - - /// @notice Verifies batchBurn treats zero-amount elements as valid no-ops - /// @dev Mirrors ERC-20 conventions (transfer(0) is valid). A zero in any slot is a no-op: - /// the balance for that element is unchanged, and `_burnRaw` emits the canonical - /// `Transfer(account, 0, 0)` for it. Non-zero elements in the same batch are processed - /// normally. The "all-or-nothing for non-zero failures" invariant is preserved by the - /// InsufficientBalance check inside `_burnRaw`. - function test_batchBurn_success_zeroAmountElementsAreNoOps() public { - _grantBurnFrom(); - _mint(alice, 100); - _mint(bob, 100); - - address[] memory accounts = new address[](2); - accounts[0] = alice; - accounts[1] = bob; - uint256[] memory amounts = new uint256[](2); - amounts[0] = 50; - amounts[1] = 0; // zero is a valid no-op per ERC-20 conventions - - vm.prank(burnFromActor); - security().batchBurn(accounts, amounts); - - assertEq(token.balanceOf(alice), 50, "alice debited by 50"); - assertEq(token.balanceOf(bob), 100, "bob balance unchanged by zero-amount element"); - } - - /// @notice Verifies batchBurn surfaces per-element InsufficientBalance reverts - /// @dev Per-element invariant: `_burnRaw` reverts InsufficientBalance(account, bal, amount) - /// if any element exceeds the account's balance. All-or-nothing atomicity. - function test_batchBurn_revert_insufficientBalance(uint256 amount) public { - _grantBurnFrom(); - amount = bound(amount, 1, type(uint128).max); - // alice holds zero balance; any positive amount triggers the revert on element 0. - - vm.prank(burnFromActor); - vm.expectRevert(abi.encodeWithSelector(IB20.InsufficientBalance.selector, alice, uint256(0), amount)); - security().batchBurn(_singletonAddresses(alice), _singletonUints(amount)); - } - - /// @notice Verifies batchBurn succeeds with a single account and debits the balance - /// @dev Single-element happy path; balance and totalSupply both drop by amount. - function test_batchBurn_success_singleAccount(uint256 amount) public { - _grantBurnFrom(); - amount = bound(amount, 1, type(uint128).max); - _mint(alice, amount); - uint256 supplyBefore = token.totalSupply(); - - vm.prank(burnFromActor); - security().batchBurn(_singletonAddresses(alice), _singletonUints(amount)); - - assertEq(token.balanceOf(alice), 0, "balance must be zero after full burn"); - assertEq(token.totalSupply(), supplyBefore - amount, "totalSupply must drop by amount"); - } - - /// @notice Verifies batchBurn succeeds with multiple accounts and debits each individually - /// @dev Multi-element happy path; iteration order doesn't matter for accounting. - function test_batchBurn_success_multipleAccounts(uint64 a1, uint64 a2, uint64 a3) public { - _grantBurnFrom(); - a1 = uint64(bound(uint256(a1), 1, type(uint64).max)); - a2 = uint64(bound(uint256(a2), 1, type(uint64).max)); - a3 = uint64(bound(uint256(a3), 1, type(uint64).max)); - - address carol = makeAddr("carol"); - _mint(alice, a1); - _mint(bob, a2); - _mint(carol, a3); - uint256 supplyBefore = token.totalSupply(); - - address[] memory accounts = new address[](3); - accounts[0] = alice; - accounts[1] = bob; - accounts[2] = carol; - uint256[] memory amounts = new uint256[](3); - amounts[0] = a1; - amounts[1] = a2; - amounts[2] = a3; - - vm.prank(burnFromActor); - security().batchBurn(accounts, amounts); - - assertEq(token.balanceOf(alice), 0, "alice balance must be zero"); - assertEq(token.balanceOf(bob), 0, "bob balance must be zero"); - assertEq(token.balanceOf(carol), 0, "carol balance must be zero"); - assertEq( - token.totalSupply(), - supplyBefore - uint256(a1) - uint256(a2) - uint256(a3), - "totalSupply must drop by the sum of amounts" - ); - } - - /// @notice Verifies batchBurn emits Transfer(account, address(0), amount) per element - /// @dev Event integrity: the canonical burn-as-transfer-to-zero signal fires per element, - /// in iteration order. (`BurnedBlocked` is NOT emitted — that is reserved for the - /// sanctions-style `burnBlocked` path per the IB20Security natspec.) - function test_batchBurn_success_emitsTransferPerElement() public { - _grantBurnFrom(); - _mint(alice, 100); - _mint(bob, 200); - - address[] memory accounts = new address[](2); - accounts[0] = alice; - accounts[1] = bob; - uint256[] memory amounts = new uint256[](2); - amounts[0] = 100; - amounts[1] = 200; - - vm.expectEmit(true, true, false, true, address(token)); - emit IB20.Transfer(alice, address(0), 100); - vm.expectEmit(true, true, false, true, address(token)); - emit IB20.Transfer(bob, address(0), 200); - vm.prank(burnFromActor); - security().batchBurn(accounts, amounts); - } -} diff --git a/test/unit/B20Security/batch/batchBurn_revertOrder.t.sol b/test/unit/B20Security/batch/batchBurn_revertOrder.t.sol deleted file mode 100644 index e0ff912..0000000 --- a/test/unit/B20Security/batch/batchBurn_revertOrder.t.sol +++ /dev/null @@ -1,124 +0,0 @@ -// 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"; - -/// @title Differential check-order tests for `batchBurn` (security variant). -/// -/// @notice **Canonical order (Solidity reference):** -/// 1. PAUSE (`whenNotPaused(BURN)` modifier) → `ContractPaused` -/// 2. ROLE (`onlyRoleStrict(BURN_FROM_ROLE)` modifier) → `AccessControlUnauthorizedAccount` -/// 3. LENGTH_MISMATCH (`accounts.length != amounts.length`) → `LengthMismatch` -/// 4. EMPTY_BATCH (`accounts.length == 0`) → `EmptyBatch` -/// 5. BALANCE (per-element `_burnRaw`) → `InsufficientBalance` -/// -/// Some pairs are not reachable because the violations are mutually -/// exclusive (LENGTH_MISMATCH vs EMPTY_BATCH) or because one check -/// short-circuits the loop where the other would fire -/// (LENGTH_MISMATCH/EMPTY_BATCH vs BALANCE — the loop never runs). -/// Reachable pairs: 7. -contract B20SecurityBatchBurnRevertOrderTest is B20SecurityTest { - address[] internal _emptyAddrs; - uint256[] internal _emptyUints; - address[] internal _twoAddrs; - uint256[] internal _twoUints; - - function setUp() public override { - super.setUp(); - _twoAddrs.push(alice); - _twoAddrs.push(bob); - _twoUints.push(1); - } - - // --- Pairs where PAUSE wins (PAUSE is canonical first) --- - - /// @notice PAUSE beats ROLE. - /// @dev Pause modifier is listed before the role-strict modifier; fires first. - function test_batchBurn_revertOrder_pause_beats_role(address caller, uint256 amount) public { - _assumeValidCaller(caller); - vm.assume(caller != admin); - vm.assume(caller != burnFromActor); - _pause(IB20.PausableFeature.BURN); - - vm.prank(caller); - vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.BURN)); - security().batchBurn(_singletonAddresses(alice), _singletonUints(amount)); - } - - /// @notice PAUSE beats LENGTH_MISMATCH. - function test_batchBurn_revertOrder_pause_beats_lengthMismatch() public { - _grantBurnFrom(); - _pause(IB20.PausableFeature.BURN); - - vm.prank(burnFromActor); - vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.BURN)); - security().batchBurn(_twoAddrs, _twoUints); - } - - /// @notice PAUSE beats EMPTY_BATCH. - function test_batchBurn_revertOrder_pause_beats_emptyBatch() public { - _grantBurnFrom(); - _pause(IB20.PausableFeature.BURN); - - vm.prank(burnFromActor); - vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.BURN)); - security().batchBurn(_emptyAddrs, _emptyUints); - } - - /// @notice PAUSE beats BALANCE. - function test_batchBurn_revertOrder_pause_beats_balance(uint256 amount) public { - amount = bound(amount, 1, type(uint128).max); - _grantBurnFrom(); - _pause(IB20.PausableFeature.BURN); - // alice has zero balance → BALANCE would fire if PAUSE didn't. - - vm.prank(burnFromActor); - vm.expectRevert(abi.encodeWithSelector(IB20.ContractPaused.selector, IB20.PausableFeature.BURN)); - security().batchBurn(_singletonAddresses(alice), _singletonUints(amount)); - } - - // --- Pairs where ROLE wins (PAUSE not violated) --- - // - // Note: BURN_FROM_ROLE() is resolved before vm.prank because the view call - // would otherwise consume the prank intended for batchBurn (same pattern as - // _setRedeemPolicy in B20SecurityTest). - - function test_batchBurn_revertOrder_role_beats_lengthMismatch(address caller) public { - _assumeValidCaller(caller); - vm.assume(caller != admin); - vm.assume(caller != burnFromActor); - bytes32 role = security().BURN_FROM_ROLE(); - // _twoAddrs has length 2, _twoUints has length 1 → length mismatch. - - vm.prank(caller); - vm.expectRevert(abi.encodeWithSelector(IB20.AccessControlUnauthorizedAccount.selector, caller, role)); - security().batchBurn(_twoAddrs, _twoUints); - } - - function test_batchBurn_revertOrder_role_beats_emptyBatch(address caller) public { - _assumeValidCaller(caller); - vm.assume(caller != admin); - vm.assume(caller != burnFromActor); - bytes32 role = security().BURN_FROM_ROLE(); - - vm.prank(caller); - vm.expectRevert(abi.encodeWithSelector(IB20.AccessControlUnauthorizedAccount.selector, caller, role)); - security().batchBurn(_emptyAddrs, _emptyUints); - } - - function test_batchBurn_revertOrder_role_beats_balance(address caller, uint256 amount) public { - _assumeValidCaller(caller); - vm.assume(caller != admin); - vm.assume(caller != burnFromActor); - amount = bound(amount, 1, type(uint128).max); - bytes32 role = security().BURN_FROM_ROLE(); - // alice has zero balance. - - vm.prank(caller); - vm.expectRevert(abi.encodeWithSelector(IB20.AccessControlUnauthorizedAccount.selector, caller, role)); - security().batchBurn(_singletonAddresses(alice), _singletonUints(amount)); - } -} diff --git a/test/unit/B20Security/batch/batchBurn_rollback.t.sol b/test/unit/B20Security/batch/batchBurn_rollback.t.sol deleted file mode 100644 index be5e396..0000000 --- a/test/unit/B20Security/batch/batchBurn_rollback.t.sol +++ /dev/null @@ -1,62 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; - -import {B20SecurityTest} from "test/lib/B20SecurityTest.sol"; - -import {IB20} from "src/interfaces/IB20.sol"; - -/// @title B20SecurityBatchBurnRollbackTest -/// @notice Verifies a revert mid-batchBurn leaves no orphan token state. -/// -/// @dev Sibling of `B20SecurityBatchBurnTest` (happy path) and -/// `B20SecurityBatchBurnRevertOrderTest` (check-order). This file -/// covers the precompile-storage checkpoint integration: when an -/// element past index 0 reverts, the per-element debits from the -/// earlier successful iterations must be rolled back. -/// -/// Mock mode gets this for free from revm's journaled storage. -/// The signal is in fork mode against the real Rust precompile, -/// where any break in the `precompile-storage` journal hookup -/// would leave the earlier debits persisted across the revert. -/// Companion to BOP-176 (Rust-side unit tests). -contract B20SecurityBatchBurnRollbackTest is B20SecurityTest { - /// @notice batchBurn that reverts on element 1 via InsufficientBalance leaves balances + supply unchanged - /// @dev Element 0 burns exactly alice's balance to 0; element 1 - /// then attempts to burn more than bob holds and reverts. - /// The journal must roll element 0's debit back, restoring - /// alice to her pre-call balance and totalSupply to its - /// pre-call value. Fuzzes alice's burn amount and bob's - /// shortfall so the test exercises a range of pre-revert - /// write sizes. - function test_batchBurn_rollback_revertMidBatch_insufficientBalance(uint64 a1Seed, uint64 bobBalSeed) public { - uint256 a1 = bound(uint256(a1Seed), 1, type(uint64).max); - // bob is mid-batch and underfunded relative to amounts[1]. - uint256 bobBurnAmount = bound(uint256(bobBalSeed), 1, type(uint64).max); - uint256 bobBal = bound(uint256(bobBalSeed), 0, bobBurnAmount - 1); - - _grantBurnFrom(); - _mint(alice, a1); // alice has exactly the burn amount → element 0 zeroes her - _mint(bob, bobBal); // bob is short by at least 1 → element 1 reverts - - address[] memory accounts = new address[](2); - accounts[0] = alice; - accounts[1] = bob; - uint256[] memory amounts = new uint256[](2); - amounts[0] = a1; - amounts[1] = bobBurnAmount; - - // Snapshot the state the revert must leave untouched. - uint256 aliceBefore = token.balanceOf(alice); - uint256 bobBefore = token.balanceOf(bob); - uint256 supplyBefore = token.totalSupply(); - - vm.prank(burnFromActor); - vm.expectRevert(abi.encodeWithSelector(IB20.InsufficientBalance.selector, bob, bobBal, bobBurnAmount)); - security().batchBurn(accounts, amounts); - - // Element 0's debit must have been rolled back by the journal. - assertEq(token.balanceOf(alice), aliceBefore, "alice balance must be unchanged after rollback"); - assertEq(token.balanceOf(bob), bobBefore, "bob balance must be unchanged after rollback"); - assertEq(token.totalSupply(), supplyBefore, "totalSupply must be unchanged after rollback"); - } -} diff --git a/test/unit/B20Security/constants/roleConstants.t.sol b/test/unit/B20Security/constants/roleConstants.t.sol index c9ada85..196a540 100644 --- a/test/unit/B20Security/constants/roleConstants.t.sol +++ b/test/unit/B20Security/constants/roleConstants.t.sol @@ -2,7 +2,6 @@ pragma solidity ^0.8.20; import {B20SecurityTest} from "test/lib/B20SecurityTest.sol"; -import {B20Constants} from "src/lib/B20Constants.sol"; contract B20SecurityRoleConstantsTest is B20SecurityTest { /// @notice Verifies SECURITY_OPERATOR_ROLE equals keccak256("SECURITY_OPERATOR_ROLE") @@ -20,30 +19,4 @@ contract B20SecurityRoleConstantsTest is B20SecurityTest { "compile-time copy in B20SecurityTest must match the contract value" ); } - - /// @notice Verifies BURN_FROM_ROLE equals keccak256("BURN_FROM_ROLE") - /// @dev Same wire-format invariant for the corp-actions clawback role. - function test_burnFromRole_success_matchesKeccak() public view { - assertEq( - security().BURN_FROM_ROLE(), - keccak256("BURN_FROM_ROLE"), - "BURN_FROM_ROLE must equal keccak256(\"BURN_FROM_ROLE\")" - ); - assertEq(security().BURN_FROM_ROLE(), BURN_FROM_ROLE, "compile-time copy in B20SecurityTest must match"); - assertEq( - security().BURN_FROM_ROLE(), - B20Constants.BURN_FROM_ROLE, - "B20Constants.BURN_FROM_ROLE library source-of-truth must match" - ); - } - - /// @notice Verifies the two variant role identifiers are distinct - /// @dev Sanity check: a collision would let a single role grant accidentally authorize both - /// operator and burn-from paths. - function test_securityRoles_success_distinct() public view { - assertTrue( - security().SECURITY_OPERATOR_ROLE() != security().BURN_FROM_ROLE(), - "operator and burn-from role identifiers must be distinct" - ); - } }