test(invariant): make protocol-fee conservation independent of on-chain state

README.md

@@ -38,9 +38,9 @@ The price doubles for each character below nine and flattens to D from nine upwa
 
 ### Bands and who pays
 
-Three bands share the one curve. Names of nine characters or more are open to anyone at the flat fee D. Names of six to eight characters are the premium band: only a verified person may register there, and they pay the curve for the length, 8D, 4D or 2D. Personhood buys access to the premium band, not a discount inside it. Names of five characters or fewer are reserved to governance; no user registers them, and governance releases them itself at the curve floor into the treasury it already controls.
+Three bands share the one curve. Names of nine characters or more are open to anyone at the flat fee D. Names of six to eight characters are the premium band: only a verified person may register there, and they pay the curve for the length, 8D, 4D or 2D. Personhood buys access to the premium band, not a discount inside it. Names of five characters or fewer are reserved to governance; no user registers them, and governance releases them itself at the price their length sets, into the treasury it already controls.
 
-Every caller pays the same curve for a given length. The two zero-cost paths are both bounded. Each verified person gets one free name through the personhood gateway, which is the unpriced lane: the gateway issues that single grant and charges nothing for it. Governance releasing a reserved name pays itself the floor, which settles in a circle into its own treasury and nets nothing.
+Every caller pays the same curve for a given length. The two zero-cost paths are both bounded. Each verified person gets one free name through the personhood gateway, which is the unpriced lane: the gateway issues that single grant and charges nothing for it. Governance releasing a reserved name charges itself that length's price, which settles in a circle into its own treasury and nets nothing.
 
 ### Deposits and protocol fees
 

contracts/escrow/DotnsNameEscrow.sol

@@ -204,8 +204,8 @@ contract DotnsNameEscrow is
         ReleasePosition storage position = _positions[params.tokenId];
 
         // Use `recipient` as the "is this slot funded?" sentinel so zero-amount
-        // positions (seeded by free PopFull / PopLite registrations) still count
-        // as funded and cannot be re-seeded with a different recipient.
+        // positions (seeded by cross-paid registrations, which pay a fee rather than a deposit)
+        // still count as present and cannot be re-seeded with a different recipient.
         require(position.recipient == address(0), PositionAlreadyFunded(params.tokenId));
         require(!position.released, AlreadyReleased(params.tokenId));
 
@@ -304,8 +304,8 @@ contract DotnsNameEscrow is
 
         ReleasePosition storage position = _positions[tokenId];
         // Recipient is the canonical "is this position present?" sentinel; zero-amount positions
-        // seeded for free PopFull / PopLite registrations are still releasable so every minted
-        // name has a reachable lifecycle.
+        // seeded for cross-paid registrations are still releasable so every minted name has a
+        // reachable lifecycle.
         require(position.recipient != address(0), DepositNotConfigured(tokenId));
         require(!position.released, AlreadyReleased(tokenId));
 

contracts/pop/PopRules.sol

@@ -269,8 +269,8 @@ contract PopRules is
         return userStatus >= required;
     }
 
