report

Identity-md/research/blob/main/jobs/433ff914-5d84-4202-acb5-a46578b05a7d/_identitymd/README.md

Four agents audited the code as it is at d5eeb4e, each in one area, and a judge reproduced, merged and ranked what they found, then read the code once more itself. Nothing in the code was changed or deployed.

Download the report (Markdown) · archived copy on GitHub

3 low1 info

• 1.lowraiseLevel authorises against the current office only, while unlock honours lockedBy: after setOffice the old office can unassign a Messenger it holds but can no longer level itp2e/contracts/src/MessengerNFT.sol:260

          if (msg.sender != office) revert NotOffice();

  MessengerNFT.setLocked(id, false, ...) authorises the unlock against lockedBy[id] (line 250), which is what lets a Messenger locked by an old office keep working there after the owner calls setOffice(newOffice) (AUDIT.md guarantee 9, IMD finding 8). Its paired game hook raiseLevel (line 260) was not mirrored: it authorises against the current office only.

  After a migration the Messenger is still assigned at the old office (MailOffice.assignedBy[id] == holder), the new office refuses it (setLocked(true) reverts AlreadyLocked), so the old office is the only office allowed to touch it, yet MailOffice.levelUp there reverts NotOffice() at the raiseLevel call. The holder must unassign at the old office and re-assign at the new one before any level can be bought; a pending levelUp transaction sent before the migration reverts.

  No MAIL is lost (the whole levelUp reverts, so _spend is undone) and no unauthorised actor gains anything, so this is a low: an access asymmetry between the two writers of a locked Messenger's game state, which AUDIT.md describes as one trust surface.

  Fix: authorise raiseLevel against the office that holds the lock, if (msg.sender != lockedBy[id]) revert NotOffice();. This keeps the agreed design (only an office levels, only through MailOffice.levelUp, which already requires assignedBy[id] == msg.sender). If levelling unassigned Messengers by the current office is wanted, use msg.sender != office && msg.sender != lockedBy[id] instead.

  Reported by the permissions specialist; reproduced here.

  whole finding

  State: nft.office == office1; alice opened office1 (open{value: 0.005 ether}), assigned Messenger 1 there (nft.locked(1) == true, nft.lockedBy(1) == office1, office1.assignedBy(1) == alice), approved office1 for MAIL and holds 10,000 MAIL; office1.levelUp(1, type(uint256).max) succeeded once (level 1).

  Then owner calls nft.setOffice(office2).

  Input: alice calls office1.levelUp(1, type(uint256).max).

  Expected: level 1 -> 2 (same call succeeded one transaction earlier; office2.assign(1) reverts AlreadyLocked and office1.unassign() succeeds, so office1 is the only office the NFT lets act on it).

  Actual: reverts NotOffice() from MessengerNFT.raiseLevel line 260 because msg.sender (office1) != office (office2).

  Verified: the attached test fails with NotOffice() on the current code (forge test --match-path test/scratch/Proof_5f2633d3ca08.t.sol).

  whole reproduction
  proof · a Foundry test that fails on this code and passes once it is fixed

  // SPDX-License-Identifier: MIT
  pragma solidity 0.8.26;
  
  import {Test} from "forge-std/Test.sol";
  import {MailToken} from "src/MailToken.sol";
  import {MessengerNFT} from "src/MessengerNFT.sol";
  import {MailOffice, IRewardDay} from "src/MailOffice.sol";
  import {DeliveryRewards} from "src/DeliveryRewards.sol";
  import {DevTransferValidator} from "src/DevTransferValidator.sol";
  
  /// Asymmetry between MessengerNFT.setLocked (unlock honours `lockedBy[id]`, so the office that locked a Messenger
  /// keeps working after `setOffice`) and MessengerNFT.raiseLevel (honours only the current `office`). After a
  /// migration the holder of a Messenger still locked by the old office can unassign there but cannot level it there,
  /// although the old office is the only one that may touch it (the new office reverts AlreadyLocked on assign).
  contract OfficeMigrationLevelUpTest is Test {
      uint256 constant DAILY = uint256(16_800_000e18) / 102;
      address owner = makeAddr("owner");
      address treasury = makeAddr("treasury");
      address poster = makeAddr("poster");
      address alice = makeAddr("alice");
  
      MailToken mail;
      MessengerNFT nft;
      DeliveryRewards rewards;
      MailOffice office1;
      MailOffice office2;
  
      function setUp() public {
          vm.startPrank(owner);
          mail = new MailToken(makeAddr("liquidity"), owner);
          nft = new MessengerNFT(
              owner, treasury, makeAddr("dev"), 0.01 ether, bytes32(uint256(1)), address(new DevTransferValidator()), ""
          );
          rewards = new DeliveryRewards(mail, owner, poster, 1 days, DAILY, 51);
          office1 = new MailOffice(mail, nft, IRewardDay(address(rewards)), treasury, owner);
          office2 = new MailOffice(mail, nft, IRewardDay(address(rewards)), treasury, owner);
          mail.setMinter(address(rewards));
          nft.setOffice(address(office1));
          nft.ownerMint(alice, 1);
          vm.stopPrank();
          vm.deal(alice, 1 ether);
          vm.prank(address(rewards));
          mail.mint(alice, 10_000e18);
      }
  
      function test_OldOfficeCanUnlockButNotLevelAfterMigration() public {
          vm.startPrank(alice);
          office1.open{value: 0.005 ether}(address(0));
          office1.assign(1);
          mail.approve(address(office1), type(uint256).max);
          office1.levelUp(1, type(uint256).max); // works before the migration
          vm.stopPrank();
          assertEq(nft.levelOf(1), 1);
          assertEq(nft.lockedBy(1), address(office1));
  
          vm.prank(owner);
          nft.setOffice(address(office2));
  
          // The Messenger is still locked by office1 and assigned to alice there; office2 can't take it.
          vm.startPrank(alice);
          office2.open{value: 0.005 ether}(address(0));
          vm.expectRevert(MessengerNFT.AlreadyLocked.selector);
          office2.assign(1);
  
          // Same wallet, same assignment, same approval: the level-up that worked a moment ago now reverts, while
          // the mirror operation (unassign through the same office) is still accepted by the NFT.
          office1.levelUp(1, type(uint256).max);
          vm.stopPrank();
          assertEq(nft.levelOf(1), 2, "the office holding the lock can still level its Messenger");
      }
  }

  whole proof

• 2.lowBatched claims are all-or-nothing: one already-claimed day, which anyone can trigger after maturity, reverts the account's whole claimMany/claimNowManyp2e/contracts/src/DeliveryRewards.sol:196

          for (uint256 i; i < dayList.length; ++i) claimNow(dayList[i], amounts[i], proofs[i]);

  claimMany (line 172) and claimNowMany (line 196) loop over the public single-day functions and revert as a whole if any entry fails in _settle (AlreadyClaimed at line 204, OverTotal at line 206).

  Because claim(account, ...) is open to anyone once a day has matured (lines 158-164), a third party can settle one of the days in an account's pending batch (the tokens go to the account, so this costs the griefer only gas) and the account's claimNowMany / claimMany transaction then reverts with AlreadyClaimed, delivering nothing for the other days in the batch. The same happens when the batch includes a day whose total was exhausted by earlier claimants (OverTotal).

  There is no partial-success path, so a wallet claiming several days at once must detect the failure off-chain and resubmit without the offending day, and a griefer can repeat this each time a new matured day is included; a relayer paying gas for players loses the gas of every reverted batch. No funds are lost.

  Fix: in claimMany/claimNowMany skip days whose flag claimedFor[day][days[day].root][account] is already set (continue instead of revert), keeping the single-day functions strict. Reported by the economics specialist; reproduced here.

  whole finding

  State: DeliveryRewards(dayLength 1 days, initialDaily 164_705e18, halving 51); poster posts day 0 with a two-leaf tree containing (A, 0, 100e18) and day 1 with one containing (A, 1, 100e18), both at startTime + 2 days; warp to maturesAt(1) (past both claimableFrom values).

  Inputs: (1) relayer R calls claim(A, 0, 100e18, proof0): succeeds, A receives 100e18.

  (2) A calls claimNowMany([0,1], [100e18,100e18], [proof0,proof1]).

  Expected (a batch entry point usable for several days): A is paid day 1 (balance 200e18) and day 0 is skipped as already claimed.

  Actual: the call reverts with AlreadyClaimed() from _settle on day 0; A receives nothing for day 1 in that transaction.

  Same revert with claimMany(A, [0,1], ...).

  Verified: the attached test fails with AlreadyClaimed() on the current code (forge test --match-path test/scratch/Proof_5b3226fce876.t.sol).

  whole reproduction
  proof · a Foundry test that fails on this code and passes once it is fixed

  // SPDX-License-Identifier: MIT
  pragma solidity 0.8.26;
  
  import {Test} from "forge-std/Test.sol";
  import {MailToken} from "src/MailToken.sol";
  import {DeliveryRewards} from "src/DeliveryRewards.sol";
  
  /// A third party settles one matured day of A's batch with `claim`; A's `claimNowMany` for [0,1] then reverts
  /// as a whole and pays nothing for day 1.
  contract BatchClaimTest is Test {
      MailToken mail;
      DeliveryRewards rewards;
      address owner = address(0xA11CE);
      address poster = address(0xB0B);
      address A = address(0xAAAA);
      address relayer = address(0xCAFE);
  
      function setUp() public {
          mail = new MailToken(address(0xDEAD), address(this));
          rewards = new DeliveryRewards(mail, owner, poster, 1 days, 164_705e18, 51);
          mail.setMinter(address(rewards));
      }
  
      function _leaf(address a, uint256 day, uint256 amt) internal pure returns (bytes32) {
          return keccak256(bytes.concat(keccak256(abi.encode(a, day, amt))));
      }
  
      function _hash(bytes32 x, bytes32 y) internal pure returns (bytes32) {
          return x < y ? keccak256(abi.encodePacked(x, y)) : keccak256(abi.encodePacked(y, x));
      }
  
      function test_batchClaimSurvivesAThirdPartySingleClaim() public {
          // two-leaf trees per day: A and a filler account
          bytes32 l0 = _leaf(A, 0, 100e18);
          bytes32 f0 = _leaf(address(0xF111), 0, 1e18);
          bytes32 root0 = _hash(l0, f0);
          bytes32 l1 = _leaf(A, 1, 100e18);
          bytes32 f1 = _leaf(address(0xF111), 1, 1e18);
          bytes32 root1 = _hash(l1, f1);
  
          vm.warp(rewards.startTime() + 2 days);
          vm.startPrank(poster);
          rewards.postRoot(0, root0, 101e18);
          rewards.postRoot(1, root1, 101e18);
          vm.stopPrank();
  
          vm.warp(rewards.maturesAt(1));
  
          bytes32[] memory p0 = new bytes32[](1);
          p0[0] = f0;
          bytes32[] memory p1 = new bytes32[](1);
          p1[0] = f1;
  
          // anyone settles day 0 for A
          vm.prank(relayer);
          rewards.claim(A, 0, 100e18, p0);
          assertEq(mail.balanceOf(A), 100e18);
  
          uint256[] memory dayList = new uint256[](2);
          dayList[0] = 0;
          dayList[1] = 1;
          uint256[] memory amounts = new uint256[](2);
          amounts[0] = 100e18;
          amounts[1] = 100e18;
          bytes32[][] memory proofs = new bytes32[][](2);
          proofs[0] = p0;
          proofs[1] = p1;
  
          // Expected: day 1 is paid (day 0 skipped). Actual on current code: the whole batch reverts.
          vm.prank(A);
          rewards.claimNowMany(dayList, amounts, proofs);
          assertEq(mail.balanceOf(A), 200e18, "day 1 was not paid by the batch");
      }
  }

  whole proof

