From 66f4afe17a9c576ad9c4bb007d6081e129e29498 Mon Sep 17 00:00:00 2001 From: Amie Corso Date: Wed, 27 May 2026 19:30:31 -0700 Subject: [PATCH 01/10] claim 2: reject empty stablecoin currency with MissingRequiredField Mirrors the Rust precompile's empty-currency rejection. The format-check loop was vacuously safe on empty input (no bytes to inspect). Adds an explicit length check matching the SECURITY-variant isin guard. Also fixes the test helper _isValidFiatCode so empty returns false (it was returning vacuously true, causing every fuzz test that filtered with vm.assume(!_isValidFiatCode) to silently exclude empty). --- test/lib/mocks/MockB20Factory.sol | 6 +++++- test/unit/B20Factory/createToken.t.sol | 23 +++++++++++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/test/lib/mocks/MockB20Factory.sol b/test/lib/mocks/MockB20Factory.sol index 7cdec8f..6d4a66f 100644 --- a/test/lib/mocks/MockB20Factory.sol +++ b/test/lib/mocks/MockB20Factory.sol @@ -119,8 +119,12 @@ contract MockB20Factory is IB20Factory { if (p.version != B20FactoryLib.B20_STABLECOIN_CREATE_PARAMS_VERSION) { revert UnsupportedVersion(p.version, variant); } - // Format check: every byte must be an uppercase ASCII letter (A-Z). + // Empty currency is a missing-required-field, not a format error: an empty byte + // string has no bytes to inspect so the format-check loop below would vacuously + // succeed without this explicit guard. Mirrors the SECURITY-variant isin check. bytes memory cb = bytes(p.currency); + if (cb.length == 0) revert MissingRequiredField(); + // Format check: every byte must be an uppercase ASCII letter (A-Z). for (uint256 i = 0; i < cb.length; ++i) { if (cb[i] < 0x41 || cb[i] > 0x5A) revert InvalidCurrency(p.currency); } diff --git a/test/unit/B20Factory/createToken.t.sol b/test/unit/B20Factory/createToken.t.sol index 30fed40..3f331a6 100644 --- a/test/unit/B20Factory/createToken.t.sol +++ b/test/unit/B20Factory/createToken.t.sol @@ -90,13 +90,15 @@ contract B20FactoryCreateB20Test is B20FactoryTest { // STABLECOIN currency validation: every byte must be an uppercase ASCII letter (A-Z). - /// @notice Any string containing a non-`A`–`Z` byte reverts with `InvalidCurrency(code)`. + /// @notice Any non-empty string containing a non-`A`–`Z` byte reverts with `InvalidCurrency(code)`. /// @dev Subsumes every point case (lowercase, digits, symbols, multi-byte UTF-8) via - /// `vm.assume(!_isValidFiatCode)`. Length is unconstrained. + /// `vm.assume(!_isValidFiatCode)`. Empty input is covered by + /// `test_createB20_revert_missingCurrency` (rejected with MissingRequiredField). function test_createB20_revert_currency_rejectsInvalidFormat(string memory code, address caller, bytes32 salt) public { _assumeValidCaller(caller); + vm.assume(bytes(code).length > 0); vm.assume(!_isValidFiatCode(code)); IB20Factory.B20StablecoinCreateParams memory p = _stablecoinParams("Test", "TST", admin, code); vm.prank(caller); @@ -106,6 +108,10 @@ contract B20FactoryCreateB20Test is B20FactoryTest { function _isValidFiatCode(string memory code) private pure returns (bool) { bytes memory b = bytes(code); + // Empty is not a valid format. Returning true here would make every fuzz test + // that filters with `vm.assume(!_isValidFiatCode(code))` silently discard empty, + // hiding the missing-currency case from the suite. + if (b.length == 0) return false; for (uint256 i = 0; i < b.length; ++i) { if (b[i] < 0x41 || b[i] > 0x5A) return false; } @@ -136,6 +142,19 @@ contract B20FactoryCreateB20Test is B20FactoryTest { factory.createB20(IB20Factory.B20Variant.SECURITY, salt, abi.encode(p), new bytes[](0)); } + /// @notice Verifies stablecoin createToken reverts when currency is the empty string + /// @dev Per-variant required-field check; mirrors the SECURITY isin guard. The format-check + /// loop on `currency` is vacuously safe on empty input (no bytes to inspect), so an + /// explicit length check is required to reject empty up front. Checks + /// MissingRequiredField() error, matching the Rust precompile. + function test_createB20_revert_missingCurrency(address caller, bytes32 salt) public { + _assumeValidCaller(caller); + IB20Factory.B20StablecoinCreateParams memory p = _stablecoinParams("Stablecoin Test", "USD", admin, ""); + vm.prank(caller); + vm.expectRevert(IB20Factory.MissingRequiredField.selector); + factory.createB20(IB20Factory.B20Variant.STABLECOIN, salt, abi.encode(p), new bytes[](0)); + } + /// @notice Verifies createToken reverts when (variant, sender, salt) collides /// @dev Deterministic-address uniqueness; checks TokenAlreadyExists(token) error function test_createB20_revert_tokenAlreadyExists(address caller, bytes32 salt) public { From eaeb5fd59a034feeb098475d8e63247a3b29b76f Mon Sep 17 00:00:00 2001 From: Amie Corso Date: Wed, 27 May 2026 19:31:23 -0700 Subject: [PATCH 02/10] claim 3: add raw-bytes test for getB20Address out-of-range variant Mirrors test_createB20_revert_outOfRangeVariant. No source change: Solidity's ABI decoder rejects out-of-range enum bytes with Panic(0x21) before any function body runs, so an explicit InvalidVariant guard at the body level would be unreachable. The Rust precompile rejects the same input with the typed InvalidVariant() revert. Observable behavior matches (both backends revert); the specific selectors differ. --- test/unit/B20Factory/getTokenAddress.t.sol | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/unit/B20Factory/getTokenAddress.t.sol b/test/unit/B20Factory/getTokenAddress.t.sol index 8a69761..cee19b8 100644 --- a/test/unit/B20Factory/getTokenAddress.t.sol +++ b/test/unit/B20Factory/getTokenAddress.t.sol @@ -117,4 +117,20 @@ contract B20FactoryGetTokenAddressTest is B20FactoryTest { uint8 byteAt11 = uint8(uint160(a) >> 64); assertEq(byteAt11, expectedByte11, "address byte [11] must come from tail entropy"); } + + /// @notice Verifies getB20Address rejects raw variant bytes outside the B20Variant enum range + /// @dev Mirrors the createB20 raw-bytes test. B20Variant has no "NONE" sentinel; typed + /// callers cannot construct an out-of-range value, so this path is only reachable via + /// raw calldata. Solidity rejects via the ABI decoder with Panic(0x21); the Rust + /// precompile rejects with the typed `InvalidVariant()` revert. The observable + /// contract from a raw-bytes caller is "the call reverts" on both backends; the + /// specific selector differs because Solidity has no opportunity to run a function + /// body before the decoder rejects. + function test_getB20Address_revert_outOfRangeVariant(address sender, bytes32 salt, uint8 badVariant) public { + badVariant = uint8(bound(uint256(badVariant), uint256(type(IB20Factory.B20Variant).max) + 1, 255)); + vm.expectRevert(); + (bool ok,) = address(factory) + .call(abi.encodeWithSelector(IB20Factory.getB20Address.selector, badVariant, sender, salt)); + ok; // silence unused warning; the revert is asserted via vm.expectRevert. + } } From 3661d00aaac65a93a7d7f153b5557c2511f9b08b Mon Sep 17 00:00:00 2001 From: Amie Corso Date: Wed, 27 May 2026 19:32:28 -0700 Subject: [PATCH 03/10] claim 4: reject zero amount per-element in batchBurn Mirrors the Rust precompile's per-element zero-amount guard. Fires inside the loop before the balance check, so a zero in any slot reverts the whole batch (all-or-nothing atomicity). New test test_batchBurn_revert_zeroAmountInBatch exercises the canonical case (zero in the second slot) and asserts both balances are unchanged. --- test/lib/mocks/MockB20Security.sol | 4 ++++ test/unit/B20Security/batch/batchBurn.t.sol | 26 +++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/test/lib/mocks/MockB20Security.sol b/test/lib/mocks/MockB20Security.sol index 3713012..ba049a2 100644 --- a/test/lib/mocks/MockB20Security.sol +++ b/test/lib/mocks/MockB20Security.sol @@ -199,6 +199,10 @@ contract MockB20Security is MockB20, IB20Security { if (accounts.length == 0) revert EmptyBatch(); if (_isPaused(PausableFeature.BURN)) revert ContractPaused(PausableFeature.BURN); for (uint256 i = 0; i < accounts.length; i++) { + // Per-element zero-amount guard, matching the Rust precompile. Fires inside + // the loop before the balance check, so a zero in any slot reverts the whole + // batch (all-or-nothing atomicity). + if (amounts[i] == 0) revert InvalidAmount(); _burnRaw(accounts[i], amounts[i]); } } diff --git a/test/unit/B20Security/batch/batchBurn.t.sol b/test/unit/B20Security/batch/batchBurn.t.sol index df6cf51..b9654a2 100644 --- a/test/unit/B20Security/batch/batchBurn.t.sol +++ b/test/unit/B20Security/batch/batchBurn.t.sol @@ -75,6 +75,32 @@ contract B20SecurityBatchBurnTest is B20SecurityTest { security().batchBurn(_singletonAddresses(alice), _singletonUints(amount)); } + /// @notice Verifies batchBurn reverts when any element's amount is zero + /// @dev Per-element zero-amount guard, mirroring the Rust precompile. Fires inside the + /// loop before the balance check, so the position of the zero in the array determines + /// whether any preceding elements were processed (they weren't — the revert unwinds + /// the whole batch). Checks IB20.InvalidAmount() error. + function test_batchBurn_revert_zeroAmountInBatch() 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 in any slot must revert the whole batch + + vm.prank(burnFromActor); + vm.expectRevert(IB20.InvalidAmount.selector); + security().batchBurn(accounts, amounts); + + // Atomicity: alice's balance must NOT have been debited by the preceding element. + assertEq(token.balanceOf(alice), 100, "alice balance must be unchanged after reverted batch"); + assertEq(token.balanceOf(bob), 100, "bob balance must be unchanged after reverted batch"); + } + /// @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. From 84c4a841bdc89d9a72554389ccb49f01877353b3 Mon Sep 17 00:00:00 2001 From: Amie Corso Date: Wed, 27 May 2026 19:33:24 -0700 Subject: [PATCH 04/10] claim 5: reject zero amount in security redeem with InvalidAmount Mirrors the Rust precompile's explicit amount.is_zero() guard. Fires AFTER pause/policy and BEFORE the shares math, so zero-amount redeems revert with IB20.InvalidAmount() (matching Rust) rather than being absorbed by the downstream shares == 0 check (which would surface as BelowMinimumRedeemable). The distinction matters for integrators that distinguish 'amount must be non-zero' from 'amount too small to clear the floor'. --- test/lib/mocks/MockB20Security.sol | 5 +++++ test/unit/B20Security/redeem/redeem.t.sol | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/test/lib/mocks/MockB20Security.sol b/test/lib/mocks/MockB20Security.sol index ba049a2..2df70f2 100644 --- a/test/lib/mocks/MockB20Security.sol +++ b/test/lib/mocks/MockB20Security.sol @@ -296,6 +296,11 @@ contract MockB20Security is MockB20, IB20Security { if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(REDEEMSenderPolicyId, msg.sender)) { revert PolicyForbids(REDEEM_SENDER_POLICY, REDEEMSenderPolicyId); } + // Explicit zero-amount guard matching the Rust precompile. Fires AFTER pause/policy + // and BEFORE the shares math, so zero-amount surfaces as InvalidAmount() rather than + // being absorbed by the shares == 0 path below (which is reserved for nonzero amounts + // that round to zero shares under low ratios). + if (amount == 0) revert InvalidAmount(); ratio = _sharesToTokensRatio(); uint256 shares = (amount * ratio) / WAD_PRECISION; uint256 minimum = $.minimumRedeemable; diff --git a/test/unit/B20Security/redeem/redeem.t.sol b/test/unit/B20Security/redeem/redeem.t.sol index 889807e..d6768d9 100644 --- a/test/unit/B20Security/redeem/redeem.t.sol +++ b/test/unit/B20Security/redeem/redeem.t.sol @@ -48,6 +48,18 @@ contract B20SecurityRedeemTest is B20SecurityTest { security().redeem(amount); } + /// @notice Verifies redeem reverts when the caller passes amount == 0 + /// @dev Explicit zero-amount guard mirroring the Rust precompile. Fires AFTER pause/policy + /// but BEFORE the shares math, so the error is IB20.InvalidAmount() (not the + /// BelowMinimumRedeemable that would otherwise catch zero shares downstream). The + /// distinction matters for integrators that distinguish "amount must be non-zero" + /// from "amount too small to clear the minimum-shares floor". + function test_redeem_revert_zeroAmount() public { + vm.prank(alice); + vm.expectRevert(IB20.InvalidAmount.selector); + security().redeem(0); + } + /// @notice Verifies redeem reverts when the resulting share count is below the configured floor /// @dev Error message reports the computed shares and the configured minimum. Test sets the /// floor above the requested share count and checks BelowMinimumRedeemable(shares, minimum). From 41066300556d088f738afba2414e023cb25407db Mon Sep 17 00:00:00 2001 From: Amie Corso Date: Wed, 27 May 2026 19:34:24 -0700 Subject: [PATCH 05/10] claim 6: parity test for malformed-signature permit recovery Pins the cross-backend equivalence on the recovered == address(0) path. EVM's ecrecover returns address(0) for malformed (v, r, s); alloy in the Rust precompile returns Err from recover_address_from_prehash and maps it to InvalidSigner(address(0), owner). Same observable result, different code paths. No source change: the behavior is already correct on both sides; the new test prevents silent drift. --- test/unit/B20/permit/permit.t.sol | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/unit/B20/permit/permit.t.sol b/test/unit/B20/permit/permit.t.sol index 7dc5161..8250cec 100644 --- a/test/unit/B20/permit/permit.t.sol +++ b/test/unit/B20/permit/permit.t.sol @@ -43,6 +43,28 @@ contract B20PermitTest is B20Test { token.permit(bob, spender, amount, deadline, v, r, s); } + /// @notice Verifies permit reverts for malformed (v, r, s) that causes recovery to fail + /// @dev Backend-parity test for the recovered == address(0) path. EVM's `ecrecover` returns + /// `address(0)` on malformed signatures (invalid `v`, etc.); alloy in the Rust + /// precompile returns `Err` from `recover_address_from_prehash` on the same input, + /// which is mapped to `InvalidSigner(address(0), owner)`. Both backends therefore + /// revert with the same selector on a malformed signature, but via different code + /// paths. This test pins that parity so neither side can silently drift. + /// + /// We pick v = 0 (only 27 and 28 are valid), so ecrecover deterministically returns + /// `address(0)` regardless of r and s. + function test_permit_revert_malformedSignature(address spender, uint256 amount) public { + vm.assume(spender != address(0)); + uint256 deadline = type(uint256).max; + address owner = alice; + uint8 v = 0; // invalid: only 27 and 28 produce a valid recovery + bytes32 r = bytes32(uint256(1)); + bytes32 s = bytes32(uint256(2)); + + vm.expectRevert(abi.encodeWithSelector(IB20.InvalidSigner.selector, address(0), owner)); + token.permit(owner, spender, amount, deadline, v, r, s); + } + /// @notice Verifies permit reverts when the same signature is replayed /// @dev Nonce monotonicity guards replay: after the first call advances the nonce, /// the second call's digest is computed against the OLD nonce, so ecrecover From 9056a14db43a8bb2207ffef485a2c469844fa240 Mon Sep 17 00:00:00 2001 From: Amie Corso Date: Wed, 27 May 2026 19:36:16 -0700 Subject: [PATCH 06/10] fixup! claim 2: align empty-currency selector to InvalidCurrency Fork test confirmed the Rust precompile reverts InvalidCurrency("") on empty currency, not MissingRequiredField(). Switch Solidity guard to match. Test renamed test_createB20_revert_missingCurrency -> test_createB20_revert_emptyCurrency to reflect the actual selector. Squash this into the claim 2 commit when reviewing. --- test/lib/mocks/MockB20Factory.sol | 8 ++++---- test/unit/B20Factory/createToken.t.sol | 11 +++++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/test/lib/mocks/MockB20Factory.sol b/test/lib/mocks/MockB20Factory.sol index 6d4a66f..d11becd 100644 --- a/test/lib/mocks/MockB20Factory.sol +++ b/test/lib/mocks/MockB20Factory.sol @@ -119,11 +119,11 @@ contract MockB20Factory is IB20Factory { if (p.version != B20FactoryLib.B20_STABLECOIN_CREATE_PARAMS_VERSION) { revert UnsupportedVersion(p.version, variant); } - // Empty currency is a missing-required-field, not a format error: an empty byte - // string has no bytes to inspect so the format-check loop below would vacuously - // succeed without this explicit guard. Mirrors the SECURITY-variant isin check. + // Empty currency must be rejected explicitly: the format-check loop below has + // no bytes to inspect on empty input and would vacuously succeed otherwise. + // Reverts InvalidCurrency("") to match the Rust precompile's selector. bytes memory cb = bytes(p.currency); - if (cb.length == 0) revert MissingRequiredField(); + if (cb.length == 0) revert InvalidCurrency(p.currency); // Format check: every byte must be an uppercase ASCII letter (A-Z). for (uint256 i = 0; i < cb.length; ++i) { if (cb[i] < 0x41 || cb[i] > 0x5A) revert InvalidCurrency(p.currency); diff --git a/test/unit/B20Factory/createToken.t.sol b/test/unit/B20Factory/createToken.t.sol index 3f331a6..c8ef5fa 100644 --- a/test/unit/B20Factory/createToken.t.sol +++ b/test/unit/B20Factory/createToken.t.sol @@ -143,15 +143,14 @@ contract B20FactoryCreateB20Test is B20FactoryTest { } /// @notice Verifies stablecoin createToken reverts when currency is the empty string - /// @dev Per-variant required-field check; mirrors the SECURITY isin guard. The format-check - /// loop on `currency` is vacuously safe on empty input (no bytes to inspect), so an - /// explicit length check is required to reject empty up front. Checks - /// MissingRequiredField() error, matching the Rust precompile. - function test_createB20_revert_missingCurrency(address caller, bytes32 salt) public { + /// @dev The format-check loop on `currency` is vacuously safe on empty input (no bytes + /// to inspect), so an explicit length check is required to reject empty up front. + /// Checks InvalidCurrency("") error, matching the Rust precompile's selector. + function test_createB20_revert_emptyCurrency(address caller, bytes32 salt) public { _assumeValidCaller(caller); IB20Factory.B20StablecoinCreateParams memory p = _stablecoinParams("Stablecoin Test", "USD", admin, ""); vm.prank(caller); - vm.expectRevert(IB20Factory.MissingRequiredField.selector); + vm.expectRevert(abi.encodeWithSelector(IB20Factory.InvalidCurrency.selector, "")); factory.createB20(IB20Factory.B20Variant.STABLECOIN, salt, abi.encode(p), new bytes[](0)); } From 2e334cdb904e125288937776f609df1e673746e1 Mon Sep 17 00:00:00 2001 From: Amie Corso Date: Wed, 27 May 2026 19:58:41 -0700 Subject: [PATCH 07/10] fuzz: extend success-test amount bounds to cover zero on non-rejecting paths The Rust precompile does not reject zero amount on regular mint/burn/transfer/transferFrom (only batchBurn and security_redeem have explicit zero guards, both covered by claims 4 and 5). Success tests for non-rejecting paths should therefore fuzz the full valid input domain rather than defensively excluding zero. Relaxes four success-test bounds: allowance decreasesAfterTransferFrom (spendAmount), transferFrom decreasesAllowance (spendAmount), transferFrom infiniteAllowanceUnchanged (amount), transferFromWithMemo movesBalanceAndDecreasesAllowance (amount). All assertions hold at zero (x - 0 == x, 0 == 0). Full fork suite passes 502/0/2 against the Rust precompile with the broader fuzz domain. --- test/unit/B20/erc20/allowance.t.sol | 6 ++++-- test/unit/B20/erc20/transferFrom.t.sol | 8 ++++++-- test/unit/B20/memo/transferFromWithMemo.t.sol | 4 +++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/test/unit/B20/erc20/allowance.t.sol b/test/unit/B20/erc20/allowance.t.sol index 82f8c9b..47a6c19 100644 --- a/test/unit/B20/erc20/allowance.t.sol +++ b/test/unit/B20/erc20/allowance.t.sol @@ -38,9 +38,11 @@ contract B20AllowanceTest is B20Test { // transferFrom only consumes allowance when msg.sender != from (see MockB20._transferFrom); // owner == spender skips the consumption path, so filter it out. vm.assume(owner != spender); - // Bound spendAmount to <= allowanceAmount, both above zero so the transfer is meaningful. + // allowanceAmount > 0 keeps the allowance setup meaningful; spendAmount includes 0 + // so the assertion (allowance decreases by spendAmount) is exercised across the full + // valid input domain, including the no-op zero-spend case. allowanceAmount = bound(allowanceAmount, 1, type(uint128).max); - spendAmount = bound(spendAmount, 1, allowanceAmount); + spendAmount = bound(spendAmount, 0, allowanceAmount); _mint(owner, spendAmount); vm.prank(owner); diff --git a/test/unit/B20/erc20/transferFrom.t.sol b/test/unit/B20/erc20/transferFrom.t.sol index 59f83ed..af837bc 100644 --- a/test/unit/B20/erc20/transferFrom.t.sol +++ b/test/unit/B20/erc20/transferFrom.t.sol @@ -197,7 +197,9 @@ contract B20TransferFromTest is B20Test { allowanceAmount = bound(allowanceAmount, 1, type(uint128).max); // Cap below type(uint256).max so we exercise the consume path (not the infinite-allowance bypass). vm.assume(allowanceAmount != type(uint256).max); - spendAmount = bound(spendAmount, 1, allowanceAmount); + // spendAmount includes 0 so the assertion (allowance decreases by spendAmount) is + // exercised across the full valid input domain, including the no-op zero-spend case. + spendAmount = bound(spendAmount, 0, allowanceAmount); _mint(from, spendAmount); vm.prank(from); @@ -230,7 +232,9 @@ contract B20TransferFromTest is B20Test { _assumeValidActor(to); vm.assume(caller != from); vm.assume(from != to); - amount = bound(amount, 1, type(uint128).max); + // Include amount = 0: the infinite-allowance invariant must hold across the full + // valid input domain, including the no-op zero-transfer case. + amount = bound(amount, 0, type(uint128).max); _mint(from, amount); vm.prank(from); diff --git a/test/unit/B20/memo/transferFromWithMemo.t.sol b/test/unit/B20/memo/transferFromWithMemo.t.sol index 0adf8dd..138be30 100644 --- a/test/unit/B20/memo/transferFromWithMemo.t.sol +++ b/test/unit/B20/memo/transferFromWithMemo.t.sol @@ -45,7 +45,9 @@ contract B20TransferFromWithMemoTest is B20Test { _assumeValidActor(to); vm.assume(caller != from); vm.assume(from != to); - amount = bound(amount, 1, type(uint128).max); + // Include amount = 0: the balance/allowance invariants must hold across the full + // valid input domain, including the no-op zero-transfer case. + amount = bound(amount, 0, type(uint128).max); _mint(from, amount); vm.prank(from); From 70a6df132d63c88b8c442cc8c2379a3186af58fc Mon Sep 17 00:00:00 2001 From: Amie Corso Date: Wed, 27 May 2026 20:09:13 -0700 Subject: [PATCH 08/10] gap 2: cover stored-zero share-ratio fallback at toShares + sharesOf level The documented behavior is that a stored sharesToTokensRatio of zero resolves as WAD_PRECISION on the read surface. The base-level fallback is already covered by test_sharesToTokensRatio_success_zeroRestoresWadFallback. These two tests pin the same property at the derived-function level so a refactor that bypasses _sharesToTokensRatio() and reads the storage slot directly would fail. Both backends behave identically (verified via fork suite). --- test/unit/B20Security/shareRatio/sharesOf.t.sol | 15 +++++++++++++++ test/unit/B20Security/shareRatio/toShares.t.sol | 13 +++++++++++++ 2 files changed, 28 insertions(+) diff --git a/test/unit/B20Security/shareRatio/sharesOf.t.sol b/test/unit/B20Security/shareRatio/sharesOf.t.sol index 7ae2be2..3792d9a 100644 --- a/test/unit/B20Security/shareRatio/sharesOf.t.sol +++ b/test/unit/B20Security/shareRatio/sharesOf.t.sol @@ -38,4 +38,19 @@ contract B20SecuritySharesOfTest is B20SecurityTest { "sharesOf must apply balance * ratio / WAD" ); } + + /// @notice Verifies sharesOf applies the WAD fallback when the stored ratio is explicitly zero + /// @dev A stored `sharesToTokensRatio` of zero is documented to resolve as `WAD_PRECISION` on + /// the read surface, both pre-write (fresh slot) and post-explicit-zero-write. Tests the + /// latter at the derived-function level so a refactor that reads the slot directly here + /// would fail. See test_sharesToTokensRatio_success_zeroRestoresWadFallback for the + /// base-level fallback assertion. + function test_sharesOf_success_explicitZeroRatioFallsBackToWad(address account, uint256 amount) public { + _assumeValidActor(account); + amount = bound(amount, 0, type(uint128).max); + if (amount > 0) _mint(account, amount); + _updateShareRatio(5e18); // seed a non-zero value first + _updateShareRatio(0); // then explicitly clear back to zero + assertEq(security().sharesOf(account), amount, "stored zero ratio must produce identity (WAD fallback)"); + } } diff --git a/test/unit/B20Security/shareRatio/toShares.t.sol b/test/unit/B20Security/shareRatio/toShares.t.sol index 7cc9ee1..123ed78 100644 --- a/test/unit/B20Security/shareRatio/toShares.t.sol +++ b/test/unit/B20Security/shareRatio/toShares.t.sol @@ -32,4 +32,17 @@ contract B20SecurityToSharesTest is B20SecurityTest { _updateShareRatio(ratio); assertEq(security().toShares(0), 0, "zero balance must produce zero shares"); } + + /// @notice Verifies toShares applies the WAD fallback when the stored ratio is explicitly zero + /// @dev A stored `sharesToTokensRatio` of zero is documented to resolve as `WAD_PRECISION` on + /// the read surface, both pre-write (fresh slot) and post-explicit-zero-write. Tests the + /// latter: after writing zero, toShares must behave as if the ratio were WAD (identity). + /// Cross-references test_sharesToTokensRatio_success_zeroRestoresWadFallback at the + /// derived-function level so a refactor that reads the slot directly here would fail. + function test_toShares_success_explicitZeroRatioFallsBackToWad(uint256 balance) public { + balance = bound(balance, 0, type(uint128).max); + _updateShareRatio(5e18); // seed a non-zero value first + _updateShareRatio(0); // then explicitly clear back to zero + assertEq(security().toShares(balance), balance, "stored zero ratio must produce identity (WAD fallback)"); + } } From 1d8a44c3a526dda4bccd33919641bb773433d93d Mon Sep 17 00:00:00 2001 From: Amie Corso Date: Thu, 28 May 2026 11:35:35 -0700 Subject: [PATCH 09/10] style: forge fmt Collapse aligned trailing comments and wrap a long .call(...) expression per forge fmt. Fixes the CI format check on PR #89. Test behavior is unchanged. --- test/unit/B20Factory/getTokenAddress.t.sol | 4 ++-- test/unit/B20Security/shareRatio/sharesOf.t.sol | 2 +- test/unit/B20Security/shareRatio/toShares.t.sol | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/B20Factory/getTokenAddress.t.sol b/test/unit/B20Factory/getTokenAddress.t.sol index cee19b8..b051d82 100644 --- a/test/unit/B20Factory/getTokenAddress.t.sol +++ b/test/unit/B20Factory/getTokenAddress.t.sol @@ -129,8 +129,8 @@ contract B20FactoryGetTokenAddressTest is B20FactoryTest { function test_getB20Address_revert_outOfRangeVariant(address sender, bytes32 salt, uint8 badVariant) public { badVariant = uint8(bound(uint256(badVariant), uint256(type(IB20Factory.B20Variant).max) + 1, 255)); vm.expectRevert(); - (bool ok,) = address(factory) - .call(abi.encodeWithSelector(IB20Factory.getB20Address.selector, badVariant, sender, salt)); + (bool ok,) = + address(factory).call(abi.encodeWithSelector(IB20Factory.getB20Address.selector, badVariant, sender, salt)); ok; // silence unused warning; the revert is asserted via vm.expectRevert. } } diff --git a/test/unit/B20Security/shareRatio/sharesOf.t.sol b/test/unit/B20Security/shareRatio/sharesOf.t.sol index 3792d9a..f19104c 100644 --- a/test/unit/B20Security/shareRatio/sharesOf.t.sol +++ b/test/unit/B20Security/shareRatio/sharesOf.t.sol @@ -50,7 +50,7 @@ contract B20SecuritySharesOfTest is B20SecurityTest { amount = bound(amount, 0, type(uint128).max); if (amount > 0) _mint(account, amount); _updateShareRatio(5e18); // seed a non-zero value first - _updateShareRatio(0); // then explicitly clear back to zero + _updateShareRatio(0); // then explicitly clear back to zero assertEq(security().sharesOf(account), amount, "stored zero ratio must produce identity (WAD fallback)"); } } diff --git a/test/unit/B20Security/shareRatio/toShares.t.sol b/test/unit/B20Security/shareRatio/toShares.t.sol index 123ed78..7745785 100644 --- a/test/unit/B20Security/shareRatio/toShares.t.sol +++ b/test/unit/B20Security/shareRatio/toShares.t.sol @@ -42,7 +42,7 @@ contract B20SecurityToSharesTest is B20SecurityTest { function test_toShares_success_explicitZeroRatioFallsBackToWad(uint256 balance) public { balance = bound(balance, 0, type(uint128).max); _updateShareRatio(5e18); // seed a non-zero value first - _updateShareRatio(0); // then explicitly clear back to zero + _updateShareRatio(0); // then explicitly clear back to zero assertEq(security().toShares(balance), balance, "stored zero ratio must produce identity (WAD fallback)"); } } From 8c223baac6c303c33ace04d30617bea1d91092d2 Mon Sep 17 00:00:00 2001 From: Amie Corso Date: Thu, 28 May 2026 14:14:42 -0700 Subject: [PATCH 10/10] fix: align batchBurn + redeem zero-amount semantics to ERC-20 conventions (Katzman review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Katzman's review on PR #89: 'Allow 0' as a rule more closely follows ERC20 'treat 0-amount transfers as valid'. Reverses the per-element zero-amount guard added to batchBurn (claim 4) and the explicit zero-amount guard added to security_redeem_burn (claim 5). For redeem, keeps the existing dust-prevention guard (shares == 0 || shares < minimum) but adds an 'amount > 0 &&' precondition so the guard only fires when the holder actually requested a positive amount that rounds to zero shares — explicit zero-amount redemptions fall through as no-ops. Replaces the two revert tests with success-no-op tests that pin the new ERC-20-aligned semantics. Creates a divergence with the Rust precompile (which currently rejects zero amount with InvalidAmount in both batchBurn and security_redeem_burn) — Rust mirror tickets to be filed under Internal Audit Fixes. --- test/lib/mocks/MockB20Security.sol | 24 ++++++++++----------- test/unit/B20Security/batch/batchBurn.t.sol | 21 +++++++++--------- test/unit/B20Security/redeem/redeem.t.sol | 20 ++++++++++------- 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/test/lib/mocks/MockB20Security.sol b/test/lib/mocks/MockB20Security.sol index 2df70f2..fe5a72e 100644 --- a/test/lib/mocks/MockB20Security.sol +++ b/test/lib/mocks/MockB20Security.sol @@ -199,10 +199,9 @@ contract MockB20Security is MockB20, IB20Security { if (accounts.length == 0) revert EmptyBatch(); if (_isPaused(PausableFeature.BURN)) revert ContractPaused(PausableFeature.BURN); for (uint256 i = 0; i < accounts.length; i++) { - // Per-element zero-amount guard, matching the Rust precompile. Fires inside - // the loop before the balance check, so a zero in any slot reverts the whole - // batch (all-or-nothing atomicity). - if (amounts[i] == 0) revert InvalidAmount(); + // 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]); } } @@ -296,18 +295,17 @@ contract MockB20Security is MockB20, IB20Security { if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(REDEEMSenderPolicyId, msg.sender)) { revert PolicyForbids(REDEEM_SENDER_POLICY, REDEEMSenderPolicyId); } - // Explicit zero-amount guard matching the Rust precompile. Fires AFTER pause/policy - // and BEFORE the shares math, so zero-amount surfaces as InvalidAmount() rather than - // being absorbed by the shares == 0 path below (which is reserved for nonzero amounts - // that round to zero shares under low ratios). - if (amount == 0) revert InvalidAmount(); ratio = _sharesToTokensRatio(); uint256 shares = (amount * ratio) / WAD_PRECISION; uint256 minimum = $.minimumRedeemable; - // Always reject zero-share redemptions, even if the configured - // minimum is 0 — burning token dust that resolves to no shares - // is never the holder's intent. - if (shares == 0 || shares < minimum) revert BelowMinimumRedeemable(shares, minimum); + // Zero amounts are allowed per ERC-20 conventions (`transfer(0)` is valid). + // For amount > 0, reject dust burns that round to zero shares OR fall below the + // configured minimum — burning a positive amount that resolves to no shares is + // never the holder's intent. The `amount > 0` guard is what keeps explicit + // zero-amount redemptions from being absorbed by the dust path. + if (amount > 0 && (shares == 0 || shares < minimum)) { + revert BelowMinimumRedeemable(shares, minimum); + } _burnRaw(msg.sender, amount); } diff --git a/test/unit/B20Security/batch/batchBurn.t.sol b/test/unit/B20Security/batch/batchBurn.t.sol index b9654a2..ede3840 100644 --- a/test/unit/B20Security/batch/batchBurn.t.sol +++ b/test/unit/B20Security/batch/batchBurn.t.sol @@ -75,12 +75,13 @@ contract B20SecurityBatchBurnTest is B20SecurityTest { security().batchBurn(_singletonAddresses(alice), _singletonUints(amount)); } - /// @notice Verifies batchBurn reverts when any element's amount is zero - /// @dev Per-element zero-amount guard, mirroring the Rust precompile. Fires inside the - /// loop before the balance check, so the position of the zero in the array determines - /// whether any preceding elements were processed (they weren't — the revert unwinds - /// the whole batch). Checks IB20.InvalidAmount() error. - function test_batchBurn_revert_zeroAmountInBatch() public { + /// @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); @@ -90,15 +91,13 @@ contract B20SecurityBatchBurnTest is B20SecurityTest { accounts[1] = bob; uint256[] memory amounts = new uint256[](2); amounts[0] = 50; - amounts[1] = 0; // zero in any slot must revert the whole batch + amounts[1] = 0; // zero is a valid no-op per ERC-20 conventions vm.prank(burnFromActor); - vm.expectRevert(IB20.InvalidAmount.selector); security().batchBurn(accounts, amounts); - // Atomicity: alice's balance must NOT have been debited by the preceding element. - assertEq(token.balanceOf(alice), 100, "alice balance must be unchanged after reverted batch"); - assertEq(token.balanceOf(bob), 100, "bob balance must be unchanged after reverted batch"); + 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 diff --git a/test/unit/B20Security/redeem/redeem.t.sol b/test/unit/B20Security/redeem/redeem.t.sol index d6768d9..ec7a1d3 100644 --- a/test/unit/B20Security/redeem/redeem.t.sol +++ b/test/unit/B20Security/redeem/redeem.t.sol @@ -48,16 +48,20 @@ contract B20SecurityRedeemTest is B20SecurityTest { security().redeem(amount); } - /// @notice Verifies redeem reverts when the caller passes amount == 0 - /// @dev Explicit zero-amount guard mirroring the Rust precompile. Fires AFTER pause/policy - /// but BEFORE the shares math, so the error is IB20.InvalidAmount() (not the - /// BelowMinimumRedeemable that would otherwise catch zero shares downstream). The - /// distinction matters for integrators that distinguish "amount must be non-zero" - /// from "amount too small to clear the minimum-shares floor". - function test_redeem_revert_zeroAmount() public { + /// @notice Verifies redeem treats amount == 0 as a valid no-op + /// @dev Mirrors ERC-20 conventions (transfer(0) is valid). The dust-prevention guard + /// (`shares == 0 || shares < minimum`) only fires for amount > 0, so an explicit + /// zero-amount redemption succeeds without burning or reverting. Emits the canonical + /// `Transfer(caller, 0, 0)` (from `_burnRaw`) followed by `Redeemed(caller, 0, ratio)`. + function test_redeem_success_zeroAmountIsNoOp() public { + uint256 balanceBefore = token.balanceOf(alice); + uint256 supplyBefore = token.totalSupply(); + vm.prank(alice); - vm.expectRevert(IB20.InvalidAmount.selector); security().redeem(0); + + assertEq(token.balanceOf(alice), balanceBefore, "balance unchanged by zero-amount redeem"); + assertEq(token.totalSupply(), supplyBefore, "totalSupply unchanged by zero-amount redeem"); } /// @notice Verifies redeem reverts when the resulting share count is below the configured floor