-    /// @notice Scarcity price for a base length: `D * 2 ** (9 - n)` below nine, else the base fee
-    /// D. @dev The multiplier is at most 512 and arithmetic is checked, so an oversized base fee
+    /// @notice Scarcity price for a base length: D * 2 ** (9 - n) below nine, else the base fee D.
+    /// @dev The multiplier is at most 512 and arithmetic is checked, so an oversized base fee
     ///      reverts rather than truncating.
     function _priceValidatedName(uint256 baseLength) internal view returns (uint256 priceValue) {
         if (baseLength >= 9) return startingPrice;

test/fuzz/registrar/DotnsRegistrarControllerFuzz.t.sol

@@ -88,7 +88,7 @@ contract DotnsRegistrarControllerFuzzTest is BaseDotns {
 
     function testFuzz_register_refunds_overpayment_inline(uint256 extra, uint256 salt) public {
         address registrant = tiago;
-        string memory nameLabel = _labelPriceZero(bound(salt, 0, 64));
+        string memory nameLabel = _labelPopLitePriced(bound(salt, 0, 64));
 
         _grantPopLite(registrant);
 
@@ -402,8 +402,9 @@ contract DotnsRegistrarControllerFuzzTest is BaseDotns {
         return string(abi.encodePacked("nostatus", _uintToAlphaFixed(salt, 2), "01"));
     }
 
-    /// @notice Generate a label that classifies as PopLite and prices to zero.
-    function _labelPriceZero(uint256 salt) internal pure returns (string memory label) {
+    /// @notice Generate an 8-char PopLite-tier label (base length 6) that prices at 8D on the
+    ///         curve, so the amount is non-zero.
+    function _labelPopLitePriced(uint256 salt) internal pure returns (string memory label) {
         return string(abi.encodePacked("free", _uintToAlphaFixed(salt, 2), "01"));
     }
 

test/intergration/BasicDotns.t.sol

@@ -5,8 +5,6 @@ import {BaseDotns} from "../base/BaseDotns.t.sol";
 import {IDotnsRegistry} from "../../contracts/registry/IDotnsRegistry.sol";
 import {IDotnsRegistrarController} from "../../contracts/registrars/IDotnsRegistrarController.sol";
 import {ILabelStore} from "../../contracts/store/ILabelStore.sol";
-import {IPersonhood} from "../../contracts/external/personhood/IPersonhood.sol";
-import {DotnsConstants} from "../../contracts/utils/DotnsConstants.sol";
 
 /// @title BasicDotnsIntegration
 /// @notice End-to-end happy-path integration coverage for registration,
@@ -91,23 +89,15 @@ contract BasicDotnsIntegration is BaseDotns {
         );
     }
 
-    /// @notice Returns true when the personhood precompile reports `account` at
-    ///         tier `Lite` (1) or `Full` (2) under the dotns context.
-    /// @dev Reads the precompile mock installed by @custom:contract BaseDotns so the integration
-    ///      flow gates pricing assertions on the same tier the controller sees.
-    function _personhoodTierIsAtLeastLite(address account) internal view returns (bool) {
-        IPersonhood.PersonhoodInfo memory info = IPersonhood(DotnsConstants.PERSONHOOD)
-            .personhoodStatus(account, DotnsConstants.PERSONHOOD_CONTEXT);
-        return info.status >= 1;
-    }
-
     /// @notice Drives a full end-to-end registration, records, subname, and
     ///         transfer flow under the configuration described by `flow`.
     /// @dev Aggregates the assertions that every PoP-tier-specific test case
     ///      shares so each tier-level test only needs to supply the parameter
     ///      bag.
     function _flowEndToEnd(FlowParams memory flow) internal {
         uint256 quotedPriceBefore = popRules.priceWithCheck(flow.name, flow.nameOwner).price;
+        // Cross-entry-point consistency: the concrete curve amounts are covered by the PopRules
+        // unit tests; this only asserts that priceWithCheck and price agree.
         assertEq(quotedPriceBefore, popRules.price(flow.name));
 
         _commitAndRegister(flow.name, flow.nameOwner, flow.reserved);
@@ -183,6 +173,8 @@ contract BasicDotnsIntegration is BaseDotns {
 
         uint256 transferRecipientQuotedPrice =
             popRules.priceWithCheck(flow.transferRecipientNewName, flow.transferTo).price;
+        // Cross-entry-point consistency: the concrete curve amounts are covered by the PopRules
+        // unit tests; this only asserts that priceWithCheck and price agree.
         assertEq(transferRecipientQuotedPrice, popRules.price(flow.transferRecipientNewName));
 
         _commitAndRegister(flow.transferRecipientNewName, flow.transferTo, false);

test/invariant/escrow/DotnsNameEscrowInvariant.t.sol

@@ -52,21 +52,32 @@ contract DotnsNameEscrowInvariantTest is BaseDotns {
     }
 
     /// @notice Escrow native balance must always cover the full liability set: tracked
-    /// reserves, the insurance fund, unclaimed pull-payment balances, and the time-locked
+    /// reserves, the protocol fees, unclaimed pull-payment balances, and the time-locked
     /// refund-ledger entries credited by the refund-on-leave path. Under the deposit-binds-
     /// to-depositor model these four flows are economically distinct; solvency is only
     /// meaningful against their sum.
     function invariant_solvency() public view {
         uint256 escrowBalance = address(dotnsNameEscrow).balance;
         uint256 reservedAmount = dotnsNameEscrow.reserves(address(0));
-        uint256 insurance = dotnsNameEscrow.protocolFees();
+        uint256 protocolFees = dotnsNameEscrow.protocolFees();
         uint256 pending = handler.totalPendingWithdrawals();
         uint256 refundEntries = handler.totalPendingRefundEntries();
 
         assertGe(
             escrowBalance,
-            reservedAmount + insurance + pending + refundEntries,
-            "Escrow balance must cover reserves + insurance + pending withdrawals + refund entries"
+            reservedAmount + protocolFees + pending + refundEntries,
+            "Escrow balance must cover reserves + protocol fees + pending withdrawals + refund entries"
+        );
+    }
+
+    /// @notice The handler's mirror of protocol-fee inflows must equal the escrow's on-chain
+    ///         protocol-fee balance. Protocol fees only ever accrue, so every inflow the handler
+    ///         tracks (cross-tier register and payable transfer) must sum to exactly the balance.
+    function invariant_protocol_fees_match_tracked_inflows() public view {
+        assertEq(
+            handler.ghost_protocolFeesPaidIn(),
+            dotnsNameEscrow.protocolFees(),
+            "Tracked protocol-fee inflows must equal on-chain protocol fees"
         );
     }
 

test/invariant/escrow/EscrowHandler.t.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.34;
 
-import {Test, Vm} from "forge-std/Test.sol";
+import {Test} from "forge-std/Test.sol";
 import {
     DotnsRegistrarController,
     IDotnsRegistrarController
@@ -56,16 +56,11 @@ contract EscrowHandler is Test {
     /// @notice Label used to register each tokenId; required for re-registration after finalise.
     mapping(uint256 tokenId => string label) public labelByTokenId;
 
-    /// @notice Cumulative native amount credited into the insurance fund by handler-driven flows.
+    /// @notice Cumulative native amount credited into protocol fees by handler-driven flows.
     /// @dev Increments via cross-tier register (`depositProtocolFee`) and payable `transferFrom`
-    ///      (`chargeTransferFee`). Counterpart to `ghost_insurancePaidOut`.
-    uint256 public ghost_insurancePaidIn;
-
-    /// @notice Cumulative native amount drawn out of the insurance fund.
-    /// @dev Updated by parsing `InsuranceDraw` events emitted from `withdraw()`. The
-    ///      conservation invariant asserts `ghost_insurancePaidIn - ghost_insurancePaidOut
-    ///      == escrow.protocolFees()`.
-    uint256 public ghost_insurancePaidOut;
+    ///      (`chargeTransferFee`). Protocol fees only ever accrue, so a conservation invariant
+    ///      asserts this equals `escrow.protocolFees()`.
+    uint256 public ghost_protocolFeesPaidIn;
 
     /// @notice Cumulative native amount credited to recipients via `withdraw()`.
     uint256 public ghost_pendingCredits;
@@ -214,10 +209,9 @@ contract EscrowHandler is Test {
 
         // Under the A1 max-not-sum rule the controller charges
         // `max(priced.price, friction)` on the cross-payer path and routes the
-        // whole charge into the insurance fund via `depositProtocolFee`. The
+        // whole charge into protocol fees via `depositProtocolFee`. The
         // refundable deposit position is seeded at zero amount, so the only
-        // mutation invariant tracking has to mirror here is the insurance leg.
-        uint256 priorProtocolFees = escrow.protocolFees();
+        // mutation invariant tracking has to mirror here is the protocol-fee leg.
         bytes32 labelhash = keccak256(bytes(label));
         bytes32 node = keccak256(abi.encodePacked(DOT_NODE, labelhash));
         uint256 tokenId = uint256(node);
@@ -231,29 +225,22 @@ contract EscrowHandler is Test {
         uint256 charge = ownerPrice > frictionForCharge ? ownerPrice : frictionForCharge;
 
         // Skip when no value moves: a zero charge produces a free zero-amount
-        // position with no insurance or reserves delta, so adding it to the
+        // position with no protocol-fee or reserves delta, so adding it to the
         // ghost-state token set adds noise without exercising any new branch.
         if (charge == 0) return;
 
-        vm.recordLogs();
         vm.prank(payer);
         try controller.register{value: charge}(registration) {
-            Vm.Log[] memory logs = vm.getRecordedLogs();
-            uint256 newInsurance = escrow.protocolFees();
-
             _depositedTokenIds.push(tokenId);
             labelByTokenId[tokenId] = label;
             // Cross-payer registrations seed a zero-amount refundable position;
             // ghost-state mirrors that by leaving `depositAmounts` at zero.
             depositAmounts[tokenId] = 0;
 
-            if (newInsurance > priorProtocolFees) {
-                ghost_insurancePaidIn += (newInsurance - priorProtocolFees);
-            }
-
-            // Track InsuranceDraw outflows surfaced by this transaction (defensive; the
-            // register path itself does not draw insurance, but recordLogs is already on).
-            _accountInsuranceDraws(logs);
+            // The whole cross-payer charge becomes protocol fee. Accumulate the
+            // independently-computed `charge` so the conservation invariant verifies the
+            // escrow credited exactly what the caller was charged, not an echo of its own state.
+            ghost_protocolFeesPaidIn += charge;
         } catch {
             return;
         }
@@ -307,8 +294,7 @@ contract EscrowHandler is Test {
 
     /// @notice Withdraws refund for a released token after cooldown.
     /// @dev Picks from _releasedTokenIds, warps past cooldown, withdraws.
-    ///      Moves the token to _withdrawnTokenIds. Records pending credits and any
-    ///      `InsuranceDraw` event amounts via `vm.recordLogs`.
+    ///      Moves the token to _withdrawnTokenIds and records the pending credit.
     /// @param tokenSeed Seed for selecting which released token to withdraw.
     function withdrawRefund(uint256 tokenSeed) external {
         if (_releasedTokenIds.length == 0) return;
@@ -326,16 +312,13 @@ contract EscrowHandler is Test {
 
         uint256 owed = position.amount;
 
-        vm.recordLogs();
         vm.prank(recipient);
         escrow.withdraw(tokenId);
-        Vm.Log[] memory logs = vm.getRecordedLogs();
 
         // Update ghost state after the inner call so a revert leaves accounting intact.
         _withdrawnTokenIds.push(tokenId);
         _removeReleased(index);
         ghost_pendingCredits += owed;
-        _accountInsuranceDraws(logs);
     }
 
     /// @notice Pulls the caller's accumulated pending refund balance.
@@ -514,24 +497,13 @@ contract EscrowHandler is Test {
         uint256 requiredFee = registrar.quoteTransferFee(tokenId, to);
         if (requiredFee == 0) return;
 
-        uint256 priorProtocolFees = escrow.protocolFees();
-
-        vm.recordLogs();
         vm.prank(currentOwner);
         try registrar.transferFrom{value: requiredFee}(currentOwner, to, tokenId) {
-            Vm.Log[] memory logs = vm.getRecordedLogs();
-
-            // Read the on-chain insurance delta rather than predicting it. The
-            // chargeTransferFee path credits the reach floor to insurance; when
-            // the NFT is leaving its prior position recipient the position is
-            // rebound to the new holder rather than refunded, so reserves stay
-            // put and only insurance moves. Mirroring the formula in the handler
-            // would re-create the drift this guard is meant to prevent.
-            uint256 newInsurance = escrow.protocolFees();
-            if (newInsurance > priorProtocolFees) {
-                ghost_insurancePaidIn += (newInsurance - priorProtocolFees);
-            }
-            _accountInsuranceDraws(logs);
+            // The transfer fee is credited in full to protocol fees: the position rebinds
+            // to the new holder rather than refunding, so reserves stay put. Accumulate the
+            // independently-quoted `requiredFee` so the conservation invariant verifies the
+            // escrow credited exactly the quoted fee.
+            ghost_protocolFeesPaidIn += requiredFee;
 
             // Sync the amount as a safety net against future downgrade paths.
             // Under the deposit-follows-name design the leaving-recipient branch
@@ -667,25 +639,6 @@ contract EscrowHandler is Test {
         return address(0);
     }
 
-    /// @notice Scans recorded logs for `InsuranceDraw` events and accumulates the amount
-    ///         drawn into `ghost_insurancePaidOut`.
-    /// @dev Single canonical accounting helper used by every handler call that may trigger a draw
-    ///      (currently `withdraw()`). Other inner calls forward an empty log array, which
-    ///      is a no-op.
-    /// @param logs Recorded logs from the most recent inner call.
-    function _accountInsuranceDraws(Vm.Log[] memory logs) internal {
-        // keccak256("InsuranceDraw(uint256,uint256)")
-        bytes32 sig = keccak256("InsuranceDraw(uint256,uint256)");
-        uint256 length = logs.length;
-        for (uint256 i; i < length; ++i) {
-            Vm.Log memory entry = logs[i];
-            if (entry.emitter != address(escrow)) continue;
-            if (entry.topics.length == 0 || entry.topics[0] != sig) continue;
-            uint256 amount = abi.decode(entry.data, (uint256));
-            ghost_insurancePaidOut += amount;
-        }
-    }
-
     /// @notice Allows the handler to receive ETH refunds.
     receive() external payable {}
 }

test/invariant/registrar/DotnsRegistrarControllerInvariant.t.sol

@@ -101,9 +101,9 @@ contract DotnsRegistrarControllerInvariantTest is BaseDotns {
     }
 
     function invariant_value_conservation() public view {
-        // Escrow balance equals reserves + insurance + pending withdrawals.
+        // Escrow balance equals reserves + protocol fees + pending withdrawals.
         uint256 reservedAmount = dotnsNameEscrow.reserves(address(0));
-        uint256 insurance = dotnsNameEscrow.protocolFees();
+        uint256 protocolFees = dotnsNameEscrow.protocolFees();
 
         uint256 pendingTotal;
         for (uint256 i; i < 5; ++i) {
@@ -117,8 +117,8 @@ contract DotnsRegistrarControllerInvariantTest is BaseDotns {
         uint256 escrowBalance = address(dotnsNameEscrow).balance;
         assertEq(
             escrowBalance,
-            reservedAmount + insurance + pendingTotal,
-            "Escrow balance must equal reserves + insurance + pending withdrawals"
+            reservedAmount + protocolFees + pendingTotal,
+            "Escrow balance must equal reserves + protocol fees + pending withdrawals"
         );
     }