• 3.lowsetFeeRecipient is one-step after the renounce: one wrong address strands every future 4% fee with no recovery pathp2e/contracts/src/MailHook.sol:355

          if (msg.sender != owner && msg.sender != feeRecipient) revert NotOwner();

  MailHook is renounced at deploy (Deploy.s.sol line 107), after which the only party able to redirect the pool's fees is the current feeRecipient, through a one-step setter (line 354-359). Ownership of the same contract is deliberately two-step ('so a typo can't lose the admin role', line 361), but the fee recipient, the only privileged role left after launch, has no such protection.

  If the recipient names an address that cannot itself call setFeeRecipient (the hook itself, a plain vault or splitter contract, a mistyped EOA whose key nobody holds), then every future swap still mints 4% into the hook's ETH claims, _collect pays only feeRecipient so collection either reverts forever (no receive) or pays an address nobody controls, and owner is address(0), so nobody can repoint it. The 4% of all future trading volume is lost or frozen permanently.

  AUDIT.md's accepted limit ('a fee recipient that can't receive ETH ... can point its fees at another address') assumes the recipient can still act, which is exactly what one wrong call removes.

  Fix that preserves the design (renounced hook, only the recipient moves its own fees): make the change two-step, pendingFeeRecipient set by the current recipient and acceptFeeRecipient callable only by the pending address, so the new recipient proves it can act before it becomes the only key to the fees. Reported by the flow specialist; reproduced here.

  whole finding

  State: PoolManager, MailHook at a mined 0x28EC address with owner, feeRecipient, 4.2M launch supply; MailToken minted to the hook; owner calls openPool(mail) then renounceOwnership() (as Deploy.s.sol does).

  1. feeRecipient calls hook.setFeeRecipient(address(hook)).

  2. alice calls router.buy{value: 1 ether}(mail, 1, block.timestamp): pendingProtocolFees(address(0)) == 0.04 ether.

  3. anyone calls hook.collectProtocolFees(address(0)).

  Expected: the fee reaches an address somebody controls.

  Actual: reverts inside PoolManager.take (the hook has no receive; NativeTransferFailed wrapped by the unlock), and the 0.04 ETH stays in the hook's claims.

  4. the old feeRecipient calls hook.setFeeRecipient(feeRecipient).

  Expected: a way back.

  Actual: reverts NotOwner(), and hook.owner() == address(0) so no one else can either.

  5. a second 1 ether buy raises pendingProtocolFees to 0.08 ether, all stranded.

  Verified with test/scratch/FeeRecipient.t.sol (passes: the stranding state is reachable) and the attached test, which fails on the current code at step 3.

  whole reproduction
  proof · a Foundry test that fails on this code and passes once it is fixed

  // SPDX-License-Identifier: MIT
  pragma solidity 0.8.26;
  
  import {Test} from "forge-std/Test.sol";
  import {PoolManager} from "v4-core/src/PoolManager.sol";
  import {MailHook} from "src/MailHook.sol";
  import {MailRouter} from "src/MailRouter.sol";
  import {MailToken} from "src/MailToken.sol";
  import {DeployLib} from "script/DeployLib.sol";
  
  /// After the renounce (done at deploy), setFeeRecipient is one-step. One call naming an address that cannot call
  /// setFeeRecipient back (here: the hook itself, which has no receive) strands every future 4% fee: collection
  /// reverts, the old recipient is no longer allowed to repoint, and owner is address(0).
  /// Fails on the current code (collectProtocolFees reverts). Passes once the change is two-step (the recipient stays
  /// `feeRecipient` until the new address accepts), because collection then still pays the controllable address.
  contract FeeRecipientProofTest is Test {
      PoolManager pm;
      MailHook hook;
      MailToken mail;
      MailRouter router;
  
      address owner = makeAddr("owner");
      address feeRecipient = makeAddr("feeRecipient");
      address alice = makeAddr("alice");
      address constant ETH = address(0);
      uint256 constant LAUNCH = 4_200_000e18;
  
      function setUp() public {
          vm.warp(1_800_000_000);
          pm = new PoolManager(address(this));
          bytes memory initCode = abi.encodePacked(
              type(MailHook).creationCode,
              abi.encode(pm, owner, feeRecipient, DeployLib.startTickForMarketCap(20 ether, 21_000_000e18), LAUNCH)
          );
          (bytes32 salt, address expected) = DeployLib.mineSalt(address(this), uint160(0x28EC), initCode, 0);
          address h;
          assembly {
              h := create2(0, add(initCode, 0x20), mload(initCode), salt)
          }
          require(h == expected, "hook address");
          hook = MailHook(h);
          router = MailRouter(payable(hook.router()));
          mail = new MailToken(address(hook), owner);
          vm.startPrank(owner);
          hook.openPool(address(mail));
          hook.renounceOwnership(); // as Deploy.s.sol does
          vm.stopPrank();
          vm.deal(alice, 10 ether);
      }
  
      function test_MistakenFeeRecipientDoesNotStrandTheFees() public {
          // The recipient names an address that can never call setFeeRecipient (the hook has no receive either).
          vm.prank(feeRecipient);
          hook.setFeeRecipient(address(hook));
  
          vm.prank(alice);
          router.buy{value: 1 ether}(address(mail), 1, block.timestamp);
          assertEq(hook.pendingProtocolFees(ETH), 0.04 ether);
  
          // Expected: the fees are still collectable to an address somebody controls.
          // Actual on the current code: reverts (the hook cannot receive ETH), and nobody can repoint the recipient.
          hook.collectProtocolFees(ETH);
          assertEq(feeRecipient.balance, 0.04 ether, "fees reached a controllable address");
      }
  }

  whole proof

• 4.infoThe one-character rule is per office contract: after setOffice a wallet can hold two locked, assigned Messengers, one in each officep2e/contracts/src/MailOffice.sol:157

          if (characterOf[msg.sender] != 0) revert HasCharacter();

  IMD re-check 206b22d4 finding 3 moved the once-per-reward-day record (lastPlayedDay, lastPlayer) onto MessengerNFT so that setOffice cannot reset it. The one-character-per-wallet rule (AUDIT.md guarantee 8: 'at most one Messenger assigned (characterOf)') was not moved: characterOf (checked at line 157) and assignedBy are still per MailOffice instance, and MessengerNFT.setLocked(true) checks nothing about the player's other locked Messengers.

  After the owner points the NFT at a new office, a wallet whose Messenger is still locked by the old office can open the new office and assign a second Messenger there. Both offices then report the wallet as playing a Messenger with full stamina, both tokens are locked, and lastPlayer of both is the same wallet.

  Whether this yields extra points depends on which office the game server reads (out of scope); on chain the guarantee as stated no longer holds across a migration, any consumer reading the old office sees two active characters for one wallet, and the holder cannot sell the first Messenger until they unassign it at the old office.

  Informational: it needs the owner's documented migration action, and the server is the economic guard. Fixes that keep the design: keep the wallet -> locked Messenger record on the NFT (mapping(address => uint256) characterOf written in setLocked, with setLocked(true) reverting when the player already holds a locked Messenger), or have MailOffice.assign refuse wallets that still have a character at the previous office.

  Reported by both the permissions and the flow specialists (merged); reproduced here.

  whole finding

  State: nft.office == office1; alice holds Messengers 1 and 2.

  1. alice: office1.open{value: 0.005 ether}(address(0)); office1.assign(1) (nft.locked(1) == true, nft.lockedBy(1) == office1).

  2. owner: nft.setOffice(address(office2)), office2 a fresh MailOffice on the same token, NFT and clock.

  3. alice: office2.open{value: 0.005 ether}(address(0)); office2.assign(2).

  Expected under guarantee 8: step 3 reverts (HasCharacter or AlreadyLocked) while the wallet still has a character.

  Actual: it succeeds; office1.characterOf(alice) == 1, office2.characterOf(alice) == 2, nft.locked(1) && nft.locked(2), nft.lastPlayer(1) == nft.lastPlayer(2) == alice, office1.staminaOf(alice) == 12 and office2.staminaOf(alice) == 12 (test/scratch/TwoCharacters.t.sol::test_StateAfterSecondAssign passes).

  The attached test, which expects the second assign to revert, fails on the current code.

  whole reproduction
  proof · a Foundry test that fails on this code and passes once it is fixed

  // SPDX-License-Identifier: MIT
  pragma solidity 0.8.26;
  
  import {Test} from "forge-std/Test.sol";
  import {MailToken} from "src/MailToken.sol";
  import {MessengerNFT} from "src/MessengerNFT.sol";
  import {MailOffice, IRewardDay} from "src/MailOffice.sol";
  import {DeliveryRewards} from "src/DeliveryRewards.sol";
  import {DevTransferValidator} from "src/DevTransferValidator.sol";
  
  /// The one-character-per-wallet rule lives in each MailOffice (characterOf), while the lock lives on the NFT.
  /// After setOffice, a wallet whose Messenger is still locked by the old office can assign a second one at the new
  /// office: two locked, assigned Messengers for one wallet. Fails on the current code (the second assign succeeds);
  /// passes once the NFT (or the new office) refuses a wallet that already plays a locked Messenger.
  contract TwoCharactersProofTest is Test {
      uint256 constant DAILY = uint256(16_800_000e18) / 102;
      address owner = makeAddr("owner");
      address treasury = makeAddr("treasury");
      address poster = makeAddr("poster");
      address alice = makeAddr("alice");
  
      MailToken mail;
      MessengerNFT nft;
      DeliveryRewards rewards;
      MailOffice office1;
      MailOffice office2;
  
      function setUp() public {
          vm.startPrank(owner);
          mail = new MailToken(makeAddr("liquidity"), owner);
          nft = new MessengerNFT(
              owner, treasury, makeAddr("dev"), 0.01 ether, bytes32(uint256(1)), address(new DevTransferValidator()), ""
          );
          rewards = new DeliveryRewards(mail, owner, poster, 1 days, DAILY, 51);
          office1 = new MailOffice(mail, nft, IRewardDay(address(rewards)), treasury, owner);
          office2 = new MailOffice(mail, nft, IRewardDay(address(rewards)), treasury, owner);
          mail.setMinter(address(rewards));
          nft.setOffice(address(office1));
          nft.ownerMint(alice, 2);
          vm.stopPrank();
          vm.deal(alice, 1 ether);
      }
  
      function test_OneWalletCannotPlayTwoMessengersAcrossOffices() public {
          vm.startPrank(alice);
          office1.open{value: 0.005 ether}(address(0));
          office1.assign(1);
          vm.stopPrank();
          assertTrue(nft.locked(1));
  
          vm.prank(owner);
          nft.setOffice(address(office2));
  
          vm.startPrank(alice);
          office2.open{value: 0.005 ether}(address(0));
          // Expected under guarantee 8 (one character per wallet): refused while Messenger 1 is still alice's character.
          // Actual on the current code: succeeds; both offices report alice playing, both tokens are locked.
          vm.expectRevert();
          office2.assign(2);
          vm.stopPrank();
      }
  }

  whole proof

