diff --git a/FORK_TESTING.md b/FORK_TESTING.md index aafe93a..45c8f54 100644 --- a/FORK_TESTING.md +++ b/FORK_TESTING.md @@ -7,7 +7,7 @@ ## What this does Runs base-std's existing unit test suite (~346 tests with paired -`vm.load`-based slot assertions, from PR #43) against a **local node that +`vm.load`-based slot assertions) against a **local node that hosts Base's Rust precompiles** (the patched `anvil` from the base-anvil fork). Forge dispatches calls to those precompiles instead of to base-std's Solidity mocks; the suite's slot-level assertions then surface any @@ -216,7 +216,7 @@ matching `FEATURE_*` constant). **Build fails with `error[E0599]: no variant or associated item named B20_STABLECOIN` in `b20_stablecoin/dispatch.rs`** — you bumped past base/base commit -`d7662c05e` (PR #2834, "replace feature id constants with ActivationFeature +`d7662c05e` ("replace feature id constants with ActivationFeature enum"). That refactor removed `ActivationRegistryStorage::B20_STABLECOIN` but the call site in `dispatch.rs` still references it. Until upstream fixes this, either pin the dep back to a pre-d7662c05e commit (e.g. @@ -249,7 +249,7 @@ If this returns garbage / fails, the fork's build is broken or out of date. | The test runner script | `script/run-fork-tests.sh` | bash; takes forge args through `$@` | | Forge profile config | `foundry.toml`, `[profile.fork]` | `base = true` enables Rust precompile dispatch | | Skip-etch logic | `test/lib/BaseTest.sol` | guarded by `LIVE_PRECOMPILES` env var | -| Slot assertions | `test/unit/**/*.t.sol`, `test/lib/mocks/Mock*Storage.sol` | shipped in base-std PR #43 | +| Slot assertions | `test/unit/**/*.t.sol`, `test/lib/mocks/Mock*Storage.sol` | `vm.load`-based slot-layout assertions paired with surface tests | | Storage helpers | `test/lib/mocks/MockB20Storage.sol`, `MockPolicyRegistryStorage.sol` | the slot-derivation library every assertion uses | | Patched forge + anvil | `~/code/base-anvil/target/.../{forge,anvil}` | built by `cargo build -p forge -p anvil` | | `--base` flag implementation | `~/code/base-anvil/crates/evm/networks/src/lib.rs` | edit here when precompile set changes | @@ -257,7 +257,7 @@ If this returns garbage / fails, the fork's build is broken or out of date. | Local-iteration override | `~/code/base-anvil/Cargo.toml` | commented `[patch."https://github.com/base/base.git"]` block | | Rust precompile source | `github.com/base/base`, pinned commit | fetched by cargo on build; optional local clone for [patch] | | Feature IDs | `base/crates/common/precompiles/src/activation/storage.rs` (on github) | `FEATURE_*` consts | -| ActivationRegistry default admin | `0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc` | codified in base/base PR #2811 | +| ActivationRegistry default admin | `0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc` | the canonical local-dev admin | | Vibenet chainid | 84538453 | auto-enables `--base` | ## What's NOT in scope diff --git a/docs/B20/Asset.md b/docs/B20/Asset.md index 22bd9db..c819999 100644 --- a/docs/B20/Asset.md +++ b/docs/B20/Asset.md @@ -12,7 +12,7 @@ Read the current multiplier with `multiplier()`; the value is in WAD precision ( ## Announcements -Announcements are publicly viewable notifications posted by a token operator. They can represent anything the operator wants to create a record of and can be coupled with actual state changes on the token (updating the multiplier, batched mints/burns, and so on). +Announcements are publicly viewable notifications posted by a token operator. They can represent anything the operator wants to create a record of and can be coupled with actual state changes on the token (updating the multiplier, batched mints, and so on). ### Event Topology diff --git a/script/__pycache__/mutate.cpython-312.pyc b/script/__pycache__/mutate.cpython-312.pyc new file mode 100644 index 0000000..ab3e30c Binary files /dev/null and b/script/__pycache__/mutate.cpython-312.pyc differ diff --git a/script/mutate.py b/script/mutate.py index 864eb50..f7aac80 100644 --- a/script/mutate.py +++ b/script/mutate.py @@ -123,7 +123,7 @@ class Mutation: " // mint receiver policy check elided\n if (false) revert PolicyForbids(MINT_RECEIVER_POLICY, mintReceiverPolicyId);", "_mint: drop MINT_RECEIVER_POLICY policy check entirely", ), - # === MockB20: lastAdmin guard off-by-one (post-#40 inline conjunction) === + # === MockB20: lastAdmin guard off-by-one (inline conjunction) === Mutation( MOCK_B20, "if (role == DEFAULT_ADMIN_ROLE && $.roles[DEFAULT_ADMIN_ROLE][msg.sender] && $.adminCount == 1) {", diff --git a/script/run-fork-tests.sh b/script/run-fork-tests.sh index 8403f86..a2c2dc4 100755 --- a/script/run-fork-tests.sh +++ b/script/run-fork-tests.sh @@ -26,7 +26,7 @@ # PORT local RPC port for anvil (default: 8546) # ACTIVATION_ADMIN address authorized to activate features # (default: 0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc, the -# canonical local-dev admin codified in base/base#2811) +# canonical local-dev admin) # ANVIL_LOG anvil stdout/stderr log path (default: /tmp/anvil.log) # # Exit codes: @@ -76,14 +76,12 @@ ACTIVATION_ADMIN="${ACTIVATION_ADMIN:-0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc REGISTRY=0x8453000000000000000000000000000000000001 LOG_FILE="${ANVIL_LOG:-/tmp/anvil.log}" -# Feature IDs from base/base/crates/common/precompiles/src/activation/storage.rs. -# If a new feature is added there, append its ID here. +# Feature IDs mirror the canonical set in test/lib/mocks/ActivationRegistryFeatureList.sol +# (the Solidity reference is the source of truth). If a feature is added there, append its ID here. FEATURE_IDS=( - 0x78751e29c8bcc0d609ab18e9fbc4158e73f7db25ae2ee095dad42e2578b1e800 # B20_FACTORY - 0x47a1afe8d3d691b87e090ee972d223a11f4da971ff5416c04985bb2393aca752 # B20_TOKEN + 0xcdcc772fe4cbdb1029f822861176d09e646db96723d4c1e82ddfdeb8163ef54c # B20_ASSET 0xb582ebae03f16fee49a6763f78df482fb11ae73f103ed0d330bbe556aa90a43f # POLICY_REGISTRY - 0xecfa0def2c10020caaf65e6155aa69c84b24892aaef76eeac52e0e2b3a0b8601 # B20_STABLECOIN (added base/base#2806) - 0x83d32fab502ae0e8bc4352a117767262cb5e47cc8d67a744008ed4ff03fcf5e6 # B20_SECURITY (added base/base#2813) + 0xecfa0def2c10020caaf65e6155aa69c84b24892aaef76eeac52e0e2b3a0b8601 # B20_STABLECOIN ) # ── Helpers ─────────────────────────────────────────────────────────────────── diff --git a/test/lib/mocks/MockActivationRegistry.sol b/test/lib/mocks/MockActivationRegistry.sol index 500a6b8..a1bef15 100644 --- a/test/lib/mocks/MockActivationRegistry.sol +++ b/test/lib/mocks/MockActivationRegistry.sol @@ -16,7 +16,7 @@ import {MockActivationRegistryStorage} from "test/lib/mocks/MockActivationRegist /// ERC-7201-namespaced root; see that library for the layout. /// /// **Admin model.** `admin()` returns a hardcoded constant -/// (`0xCB00…0000` per base/base#2733) — the production +/// (`0xCB00…0000`) — the production /// precompile and this mock both expose the same fixed admin /// identity. There is no storage-backed admin slot and no /// admin-rotation surface; replacing the admin requires a @@ -39,7 +39,7 @@ contract MockActivationRegistry is IActivationRegistry { // ============================================================ /// @notice The activation admin address. Hardcoded to the same value the - /// live Rust precompile returns (`0xCB00…0000`, per base/base#2733), + /// live Rust precompile returns (`0xCB00…0000`), /// so tests and consumers can pin the admin identity without /// depending on per-environment configuration. address internal constant ADMIN = 0xCB00000000000000000000000000000000000000; diff --git a/test/lib/mocks/MockB20.sol b/test/lib/mocks/MockB20.sol index 25fe739..a479881 100644 --- a/test/lib/mocks/MockB20.sol +++ b/test/lib/mocks/MockB20.sol @@ -133,19 +133,7 @@ abstract contract MockB20 is IB20 { /// / `setRoleAdmin`, where the gate is over the meta-role, /// not the role itself. modifier onlyRoleAdmin(bytes32 role) { - if (!_isPrivileged()) { - // Admin-resurrection guard: once `adminCount == 0` the token is admin-less - // by design (post-`renounceLastAdmin`). No role-mgmt mutation is permitted - // even if the caller holds a custom-admin role that would otherwise - // authorize them, because allowing such mutations would let a custom-admin - // chain (e.g. setRoleAdmin(MINT_ROLE, BURN_ROLE) → grantRole(BURN_ROLE, X)) - // re-establish admin power on a token that has explicitly relinquished it. - // Mirrors base/base PR #2961 (`ensure_role_admin_mutations_available`). - if (MockB20Storage.layout().adminCount == 0) { - revert AccessControlUnauthorizedAccount(msg.sender, DEFAULT_ADMIN_ROLE); - } - _requireRoleAdmin(role); - } + if (!_isPrivileged()) _requireRoleAdmin(role); _; } @@ -408,8 +396,7 @@ abstract contract MockB20 is IB20 { function pausedFeatures() external view returns (PausableFeature[] memory) { uint256 vectors = MockB20Storage.layout().pausedVectors; uint256 count; - // Bound the scan off `type(PausableFeature).max` so it tracks the enum as variants are - // added or removed (the REDEEM variant was removed in BOP-251) rather than a stale literal. + // Derive the bound from the enum so it can't drift out of sync with PausableFeature. uint256 featureCount = uint256(type(PausableFeature).max) + 1; for (uint256 i = 0; i < featureCount; i++) { if (((vectors >> i) & uint256(1)) == 1) count++; @@ -496,16 +483,16 @@ abstract contract MockB20 is IB20 { /// falling through to `super`. function _writePolicyId(bytes32 policyScope, uint64 newPolicyId) internal virtual { MockB20Storage.Layout storage $ = MockB20Storage.layout(); + // `updatePolicy` validates the scope via `_readPolicyId` before calling this, so it is + // guaranteed to be one of the four supported scopes — the last arm needs no guard. if (policyScope == TRANSFER_SENDER_POLICY) { $.transferPolicyIds.sender = newPolicyId; } else if (policyScope == TRANSFER_RECEIVER_POLICY) { $.transferPolicyIds.receiver = newPolicyId; } else if (policyScope == TRANSFER_EXECUTOR_POLICY) { $.transferPolicyIds.executor = newPolicyId; - } else if (policyScope == MINT_RECEIVER_POLICY) { - $.mintPolicyIds.receiver = newPolicyId; } else { - revert UnsupportedPolicyType(policyScope); + $.mintPolicyIds.receiver = newPolicyId; } } @@ -655,6 +642,16 @@ abstract contract MockB20 is IB20 { /// unconfigured roles it's `bytes32(0) == DEFAULT_ADMIN_ROLE`, /// which is the role the caller actually needs to hold. function _requireRoleAdmin(bytes32 role) internal view { + // Admin-resurrection guard: once `adminCount == 0` the token is admin-less + // by design (post-`renounceLastAdmin`). No role-mgmt mutation is permitted + // even if the caller holds a custom-admin role that would otherwise + // authorize them, because allowing such mutations would let a custom-admin + // chain (e.g. setRoleAdmin(MINT_ROLE, BURN_ROLE) → grantRole(BURN_ROLE, X)) + // re-establish admin power on a token that has explicitly relinquished it. + // Mirrors the Rust precompile's `ensure_role_admin_mutations_available`. + if (MockB20Storage.layout().adminCount == 0) { + revert AccessControlUnauthorizedAccount(msg.sender, DEFAULT_ADMIN_ROLE); + } bytes32 adminRole = MockB20Storage.layout().roleAdmins[role]; if (!hasRole(adminRole, msg.sender)) revert AccessControlUnauthorizedAccount(msg.sender, adminRole); } diff --git a/test/lib/mocks/MockB20Factory.sol b/test/lib/mocks/MockB20Factory.sol index 32cf295..2bfc55b 100644 --- a/test/lib/mocks/MockB20Factory.sol +++ b/test/lib/mocks/MockB20Factory.sol @@ -101,8 +101,6 @@ contract MockB20Factory is IB20Factory { // any moment. Variant-feature mapping: // STABLECOIN → B20_STABLECOIN // ASSET → B20_ASSET - // (The dedicated `B20_TOKEN` feature was retired by BOP-257 - // and the `DEFAULT` variant by BOP-253.) _enforceActivationGates(variant); // -- 1. Decode + validate, get the common params -- @@ -146,9 +144,11 @@ contract MockB20Factory is IB20Factory { decimals = 6; currency_ = p.currency; } else { - // Unreachable in Solidity: an out-of-range `B20Variant` is rejected by ABI - // enum-decoding (Panic 0x21) before this body runs. Retained to mirror the Rust - // precompile, which decodes the raw discriminator and surfaces this typed revert. + // Unreachable in Solidity, and intentionally so: an out-of-range `B20Variant` is + // rejected by ABI enum-decoding (Panic 0x21) before this body runs, so no test can + // reach it — this is the single line the coverage report shows uncovered, by design. + // It is retained to mirror the Rust precompile, which decodes the raw discriminator + // and surfaces this typed `InvalidVariant` revert. revert InvalidVariant(); } diff --git a/test/lib/mocks/MockPolicyRegistry.sol b/test/lib/mocks/MockPolicyRegistry.sol index 24c8249..bec7f49 100644 --- a/test/lib/mocks/MockPolicyRegistry.sol +++ b/test/lib/mocks/MockPolicyRegistry.sol @@ -74,7 +74,7 @@ contract MockPolicyRegistry is IPolicyRegistry { /// `updateAllowlist`, and `updateBlocklist` revert with /// `BatchSizeTooLarge(MAX_BATCH_SIZE)` when `accounts.length` /// exceeds this value. Mirrors the Rust PolicyRegistry - /// precompile (base/base#2876). + /// precompile. uint256 internal constant MAX_BATCH_SIZE = 64; // ============================================================ diff --git a/test/regression/B20Removals.t.sol b/test/regression/B20Removals.t.sol index ff59b1a..110efd1 100644 --- a/test/regression/B20Removals.t.sol +++ b/test/regression/B20Removals.t.sol @@ -9,30 +9,21 @@ import {B20AssetTest} from "test/lib/B20AssetTest.sol"; /// @title B20 removal regression suite /// -/// @notice Locks in the *removals* from the B-20 asset rework (the -/// redemption subsystem, batched burn / burn-from, and the `DEFAULT` token variant). -/// Each test asserts that a function/selector that USED to exist -/// on the surface no longer resolves, so a future reintroduction -/// — or a Rust precompile in `base/base` that still exposes the -/// retired surface — fails here. +/// @notice Locks in the *removals* from the B-20 asset rework (the redemption subsystem, +/// batched burn / burn-from, and the `DEFAULT` token variant). Each test asserts that a +/// function/selector that USED to exist on the surface no longer resolves, so a future +/// reintroduction — or a Rust precompile in `base/base` that still exposes the retired +/// surface — fails here. /// -/// @dev These are *selector-absence* assertions: the token mock (and -/// the live precompile in fork mode) carries no fallback, so a -/// call to a removed selector cannot succeed. The same test body -/// is meaningful against the mock (proves the Solidity reference -/// dropped the surface) and against the live precompile under -/// `LIVE_PRECOMPILES=true ... --fork-url` (proves `base/base` -/// dropped it too). Args are irrelevant — dispatch fails before -/// decoding for an unknown selector — but old signatures are -/// reproduced verbatim so each `bytes4` matches the historical -/// selector exactly. +/// @dev These are *selector-absence* assertions: the token mock (and the live precompile in +/// fork mode) carries no fallback, so a call to a removed selector cannot succeed. The +/// same test body is meaningful against the mock (proves the Solidity reference dropped +/// the surface) and against the live precompile under `LIVE_PRECOMPILES=true ... --fork-url` +/// (proves `base/base` dropped it too). Args are irrelevant — dispatch fails before decoding +/// for an unknown selector — but old signatures are reproduced verbatim so each `bytes4` +/// matches the historical selector exactly. /// -/// Removal provenance: -/// - Redemption subsystem (`redeem`, `redeemWithMemo`, -/// `minimumRedeemable`, `updateMinimumRedeemable`, -/// `REDEEM_SENDER_POLICY`, the `REDEEM` pausable feature) — removed in BOP-251. -/// - `batchBurn` + `BURN_FROM_ROLE` — removed in BOP-250. -/// - `DEFAULT` B-20 variant — removed in BOP-253. +/// Each test tags the change it guards with a trailing `Regression: BOP-XXX.` line. contract B20RemovalsTest is B20AssetTest { /// @dev Asserts a low-level call of `signature` (with `args` appended) to the token reverts, /// i.e. the selector no longer resolves on the B-20 surface. @@ -46,56 +37,51 @@ contract B20RemovalsTest is B20AssetTest { // ============================================================ /// @notice Verifies the `redeem(uint256)` selector no longer resolves on the B-20 surface - /// @dev Redemption subsystem removed (BOP-251); a reintroduced `redeem` entrypoint trips this. + /// @dev A reintroduced `redeem` entrypoint trips this. Regression: BOP-251. function test_redeem_revert_selectorRemoved(uint256 amount) public { - _assertSelectorRemoved( - abi.encodeWithSignature("redeem(uint256)", amount), "redeem(uint256) must not resolve (removed BOP-251)" - ); + _assertSelectorRemoved(abi.encodeWithSignature("redeem(uint256)", amount), "redeem(uint256) must not resolve"); } /// @notice Verifies the `redeemWithMemo(uint256,bytes32)` selector no longer resolves - /// @dev Memo'd redemption removed alongside the redemption subsystem (BOP-251). + /// @dev Memo'd variant of the removed redeem path. Regression: BOP-251. function test_redeemWithMemo_revert_selectorRemoved(uint256 amount, bytes32 memo) public { _assertSelectorRemoved( abi.encodeWithSignature("redeemWithMemo(uint256,bytes32)", amount, memo), - "redeemWithMemo(uint256,bytes32) must not resolve (removed BOP-251)" + "redeemWithMemo(uint256,bytes32) must not resolve" ); } /// @notice Verifies the `minimumRedeemable()` getter no longer resolves - /// @dev The redemption floor parameter was removed with the redemption subsystem (BOP-251). + /// @dev The redemption floor getter was removed with the redemption subsystem. Regression: BOP-251. function test_minimumRedeemable_revert_selectorRemoved() public { - _assertSelectorRemoved( - abi.encodeWithSignature("minimumRedeemable()"), "minimumRedeemable() must not resolve (removed BOP-251)" - ); + _assertSelectorRemoved(abi.encodeWithSignature("minimumRedeemable()"), "minimumRedeemable() must not resolve"); } /// @notice Verifies the `updateMinimumRedeemable(uint256)` setter no longer resolves - /// @dev The redemption floor setter was removed with the redemption subsystem (BOP-251). + /// @dev The redemption floor setter was removed with the redemption subsystem. Regression: BOP-251. function test_updateMinimumRedeemable_revert_selectorRemoved(uint256 newMin) public { _assertSelectorRemoved( abi.encodeWithSignature("updateMinimumRedeemable(uint256)", newMin), - "updateMinimumRedeemable(uint256) must not resolve (removed BOP-251)" + "updateMinimumRedeemable(uint256) must not resolve" ); } /// @notice Verifies the `REDEEM_SENDER_POLICY()` policy-scope constant no longer resolves - /// @dev The redemption policy lane was removed with the redemption subsystem (BOP-251). + /// @dev The redemption policy lane was removed with the redemption subsystem. Regression: BOP-251. function test_redeemSenderPolicy_revert_selectorRemoved() public { _assertSelectorRemoved( - abi.encodeWithSignature("REDEEM_SENDER_POLICY()"), - "REDEEM_SENDER_POLICY() must not resolve (removed BOP-251)" + abi.encodeWithSignature("REDEEM_SENDER_POLICY()"), "REDEEM_SENDER_POLICY() must not resolve" ); } /// @notice Verifies the `PausableFeature` enum has no fourth (`REDEEM`) member /// @dev `isPaused(PausableFeature)` ABI-decodes its arg as the enum; an out-of-range value (3) /// reverts (Panic 0x21). Index 2 (`BURN`) still decodes, bracketing the enum at exactly - /// {TRANSFER, MINT, BURN}. The `REDEEM` feature was removed in BOP-251. + /// {TRANSFER, MINT, BURN}. Regression: BOP-251. function test_isPaused_revert_noRedeemFeature() public { _assertSelectorRemoved( abi.encodeWithSignature("isPaused(uint8)", uint8(3)), - "isPaused must reject enum index 3 (REDEEM removed BOP-251)" + "isPaused must reject enum index 3 (out of PausableFeature range)" ); (bool ok,) = address(token).call(abi.encodeWithSignature("isPaused(uint8)", uint8(2))); @@ -103,8 +89,8 @@ contract B20RemovalsTest is B20AssetTest { } /// @notice Verifies the all-features-paused bitmask covers exactly three features - /// @dev `ALL_FEATURES_PAUSED == 7` is `TRANSFER | MINT | BURN` (0b111); a fourth feature - /// (REDEEM) would push it to 15. Pins the feature count at the library source-of-truth. + /// @dev `ALL_FEATURES_PAUSED == 7` is `TRANSFER | MINT | BURN` (0b111); a fourth feature would + /// push it to 15. Pins the feature count at the library source-of-truth. Regression: BOP-251. function test_allFeaturesPaused_success_threeFeatureBitmask() public pure { assertEq(B20Constants.ALL_FEATURES_PAUSED, 7, "ALL_FEATURES_PAUSED must be TRANSFER|MINT|BURN (0b111)"); } @@ -114,23 +100,20 @@ contract B20RemovalsTest is B20AssetTest { // ============================================================ /// @notice Verifies the `batchBurn(address[],uint256[])` selector no longer resolves - /// @dev Batched burn was removed in BOP-250; only `batchMint` remains on the asset variant. + /// @dev Only `batchMint` remains on the asset variant. Regression: BOP-250. function test_batchBurn_revert_selectorRemoved(address account, uint256 amount) public { address[] memory accounts = _singletonAddresses(account); uint256[] memory amounts = _singletonUints(amount); _assertSelectorRemoved( abi.encodeWithSignature("batchBurn(address[],uint256[])", accounts, amounts), - "batchBurn(address[],uint256[]) must not resolve (removed BOP-250)" + "batchBurn(address[],uint256[]) must not resolve" ); } /// @notice Verifies the `BURN_FROM_ROLE()` role constant no longer resolves - /// @dev The burn-from role was removed with batched burn in BOP-250; `burnBlocked` is the - /// remaining seize path and is gated by `BURN_BLOCKED_ROLE`. + /// @dev `burnBlocked` (gated by `BURN_BLOCKED_ROLE`) is the remaining seize path. Regression: BOP-250. function test_burnFromRole_revert_selectorRemoved() public { - _assertSelectorRemoved( - abi.encodeWithSignature("BURN_FROM_ROLE()"), "BURN_FROM_ROLE() must not resolve (removed BOP-250)" - ); + _assertSelectorRemoved(abi.encodeWithSignature("BURN_FROM_ROLE()"), "BURN_FROM_ROLE() must not resolve"); } // ============================================================ @@ -138,13 +121,13 @@ contract B20RemovalsTest is B20AssetTest { // ============================================================ /// @notice Verifies the factory rejects a third (`DEFAULT`) variant discriminator - /// @dev `B20Variant` is now exactly {ASSET=0, STABLECOIN=1} (BOP-253 removed DEFAULT). - /// `createB20` ABI-decodes `variant` as the enum, so discriminator 2 reverts (Panic 0x21) - /// before reaching the factory body. A reintroduced third variant would let this succeed. + /// @dev `B20Variant` is now exactly {ASSET=0, STABLECOIN=1}. `createB20` ABI-decodes `variant` + /// as the enum, so discriminator 2 reverts (Panic 0x21) before reaching the factory body; + /// a reintroduced third variant would let this succeed. Regression: BOP-253. function test_createB20_revert_defaultVariantRemoved(bytes32 salt) public { bytes memory params = abi.encode(_assetParams()); (bool ok,) = address(factory) .call(abi.encodeWithSelector(IB20Factory.createB20.selector, uint8(2), salt, params, new bytes[](0))); - assertFalse(ok, "createB20 must reject variant discriminator 2 (DEFAULT removed BOP-253)"); + assertFalse(ok, "createB20 must reject variant discriminator 2 (out of B20Variant range)"); } } diff --git a/test/regression/B20Renames.t.sol b/test/regression/B20Renames.t.sol index f2b7e59..05e8c38 100644 --- a/test/regression/B20Renames.t.sol +++ b/test/regression/B20Renames.t.sol @@ -10,21 +10,19 @@ import {ActivationRegistryFeatureList} from "test/lib/mocks/ActivationRegistryFe /// @title B20 rename regression suite /// -/// @notice Locks in the *renames* from the B-20 asset rework: the -/// operator role (`SECURITY_OPERATOR_ROLE` → `OPERATOR_ROLE`, -/// BOP-248), share-ratio scaling (`sharesToTokensRatio`/`toShares`/ -/// `sharesOf`/`updateShareRatio` → `multiplier`/`toScaledBalance`/ -/// `scaledBalanceOf`/`updateMultiplier`, BOP-249), and the -/// activation feature namespace (`base.b20_security` → `base.b20_asset`, -/// `base.b20_token` removed, BOP-257). Each test asserts the new -/// surface is present and correct AND the old surface is gone, so the -/// rename cannot silently regress in either the Solidity reference or -/// the `base/base` Rust precompile (under fork mode). +/// @notice Locks in the *renames* from the B-20 asset rework: the operator role +/// (`SECURITY_OPERATOR_ROLE` → `OPERATOR_ROLE`), share-ratio scaling +/// (`sharesToTokensRatio`/`toShares`/`sharesOf`/`updateShareRatio` → `multiplier`/ +/// `toScaledBalance`/`scaledBalanceOf`/`updateMultiplier`), the METADATA/OPERATOR +/// authority split, and the `base.b20_asset` activation namespace. Each test asserts the +/// new surface is present and correct AND the old surface is gone, so the rename cannot +/// silently regress in either the Solidity reference or the `base/base` Rust precompile +/// (under fork mode). /// -/// @dev Old-selector absence is checked with low-level calls (the token -/// carries no fallback, so a retired selector cannot resolve). New -/// surface is checked with typed calls that only compile against the -/// current interface. +/// @dev Old-selector absence is checked with low-level calls (the token carries no fallback, so +/// a retired selector cannot resolve); new surface is checked with typed calls that only +/// compile against the current interface. Each test tags the change it guards with a +/// trailing `Regression: BOP-XXX.` line. contract B20RenamesTest is B20AssetTest { /// @dev Asserts a removed selector no longer resolves on the token surface. function _assertSelectorRemoved(bytes memory callData, string memory err) internal { @@ -33,31 +31,30 @@ contract B20RenamesTest is B20AssetTest { } // ============================================================ - // OPERATOR ROLE (BOP-248 rename) + // OPERATOR ROLE // ============================================================ /// @notice Verifies the operator role is exposed as `OPERATOR_ROLE` and the legacy /// `SECURITY_OPERATOR_ROLE` selector is gone - /// @dev BOP-248 renamed `SECURITY_OPERATOR_ROLE` → `OPERATOR_ROLE`; the wire value - /// (`keccak256("OPERATOR_ROLE")`) and library source-of-truth must agree, and the old - /// getter must not resolve. + /// @dev The wire value (`keccak256("OPERATOR_ROLE")`) and library source-of-truth must agree, + /// and the old getter must not resolve. Regression: BOP-248. function test_operatorRole_success_renamedFromSecurityOperator() public { assertEq(asset().OPERATOR_ROLE(), keccak256("OPERATOR_ROLE"), "OPERATOR_ROLE must equal its keccak preimage"); assertEq(asset().OPERATOR_ROLE(), B20Constants.OPERATOR_ROLE, "OPERATOR_ROLE must match B20Constants"); _assertSelectorRemoved( abi.encodeWithSignature("SECURITY_OPERATOR_ROLE()"), - "SECURITY_OPERATOR_ROLE() must not resolve (renamed to OPERATOR_ROLE in BOP-248)" + "SECURITY_OPERATOR_ROLE() must not resolve (renamed to OPERATOR_ROLE)" ); } // ============================================================ - // MULTIPLIER (BOP-249 rename) + // MULTIPLIER // ============================================================ /// @notice Verifies share-ratio scaling is exposed under the `multiplier` names and the legacy /// share-ratio selectors are gone - /// @dev BOP-249 renamed the share-ratio surface to multiplier scaling. The new getters resolve - /// (a fresh token reports a WAD multiplier), and every legacy selector must not resolve. + /// @dev The new getters resolve (a fresh token reports a WAD multiplier) and every legacy + /// selector must not resolve. Regression: BOP-249. function test_multiplier_success_renamedFromShareRatio(uint256 rawBalance) public { rawBalance = bound(rawBalance, 0, type(uint128).max); @@ -69,34 +66,33 @@ contract B20RenamesTest is B20AssetTest { // Legacy share-ratio surface is gone. _assertSelectorRemoved( abi.encodeWithSignature("sharesToTokensRatio()"), - "sharesToTokensRatio() must not resolve (renamed to multiplier() in BOP-249)" + "sharesToTokensRatio() must not resolve (renamed to multiplier())" ); _assertSelectorRemoved( abi.encodeWithSignature("toShares(uint256)", rawBalance), - "toShares(uint256) must not resolve (renamed to toScaledBalance in BOP-249)" + "toShares(uint256) must not resolve (renamed to toScaledBalance)" ); _assertSelectorRemoved( abi.encodeWithSignature("sharesOf(address)", alice), - "sharesOf(address) must not resolve (renamed to scaledBalanceOf in BOP-249)" + "sharesOf(address) must not resolve (renamed to scaledBalanceOf)" ); _assertSelectorRemoved( abi.encodeWithSignature("updateShareRatio(uint256)", rawBalance), - "updateShareRatio(uint256) must not resolve (renamed to updateMultiplier in BOP-249)" + "updateShareRatio(uint256) must not resolve (renamed to updateMultiplier)" ); } // ============================================================ - // METADATA vs OPERATOR GATING (BOP-248) + // METADATA vs OPERATOR GATING // ============================================================ - // BOP-248 split the asset variant's authority: the metadata - // setters (updateName / updateSymbol / updateContractURI / - // updateExtraMetadata) are gated by METADATA_ROLE, while the - // operator actions (announce / updateMultiplier) are gated by - // OPERATOR_ROLE. The two tests below pin that split from both sides. + // The asset variant splits authority: the metadata setters (updateName / updateSymbol / + // updateContractURI / updateExtraMetadata) are gated by METADATA_ROLE, while the operator + // actions (announce / updateMultiplier) are gated by OPERATOR_ROLE. The tests below pin that + // split from both sides. /// @notice Verifies `updateExtraMetadata` is gated by METADATA_ROLE, not OPERATOR_ROLE /// @dev An OPERATOR_ROLE-only holder is rejected with the METADATA_ROLE selector; a - /// METADATA_ROLE holder succeeds. Locks the BOP-248 gating split for metadata writes. + /// METADATA_ROLE holder succeeds. Regression: BOP-248. function test_updateExtraMetadata_success_gatedByMetadataRole(string calldata value) public { // Operator (OPERATOR_ROLE only) cannot write metadata. _grantOperator(); @@ -115,7 +111,7 @@ contract B20RenamesTest is B20AssetTest { /// @notice Verifies `updateMultiplier` is gated by OPERATOR_ROLE, not METADATA_ROLE /// @dev A METADATA_ROLE-only holder is rejected with the OPERATOR_ROLE selector — the inverse - /// of the metadata-gating test, confirming the two authorities are genuinely distinct. + /// of the metadata-gating test, confirming the two authorities are distinct. Regression: BOP-248. function test_updateMultiplier_revert_metadataRoleInsufficient(uint256 newMultiplier) public { newMultiplier = bound(newMultiplier, 1, type(uint128).max); _grantRole(B20Constants.METADATA_ROLE, bob); @@ -127,10 +123,9 @@ contract B20RenamesTest is B20AssetTest { } /// @notice Verifies METADATA_ROLE is administered by DEFAULT_ADMIN_ROLE on a freshly created token - /// @dev Pins the role-admin wiring: the asset variant does not set a custom admin for METADATA_ROLE, - /// so it defaults to DEFAULT_ADMIN_ROLE (the default admin grants/revokes METADATA_ROLE). - /// Authority over metadata *operations* is separate and split per the two tests above - /// (writes need METADATA_ROLE; operator actions need OPERATOR_ROLE). + /// @dev The asset variant does not set a custom admin for METADATA_ROLE, so it defaults to + /// DEFAULT_ADMIN_ROLE (the default admin grants/revokes METADATA_ROLE). Authority over + /// metadata *operations* is separate and split per the two tests above. Regression: BOP-248. function test_metadataRole_success_administeredByDefaultAdmin() public view { assertEq( token.getRoleAdmin(B20Constants.METADATA_ROLE), @@ -140,12 +135,12 @@ contract B20RenamesTest is B20AssetTest { } // ============================================================ - // ACTIVATION FEATURE NAMESPACE (BOP-257 rename) + // ACTIVATION FEATURE NAMESPACE // ============================================================ /// @notice Verifies the asset activation feature is keyed on the `base.b20_asset` namespace - /// @dev BOP-257 renamed `base.b20_security` → `base.b20_asset`. This is the cross-language - /// contract with the Rust `ActivationFeature` enum; a preimage drift desyncs the gate. + /// @dev This is the cross-language contract with the Rust `ActivationFeature` enum; a preimage + /// drift desyncs the gate. Regression: BOP-257. function test_b20Asset_success_keyedOnAssetNamespace() public pure { assertEq( ActivationRegistryFeatureList.B20_ASSET, @@ -155,17 +150,16 @@ contract B20RenamesTest is B20AssetTest { } /// @notice Verifies the asset feature is not keyed on either retired namespace - /// @dev BOP-257 renamed `base.b20_security` and removed `base.b20_token`. Asserting the asset - /// id differs from both retired preimages locks against an accidental revert to the old - /// namespace string. + /// @dev Asserting the asset id differs from both retired preimages locks against an accidental + /// revert to the old `base.b20_security` / `base.b20_token` namespace string. Regression: BOP-257. function test_b20Asset_success_notKeyedOnLegacyNamespaces() public pure { assertTrue( ActivationRegistryFeatureList.B20_ASSET != keccak256("base.b20_security"), - "B20_ASSET must not use the retired base.b20_security namespace (renamed BOP-257)" + "B20_ASSET must not use the retired base.b20_security namespace" ); assertTrue( ActivationRegistryFeatureList.B20_ASSET != keccak256("base.b20_token"), - "B20_ASSET must not use the retired base.b20_token namespace (removed BOP-257)" + "B20_ASSET must not use the retired base.b20_token namespace" ); } } diff --git a/test/unit/ActivationRegistry/admin.t.sol b/test/unit/ActivationRegistry/admin.t.sol index f7dda50..66c2be4 100644 --- a/test/unit/ActivationRegistry/admin.t.sol +++ b/test/unit/ActivationRegistry/admin.t.sol @@ -7,7 +7,7 @@ contract ActivationRegistryAdminTest is ActivationRegistryTest { /// @notice Verifies admin returns the configured activation admin address /// @dev Constant readback; the mock-vs-live boundary returns the same address either way. /// The setUp-resolved `activationAdmin` is the same value the mock hardcodes - /// (`0xCB00…0000`, per base/base#2733), so a second readback must agree. + /// (`0xCB00…0000`), so a second readback must agree. function test_admin_success_returnsConfigured() public view { assertEq(activationRegistry.admin(), activationAdmin, "admin() must return the address resolved during setUp"); assertTrue(activationAdmin != address(0), "activation admin must be non-zero"); diff --git a/test/unit/B20/roles/grantRole.t.sol b/test/unit/B20/roles/grantRole.t.sol index 161453c..acdfe17 100644 --- a/test/unit/B20/roles/grantRole.t.sol +++ b/test/unit/B20/roles/grantRole.t.sol @@ -67,8 +67,8 @@ contract B20GrantRoleTest is B20Test { /// @notice Verifies grantRole reverts on an admin-less token even when the caller holds a /// custom-admin role that would otherwise authorize the grant. - /// @dev Admin-role-resurrection guard — mirrors base/base PR #2961 - /// (`ensure_role_admin_mutations_available`). Without this guard a custom-admin + /// @dev Admin-role-resurrection guard — mirrors the Rust precompile's + /// `ensure_role_admin_mutations_available`. Without this guard a custom-admin /// role chain set up while an admin existed (e.g. setRoleAdmin(MINT_ROLE, BURN_ROLE) /// + grantRole(BURN_ROLE, alice)) survives `renounceLastAdmin()` and lets BURN_ROLE /// holders keep mutating the role graph on a token that has no admin. The shared diff --git a/test/unit/B20/roles/revokeRole.t.sol b/test/unit/B20/roles/revokeRole.t.sol index 086b643..98a7875 100644 --- a/test/unit/B20/roles/revokeRole.t.sol +++ b/test/unit/B20/roles/revokeRole.t.sol @@ -24,7 +24,7 @@ contract B20RevokeRoleTest is B20Test { } /// @notice Verifies revokeRole reverts when the call would remove the sole DEFAULT_ADMIN_ROLE holder - /// @dev BOP-196 regression. Without this guard, `revokeRole(DEFAULT_ADMIN_ROLE, soleAdmin)` would + /// @dev Without this guard, `revokeRole(DEFAULT_ADMIN_ROLE, soleAdmin)` would /// succeed silently, bricking the token: admin operations become unreachable, and no /// `LastAdminRenounced` event is emitted to signal the terminal state. The dedicated /// `renounceLastAdmin()` path remains the only legitimate way to remove the final admin. @@ -42,7 +42,7 @@ contract B20RevokeRoleTest is B20Test { /// @notice Verifies revokeRole sets hasRole(role, account) to false /// @dev Read-after-write; canonical hasRole readback test lives in hasRole.t.sol. /// Skips revoking DEFAULT_ADMIN_ROLE from the sole bootstrap admin - /// (would revert with LastAdminCannotRenounce per the BOP-196 guard; + /// (would revert with LastAdminCannotRenounce per the last-admin guard; /// see test_revokeRole_revert_lastAdmin and renounceLastAdmin.t.sol /// for the terminal-admin removal path). /// Paired slot assertion: the `roles[role][account]` slot diff --git a/test/unit/B20/roles/revokeRole_revertOrder.t.sol b/test/unit/B20/roles/revokeRole_revertOrder.t.sol index 335922c..4fbe6bb 100644 --- a/test/unit/B20/roles/revokeRole_revertOrder.t.sol +++ b/test/unit/B20/roles/revokeRole_revertOrder.t.sol @@ -8,14 +8,13 @@ import {MockB20, B20Constants} from "test/lib/mocks/MockB20.sol"; /// @title Differential check-order tests for `revokeRole`. /// -/// @notice **Canonical order (Solidity reference, post-BOP-196):** +/// @notice **Canonical order (Solidity reference):** /// 1. ROLE (`onlyRoleAdmin(role)` modifier) → `AccessControlUnauthorizedAccount` /// 2. LAST-ADMIN (`role == DEFAULT_ADMIN && account holds it && adminCount == 1`) /// → `LastAdminCannotRenounce` /// -/// C(2, 2) = 1 pair. The LAST-ADMIN guard was added in -/// [PR #91 (BOP-196)](https://github.com/base/base-std/pull/91); before that fix -/// `revokeRole` would silently brick the token by removing the sole admin. +/// C(2, 2) = 1 pair. Without the LAST-ADMIN guard, `revokeRole` would silently +/// brick the token by removing the sole admin. contract B20RevokeRoleRevertOrderTest is B20Test { /// @notice ROLE beats LAST-ADMIN. /// @dev Caller lacks the role-admin role AND the target is the sole DEFAULT_ADMIN_ROLE holder. diff --git a/test/unit/B20Asset/batch/batchMint_rollback.t.sol b/test/unit/B20Asset/batch/batchMint_rollback.t.sol index 3a5285f..bf8996d 100644 --- a/test/unit/B20Asset/batch/batchMint_rollback.t.sol +++ b/test/unit/B20Asset/batch/batchMint_rollback.t.sol @@ -21,7 +21,7 @@ import {B20Constants} from "src/lib/B20Constants.sol"; /// signal is in fork mode against the real Rust precompile, /// where any break in the `precompile-storage` journal hookup /// would leave the earlier writes persisted across the revert. -/// Companion to BOP-176 (Rust-side unit tests). +/// Companion to the Rust-side unit tests. contract B20AssetBatchMintRollbackTest is B20AssetTest { /// @notice batchMint that reverts on element 1 via supply-cap leaves balances + supply unchanged /// @dev Element 0 mints under the cap (writes alice's balance and diff --git a/test/unit/B20Factory/createB20_asset_decimals.t.sol b/test/unit/B20Factory/createB20_asset_decimals.t.sol index 7fd3955..773ae06 100644 --- a/test/unit/B20Factory/createB20_asset_decimals.t.sol +++ b/test/unit/B20Factory/createB20_asset_decimals.t.sol @@ -11,8 +11,7 @@ import {MockB20} from "test/lib/mocks/MockB20.sol"; import {MockB20AssetStorage} from "test/lib/mocks/MockB20Storage.sol"; import {B20FactoryTest} from "test/lib/B20FactoryTest.sol"; -/// @notice Coverage for the asset variant's configurable `decimals` field -/// introduced in BOP-252 / BOP-255 (storage) and BOP-259 (tests). +/// @notice Coverage for the asset variant's configurable `decimals` field. /// /// @dev Asserts the boundary values, the out-of-range revert with the new /// `InvalidDecimals(uint8)` error, fuzz coverage over the full @@ -213,7 +212,7 @@ contract B20FactoryCreateB20AssetDecimalsTest is B20FactoryTest { //////////////////////////////////////////////////////////////*/ /// @notice Verifies the stablecoin variant still hardcodes `decimals() == 6`. - /// BOP-255 is an asset-only change; the stablecoin path must be untouched. + /// @dev Configurable decimals is an asset-only change; the stablecoin path must be untouched. function test_createB20_success_stablecoin_decimalsStillHardcoded(address caller, bytes32 salt) public { _assumeValidCaller(caller); address token = _createStablecoin(caller, salt, _stablecoinParams(), new bytes[](0)); diff --git a/test/unit/B20Factory/createB20_revertOrder.t.sol b/test/unit/B20Factory/createB20_revertOrder.t.sol index b88070f..0adeab8 100644 --- a/test/unit/B20Factory/createB20_revertOrder.t.sol +++ b/test/unit/B20Factory/createB20_revertOrder.t.sol @@ -12,7 +12,7 @@ import {B20FactoryTest} from "test/lib/B20FactoryTest.sol"; /// ordering: VERSION beats each field-validation check inside an arm. /// The cross-arm UNSUPPORTED-VARIANT-beats-arm-body ordering is pinned by /// `test_createB20_revert_outOfRangeVariant` in `getTokenAddress.t.sol` -/// (raw-bytes ABI-decoder panic; see audit-response PR #89). +/// (raw-bytes ABI-decoder panic). /// /// **Canonical order per variant arm (Solidity reference):** /// - STABLECOIN: VERSION → INVALID-CURRENCY (format check on each byte) diff --git a/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol b/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol index 4738d7a..d8aa7dc 100644 --- a/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol +++ b/test/unit/PolicyRegistry/createPolicyWithAccounts.t.sol @@ -121,7 +121,7 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { } /// @notice Verifies createPolicyWithAccounts reverts when the batch exceeds MAX_BATCH_SIZE - /// @dev Mirrors the Rust precompile's batch limit (base/base#2876); checks + /// @dev Mirrors the Rust precompile's batch limit; checks /// BatchSizeTooLarge(maxBatchSize). Fuzz drives `overflow` so the test exercises /// arbitrary over-the-limit sizes, not just the immediate neighbor. function test_createPolicyWithAccounts_revert_batchSizeTooLarge( @@ -156,9 +156,8 @@ contract PolicyRegistryCreatePolicyWithAccountsTest is PolicyRegistryTest { /// @dev Pins the Rust precompile's check precedence (`validate_create_policy_inputs` /// → `require_account_batch_size`). Mirrors Rust test /// `create_policy_with_accounts_zero_admin_precedes_batch_size_revert` in - /// `crates/common/precompiles/src/policy/storage.rs`. Regression guard for the - /// BOP-207 hoist: if a later refactor reorders these two entry-point checks, - /// this test fails. + /// `crates/common/precompiles/src/policy/storage.rs`. Guards the entry-point check + /// order: if a later refactor reorders these two checks, this test fails. function test_createPolicyWithAccounts_revert_zeroAdmin_precedes_batchSizeTooLarge( address caller, uint8 typeIdx, diff --git a/test/unit/PolicyRegistry/createPolicyWithAccounts_rollback.t.sol b/test/unit/PolicyRegistry/createPolicyWithAccounts_rollback.t.sol index 7279777..5249628 100644 --- a/test/unit/PolicyRegistry/createPolicyWithAccounts_rollback.t.sol +++ b/test/unit/PolicyRegistry/createPolicyWithAccounts_rollback.t.sol @@ -11,8 +11,8 @@ import {MockPolicyRegistryStorage} from "test/lib/mocks/MockPolicyRegistryStorag /// registry state. /// /// @dev Sibling of `PolicyRegistryCreatePolicyWithAccountsTest` (happy -/// path + revert reasons). Distinct from BOP-176 (Rust-side unit -/// tests of `precompile-storage`): this exercises the precompile +/// path + revert reasons). Distinct from the Rust-side unit +/// tests of `precompile-storage`: this exercises the precompile /// in a real EVM execution context to verify the JournaledState /// integration rolls back any partial writes — or, in the case /// of an impl that validates before creating, that no writes @@ -22,8 +22,7 @@ import {MockPolicyRegistryStorage} from "test/lib/mocks/MockPolicyRegistryStorag /// different paths: /// - The Solidity mock advances `nextCounter` and writes the /// policy slot before checking the batch size, then relies on -/// revm's journal to roll back on revert (BOP-207 tracks the -/// reorder). +/// revm's journal to roll back on revert. /// - The Rust precompile validates batch size first and never /// writes. /// End state is identical in both: counter unchanged, predicted diff --git a/test/unit/PolicyRegistry/updateAllowlist.t.sol b/test/unit/PolicyRegistry/updateAllowlist.t.sol index 43b955d..11a1cb3 100644 --- a/test/unit/PolicyRegistry/updateAllowlist.t.sol +++ b/test/unit/PolicyRegistry/updateAllowlist.t.sol @@ -129,7 +129,7 @@ contract PolicyRegistryUpdateAllowlistTest is PolicyRegistryTest { } /// @notice Verifies updateAllowlist reverts when the batch exceeds MAX_BATCH_SIZE - /// @dev Mirrors the Rust precompile's batch limit (base/base#2876); checks + /// @dev Mirrors the Rust precompile's batch limit; checks /// BatchSizeTooLarge(maxBatchSize). Fuzz drives `overflow` so the test exercises /// arbitrary over-the-limit sizes. function test_updateAllowlist_revert_batchSizeTooLarge(address currentAdmin, bool allowed, uint8 overflow) public { diff --git a/test/unit/PolicyRegistry/updateBlocklist.t.sol b/test/unit/PolicyRegistry/updateBlocklist.t.sol index b6d15cd..44fb275 100644 --- a/test/unit/PolicyRegistry/updateBlocklist.t.sol +++ b/test/unit/PolicyRegistry/updateBlocklist.t.sol @@ -129,7 +129,7 @@ contract PolicyRegistryUpdateBlocklistTest is PolicyRegistryTest { } /// @notice Verifies updateBlocklist reverts when the batch exceeds MAX_BATCH_SIZE - /// @dev Mirrors the Rust precompile's batch limit (base/base#2876); checks + /// @dev Mirrors the Rust precompile's batch limit; checks /// BatchSizeTooLarge(maxBatchSize). Fuzz drives `overflow` so the test exercises /// arbitrary over-the-limit sizes. function test_updateBlocklist_revert_batchSizeTooLarge(address currentAdmin, bool blocked, uint8 overflow) public {