1. Posted13 minto the first attempt
2. Audit mathAgent #999found 2 info

   The review is complete and .imd-findings.json holds the result. The working tree is clean apart from that file.

   Outcome. The math in the eight contracts under p2e/contracts/src holds up against the Math Precision, Boundary and Numerical Gap guides. No critical, high or medium defect was found, so no proof tests are attached. Two informational items are recorded.

   What was checked, with evidence

   • MailHook fee arithmetic in all four swap modes (exact-in and exact-out, buy and sell). A 400-run fuzz per mode confirmed the fee equals 4% of the ETH side rounded up, the buyer pays exactly the requested ETH, and exact-out sells deliver exactly the requested ETH. The partial-fill guard, the dust floor and the sell-past-launch-price path behaved as documented.
   • Launch liquidity bounds. The liquidity formula at the extreme allowed ticks and supplies stays positive and under the per-tick cap, as the constructor comment claims. The tick rounding in DeployLib.startTickForMarketCap lands the market cap within the stated 2% above target.
   • DeliveryRewards schedule and windows. Halving budget, allocation cap, claim-open, revoke and maturity boundaries are mutually exclusive with no gap or overlap at the exact timestamps. Minted amounts cannot exceed the token cap.
   • MailOffice, Wardrobe, MessengerNFT. Burn rounding, tier and level indexing, narrow casts and payment checks have no overflow, underflow or zero-division path reachable by an unprivileged caller.

   Findings recorded (both info)

   1. Early-claim burn rounds down. DeliveryRewards.claimNow truncates the 20% burn, unlike the office and wardrobe burns that round up. Amount 4 wei burns 0. Impact is under 1 wei per claim and cannot be repeated.
   2. Trust assumption on reveal. The owner holds the secret and picks the reveal block, so the blockhash adds no entropy against them. This is outside the math area and is reported as a documented owner power, not an exploit.

   Limits. No fork test was run. Dependencies were installed in a temporary directory outside the repository and nothing in the tree was changed except the findings file.

   ran onclaude · claude-fable-5-1 · 34 turns · 12m 22s · 514 in · 45.4K out · 1.6M cached

   submission839dde803688cb831f7ca50141652aa86d75451d90df4afd4b5fa559b568d296

   device99c6d0bcc495ad613a6a5093465f2cc2d3ac6a53d90273d31b81cfc62f92c524

   started fromd5eeb4e003f05ce93d19e4eb2d3e46c6d97a19b0

   bundlenone

   • infoEarly-claim burn rounds down, so the 20% penalty truncates to 0 on tiny allocations (opposite direction to every other burn in the system)p2e/contracts/src/DeliveryRewards.sol:184

     Math Precision guide, 'fees round UP' / 'zero-rounding at minimum inputs'. claimNow() is the one place a penalty is computed and it truncates: burned = amount * 2000 / 10_000 rounds toward the player.

     MailOffice._spend (line 281) and Wardrobe.buy (line 60) deliberately round their 75% burn up ((x*7500+9_999)/10_000, 'so the burn is never below 75%', IMD audit finding 6); the early-claim burn was not given the same treatment, so for any amount < 5 wei the 20% burn is exactly 0 and the player is paid 100% early, and for every other amount the player keeps up to 1 wei that should have burned.

     Amounts are fixed by the poster's Merkle tree and each (day, account) claims once, so this cannot be farmed by repetition: impact is dust (< 1 wei per claim, non-compounding). Reported for consistency with the documented rounding policy rather than as a loss.

     Fix: burned = (amount * EARLY_BURN_BPS + 9_999) / 10_000, which keeps amount - burned >= 0 for all amount >= 1 (amount = 1 gives burned = 1, paid = 0, matching the office's behaviour for a 1-wei spend).

     whole finding

     State: poster posts day 0 with a tree containing leaf(alice, 0, 4) and total 4; warp past postedAt + dayLength but before maturesAt(0). alice calls claimNow(0, 4, proof).

     Expected (20% burned, rounded like the other burns): burned = 1, alice minted 3.

     Actual: burned = 4 * 2000 / 10_000 = 0, alice minted 4, ClaimedEarly(0, alice, 4, 0).

     Same with amount = 9: expected burn 2, actual 1.

   • infoTrust assumption: the reveal seed is grindable by the owner, who holds the secret and chooses the reveal block (blockhash adds no entropy against them)p2e/contracts/src/MessengerNFT.sol:193

     Outside the assigned math area; recorded as a privileged-power trust assumption, not an exploit by an unprivileged party. The commit/reveal scheme protects buyers from the owner changing the secret, and the 'later blockhash' is meant to stop the owner from knowing the seed when the sale closes.

     But blockhash(block.number - 1) is already fixed when the owner submits reveal(), and the owner may submit in any block after saleClosedAt: each new block yields a different candidate seed = keccak(secret, prevhash) that the owner can evaluate off-chain against traitsOf() for the token ids they (or an ally) minted during the sale, and simply not send the transaction until a block whose seed gives those ids rare 'Golden Letter' / preferred zone traits.

     On an Arbitrum-style chain (the deploy target) block.number is the L1 block estimate and blockhash() is a pseudo-random hash tied to it, so candidate seeds change roughly every L1 block; the owner has unlimited attempts. ownerMint after the close is already blocked (re-check finding 5), which closes the 'mint after knowing traits' path, but it does not close 'pick the seed for tokens already held'.

     This is only exploitable by the owner, so it is a trust assumption to document for buyers, not a defect in the requested design. If the requester wants to remove it: commit the reveal to a specific future block (store targetBlock = block.number + N at close, require block.number > targetBlock and use blockhash(targetBlock), with a fallback if it falls out of the 256-block window), or use a VRF.

     whole finding

     State: sale closed at block B; owner wallet minted ids 1..10 during the sale.

     Loop off-chain over successive blocks b > B: candidate = keccak256(abi.encode(secret, blockhash(b-1))) | 1; compute traitsOf-style rolls for ids 1..10; send reveal(secret) only in a block where e.g. (uint256(keccak256(abi.encode(candidate, 1))) >> 64) % 1000 >= 980 (carry = Golden Letter, nominally 2%).

     Expected per the fair-distribution intent: 2% chance for id 1.

     Actual: the owner reaches it with certainty after ~50 blocks on average.

     In a Foundry test: vm.roll forward one block at a time, compute the candidate seed from blockhash(block.number-1) with the committed secret, and call reveal() only when the chosen id would get carry == 4; traitsOf(1).carry then equals 4 deterministically.

     whole reproduction
3. Audit permissionsAgent #1176found 1 low, 1 info

   The review is complete and the findings file is in place at the repository root.

   Scope covered. I read every contract in p2e/contracts/src, both deploy scripts, and the existing test suites, then walked the full permission model: every state-changing entry point, its guard, the roles handed out in constructors and the deploy script, the hook and unlock callbacks, and the Ownable2Step paths. I verified against the vendored libraries that only the hook can initialize or add liquidity to a pool on itself, that the pool manager only calls back its own caller (so neither router nor hook callbacks can be fed outside data), and that two-step ownership cannot zero an owner. The existing 90 tests pass on the installed dependencies.

   Result: two findings, no critical or high. The access-control surface is tight after the four prior audit rounds. What remains are two asymmetries in the one area the earlier fixes touched repeatedly, office migration:

   • Low, MessengerNFT.raiseLevel. Unlocking honours lockedBy, so an old office keeps working after setOffice, but levelling honours only the current office. After a migration the old office is the only one the NFT lets act on a still-locked Messenger, yet levelUp there reverts with NotOffice. The holder must unassign and re-assign before buying any level. No funds are lost. The scratch test fails on the current code and passes with a one-line fix that checks lockedBy[id], which also narrows the documented "office can level any Messenger" power.
   • Info, MailOffice.characterOf. The once-per-day play record was moved onto the NFT so migration cannot reset it, but the one-character record stayed per office. After setOffice, a wallet can hold two locked Messengers, one assigned in each office, and both offices report full stamina for it. Whether this earns extra points depends on which office the server reads, which is out of scope.

   Things I checked and found clean, so the requester does not need to revisit them: relayer claims, pause and revoke windows, the minter one-shot, confused-deputy paths through the approvals held by the office, wardrobe and router, fee collection inside someone else's unlock, admin setters that could redirect in-flight value, and the sale-close and reveal ordering. Fork and Slither checks did not run, and the live OpenSea validator's policy for an unconfigured collection is not verifiable offline, so that remains a deployment item.

   The proof test lives under test/scratch/, which is not kept. Nothing else in the tree was changed.

   ran onclaude · claude-fable-5-1 · 37 turns · 13m 13s · 578 in · 46.2K out · 2.1M cached

   submissionf52f2276f0ccb4f1473166661149d7f0f483d62847042c0da7c45f92a07607ef

   device707624586655cf74d5de4fda43bf023a65c7c04fc91591b489b99cd88cf6bf83

   started fromd5eeb4e003f05ce93d19e4eb2d3e46c6d97a19b0

   bundlenone

   • lowraiseLevel honours only the current office while setLocked's unlock honours lockedBy: after setOffice, the office that still holds a Messenger can unassign it but can no longer level itp2e/contracts/src/MessengerNFT.sol:260

     MessengerNFT.setLocked(id, false, ...) authorises the unlock against lockedBy[id] (line 250), so a Messenger locked by an old office keeps working there after the owner calls setOffice(newOffice) (IMD audit finding 8 / re-check finding 8). Its paired game hook raiseLevel (line 260) was not mirrored: it authorises against the current office only. The two writers of a locked Messenger's game state therefore disagree about who may act on it.

     After a migration the Messenger is still assigned in the old office (MailOffice.assignedBy[id] == holder) and the new office refuses it (setLocked(true) reverts AlreadyLocked), so the only office allowed to touch it is the old one, yet MailOffice.levelUp there reverts NotOffice at the raiseLevel call.

     The holder has to unassign from the old office and re-assign at the new one (allowed the same day only because lastPlayer is the same wallet) before any level can be bought; a pending levelUp transaction sent before the migration reverts.

     No MAIL is lost (the whole levelUp reverts, so _spend is undone), and no unauthorised actor gains anything, so this is a low: an access asymmetry between two functions that are documented as the same trust surface ("only the office that locked a Messenger can unlock it"), and the same class of defect as the fixed finding 8.

     Fix: authorise raiseLevel against the office that holds the lock, if (msg.sender != lockedBy[id]) revert NotOffice();. This keeps the agreed design (only an office levels, only through MailOffice.levelUp, which already requires assignedBy[id] == msg.sender) and additionally narrows the documented office power so a (new or malicious) office can no longer level a Messenger it does not hold, which AUDIT.md currently lists as a trust assumption.

     If levelling unassigned Messengers by the current office is wanted, use msg.sender != office && msg.sender != lockedBy[id] instead.

     whole finding

     State: nft.office == office1; alice opened office1, assigned Messenger 1 there (nft.locked(1) == true, nft.lockedBy(1) == office1, office1.assignedBy(1) == alice), approved office1 for MAIL and holds 10,000 MAIL.

     Then owner calls nft.setOffice(office2).

     Input: alice calls office1.levelUp(1, type(uint256).max).

     Expected: level 1 -> 2 (the same call succeeded one transaction earlier, the Messenger is still assigned at office1, and office1 is the only office the NFT lets act on it: office2.assign(1) reverts AlreadyLocked, office1.unassign() succeeds).

     Actual: reverts NotOffice() from MessengerNFT.raiseLevel line 260 because msg.sender (office1) != office (office2).

     The scratch test test/scratch/OfficeMigrationLevelUp.t.sol fails with NotOffice() on the current code and passes once raiseLevel checks lockedBy[id].

     whole reproduction
     proof · a Foundry test the fix has to pass

     // SPDX-License-Identifier: MIT
     pragma solidity 0.8.26;
     
     import {Test} from "forge-std/Test.sol";
     import {MailToken} from "src/MailToken.sol";
     import {MessengerNFT} from "src/MessengerNFT.sol";
     import {MailOffice, IRewardDay} from "src/MailOffice.sol";
     import {DeliveryRewards} from "src/DeliveryRewards.sol";
     import {DevTransferValidator} from "src/DevTransferValidator.sol";
     
     /// Asymmetry between MessengerNFT.setLocked (unlock honours `lockedBy[id]`, so the office that locked a Messenger
     /// keeps working after `setOffice`) and MessengerNFT.raiseLevel (honours only the current `office`). After a
     /// migration the holder of a Messenger still locked by the old office can unassign there but cannot level it there,
     /// although the old office is the only one that may touch it (the new office reverts AlreadyLocked on assign).
     contract OfficeMigrationLevelUpTest is Test {
         uint256 constant DAILY = uint256(16_800_000e18) / 102;
         address owner = makeAddr("owner");
         address treasury = makeAddr("treasury");
         address poster = makeAddr("poster");
         address alice = makeAddr("alice");
     
         MailToken mail;
         MessengerNFT nft;
         DeliveryRewards rewards;
         MailOffice office1;
         MailOffice office2;
     
         function setUp() public {
             vm.startPrank(owner);
             mail = new MailToken(makeAddr("liquidity"), owner);
             nft = new MessengerNFT(
                 owner, treasury, makeAddr("dev"), 0.01 ether, bytes32(uint256(1)), address(new DevTransferValidator()), ""
             );
             rewards = new DeliveryRewards(mail, owner, poster, 1 days, DAILY, 51);
             office1 = new MailOffice(mail, nft, IRewardDay(address(rewards)), treasury, owner);
             office2 = new MailOffice(mail, nft, IRewardDay(address(rewards)), treasury, owner);
             mail.setMinter(address(rewards));
             nft.setOffice(address(office1));
             nft.ownerMint(alice, 1);
             vm.stopPrank();
             vm.deal(alice, 1 ether);
             vm.prank(address(rewards));
             mail.mint(alice, 10_000e18);
         }
     
         function test_OldOfficeCanUnlockButNotLevelAfterMigration() public {
             vm.startPrank(alice);
             office1.open{value: 0.005 ether}(address(0));
             office1.assign(1);
             mail.approve(address(office1), type(uint256).max);
             office1.levelUp(1, type(uint256).max); // works before the migration
             vm.stopPrank();
             assertEq(nft.levelOf(1), 1);
             assertEq(nft.lockedBy(1), address(office1));
     
             vm.prank(owner);
             nft.setOffice(address(office2));
     
             // The Messenger is still locked by office1 and assigned to alice there; office2 can't take it.
             vm.startPrank(alice);
             office2.open{value: 0.005 ether}(address(0));
             vm.expectRevert(MessengerNFT.AlreadyLocked.selector);
             office2.assign(1);
     
             // Same wallet, same assignment, same approval: the level-up that worked a moment ago now reverts, while
             // the mirror operation (unassign through the same office) is still accepted by the NFT.
             office1.levelUp(1, type(uint256).max);
             vm.stopPrank();
             assertEq(nft.levelOf(1), 2, "the office holding the lock can still level its Messenger");
         }
     }

     whole proof

   • infoThe one-character record (characterOf) stays per office while the play record moved to the NFT, so after setOffice a wallet can hold two locked Messengers, one assigned in each officep2e/contracts/src/MailOffice.sol:54

     IMD re-check 206b22d4 finding 3 moved the once-per-reward-day record (lastPlayedDay, lastPlayer) from the office onto MessengerNFT precisely so that setOffice cannot reset it. The one-character-per-wallet rule (AUDIT.md guarantee 8: "at most one Messenger assigned (characterOf)") was not moved: characterOf and assignedBy are still per MailOffice instance, and MessengerNFT.setLocked(true) checks nothing about the player's other locked Messengers.

     After the owner points the NFT at a new office, a wallet whose Messenger is still locked by the old office can open the new office and assign a second Messenger there. Both offices then report that wallet as playing a Messenger with full stamina (office1.staminaOf(alice) == 12 and office2.staminaOf(alice) == 12), both tokens are locked, and lastPlayer of both is the same wallet.

     Whether this yields extra points depends on which office(s) the game server reads, which is out of scope; on chain the guarantee as stated (one character per wallet) no longer holds across a migration, and the holder also loses the ability to sell the first Messenger until they unassign it at the old office.

     Reported as informational: it needs the owner's documented migration action, and the server-side split is the economic guard.

     Fix options that preserve the design: keep the wallet -> locked Messenger record on the NFT (e.g. mapping(address => uint256) characterOf written in setLocked, with setLocked(true) reverting HasCharacter when the player already holds a locked Messenger), or have MailOffice.assign refuse wallets that still have a character at the previous office (the NFT can expose the old office via lockedBy).

     whole finding

     State: nft.office == office1; alice holds Messengers 1 and 2; alice calls office1.open{value: 0.005 ether}(0) and office1.assign(1) (nft.locked(1) == true, lockedBy(1) == office1).

     Owner calls nft.setOffice(office2).

     Input: alice calls office2.open{value: 0.005 ether}(0) then office2.assign(2).

     Expected under guarantee 8: the second assignment is refused while the wallet still has a character (HasCharacter), or the first is released.

     Actual: both succeed; office1.characterOf(alice) == 1, office2.characterOf(alice) == 2, nft.locked(1) && nft.locked(2), nft.lastPlayer(1) == nft.lastPlayer(2) == alice, office1.staminaOf(alice) == 12 and office2.staminaOf(alice) == 12.

     Verified with test/scratch/TwoCharacters.t.sol (passes, i.e. the state above is reachable).

     whole reproduction
4. Audit flowAgent #415found 1 low, 1 info

   The review is complete and the findings file is in place at the repository root, valid JSON with two substantiated findings.

   Outcome. No critical, high or medium defects. The ten in-scope contracts hold the guarantees stated in the audit brief under every failing input I could construct, including the Uniswap v4 fee accounting in all four swap modes, partial-fill and dust rejection, the locked launch liquidity, the Merkle claim windows, the once-per-reward-day Messenger rule, the 75% burn, and the supply cap. The full suite passes (90 tests), and the fork test, which the suite normally skips, also passes against live Robinhood Chain. All four hardcoded chain addresses (PoolManager, Chainlink ETH/USD, OpenSea validator, CREATE2 factory) have code on chain 4663 and the feed is fresh.

   Findings recorded

   • Low, MailHook.setFeeRecipient (line 355). After the deploy-time renounce, the fee recipient is the only remaining privileged role, and its setter is one-step. Pointing it at any address that cannot call back (the hook itself, a passive vault, a typo) strands every future 4% fee forever, since _collect pays only feeRecipient and owner is zero. Ownership on the same contract is two-step for exactly this reason. Proven by test/scratch/FeeRecipient.t.sol. Suggested fix: pending-and-accept for the recipient.
   • Info, MailOffice.assign (line 157). The one-character rule lives in each office's own mapping. After an office migration a wallet can keep Messenger A locked under the old office and assign Messenger B under the new one, so both report stamina for the same wallet. The server reads one office, so no double earning follows from the contracts alone. Proven by test/scratch/Probe.t.sol.

   Edges probed that behaved correctly and are not covered by the existing suite: an exact-out sell asking for more ETH than the pool holds reverts without charging a fee, and an exact-out buy from a price stuck above the launch tick fills and pays exactly 4%. A late root post uses the historic day's budget, which matches the documented trust in the poster.

   Not covered. Slither did not run (not provided). The game server and client are out of scope, as is the OpenSea validator's own policy configuration for this collection, which the owner must still set on chain. Nothing in the repository was changed; the only new files are under test/scratch/, which is discarded.

   ran onclaude · claude-fable-5-1 · 36 turns · 15m 25s · 450 in · 55.1K out · 1.9M cached

   submission5220ecb342409ed1036d6fc54ddc1f0bdb377ebbca09e3bf02c335adf2cb37e3

   devicef9a12c57a094ca8d32b656994c73e3726e389499d3029a76e3d2aa941e900d16

   started fromd5eeb4e003f05ce93d19e4eb2d3e46c6d97a19b0

   bundlenone

   • lowsetFeeRecipient is one-step after renounce: a single wrong address strands every future 4% fee with no recovery pathp2e/contracts/src/MailHook.sol:355

     MailHook is renounced at deploy (Deploy.s.sol line 107), after which the only party able to redirect the pool's fees is the current feeRecipient, through a one-step setter. Ownership of the same contract is deliberately two-step ('so a typo can't lose the admin role', line 361), but the fee recipient, which is the only privileged role left after launch, has no such protection.

     If the recipient points the fees at any address that cannot itself call setFeeRecipient (the hook itself, a plain vault or splitter contract, a mistyped EOA whose key nobody holds), then (1) every future swap still mints 4% into the hook's ETH claims, (2) _collect pays only feeRecipient, so collection either reverts forever (recipient has no receive) or pays an address nobody controls, and (3) owner is address(0), so nobody can repoint it.

     The 4% of all future trading volume is lost or frozen permanently. This contradicts guarantee 19 as the team intends it (only feeRecipient 'itself can move where its fees go': after one wrong move, nobody can). Fix that preserves the design: make the change two-step (pendingFeeRecipient + acceptFeeRecipient, callable only by the pending address), so the new recipient proves it can act before it becomes the only key to the fees.

     whole finding

     State: pool opened and hook renounced (as Deploy.s.sol does).

     1. feeRecipient calls hook.setFeeRecipient(address(hook)) (or any contract without a path to call setFeeRecipient).

     2. alice calls router.buy{value: 1 ether}(mail, 1, deadline): pendingProtocolFees(ETH) == 0.04 ether.

     3. anyone calls hook.collectProtocolFees(address(0)): expected the fee to reach a controllable address; actual: revert (the hook has no receive) and the 0.04 ETH stays in the hook's claims.

     4. the old feeRecipient calls hook.setFeeRecipient(oldRecipient): expected a way back; actual: revert NotOwner, and hook.owner() == address(0) so no one else can either.

     Every later swap adds to the stranded balance.

     Scratch test test/scratch/FeeRecipient.t.sol (test_OneStepFeeRecipientChangeAfterRenounceIsIrreversible, test_PassiveContractRecipientCannotBeUndone) passes on this code, i.e. the stranding happens.

     whole reproduction

   • infoAfter setOffice, one wallet can hold two assigned, locked Messengers (one per office), so the one-character rule holds only per office contractp2e/contracts/src/MailOffice.sol:157

     The one-character rule (guarantee 8: 'at most one Messenger assigned (characterOf)') is enforced from the office's own characterOf mapping. MessengerNFT keeps the lock, lockedBy and play record per token, but nothing per wallet.

     When the owner migrates to a new office with setOffice (an anticipated path: lockedBy exists exactly so that migrations don't strand Messengers), a wallet that still has Messenger A locked by the old office can open the new office and assign Messenger B there. Both tokens are locked, both offices report staminaOf(wallet) == 12 and characterOf(wallet) != 0.

     The game server only reads one office, so no double earning follows from the contracts alone, but the on-chain statement 'a wallet never plays more than one character' is false during and after a migration, and any consumer that reads the old office (the public ledger tooling, a re-check) sees two active characters for one wallet.

     Minimal fix that keeps the design: track the wallet's active character on MessengerNFT (e.g. mapping(address => uint256) characterOf written in setLocked) and have setLocked(true) revert when the player already has a locked Messenger, whichever office locked it; or require the new office's assign to see no Messenger locked for msg.sender.

     whole finding

     State: NFT office = office1; alice owns tokens 1 and 2.

     1. alice: office1.open{0.005 ether}(0); office1.assign(1).

     2. owner: nft.setOffice(address(office2)) where office2 is a fresh MailOffice on the same token, NFT and clock.

     3. alice: office2.open{0.005 ether}(0); office2.assign(2).

     Expected: step 3 reverts (HasCharacter or AlreadyLocked), since alice already plays Messenger 1.

     Actual: it succeeds; office1.characterOf(alice) == 1, office2.characterOf(alice) == 2, nft.locked(1) && nft.locked(2), office1.staminaOf(alice) == office2.staminaOf(alice) == 12.

     Scratch test test/scratch/Probe.t.sol::test_TwoCharactersAcrossOfficeMigration passes on this code.

     whole reproduction
5. Audit economicsAgent #326found 1 low, 1 info

   The review is complete and the findings file is written.

   Outcome: no critical or high defects. The economics of this tree hold up against the Economic Security, Invariant and Flow Gap checks: the 21M cap, the 16.8M play allocation, per-day budgets, per-(day, root) claim flags, the 24-hour revoke window, the 80/20 early-claim burn, the 75% spend burn, the one-Messenger-per-wallet-per-reward-day rule, and the 4% ETH-side fee in every swap mode (exact-in and exact-out, buy and sell, including partial-fill rejection) all traced correctly from caller to effect. The earlier four audit rounds' fixes are present as described.

   Findings written to .imd-findings.json:

   • Low, DeliveryRewards batched claims are all-or-nothing. After a day matures anyone may call the single-day claim for an account. A third party can settle one day of an account's pending batch, after which the account's claimNowMany or claimMany reverts with AlreadyClaimed and pays nothing for the other days. A scratch Foundry test under test/scratch/BatchClaim.t.sol reproduces it and is attached as proof. It fails on the current code and passes once already-claimed days are skipped.
   • Info, stray MAIL is unrecoverable. MAIL transferred directly to DeliveryRewards, MailOffice or Wardrobe is locked forever and not burned. The "never holds MAIL" guarantee describes the contracts' own behaviour, not a state anyone can rely on.

   Coverage and limits:

   • All ten in-scope files read in full, with the deploy scripts and the server's stamina metering consulted to confirm that Messenger rotation within one wallet gives no on-chain or off-chain gain.
   • The full test suite could not be run here. Compiling the hook tests with via_ir exceeded the ten-minute command limit, so hook behaviour was verified by tracing against the pinned v4-core source rather than by execution. Source contracts compiled cleanly and the scratch test ran.
   • Owner powers (fee retuning, office price, validator replacement, reveal block choice, poster trust) match the documented trust assumptions and are not reported as defects.

   ran onclaude · claude-fable-5-1 · 32 turns · 38m 47s · 550 in · 38.1K out · 1.7M cached

   submission6f08f521a08e1a026fc7176fcced0bdf384a50963dbdd3e3d496d3d55da3e464

   deviced2d0460994f470140c4a2ff8a5cc8719ff4a9a3c9f7beea3edd00ca492c69963

   started fromd5eeb4e003f05ce93d19e4eb2d3e46c6d97a19b0

   bundlenone

   • lowBatched claims are all-or-nothing: one already-claimed day (which anyone can trigger after maturity) reverts the account's whole claimMany/claimNowManyp2e/contracts/src/DeliveryRewards.sol:196

     claimMany (line 172) and claimNowMany (line 196) loop over the public single-day functions and revert as a whole if any entry fails in _settle (AlreadyClaimed at line 204, OverTotal at line 206).

     Because claim(account, ...) is open to anyone once a day has matured (line 158-164), a third party can settle one of the days in an account's pending batch (the tokens go to the account, so this costs the griefer only gas) and the account's claimNowMany / claimMany transaction then reverts with AlreadyClaimed, delivering nothing for the other days in the batch.

     The same happens when the batch mixes a day whose total was exhausted by earlier claimants (OverTotal): the valid days in the batch are not paid. There is no partial-success path (skip-already-claimed or return per-day results), so a wallet claiming several days at once has to detect the failure off-chain and resubmit without the offending day; a griefer can repeat this each time a new matured day is included.

     No funds are lost, but the batched claim entry points cannot be relied on, and a relayer service paying gas for players loses the gas of every reverted batch.

     Fix: in claimMany/claimNowMany skip days whose flag _claimedFor[day][root][account] is already set (continue instead of revert) or expose a per-day try pattern, keeping the single-day functions strict.

     whole finding

     State: DeliveryRewards deployed with dayLength 86400; poster posts for day 0 a root containing leaves (A, 0, 100e18) and for day 1 a root containing (A, 1, 100e18); warp to maturesAt(1) (startTime + 9 days, past both claimableFrom values).

     Inputs: (1) any address R calls claim(A, 0, 100e18, proof0) -> succeeds, A receives 100e18.

     (2) A calls claimNowMany([0,1], [100e18,100e18], [proof0,proof1]).

     Expected (a batch entry point usable for several days): A is paid day 1 (100e18) and day 0 is skipped as already claimed.

     Actual: the call reverts with AlreadyClaimed() from _settle on day 0; A receives nothing for day 1 in that transaction and must resubmit with [1] only.

     Same revert with claimMany(A, [0,1], ...).

     whole reproduction
     proof · a Foundry test the fix has to pass

     // SPDX-License-Identifier: MIT
     pragma solidity 0.8.26;
     
     import {Test} from "forge-std/Test.sol";
     import {MailToken} from "src/MailToken.sol";
     import {DeliveryRewards} from "src/DeliveryRewards.sol";
     
     /// A third party settles one matured day of A's batch with `claim`; A's `claimNowMany` for [0,1] then reverts
     /// as a whole and pays nothing for day 1.
     contract BatchClaimTest is Test {
         MailToken mail;
         DeliveryRewards rewards;
         address owner = address(0xA11CE);
         address poster = address(0xB0B);
         address A = address(0xAAAA);
         address relayer = address(0xCAFE);
     
         function setUp() public {
             mail = new MailToken(address(0xDEAD), address(this));
             rewards = new DeliveryRewards(mail, owner, poster, 1 days, 164_705e18, 51);
             mail.setMinter(address(rewards));
         }
     
         function _leaf(address a, uint256 day, uint256 amt) internal pure returns (bytes32) {
             return keccak256(bytes.concat(keccak256(abi.encode(a, day, amt))));
         }
     
         function _hash(bytes32 x, bytes32 y) internal pure returns (bytes32) {
             return x < y ? keccak256(abi.encodePacked(x, y)) : keccak256(abi.encodePacked(y, x));
         }
     
         function test_batchClaimSurvivesAThirdPartySingleClaim() public {
             // two-leaf trees per day: A and a filler account
             bytes32 l0 = _leaf(A, 0, 100e18);
             bytes32 f0 = _leaf(address(0xF111), 0, 1e18);
             bytes32 root0 = _hash(l0, f0);
             bytes32 l1 = _leaf(A, 1, 100e18);
             bytes32 f1 = _leaf(address(0xF111), 1, 1e18);
             bytes32 root1 = _hash(l1, f1);
     
             vm.warp(rewards.startTime() + 2 days);
             vm.startPrank(poster);
             rewards.postRoot(0, root0, 101e18);
             rewards.postRoot(1, root1, 101e18);
             vm.stopPrank();
     
             vm.warp(rewards.maturesAt(1));
     
             bytes32[] memory p0 = new bytes32[](1);
             p0[0] = f0;
             bytes32[] memory p1 = new bytes32[](1);
             p1[0] = f1;
     
             // anyone settles day 0 for A
             vm.prank(relayer);
             rewards.claim(A, 0, 100e18, p0);
             assertEq(mail.balanceOf(A), 100e18);
     
             uint256[] memory dayList = new uint256[](2);
             dayList[0] = 0;
             dayList[1] = 1;
             uint256[] memory amounts = new uint256[](2);
             amounts[0] = 100e18;
             amounts[1] = 100e18;
             bytes32[][] memory proofs = new bytes32[][](2);
             proofs[0] = p0;
             proofs[1] = p1;
     
             // Expected: day 1 is paid (day 0 skipped). Actual on current code: the whole batch reverts.
             vm.prank(A);
             rewards.claimNowMany(dayList, amounts, proofs);
             assertEq(mail.balanceOf(A), 200e18, "day 1 was not paid by the batch");
         }
     }

     whole proof

   • infoMAIL transferred directly to DeliveryRewards, MailOffice or Wardrobe is unrecoverable (the 'never holds MAIL' invariant is breakable by anyone and there is no sweep)p2e/contracts/src/DeliveryRewards.sol:187

     The contracts only ever hold MAIL transiently (the 20% early-claim burn at lines 187-188 mints to the contract and burns it in the same call). None of DeliveryRewards, MailOffice or Wardrobe has a function that moves MAIL out of its own balance, so any MAIL sent to them by a plain ERC20 transfer (a user pasting the wrong address, or an integration that 'pays' the office by transfer instead of approve) is locked forever.

     It is also not burned, so it stays in totalSupply while being unusable. The AUDIT.md guarantee 6 and 11 ('the contract never holds MAIL', 'the office never holds MAIL') describe the contracts' own behaviour, not a state anyone can rely on, since any holder can break it with one transfer.

     Fix: either document that stray MAIL is lost, or add an owner-only sweep that can only move MAIL (not ETH owed to the treasury) to the treasury or burn it; for DeliveryRewards a sweep that burns is consistent with 'burns are permanent'.

     whole finding

     State: any deployed MailToken + DeliveryRewards.

     Input: a holder H with 1e18 MAIL calls mail.transfer(address(rewards), 1e18).

     Expected: either the transfer is refused or the owner can recover/burn it.

     Actual: mail.balanceOf(rewards) == 1e18 permanently; no function on DeliveryRewards (nor MailOffice/Wardrobe for the same input) can move or burn it, and no owner power reaches it.

6. Audit judgeAgent #531found 3 low, 1 info

   Done. The findings file at .imd-findings.json holds four reproduced findings, each with a Foundry test that fails on the current code.

   Kept (all reproduced locally after installing the gitignored libraries per the README; the existing 90-test suite passes):

   • Low, MessengerNFT.sol:260. raiseLevel checks the current office only, while unlock checks lockedBy. After setOffice, the old office can unassign a Messenger it holds but levelUp there reverts with NotOffice. The specialist's proof fails with that error on this code, so I attached it.
   • Low, DeliveryRewards.sol:196. Batched claims are all-or-nothing. Anyone can claim one matured day for an account, and that account's claimNowMany or claimMany covering that day then reverts with AlreadyClaimed and pays nothing for the other days. The specialist's proof fails as stated, attached.
   • Low, MailHook.sol:355. After the deploy-time renounce, setFeeRecipient is one-step. A recipient that names an address that cannot call back, such as the hook itself, strands all future 4% fees. I confirmed collection reverts inside the pool manager, the old recipient gets NotOwner, and owner is zero. The specialist's test only showed the state was reachable, so I wrote a proof that fails on this code and would pass with a two-step handover.
   • Info, MailOffice.sol:157. The one-character rule lives in each office contract. After setOffice, one wallet assigns a second Messenger at the new office while the first stays locked at the old one. Both offices report full stamina for the wallet. The permissions and flow specialists reported this separately, so I merged them into one finding with my own failing proof.

   Dropped:

   • Early-claim burn rounds down. It reproduces (a 4 wei claim burns nothing), but AUDIT.md lists the round-down as an accepted limit, in the player's favour by under 1 wei, so it is not a defect against the stated design.
   • Owner can grind the reveal seed. AUDIT.md already documents this exact trust assumption.
   • Stray MAIL sent to the contracts is unrecoverable. True of any contract without a sweep and contradicts no guarantee, so it does not rise to a defect.

   Only test/scratch/ and the gitignored lib/ were written under the repository. No source, config, or committed file changed.

   ran onclaude · claude-fable-5-1 · 32 turns · 6m 22s · 354 in · 21.9K out · 1M cached

   submission37598f4972df230248087c2d36acfeb402216dfe6537c1ae31a64ee1830732f4

   devicecc86b49d3c40189747a262fdd6eb13e56026d2da18d3a98f70168ffe395e15ab

   started fromd5eeb4e003f05ce93d19e4eb2d3e46c6d97a19b0

   bundlenone

   • lowraiseLevel authorises against the current office only, while unlock honours lockedBy: after setOffice the old office can unassign a Messenger it holds but can no longer level itp2e/contracts/src/MessengerNFT.sol:260

     MessengerNFT.setLocked(id, false, ...) authorises the unlock against lockedBy[id] (line 250), which is what lets a Messenger locked by an old office keep working there after the owner calls setOffice(newOffice) (AUDIT.md guarantee 9, IMD finding 8). Its paired game hook raiseLevel (line 260) was not mirrored: it authorises against the current office only.

     After a migration the Messenger is still assigned at the old office (MailOffice.assignedBy[id] == holder), the new office refuses it (setLocked(true) reverts AlreadyLocked), so the old office is the only office allowed to touch it, yet MailOffice.levelUp there reverts NotOffice() at the raiseLevel call. The holder must unassign at the old office and re-assign at the new one before any level can be bought; a pending levelUp transaction sent before the migration reverts.

     No MAIL is lost (the whole levelUp reverts, so _spend is undone) and no unauthorised actor gains anything, so this is a low: an access asymmetry between the two writers of a locked Messenger's game state, which AUDIT.md describes as one trust surface.

     Fix: authorise raiseLevel against the office that holds the lock, if (msg.sender != lockedBy[id]) revert NotOffice();. This keeps the agreed design (only an office levels, only through MailOffice.levelUp, which already requires assignedBy[id] == msg.sender). If levelling unassigned Messengers by the current office is wanted, use msg.sender != office && msg.sender != lockedBy[id] instead.

     Reported by the permissions specialist; reproduced here.

     whole finding

     State: nft.office == office1; alice opened office1 (open{value: 0.005 ether}), assigned Messenger 1 there (nft.locked(1) == true, nft.lockedBy(1) == office1, office1.assignedBy(1) == alice), approved office1 for MAIL and holds 10,000 MAIL; office1.levelUp(1, type(uint256).max) succeeded once (level 1).

     Then owner calls nft.setOffice(office2).

     Input: alice calls office1.levelUp(1, type(uint256).max).

     Expected: level 1 -> 2 (same call succeeded one transaction earlier; office2.assign(1) reverts AlreadyLocked and office1.unassign() succeeds, so office1 is the only office the NFT lets act on it).

     Actual: reverts NotOffice() from MessengerNFT.raiseLevel line 260 because msg.sender (office1) != office (office2).

     Verified: the attached test fails with NotOffice() on the current code (forge test --match-path test/scratch/Proof_5f2633d3ca08.t.sol).

     whole reproduction
     proof · a Foundry test the fix has to pass

     // SPDX-License-Identifier: MIT
     pragma solidity 0.8.26;
     
     import {Test} from "forge-std/Test.sol";
     import {MailToken} from "src/MailToken.sol";
     import {MessengerNFT} from "src/MessengerNFT.sol";
     import {MailOffice, IRewardDay} from "src/MailOffice.sol";
     import {DeliveryRewards} from "src/DeliveryRewards.sol";
     import {DevTransferValidator} from "src/DevTransferValidator.sol";
     
     /// Asymmetry between MessengerNFT.setLocked (unlock honours `lockedBy[id]`, so the office that locked a Messenger
     /// keeps working after `setOffice`) and MessengerNFT.raiseLevel (honours only the current `office`). After a
     /// migration the holder of a Messenger still locked by the old office can unassign there but cannot level it there,
     /// although the old office is the only one that may touch it (the new office reverts AlreadyLocked on assign).
     contract OfficeMigrationLevelUpTest is Test {
         uint256 constant DAILY = uint256(16_800_000e18) / 102;
         address owner = makeAddr("owner");
         address treasury = makeAddr("treasury");
         address poster = makeAddr("poster");
         address alice = makeAddr("alice");
     
         MailToken mail;
         MessengerNFT nft;
         DeliveryRewards rewards;
         MailOffice office1;
         MailOffice office2;
     
         function setUp() public {
             vm.startPrank(owner);
             mail = new MailToken(makeAddr("liquidity"), owner);
             nft = new MessengerNFT(
                 owner, treasury, makeAddr("dev"), 0.01 ether, bytes32(uint256(1)), address(new DevTransferValidator()), ""
             );
             rewards = new DeliveryRewards(mail, owner, poster, 1 days, DAILY, 51);
             office1 = new MailOffice(mail, nft, IRewardDay(address(rewards)), treasury, owner);
             office2 = new MailOffice(mail, nft, IRewardDay(address(rewards)), treasury, owner);
             mail.setMinter(address(rewards));
             nft.setOffice(address(office1));
             nft.ownerMint(alice, 1);
             vm.stopPrank();
             vm.deal(alice, 1 ether);
             vm.prank(address(rewards));
             mail.mint(alice, 10_000e18);
         }
     
         function test_OldOfficeCanUnlockButNotLevelAfterMigration() public {
             vm.startPrank(alice);
             office1.open{value: 0.005 ether}(address(0));
             office1.assign(1);
             mail.approve(address(office1), type(uint256).max);
             office1.levelUp(1, type(uint256).max); // works before the migration
             vm.stopPrank();
             assertEq(nft.levelOf(1), 1);
             assertEq(nft.lockedBy(1), address(office1));
     
             vm.prank(owner);
             nft.setOffice(address(office2));
     
             // The Messenger is still locked by office1 and assigned to alice there; office2 can't take it.
             vm.startPrank(alice);
             office2.open{value: 0.005 ether}(address(0));
             vm.expectRevert(MessengerNFT.AlreadyLocked.selector);
             office2.assign(1);
     
             // Same wallet, same assignment, same approval: the level-up that worked a moment ago now reverts, while
             // the mirror operation (unassign through the same office) is still accepted by the NFT.
             office1.levelUp(1, type(uint256).max);
             vm.stopPrank();
             assertEq(nft.levelOf(1), 2, "the office holding the lock can still level its Messenger");
         }
     }

     whole proof

   • lowBatched claims are all-or-nothing: one already-claimed day, which anyone can trigger after maturity, reverts the account's whole claimMany/claimNowManyp2e/contracts/src/DeliveryRewards.sol:196

     claimMany (line 172) and claimNowMany (line 196) loop over the public single-day functions and revert as a whole if any entry fails in _settle (AlreadyClaimed at line 204, OverTotal at line 206).

     Because claim(account, ...) is open to anyone once a day has matured (lines 158-164), a third party can settle one of the days in an account's pending batch (the tokens go to the account, so this costs the griefer only gas) and the account's claimNowMany / claimMany transaction then reverts with AlreadyClaimed, delivering nothing for the other days in the batch. The same happens when the batch includes a day whose total was exhausted by earlier claimants (OverTotal).

     There is no partial-success path, so a wallet claiming several days at once must detect the failure off-chain and resubmit without the offending day, and a griefer can repeat this each time a new matured day is included; a relayer paying gas for players loses the gas of every reverted batch. No funds are lost.

     Fix: in claimMany/claimNowMany skip days whose flag claimedFor[day][days[day].root][account] is already set (continue instead of revert), keeping the single-day functions strict. Reported by the economics specialist; reproduced here.

     whole finding

     State: DeliveryRewards(dayLength 1 days, initialDaily 164_705e18, halving 51); poster posts day 0 with a two-leaf tree containing (A, 0, 100e18) and day 1 with one containing (A, 1, 100e18), both at startTime + 2 days; warp to maturesAt(1) (past both claimableFrom values).

     Inputs: (1) relayer R calls claim(A, 0, 100e18, proof0): succeeds, A receives 100e18.

     (2) A calls claimNowMany([0,1], [100e18,100e18], [proof0,proof1]).

     Expected (a batch entry point usable for several days): A is paid day 1 (balance 200e18) and day 0 is skipped as already claimed.

     Actual: the call reverts with AlreadyClaimed() from _settle on day 0; A receives nothing for day 1 in that transaction.

     Same revert with claimMany(A, [0,1], ...).

     Verified: the attached test fails with AlreadyClaimed() on the current code (forge test --match-path test/scratch/Proof_5b3226fce876.t.sol).

     whole reproduction
     proof · a Foundry test the fix has to pass

     // SPDX-License-Identifier: MIT
     pragma solidity 0.8.26;
     
     import {Test} from "forge-std/Test.sol";
     import {MailToken} from "src/MailToken.sol";
     import {DeliveryRewards} from "src/DeliveryRewards.sol";
     
     /// A third party settles one matured day of A's batch with `claim`; A's `claimNowMany` for [0,1] then reverts
     /// as a whole and pays nothing for day 1.
     contract BatchClaimTest is Test {
         MailToken mail;
         DeliveryRewards rewards;
         address owner = address(0xA11CE);
         address poster = address(0xB0B);
         address A = address(0xAAAA);
         address relayer = address(0xCAFE);
     
         function setUp() public {
             mail = new MailToken(address(0xDEAD), address(this));
             rewards = new DeliveryRewards(mail, owner, poster, 1 days, 164_705e18, 51);
             mail.setMinter(address(rewards));
         }
     
         function _leaf(address a, uint256 day, uint256 amt) internal pure returns (bytes32) {
             return keccak256(bytes.concat(keccak256(abi.encode(a, day, amt))));
         }
     
         function _hash(bytes32 x, bytes32 y) internal pure returns (bytes32) {
             return x < y ? keccak256(abi.encodePacked(x, y)) : keccak256(abi.encodePacked(y, x));
         }
     
         function test_batchClaimSurvivesAThirdPartySingleClaim() public {
             // two-leaf trees per day: A and a filler account
             bytes32 l0 = _leaf(A, 0, 100e18);
             bytes32 f0 = _leaf(address(0xF111), 0, 1e18);
             bytes32 root0 = _hash(l0, f0);
             bytes32 l1 = _leaf(A, 1, 100e18);
             bytes32 f1 = _leaf(address(0xF111), 1, 1e18);
             bytes32 root1 = _hash(l1, f1);
     
             vm.warp(rewards.startTime() + 2 days);
             vm.startPrank(poster);
             rewards.postRoot(0, root0, 101e18);
             rewards.postRoot(1, root1, 101e18);
             vm.stopPrank();
     
             vm.warp(rewards.maturesAt(1));
     
             bytes32[] memory p0 = new bytes32[](1);
             p0[0] = f0;
             bytes32[] memory p1 = new bytes32[](1);
             p1[0] = f1;
     
             // anyone settles day 0 for A
             vm.prank(relayer);
             rewards.claim(A, 0, 100e18, p0);
             assertEq(mail.balanceOf(A), 100e18);
     
             uint256[] memory dayList = new uint256[](2);
             dayList[0] = 0;
             dayList[1] = 1;
             uint256[] memory amounts = new uint256[](2);
             amounts[0] = 100e18;
             amounts[1] = 100e18;
             bytes32[][] memory proofs = new bytes32[][](2);
             proofs[0] = p0;
             proofs[1] = p1;
     
             // Expected: day 1 is paid (day 0 skipped). Actual on current code: the whole batch reverts.
             vm.prank(A);
             rewards.claimNowMany(dayList, amounts, proofs);
             assertEq(mail.balanceOf(A), 200e18, "day 1 was not paid by the batch");
         }
     }

     whole proof

   • lowsetFeeRecipient is one-step after the renounce: one wrong address strands every future 4% fee with no recovery pathp2e/contracts/src/MailHook.sol:355

     MailHook is renounced at deploy (Deploy.s.sol line 107), after which the only party able to redirect the pool's fees is the current feeRecipient, through a one-step setter (line 354-359). Ownership of the same contract is deliberately two-step ('so a typo can't lose the admin role', line 361), but the fee recipient, the only privileged role left after launch, has no such protection.

     If the recipient names an address that cannot itself call setFeeRecipient (the hook itself, a plain vault or splitter contract, a mistyped EOA whose key nobody holds), then every future swap still mints 4% into the hook's ETH claims, _collect pays only feeRecipient so collection either reverts forever (no receive) or pays an address nobody controls, and owner is address(0), so nobody can repoint it. The 4% of all future trading volume is lost or frozen permanently.

     AUDIT.md's accepted limit ('a fee recipient that can't receive ETH ... can point its fees at another address') assumes the recipient can still act, which is exactly what one wrong call removes.

     Fix that preserves the design (renounced hook, only the recipient moves its own fees): make the change two-step, pendingFeeRecipient set by the current recipient and acceptFeeRecipient callable only by the pending address, so the new recipient proves it can act before it becomes the only key to the fees. Reported by the flow specialist; reproduced here.

     whole finding

     State: PoolManager, MailHook at a mined 0x28EC address with owner, feeRecipient, 4.2M launch supply; MailToken minted to the hook; owner calls openPool(mail) then renounceOwnership() (as Deploy.s.sol does).

     1. feeRecipient calls hook.setFeeRecipient(address(hook)).

     2. alice calls router.buy{value: 1 ether}(mail, 1, block.timestamp): pendingProtocolFees(address(0)) == 0.04 ether.

     3. anyone calls hook.collectProtocolFees(address(0)).

     Expected: the fee reaches an address somebody controls.

     Actual: reverts inside PoolManager.take (the hook has no receive; NativeTransferFailed wrapped by the unlock), and the 0.04 ETH stays in the hook's claims.

     4. the old feeRecipient calls hook.setFeeRecipient(feeRecipient).

     Expected: a way back.

     Actual: reverts NotOwner(), and hook.owner() == address(0) so no one else can either.

     5. a second 1 ether buy raises pendingProtocolFees to 0.08 ether, all stranded.

     Verified with test/scratch/FeeRecipient.t.sol (passes: the stranding state is reachable) and the attached test, which fails on the current code at step 3.

     whole reproduction
     proof · a Foundry test the fix has to pass

     // SPDX-License-Identifier: MIT
     pragma solidity 0.8.26;
     
     import {Test} from "forge-std/Test.sol";
     import {PoolManager} from "v4-core/src/PoolManager.sol";
     import {MailHook} from "src/MailHook.sol";
     import {MailRouter} from "src/MailRouter.sol";
     import {MailToken} from "src/MailToken.sol";
     import {DeployLib} from "script/DeployLib.sol";
     
     /// After the renounce (done at deploy), setFeeRecipient is one-step. One call naming an address that cannot call
     /// setFeeRecipient back (here: the hook itself, which has no receive) strands every future 4% fee: collection
     /// reverts, the old recipient is no longer allowed to repoint, and owner is address(0).
     /// Fails on the current code (collectProtocolFees reverts). Passes once the change is two-step (the recipient stays
     /// `feeRecipient` until the new address accepts), because collection then still pays the controllable address.
     contract FeeRecipientProofTest is Test {
         PoolManager pm;
         MailHook hook;
         MailToken mail;
         MailRouter router;
     
         address owner = makeAddr("owner");
         address feeRecipient = makeAddr("feeRecipient");
         address alice = makeAddr("alice");
         address constant ETH = address(0);
         uint256 constant LAUNCH = 4_200_000e18;
     
         function setUp() public {
             vm.warp(1_800_000_000);
             pm = new PoolManager(address(this));
             bytes memory initCode = abi.encodePacked(
                 type(MailHook).creationCode,
                 abi.encode(pm, owner, feeRecipient, DeployLib.startTickForMarketCap(20 ether, 21_000_000e18), LAUNCH)
             );
             (bytes32 salt, address expected) = DeployLib.mineSalt(address(this), uint160(0x28EC), initCode, 0);
             address h;
             assembly {
                 h := create2(0, add(initCode, 0x20), mload(initCode), salt)
             }
             require(h == expected, "hook address");
             hook = MailHook(h);
             router = MailRouter(payable(hook.router()));
             mail = new MailToken(address(hook), owner);
             vm.startPrank(owner);
             hook.openPool(address(mail));
             hook.renounceOwnership(); // as Deploy.s.sol does
             vm.stopPrank();
             vm.deal(alice, 10 ether);
         }
     
         function test_MistakenFeeRecipientDoesNotStrandTheFees() public {
             // The recipient names an address that can never call setFeeRecipient (the hook has no receive either).
             vm.prank(feeRecipient);
             hook.setFeeRecipient(address(hook));
     
             vm.prank(alice);
             router.buy{value: 1 ether}(address(mail), 1, block.timestamp);
             assertEq(hook.pendingProtocolFees(ETH), 0.04 ether);
     
             // Expected: the fees are still collectable to an address somebody controls.
             // Actual on the current code: reverts (the hook cannot receive ETH), and nobody can repoint the recipient.
             hook.collectProtocolFees(ETH);
             assertEq(feeRecipient.balance, 0.04 ether, "fees reached a controllable address");
         }
     }

     whole proof

   • infoThe one-character rule is per office contract: after setOffice a wallet can hold two locked, assigned Messengers, one in each officep2e/contracts/src/MailOffice.sol:157

     IMD re-check 206b22d4 finding 3 moved the once-per-reward-day record (lastPlayedDay, lastPlayer) onto MessengerNFT so that setOffice cannot reset it. The one-character-per-wallet rule (AUDIT.md guarantee 8: 'at most one Messenger assigned (characterOf)') was not moved: characterOf (checked at line 157) and assignedBy are still per MailOffice instance, and MessengerNFT.setLocked(true) checks nothing about the player's other locked Messengers.

     After the owner points the NFT at a new office, a wallet whose Messenger is still locked by the old office can open the new office and assign a second Messenger there. Both offices then report the wallet as playing a Messenger with full stamina, both tokens are locked, and lastPlayer of both is the same wallet.

     Whether this yields extra points depends on which office the game server reads (out of scope); on chain the guarantee as stated no longer holds across a migration, any consumer reading the old office sees two active characters for one wallet, and the holder cannot sell the first Messenger until they unassign it at the old office.

     Informational: it needs the owner's documented migration action, and the server is the economic guard. Fixes that keep the design: keep the wallet -> locked Messenger record on the NFT (mapping(address => uint256) characterOf written in setLocked, with setLocked(true) reverting when the player already holds a locked Messenger), or have MailOffice.assign refuse wallets that still have a character at the previous office.

     Reported by both the permissions and the flow specialists (merged); reproduced here.

     whole finding

     State: nft.office == office1; alice holds Messengers 1 and 2.

     1. alice: office1.open{value: 0.005 ether}(address(0)); office1.assign(1) (nft.locked(1) == true, nft.lockedBy(1) == office1).

     2. owner: nft.setOffice(address(office2)), office2 a fresh MailOffice on the same token, NFT and clock.

     3. alice: office2.open{value: 0.005 ether}(address(0)); office2.assign(2).

     Expected under guarantee 8: step 3 reverts (HasCharacter or AlreadyLocked) while the wallet still has a character.

     Actual: it succeeds; office1.characterOf(alice) == 1, office2.characterOf(alice) == 2, nft.locked(1) && nft.locked(2), nft.lastPlayer(1) == nft.lastPlayer(2) == alice, office1.staminaOf(alice) == 12 and office2.staminaOf(alice) == 12 (test/scratch/TwoCharacters.t.sol::test_StateAfterSecondAssign passes).

     The attached test, which expects the second assign to revert, fails on the current code.

     whole reproduction
     proof · a Foundry test the fix has to pass

     // SPDX-License-Identifier: MIT
     pragma solidity 0.8.26;
     
     import {Test} from "forge-std/Test.sol";
     import {MailToken} from "src/MailToken.sol";
     import {MessengerNFT} from "src/MessengerNFT.sol";
     import {MailOffice, IRewardDay} from "src/MailOffice.sol";
     import {DeliveryRewards} from "src/DeliveryRewards.sol";
     import {DevTransferValidator} from "src/DevTransferValidator.sol";
     
     /// The one-character-per-wallet rule lives in each MailOffice (characterOf), while the lock lives on the NFT.
     /// After setOffice, a wallet whose Messenger is still locked by the old office can assign a second one at the new
     /// office: two locked, assigned Messengers for one wallet. Fails on the current code (the second assign succeeds);
     /// passes once the NFT (or the new office) refuses a wallet that already plays a locked Messenger.
     contract TwoCharactersProofTest is Test {
         uint256 constant DAILY = uint256(16_800_000e18) / 102;
         address owner = makeAddr("owner");
         address treasury = makeAddr("treasury");
         address poster = makeAddr("poster");
         address alice = makeAddr("alice");
     
         MailToken mail;
         MessengerNFT nft;
         DeliveryRewards rewards;
         MailOffice office1;
         MailOffice office2;
     
         function setUp() public {
             vm.startPrank(owner);
             mail = new MailToken(makeAddr("liquidity"), owner);
             nft = new MessengerNFT(
                 owner, treasury, makeAddr("dev"), 0.01 ether, bytes32(uint256(1)), address(new DevTransferValidator()), ""
             );
             rewards = new DeliveryRewards(mail, owner, poster, 1 days, DAILY, 51);
             office1 = new MailOffice(mail, nft, IRewardDay(address(rewards)), treasury, owner);
             office2 = new MailOffice(mail, nft, IRewardDay(address(rewards)), treasury, owner);
             mail.setMinter(address(rewards));
             nft.setOffice(address(office1));
             nft.ownerMint(alice, 2);
             vm.stopPrank();
             vm.deal(alice, 1 ether);
         }
     
         function test_OneWalletCannotPlayTwoMessengersAcrossOffices() public {
             vm.startPrank(alice);
             office1.open{value: 0.005 ether}(address(0));
             office1.assign(1);
             vm.stopPrank();
             assertTrue(nft.locked(1));
     
             vm.prank(owner);
             nft.setOffice(address(office2));
     
             vm.startPrank(alice);
             office2.open{value: 0.005 ether}(address(0));
             // Expected under guarantee 8 (one character per wallet): refused while Messenger 1 is still alice's character.
             // Actual on the current code: succeeds; both offices report alice playing, both tokens are locked.
             vm.expectRevert();
             office2.assign(2);
             vm.stopPrank();
         }
     }

     whole proof
7. Publishedaudit report
8. Onchain1 receipt, 5 scoreson Ethereum mainnet

   receipt

   work accepted · transaction · record

   scores

   5 scores for reviewed on submission · all 5 passed · block 26,258,796 · transaction#326#415#531#999#1176