From 20db3a2468bd764ada82f46b8b04995a6d12c104 Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 8 Oct 2026 15:33:05 +0530 Subject: [PATCH 01/12] fix: make reserved name releasable Signed-off-by: GHkrishna --- .../registrars/DotnsRegistrarController.sol | 42 ++++++++++++++++++- .../registrars/IDotnsRegistrarController.sol | 12 +++++- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/contracts/registrars/DotnsRegistrarController.sol b/contracts/registrars/DotnsRegistrarController.sol index c91d4001f..3ffe3a8c0 100644 --- a/contracts/registrars/DotnsRegistrarController.sol +++ b/contracts/registrars/DotnsRegistrarController.sol @@ -304,7 +304,8 @@ contract DotnsRegistrarController is ); } - (, bytes32 labelhash, bytes32 node) = _requireAvailableLabel(registration.label); + (IDotnsRegistrar registrar, bytes32 labelhash, bytes32 node) = + _requireAvailableLabel(registration.label); _consumeCommitment(registration); // Spend the grant before minting so a grant in the wrong state fails before any name is @@ -318,12 +319,49 @@ contract DotnsRegistrarController is whitelist.consume(registration.label, registration.owner); } + IDotnsNameEscrow escrow = IDotnsNameEscrow(payable(_escrow())); + uint256 tokenId = uint256(node); + // `available` is true both for a never-minted label and for one the escrow still holds + // past its redeem window, so a grant can land on either and the fresh-mint branch alone + // is not enough: `DotnsRegistrar.register` rejects an id the escrow holds. Reserved-tier + // labels reach circulation only through this function, so without the reclaim branch a + // released grant would sit in custody, advertised as available, with no route back out. + // + // One deliberate divergence from @custom:function register: that path also clears a + // sibling controller's stale base-name reservation on reclaim. This one does not, because + // it reads no `IPopRules` state at all, and keeping it that way is what lets the whole + // function stay callable under a Root origin. + bool isReclaim = registrar.exists(tokenId); + // No reverse record. `setReverseName` overwrites unconditionally, and the gate above lets // anyone submit for the beneficiary, so writing here would let a third party relabel // another address. The owner claims their own record through `claimReverseRecord`, which // checks ownership and writes only their own key. _completeRegistration( - registration, labelhash, node, 0, false, IDotnsReverseResolver(address(0)), false + registration, labelhash, node, 0, false, IDotnsReverseResolver(address(0)), isReclaim + ); + + if (isReclaim) { + escrow.reclaim(tokenId, registration.owner); + // Mirrors @custom:function register: reclaim hands the NFT to the new holder, so the + // registry record is rewritten only afterwards, once `ownerOf` reports that holder + // rather than the escrow. Otherwise the prior owner's resolver pointer follows the + // name. + IDotnsRegistry(protocolRegistry.get(DotnsConstants.REGISTRY)) + .setOwner(node, registration.owner); + } + + // Seed the release position last, after `reclaim` has deleted whatever the previous holder + // left behind. A grant costs nothing, so there is no deposit to lock, but the position is + // also the lifecycle marker: `release` rejects a token whose position carries no recipient, + // so a name minted without one can never be released, never becomes reclaimable, and never + // returns to circulation. A zero amount keeps that lifecycle reachable while leaving + // nothing to withdraw and nothing for reclaim to settle, which is exactly what the + // cross-payer branch of @custom:function _settleEscrow seeds, for the same reason. + escrow.deposit( + IDotnsNameEscrow.DepositParams({ + tokenId: tokenId, asset: address(0), amount: 0, recipient: registration.owner + }) ); } diff --git a/contracts/registrars/IDotnsRegistrarController.sol b/contracts/registrars/IDotnsRegistrarController.sol index 254d4913f..234170eba 100644 --- a/contracts/registrars/IDotnsRegistrarController.sol +++ b/contracts/registrars/IDotnsRegistrarController.sol @@ -172,7 +172,7 @@ interface IDotnsRegistrarController is IDotnsController { function register(Registration calldata registration) external payable; /// @notice Registers a granted name after the commitment delay, at zero base cost. - /// @dev Grant-backed issuance path: skips the PoP price check and the escrow deposit, but + /// @dev Grant-backed issuance path: skips the PoP price check and charges no deposit, but /// reuses the same commit-reveal pipeline so the same anti-front-running guarantees apply. /// /// Authority is either a substrate Root dispatch or a grant naming `registration.owner` on the @@ -193,6 +193,14 @@ interface IDotnsRegistrarController is IDotnsController { /// (otherwise @custom:reverts NameNotAvailable), then consumes the prior commitment, which /// fails with @custom:reverts CommitmentNotFound, @custom:reverts CommitmentTooNew, or /// @custom:reverts CommitmentTooOld under the same conditions as @custom:function register. - /// Emits @custom:emits NameRegistered on success. + /// + /// Availability covers both a never-minted label and one the escrow still holds past its + /// redeem window, so this path reclaims from escrow when the token already exists, exactly as + /// @custom:function register does. It then seeds a zero-amount escrow release position keyed + /// to `registration.owner`. The grant is free, so there is nothing to refund, but the position + /// is what makes the name releasable: without it the holder cannot release, the name never + /// becomes reclaimable, and a reserved-tier label leaves circulation permanently. The escrow + /// must therefore be registered under `DotnsConstants.NAME_ESCROW`, otherwise + /// @custom:reverts EscrowNotConfigured. Emits @custom:emits NameRegistered on success. function registerReserved(Registration calldata registration) external; } From 80c6a5bfa33fc4b8e16d982279762246fabf6633 Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 8 Oct 2026 15:33:37 +0530 Subject: [PATCH 02/12] fix: update tests Signed-off-by: GHkrishna --- test/intergration/NameGrantFlow.t.sol | 134 ++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/test/intergration/NameGrantFlow.t.sol b/test/intergration/NameGrantFlow.t.sol index 269652e59..0f93dc31b 100644 --- a/test/intergration/NameGrantFlow.t.sol +++ b/test/intergration/NameGrantFlow.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.34; import {BaseDotns, IDotnsRegistrarController} from "../base/BaseDotns.t.sol"; import {IDotnsNameWhitelist} from "../../contracts/whitelist/IDotnsNameWhitelist.sol"; +import {IDotnsNameEscrow} from "../../contracts/escrow/IDotnsNameEscrow.sol"; import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; /// @title NameGrantFlow @@ -71,6 +72,116 @@ contract NameGrantFlow is BaseDotns { assertEq(IERC721(address(dotnsRegistrar)).ownerOf(uint256(node)), beneficiary); } + // release lifecycle: a granted name must not be a one-way exit from circulation + + /// @dev The mint is free, so the position carries no value, but it must exist: `release` gates + /// on `position.recipient`, so a granted name minted without one could never be released. + function test_a_granted_name_seeds_a_zero_amount_release_position() public { + address user = ed; + string memory nameLabel = "releasablegrant01"; + + _grantName(nameLabel, user); + _registerReserved(nameLabel, user, user); + + IDotnsNameEscrow.ReleasePosition memory position = + dotnsNameEscrow.getReleasePosition(_tokenIdForLabel(nameLabel)); + + assertEq(position.recipient, user, "the grant must seed a position keyed to the owner"); + assertEq(position.amount, 0, "a free mint must not seed a refundable deposit"); + assertFalse(position.released, "a fresh position is not in the released phase"); + } + + /// @dev A Root dispatch mints without a grant and must seed the same position: the lifecycle + /// cannot depend on which of the two authorities issued the name. + function test_a_root_minted_reserved_name_seeds_the_same_position() public { + address user = ed; + string memory nameLabel = "rootmintedgrant01"; + + _registerReservedAsRoot(nameLabel, user); + + IDotnsNameEscrow.ReleasePosition memory position = + dotnsNameEscrow.getReleasePosition(_tokenIdForLabel(nameLabel)); + + assertEq(position.recipient, user, "a Root mint must seed a position too"); + assertEq(position.amount, 0, "a Root mint is free, so nothing is refundable"); + } + + /// @dev The headline regression. Grants are the only route to a reserved-tier label, so before + /// the position was seeded every one governance issued left the pool permanently: the + /// holder could not release, so the name never became reclaimable and never came back. + function test_a_granted_reserved_tier_name_returns_to_circulation() public { + // Three characters, so the label is governance-reserved and `register` cannot mint it. + // `registerReserved` is the only way in, which is what makes the exit matter. + string memory nameLabel = "gov"; + uint256 tokenId = _tokenIdForLabel(nameLabel); + + _grantName(nameLabel, ed); + _registerReserved(nameLabel, ed, ed); + assertFalse(dotnsRegistrarController.available(nameLabel), "the name is taken"); + + vm.startPrank(ed); + dotnsRegistrar.approve(address(dotnsNameEscrow), tokenId); + dotnsNameEscrow.release(tokenId); + vm.stopPrank(); + + assertEq( + IERC721(address(dotnsRegistrar)).ownerOf(tokenId), + address(dotnsNameEscrow), + "release moves the name into custody" + ); + assertFalse( + dotnsRegistrarController.available(nameLabel), + "inside the redeem window the name still belongs to its previous holder" + ); + + vm.warp(dotnsNameEscrow.getReleasePosition(tokenId).redeemableUntil); + assertTrue( + dotnsRegistrarController.available(nameLabel), "the window closed, so the name is free" + ); + + // Reserved-tier labels only mint through `registerReserved`, so a name advertised as + // available has to be reachable from here or it is not really back in circulation. + _grantName(nameLabel, tiago); + _registerReserved(nameLabel, tiago, tiago); + + assertEq(IERC721(address(dotnsRegistrar)).ownerOf(tokenId), tiago, "the name was regranted"); + assertEq(dotnsRegistry.owner(_nodeOf(nameLabel)), tiago, "the registry follows the holder"); + + IDotnsNameEscrow.ReleasePosition memory position = + dotnsNameEscrow.getReleasePosition(tokenId); + assertEq(position.recipient, tiago, "the reclaim seeds a fresh position for the new holder"); + assertFalse(position.released, "the new position starts outside the released phase"); + } + + /// @dev The position is the lifecycle marker, so it has to follow the name. Otherwise a granted + /// name becomes unreleasable again the moment its first holder passes it on. + function test_a_granted_name_keeps_its_position_through_a_transfer() public { + string memory nameLabel = "transferredgrant01"; + uint256 tokenId = _tokenIdForLabel(nameLabel); + + _grantName(nameLabel, ed); + _registerReserved(nameLabel, ed, ed); + + vm.prank(ed); + IERC721(address(dotnsRegistrar)).transferFrom(ed, tiago, tokenId); + + assertEq( + dotnsNameEscrow.getReleasePosition(tokenId).recipient, + tiago, + "the position rebinds to the new holder" + ); + + vm.startPrank(tiago); + dotnsRegistrar.approve(address(dotnsNameEscrow), tokenId); + dotnsNameEscrow.release(tokenId); + vm.stopPrank(); + + assertTrue( + dotnsNameEscrow.getReleasePosition(tokenId).released, + "the new holder can release what they were given" + ); + } + /// @notice Commit-reveal a reserved registration for `nameOwner`, submitted by `submitter`. function _registerReserved( string memory nameLabel, @@ -95,4 +206,27 @@ contract NameGrantFlow is BaseDotns { dotnsRegistrarController.registerReserved(registration); vm.stopPrank(); } + + /// @notice Commit-reveal a reserved registration for `nameOwner` under a mocked Root origin. + /// @dev The Root branch takes no grant and consumes none, so this skips `_grantName`. Only the + /// reveal runs as Root: the commit does not read the origin, and leaving the mock sticky + /// would put later registrations on the Root branch by accident. + function _registerReservedAsRoot(string memory nameLabel, address nameOwner) private { + IDotnsRegistrarController.Registration memory registration = + IDotnsRegistrarController.Registration({ + label: nameLabel, + owner: nameOwner, + secret: keccak256(abi.encodePacked(nameLabel, nameOwner, "root")), + reserved: true, + maxPrice: type(uint256).max, + pricingVersion: popRules.pricingVersion() + }); + + dotnsRegistrarController.commit(dotnsRegistrarController.makeCommitment(registration)); + vm.warp(block.timestamp + dotnsRegistrarController.minCommitmentAge() + 1); + + _mockOriginIsRoot(true); + dotnsRegistrarController.registerReserved(registration); + _mockOriginIsRoot(false); + } } From c07af17cadc0255833dc3be84167504e8d98391d Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 8 Oct 2026 15:33:56 +0530 Subject: [PATCH 03/12] fix: update comments Signed-off-by: GHkrishna --- contracts/escrow/DotnsNameEscrow.sol | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/contracts/escrow/DotnsNameEscrow.sol b/contracts/escrow/DotnsNameEscrow.sol index 99507e3e0..d4879bed9 100644 --- a/contracts/escrow/DotnsNameEscrow.sol +++ b/contracts/escrow/DotnsNameEscrow.sol @@ -274,8 +274,9 @@ contract DotnsNameEscrow is ReleasePosition storage position = _positions[params.tokenId]; // Use `recipient` as the "is this slot funded?" sentinel so zero-amount - // 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. + // positions (seeded by cross-paid registrations, which pay a fee rather than a deposit, + // and by grant-backed reserved registrations, which pay nothing) 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)); @@ -373,8 +374,8 @@ contract DotnsNameEscrow is ReleasePosition storage position = _positions[tokenId]; // Recipient is the canonical "is this position present?" sentinel; zero-amount positions - // seeded for cross-paid registrations are still releasable so every minted name has a - // reachable lifecycle. + // seeded for cross-paid and grant-backed reserved registrations are still releasable so + // every minted name has a reachable lifecycle. require(position.recipient != address(0), DepositNotConfigured(tokenId)); require(!position.released, AlreadyReleased(tokenId)); From 834e3fa2a28a623681a15533f76faed76d3a5353 Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 8 Oct 2026 15:34:11 +0530 Subject: [PATCH 04/12] fix: update readme Signed-off-by: GHkrishna --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 9a0d59333..8183e9c20 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,7 @@ Both are stamped at release time, so a governance change never moves the clocks - `redeem` returns the token and moves no value. The position keeps its recipient, asset and amount, so the deposit stays locked and the name lands back in its exact pre-release state, releasable again later on a fresh pair of clocks. This is the undo for an accidental release. - `withdraw` credits the deposit and forfeits the right to redeem. A holder paid for the name cannot also take it back, otherwise they would hold a name no deposit backs. -Once `redeemableUntil` passes, `reclaim` is permissionless through the ordinary commit-reveal path, whether or not the previous holder ever withdrew. If the position still holds value, reclaim settles it: the amount is credited to the previous holder's pull-payment balance and stays claimable with no deadline. The value follows the departing holder; the name does not wait for them. Cross-paid registrations seed zero-amount positions, so those names have nothing to withdraw and nothing to settle. +Once `redeemableUntil` passes, `reclaim` is permissionless through the ordinary commit-reveal path, whether or not the previous holder ever withdrew. If the position still holds value, reclaim settles it: the amount is credited to the previous holder's pull-payment balance and stays claimable with no deadline. The value follows the departing holder; the name does not wait for them. Cross-paid registrations and grant-backed reserved registrations seed zero-amount positions, so those names have nothing to withdraw and nothing to settle, but they move through release, redeem and reclaim like any other name. Gateway-issued (soulbound) names never enter this lifecycle. Releasing a name transfers it into escrow custody, and a soulbound name reverts on any transfer, so it holds no position and has no release, redeem, or reclaim path. It stays with the person who earned it. @@ -237,6 +237,8 @@ Name grants are the exception path for users or organisations that need to regis A grant is deliberately narrow. It names one label rather than permitting any available name, it names the address the name must mint to, and it is spent by that mint, so it cannot seed a second registration. The gate reads the intended owner rather than the caller, so a relayer may submit the registration on the beneficiary's behalf; the name still lands on the beneficiary. The reserved path writes no reverse record, because the submitter is not necessarily the beneficiary and setReverseName overwrites unconditionally; the owner claims their own primary afterwards through claimReverseRecord. +A granted name is not outside the release lifecycle. The grant is free, so the mint seeds a zero-amount escrow position rather than a refundable deposit, but the position is what `release` checks, so the holder can release the name and, once the redeem window passes, it becomes reclaimable and available again. Reclaiming a reserved-tier label still needs a fresh grant, because `registerReserved` is the only path that mints one; that path reclaims from escrow rather than minting when the escrow still holds the token. + A grant is a governance action. `grantName` is Root-only, so on a production network it is proposed and decided on-chain as a referendum, and on a test network it is a sudo-dispatched call. See [Name grants](./DEPLOYMENTS.md#name-grants-whitelisting) for the dispatch. A grant does not register a name or bypass ownership rules; it only permits the named address to register that one label without a PoP status. From 7d8a505295b23414b038e172cc03c8bb4198c8c5 Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 8 Oct 2026 15:36:22 +0530 Subject: [PATCH 05/12] fix: update deployment docs Signed-off-by: GHkrishna --- DEPLOYMENTS.md | 2 ++ DEPLOYMENT_CHECKLIST.md | 9 ++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/DEPLOYMENTS.md b/DEPLOYMENTS.md index 630e0e9f9..cb72f86d0 100644 --- a/DEPLOYMENTS.md +++ b/DEPLOYMENTS.md @@ -295,6 +295,8 @@ Before deploying to a live network, diff its manifest against `deployments/expec Reserved registration is gated on `DotnsNameWhitelist`. A grant binds one label to one beneficiary address and is single use: `registerReserved` requires a grant naming `registration.owner`, spends it on the mint, and refuses a second attempt. See the [README economics section](./README.md#economics) for what a grant does and does not confer; this section covers the mechanics. +`registerReserved` also needs `nameEscrow` registered, and reverts with `EscrowNotConfigured` without it. The mint charges nothing, but it seeds a zero-amount escrow release position so the granted name stays inside the normal release lifecycle; a name minted without one could never be released and would leave circulation permanently. For the same reason the path reclaims from escrow instead of minting when the escrow still holds the token, which is how a released reserved-tier label is granted again. + **Every admin action on the whitelist is a substrate Root dispatch.** Granting, revoking, accepting, rejecting, reserving, setting the request window and retuning the caps all require it. No signed account can do any of them, the contract owner included: the owner's authority is deployment and upgrade, not allocation. A signed call reverts with `NotGovernance`. There is no operator role and no address allowlist. That is the point of the design. As a security measure, no single key can grant a name; a grant costs a referendum, or on a test network a sudo-dispatched Root call. diff --git a/DEPLOYMENT_CHECKLIST.md b/DEPLOYMENT_CHECKLIST.md index 0cc935183..59b039f83 100644 --- a/DEPLOYMENT_CHECKLIST.md +++ b/DEPLOYMENT_CHECKLIST.md @@ -151,9 +151,12 @@ Confirm these keys are present: - [ ] `DotnsPopLens` The wiring stage already asserts every protocol-registry binding, so a green deploy means they -are set. One is worth confirming by hand, because it is the only key whose absence surfaces to -users rather than to the pipeline: with `nameWhitelist` unset, `registerReserved` reverts -`WhitelistNotConfigured` for every caller, Root included. +are set. Two are worth confirming by hand, because their absence surfaces to users rather than +to the pipeline. With `nameWhitelist` unset, `registerReserved` reverts `WhitelistNotConfigured` +for every grant-backed caller; a Root dispatch skips the whitelist read and is unaffected. With +`nameEscrow` unset it reverts `EscrowNotConfigured` for every caller, Root included, because the +mint seeds the zero-amount release position that keeps a granted name inside the release +lifecycle. ```bash cast call "$PROTOCOL_REGISTRY" "get(bytes32)(address)" \ From 56e6f9da6ab9869dd13de90a576222334d316d37 Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 8 Oct 2026 15:44:32 +0530 Subject: [PATCH 06/12] fix: abstract reclaim and seed Signed-off-by: GHkrishna --- .../registrars/DotnsRegistrarController.sol | 103 +++++++++++------- 1 file changed, 65 insertions(+), 38 deletions(-) diff --git a/contracts/registrars/DotnsRegistrarController.sol b/contracts/registrars/DotnsRegistrarController.sol index 3ffe3a8c0..49064cd27 100644 --- a/contracts/registrars/DotnsRegistrarController.sol +++ b/contracts/registrars/DotnsRegistrarController.sol @@ -235,12 +235,7 @@ contract DotnsRegistrarController is ); if (isReclaim) { - IDotnsNameEscrow(payable(escrow)).reclaim(tokenId, registration.owner); - // Reclaim hands the NFT to the new holder; rewrite the registry record so the prior - // owner's resolver pointer cannot follow the name. Must run after `escrow.reclaim` - // so the registry's `ownerOf` check sees the new holder, not the escrow. - IDotnsRegistry(protocolRegistry.get(DotnsConstants.REGISTRY)) - .setOwner(node, registration.owner); + _reclaimFromEscrow(escrow, tokenId, node, registration.owner); } _settleEscrow(escrow, tokenId, registration.owner, isDirect, totalCharged); @@ -273,11 +268,7 @@ contract DotnsRegistrarController is internal { uint256 depositAmount = isDirect ? chargeAmount : 0; - IDotnsNameEscrow(payable(escrow)).deposit{value: depositAmount}( - IDotnsNameEscrow.DepositParams({ - tokenId: tokenId, asset: address(0), amount: depositAmount, recipient: nameOwner - }) - ); + _seedReleasePosition(escrow, tokenId, nameOwner, depositAmount); if (!isDirect && chargeAmount > 0) { IDotnsNameEscrow(payable(escrow)).depositProtocolFee{value: chargeAmount}( @@ -319,18 +310,13 @@ contract DotnsRegistrarController is whitelist.consume(registration.label, registration.owner); } - IDotnsNameEscrow escrow = IDotnsNameEscrow(payable(_escrow())); + address escrow = _escrow(); uint256 tokenId = uint256(node); // `available` is true both for a never-minted label and for one the escrow still holds - // past its redeem window, so a grant can land on either and the fresh-mint branch alone - // is not enough: `DotnsRegistrar.register` rejects an id the escrow holds. Reserved-tier + // past its redeem window, so a grant can land on either and the fresh-mint branch alone is + // not enough: `DotnsRegistrar.register` rejects an id the escrow holds. Reserved-tier // labels reach circulation only through this function, so without the reclaim branch a // released grant would sit in custody, advertised as available, with no route back out. - // - // One deliberate divergence from @custom:function register: that path also clears a - // sibling controller's stale base-name reservation on reclaim. This one does not, because - // it reads no `IPopRules` state at all, and keeping it that way is what lets the whole - // function stay callable under a Root origin. bool isReclaim = registrar.exists(tokenId); // No reverse record. `setReverseName` overwrites unconditionally, and the gate above lets @@ -341,28 +327,20 @@ contract DotnsRegistrarController is registration, labelhash, node, 0, false, IDotnsReverseResolver(address(0)), isReclaim ); + // One deliberate divergence from @custom:function register, which also clears a sibling + // controller's stale base-name reservation before reclaiming. This path does not, because + // it reads no `IPopRules` state at all, and keeping it that way is what lets the whole + // function stay callable under a Root origin. if (isReclaim) { - escrow.reclaim(tokenId, registration.owner); - // Mirrors @custom:function register: reclaim hands the NFT to the new holder, so the - // registry record is rewritten only afterwards, once `ownerOf` reports that holder - // rather than the escrow. Otherwise the prior owner's resolver pointer follows the - // name. - IDotnsRegistry(protocolRegistry.get(DotnsConstants.REGISTRY)) - .setOwner(node, registration.owner); + _reclaimFromEscrow(escrow, tokenId, node, registration.owner); } - // Seed the release position last, after `reclaim` has deleted whatever the previous holder - // left behind. A grant costs nothing, so there is no deposit to lock, but the position is - // also the lifecycle marker: `release` rejects a token whose position carries no recipient, - // so a name minted without one can never be released, never becomes reclaimable, and never - // returns to circulation. A zero amount keeps that lifecycle reachable while leaving - // nothing to withdraw and nothing for reclaim to settle, which is exactly what the - // cross-payer branch of @custom:function _settleEscrow seeds, for the same reason. - escrow.deposit( - IDotnsNameEscrow.DepositParams({ - tokenId: tokenId, asset: address(0), amount: 0, recipient: registration.owner - }) - ); + // Seed the position last, after `reclaim` has deleted whatever the previous holder left + // behind. A grant costs nothing, so there is no deposit to lock, but the position is also + // the lifecycle marker: `release` rejects a token whose position carries no recipient, so + // a name minted without one can never be released, never becomes reclaimable, and never + // returns to circulation. + _seedReleasePosition(escrow, tokenId, registration.owner, 0); } /// @inheritdoc IERC165 @@ -481,6 +459,55 @@ contract DotnsRegistrarController is require(escrow != address(0), EscrowNotConfigured()); } + /// @notice Takes a name out of escrow custody and repoints the forward registry at its new + /// holder. + /// @dev Shared by both registration paths so the ordering constraint lives in one place: the + /// registry write must follow `escrow.reclaim`, because `setOwner` checks `ownerOf` and + /// would otherwise see the escrow rather than `newOwner`. Without the rewrite the prior + /// owner's resolver pointer follows the name to its new holder. + /// @param escrow Configured name escrow. + /// @param tokenId Token being reclaimed, equal to `uint256(node)`. + /// @param node Namehash of the label, as the forward registry keys it. + /// @param newOwner Address taking custody of the name. + function _reclaimFromEscrow( + address escrow, + uint256 tokenId, + bytes32 node, + address newOwner + ) + internal + { + IDotnsNameEscrow(payable(escrow)).reclaim(tokenId, newOwner); + IDotnsRegistry(protocolRegistry.get(DotnsConstants.REGISTRY)).setOwner(node, newOwner); + } + + /// @notice Opens the escrow release position that every registration path must leave behind. + /// @dev The single construction site for a position, so "every minted name is releasable" is + /// one rule rather than one per path. `amount` is the refundable deposit and may be zero: + /// `DotnsNameEscrow` treats `recipient` as the presence sentinel, so a zero-amount + /// position is still a position. Cross-payer registrations seed zero because the charge + /// goes to the protocol fee pot instead, and grant-backed reserved registrations seed + /// zero because they charge nothing at all. In both cases the position carries no value + /// and exists only so `release`, `redeem` and `reclaim` stay reachable. + /// @param escrow Configured name escrow. + /// @param tokenId Token the position is keyed to. + /// @param recipient Address the deposit (when non-zero) and the lifecycle are keyed to. + /// @param amount Refundable deposit to lock, forwarded as `msg.value`. + function _seedReleasePosition( + address escrow, + uint256 tokenId, + address recipient, + uint256 amount + ) + internal + { + IDotnsNameEscrow(payable(escrow)).deposit{value: amount}( + IDotnsNameEscrow.DepositParams({ + tokenId: tokenId, asset: address(0), amount: amount, recipient: recipient + }) + ); + } + /// @notice Returns the release this network declares it runs, read live from the protocol /// registry so every dotNS contract reports one synchronised value. /// @dev Mirror of `IDotnsProtocolRegistry.protocolVersion`, kept under the historical From 1d2004806525da1d6f4c991e1b6075cb7f21ada4 Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 8 Oct 2026 15:45:41 +0530 Subject: [PATCH 07/12] chore: test for reserved registration requires escrow configured Signed-off-by: GHkrishna --- test/intergration/NameGrantFlow.t.sol | 29 +++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/intergration/NameGrantFlow.t.sol b/test/intergration/NameGrantFlow.t.sol index 0f93dc31b..e0a14190a 100644 --- a/test/intergration/NameGrantFlow.t.sol +++ b/test/intergration/NameGrantFlow.t.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.34; import {BaseDotns, IDotnsRegistrarController} from "../base/BaseDotns.t.sol"; import {IDotnsNameWhitelist} from "../../contracts/whitelist/IDotnsNameWhitelist.sol"; import {IDotnsNameEscrow} from "../../contracts/escrow/IDotnsNameEscrow.sol"; +import {DotnsConstants} from "../../contracts/utils/DotnsConstants.sol"; import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; /// @title NameGrantFlow @@ -182,6 +183,34 @@ contract NameGrantFlow is BaseDotns { ); } + /// @dev The position seeding makes the escrow a hard dependency of this path, where it used to + /// be absent entirely. Root is no exception: it skips the whitelist read, not this one. + function test_reserved_registration_requires_a_configured_escrow() public { + string memory nameLabel = "noescrowgrant01"; + + vm.prank(owner); + protocolRegistry.remove(DotnsConstants.NAME_ESCROW); + + _grantName(nameLabel, ed); + + IDotnsRegistrarController.Registration memory registration = + IDotnsRegistrarController.Registration({ + label: nameLabel, + owner: ed, + secret: keccak256(abi.encodePacked(nameLabel)), + reserved: true, + maxPrice: type(uint256).max, + pricingVersion: popRules.pricingVersion() + }); + + vm.startPrank(ed); + dotnsRegistrarController.commit(dotnsRegistrarController.makeCommitment(registration)); + vm.warp(block.timestamp + dotnsRegistrarController.minCommitmentAge() + 1); + vm.expectRevert(IDotnsRegistrarController.EscrowNotConfigured.selector); + dotnsRegistrarController.registerReserved(registration); + vm.stopPrank(); + } + /// @notice Commit-reveal a reserved registration for `nameOwner`, submitted by `submitter`. function _registerReserved( string memory nameLabel, From f31ea3ee3885071e6409315ab266c6c3edf33d93 Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 8 Oct 2026 15:46:15 +0530 Subject: [PATCH 08/12] fix: invariant test to check every granted name is releaseable Signed-off-by: GHkrishna --- ...DotnsRegistrarReservedGrantInvariant.t.sol | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/invariant/registrar/DotnsRegistrarReservedGrantInvariant.t.sol b/test/invariant/registrar/DotnsRegistrarReservedGrantInvariant.t.sol index aff8791de..51b5f6b3f 100644 --- a/test/invariant/registrar/DotnsRegistrarReservedGrantInvariant.t.sol +++ b/test/invariant/registrar/DotnsRegistrarReservedGrantInvariant.t.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.34; import {BaseDotns} from "../../base/BaseDotns.t.sol"; +import {IDotnsNameEscrow} from "../../../contracts/escrow/IDotnsNameEscrow.sol"; import {ReservedGrantHandler} from "./ReservedGrantHandler.t.sol"; /// @title dotNS Reserved Grant Invariant Suite @@ -108,6 +109,25 @@ contract DotnsRegistrarReservedGrantInvariantTest is BaseDotns { ); } + /// @notice Every granted name stays inside the release lifecycle. The mint is free, so the + /// position it seeds carries no value, but `DotnsNameEscrow.release` gates on the recipient + /// field: a name minted without one can never be released, never becomes reclaimable and + /// never returns to circulation. Asserted over the whole minted set rather than a flag, + /// because the failure is a missing write, which leaves nothing behind to observe. + function invariant_every_granted_name_is_releasable() public view { + string[] memory labels = handler.mintedLabelsList(); + for (uint256 i = 0; i < labels.length; i++) { + uint256 tokenId = handler.tokenIdOf(labels[i]); + IDotnsNameEscrow.ReleasePosition memory position = + dotnsNameEscrow.getReleasePosition(tokenId); + + assertEq( + position.recipient, dotnsRegistrar.ownerOf(tokenId), "no position, or it drifted" + ); + assertEq(position.amount, 0, "a free mint must not lock a refundable deposit"); + } + } + /// @notice No reserved mint writes a reverse record. Beneficiaries here receive names only /// through the reserved path, so their reverse record must stay empty for the whole campaign. function invariant_reserved_path_never_writes_a_reverse_record() public view { From b643d6261abee3a9ea38c2d6f28b9630c4a45aab Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 8 Oct 2026 15:47:43 +0530 Subject: [PATCH 09/12] fix: readme wording regarding reserved name based on its size Signed-off-by: GHkrishna --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8183e9c20..15a80dde0 100644 --- a/README.md +++ b/README.md @@ -237,7 +237,7 @@ Name grants are the exception path for users or organisations that need to regis A grant is deliberately narrow. It names one label rather than permitting any available name, it names the address the name must mint to, and it is spent by that mint, so it cannot seed a second registration. The gate reads the intended owner rather than the caller, so a relayer may submit the registration on the beneficiary's behalf; the name still lands on the beneficiary. The reserved path writes no reverse record, because the submitter is not necessarily the beneficiary and setReverseName overwrites unconditionally; the owner claims their own primary afterwards through claimReverseRecord. -A granted name is not outside the release lifecycle. The grant is free, so the mint seeds a zero-amount escrow position rather than a refundable deposit, but the position is what `release` checks, so the holder can release the name and, once the redeem window passes, it becomes reclaimable and available again. Reclaiming a reserved-tier label still needs a fresh grant, because `registerReserved` is the only path that mints one; that path reclaims from escrow rather than minting when the escrow still holds the token. +A granted name is not outside the release lifecycle. The grant is free, so the mint seeds a zero-amount escrow position rather than a refundable deposit, but the position is what `release` checks, so the holder can release the name and, once the redeem window passes, it becomes reclaimable and available again. A long label that comes back this way can be re-registered by anyone through the ordinary paid path. A reserved-tier label cannot, because `register` refuses it, so it waits for a fresh grant or a direct Root dispatch; either way `registerReserved` reclaims it out of escrow rather than minting, since the token still exists. A grant is a governance action. `grantName` is Root-only, so on a production network it is proposed and decided on-chain as a referendum, and on a test network it is a sudo-dispatched call. See [Name grants](./DEPLOYMENTS.md#name-grants-whitelisting) for the dispatch. From 0f9aac674c0547d7ed4be8e709a66a30533ccd1a Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 8 Oct 2026 16:36:25 +0530 Subject: [PATCH 10/12] fix: minor wordings Signed-off-by: GHkrishna --- README.md | 2 +- .../registrars/DotnsRegistrarController.sol | 8 ++++---- test/intergration/NameGrantFlow.t.sol | 16 +++++++++------- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 15a80dde0..3c24774ff 100644 --- a/README.md +++ b/README.md @@ -237,7 +237,7 @@ Name grants are the exception path for users or organisations that need to regis A grant is deliberately narrow. It names one label rather than permitting any available name, it names the address the name must mint to, and it is spent by that mint, so it cannot seed a second registration. The gate reads the intended owner rather than the caller, so a relayer may submit the registration on the beneficiary's behalf; the name still lands on the beneficiary. The reserved path writes no reverse record, because the submitter is not necessarily the beneficiary and setReverseName overwrites unconditionally; the owner claims their own primary afterwards through claimReverseRecord. -A granted name is not outside the release lifecycle. The grant is free, so the mint seeds a zero-amount escrow position rather than a refundable deposit, but the position is what `release` checks, so the holder can release the name and, once the redeem window passes, it becomes reclaimable and available again. A long label that comes back this way can be re-registered by anyone through the ordinary paid path. A reserved-tier label cannot, because `register` refuses it, so it waits for a fresh grant or a direct Root dispatch; either way `registerReserved` reclaims it out of escrow rather than minting, since the token still exists. +A granted name moves through the same release lifecycle as any other name. The grant is free, so the mint seeds a zero-amount escrow position rather than a refundable deposit, but the position is what `release` checks, so the holder can release the name and, once the redeem window passes, it becomes reclaimable and available again. A long label that comes back this way can be re-registered by anyone through the ordinary paid path. A reserved-tier label cannot, because `register` refuses it, so it waits for a fresh grant or a direct Root dispatch; either way `registerReserved` reclaims it out of escrow rather than minting, since the token still exists. A grant is a governance action. `grantName` is Root-only, so on a production network it is proposed and decided on-chain as a referendum, and on a test network it is a sudo-dispatched call. See [Name grants](./DEPLOYMENTS.md#name-grants-whitelisting) for the dispatch. diff --git a/contracts/registrars/DotnsRegistrarController.sol b/contracts/registrars/DotnsRegistrarController.sol index 49064cd27..b1dd47d05 100644 --- a/contracts/registrars/DotnsRegistrarController.sol +++ b/contracts/registrars/DotnsRegistrarController.sol @@ -313,10 +313,10 @@ contract DotnsRegistrarController is address escrow = _escrow(); uint256 tokenId = uint256(node); // `available` is true both for a never-minted label and for one the escrow still holds - // past its redeem window, so a grant can land on either and the fresh-mint branch alone is - // not enough: `DotnsRegistrar.register` rejects an id the escrow holds. Reserved-tier - // labels reach circulation only through this function, so without the reclaim branch a - // released grant would sit in custody, advertised as available, with no route back out. + // past its redeem window, so a grant can land on either and this path has to serve both: + // `DotnsRegistrar.register` rejects an id the escrow holds, so an existing token is + // reclaimed rather than minted. Reserved-tier labels reach circulation only through this + // function, so a released grant has no other route back out of custody. bool isReclaim = registrar.exists(tokenId); // No reverse record. `setReverseName` overwrites unconditionally, and the gate above lets diff --git a/test/intergration/NameGrantFlow.t.sol b/test/intergration/NameGrantFlow.t.sol index e0a14190a..0e13170bf 100644 --- a/test/intergration/NameGrantFlow.t.sol +++ b/test/intergration/NameGrantFlow.t.sol @@ -107,9 +107,11 @@ contract NameGrantFlow is BaseDotns { assertEq(position.amount, 0, "a Root mint is free, so nothing is refundable"); } - /// @dev The headline regression. Grants are the only route to a reserved-tier label, so before - /// the position was seeded every one governance issued left the pool permanently: the - /// holder could not release, so the name never became reclaimable and never came back. + /// @dev The defect this covers: a reserved registration that seeds no release position leaves + /// its name permanently unreleasable, so the name never becomes reclaimable and never + /// comes back. Grants are the only route to a reserved-tier label, which makes that a + /// one-way exit for every short name governance issues. Walks the whole loop to prove the + /// exit is real: mint, release, wait out the redeem window, grant the label again. function test_a_granted_reserved_tier_name_returns_to_circulation() public { // Three characters, so the label is governance-reserved and `register` cannot mint it. // `registerReserved` is the only way in, which is what makes the exit matter. @@ -154,8 +156,8 @@ contract NameGrantFlow is BaseDotns { assertFalse(position.released, "the new position starts outside the released phase"); } - /// @dev The position is the lifecycle marker, so it has to follow the name. Otherwise a granted - /// name becomes unreleasable again the moment its first holder passes it on. + /// @dev The position is the lifecycle marker, so it has to follow the name. A granted name + /// whose position stayed behind would be unreleasable in its second holder's hands. function test_a_granted_name_keeps_its_position_through_a_transfer() public { string memory nameLabel = "transferredgrant01"; uint256 tokenId = _tokenIdForLabel(nameLabel); @@ -183,8 +185,8 @@ contract NameGrantFlow is BaseDotns { ); } - /// @dev The position seeding makes the escrow a hard dependency of this path, where it used to - /// be absent entirely. Root is no exception: it skips the whitelist read, not this one. + /// @dev Seeding the release position makes the escrow a hard dependency of this path. Root is + /// no exception: it skips the whitelist read, not this one. function test_reserved_registration_requires_a_configured_escrow() public { string memory nameLabel = "noescrowgrant01"; From abcdec1085192b56234150fea77f30c4343fbfe6 Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Tue, 13 Oct 2026 16:08:18 +0530 Subject: [PATCH 11/12] fix: clear stale reservation Signed-off-by: GHkrishna --- .../registrars/DotnsRegistrarController.sol | 64 +++++++++++++------ .../registrars/IDotnsRegistrarController.sol | 4 +- 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/contracts/registrars/DotnsRegistrarController.sol b/contracts/registrars/DotnsRegistrarController.sol index b1dd47d05..74b2db2ec 100644 --- a/contracts/registrars/DotnsRegistrarController.sol +++ b/contracts/registrars/DotnsRegistrarController.sol @@ -175,20 +175,13 @@ contract DotnsRegistrarController is uint256 tokenId = uint256(node); bool isReclaim = registrar.exists(tokenId); - string memory baseName = rules.stripDigits(registration.label); - bool baseNameCanonical = baseName.isSingleLabelMemory(); - // Reclaim hands the name back from a prior occupant who may hold a sibling-controller's - // base-name reservation, which is garbage once the name moves on, so clear it. Non-reclaim - // paths intentionally leave an existing reservation in place: the slot belongs to the - // sibling controller that wrote it (e.g. the PoP queue head stamp), and clearing it from - // here would brick that controller's release and advance paths. - if (baseNameCanonical && isReclaim) { - (address reservationOwner,) = rules.getBaseNameReservation(baseName); - address expectedOwner = - IDotnsNameEscrow(payable(escrow)).getReleasePosition(tokenId).recipient; - if (reservationOwner != address(0) && reservationOwner == expectedOwner) { - rules.releaseReservationForReclaim(baseName, expectedOwner); - } + // Runs before pricing: the price reads below enforce reservations against the new owner, + // so a slot the previous holder left behind would otherwise block this registration. + // Non-reclaim paths intentionally leave an existing reservation in place: the slot belongs + // to the sibling controller that wrote it (e.g. the PoP queue head stamp), and clearing it + // from here would brick that controller's release and advance paths. + if (isReclaim) { + _clearStaleReservation(rules, escrow, tokenId, registration.label); } bool isDirect = msg.sender == registration.owner; @@ -327,11 +320,13 @@ contract DotnsRegistrarController is registration, labelhash, node, 0, false, IDotnsReverseResolver(address(0)), isReclaim ); - // One deliberate divergence from @custom:function register, which also clears a sibling - // controller's stale base-name reservation before reclaiming. This path does not, because - // it reads no `IPopRules` state at all, and keeping it that way is what lets the whole - // function stay callable under a Root origin. if (isReclaim) { + _clearStaleReservation( + IPopRules(protocolRegistry.get(DotnsConstants.POP_RULES)), + escrow, + tokenId, + registration.label + ); _reclaimFromEscrow(escrow, tokenId, node, registration.owner); } @@ -459,6 +454,39 @@ contract DotnsRegistrarController is require(escrow != address(0), EscrowNotConfigured()); } + /// @notice Clears the base-name reservation that the previous holder of a reclaimed name still + /// owns. + /// @dev Shared by both registration paths so their reclaim clean-up cannot drift apart. The + /// previous holder may own a reservation on the name's base name, written by a sibling + /// controller (e.g. the PoP queue head stamp). Once the name moves on, that slot is + /// garbage, and left alone it blocks the base name for everyone else until it expires. + /// Only a slot owned by the previous holder is cleared; a slot held by anyone else is + /// still theirs and stays. Must run before `escrow.reclaim`, which deletes the position + /// the previous holder is read from. Reads no `msg.sender`, so it is safe under a Root + /// origin: `PopRules` sees this controller as its caller either way. + /// @param rules Configured PopRules. + /// @param escrow Configured name escrow. + /// @param tokenId Token being reclaimed. + /// @param label Label being registered. + function _clearStaleReservation( + IPopRules rules, + address escrow, + uint256 tokenId, + string calldata label + ) + internal + { + string memory baseName = rules.stripDigits(label); + if (!baseName.isSingleLabelMemory()) return; + + (address reservationOwner,) = rules.getBaseNameReservation(baseName); + address previousHolder = + IDotnsNameEscrow(payable(escrow)).getReleasePosition(tokenId).recipient; + if (reservationOwner != address(0) && reservationOwner == previousHolder) { + rules.releaseReservationForReclaim(baseName, previousHolder); + } + } + /// @notice Takes a name out of escrow custody and repoints the forward registry at its new /// holder. /// @dev Shared by both registration paths so the ordering constraint lives in one place: the diff --git a/contracts/registrars/IDotnsRegistrarController.sol b/contracts/registrars/IDotnsRegistrarController.sol index 234170eba..1f00d00fc 100644 --- a/contracts/registrars/IDotnsRegistrarController.sol +++ b/contracts/registrars/IDotnsRegistrarController.sol @@ -196,7 +196,9 @@ interface IDotnsRegistrarController is IDotnsController { /// /// Availability covers both a never-minted label and one the escrow still holds past its /// redeem window, so this path reclaims from escrow when the token already exists, exactly as - /// @custom:function register does. It then seeds a zero-amount escrow release position keyed + /// @custom:function register does. Like @custom:function register, a reclaim first clears a + /// base-name reservation that the previous holder still owns, so it does not outlive their + /// hold on the name. It then seeds a zero-amount escrow release position keyed /// to `registration.owner`. The grant is free, so there is nothing to refund, but the position /// is what makes the name releasable: without it the holder cannot release, the name never /// becomes reclaimable, and a reserved-tier label leaves circulation permanently. The escrow From 681ccbf35d88cfda4a09ebe6c233c41d4e9b14f2 Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Tue, 13 Oct 2026 16:08:40 +0530 Subject: [PATCH 12/12] fix: add tests Signed-off-by: GHkrishna --- test/intergration/NameGrantFlow.t.sol | 123 ++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/test/intergration/NameGrantFlow.t.sol b/test/intergration/NameGrantFlow.t.sol index 0e13170bf..e7cda3b60 100644 --- a/test/intergration/NameGrantFlow.t.sol +++ b/test/intergration/NameGrantFlow.t.sol @@ -213,6 +213,129 @@ contract NameGrantFlow is BaseDotns { vm.stopPrank(); } + /// @dev A Root dispatch takes no grant, but the released token still exists, so it has to + /// reclaim rather than mint, the same as the grant path. + function test_a_root_dispatch_reclaims_a_released_grant() public { + string memory nameLabel = "rgov"; + uint256 tokenId = _tokenIdForLabel(nameLabel); + + _grantName(nameLabel, ed); + _registerReserved(nameLabel, ed, ed); + _releaseAndWaitOutRedeemWindow(ed, tokenId); + + _registerReservedAsRoot(nameLabel, tiago); + + assertEq( + IERC721(address(dotnsRegistrar)).ownerOf(tokenId), tiago, "Root reclaimed the name" + ); + assertEq(dotnsRegistry.owner(_nodeOf(nameLabel)), tiago, "the registry follows the holder"); + + IDotnsNameEscrow.ReleasePosition memory position = + dotnsNameEscrow.getReleasePosition(tokenId); + assertEq(position.recipient, tiago, "the reclaim seeds a fresh position for the new holder"); + assertEq(position.amount, 0, "a Root mint is free, so nothing is refundable"); + assertFalse(position.released, "the new position starts outside the released phase"); + } + + /// @dev A long label can come back from a paid registration that still holds its deposit. The + /// reclaim must settle that deposit to the previous holder, not drop it or hand it on. + function test_a_granted_reclaim_credits_the_previous_holders_deposit() public { + string memory nameLabel = "depositbacked01"; + uint256 tokenId = _tokenIdForLabel(nameLabel); + + _commitAndRegister(nameLabel, ed, false); + uint256 deposit = dotnsNameEscrow.getReleasePosition(tokenId).amount; + assertGt(deposit, 0, "a direct registration locks a deposit"); + + _releaseAndWaitOutRedeemWindow(ed, tokenId); + uint256 pendingBefore = dotnsNameEscrow.pendingWithdrawal(ed); + + _grantName(nameLabel, tiago); + _registerReserved(nameLabel, tiago, tiago); + + assertEq( + dotnsNameEscrow.pendingWithdrawal(ed) - pendingBefore, + deposit, + "the previous holder can pull their unwithdrawn deposit" + ); + IDotnsNameEscrow.ReleasePosition memory position = + dotnsNameEscrow.getReleasePosition(tokenId); + assertEq(position.recipient, tiago, "the new holder gets a fresh position"); + assertEq(position.amount, 0, "the old deposit does not carry over to a free grant"); + } + + /// @dev A reservation the previous holder still owns on the name's base name is garbage once + /// the name moves on. Left alone, it blocks the base name for everyone else until it + /// expires. + function test_a_granted_reclaim_clears_the_previous_holders_reservation() public { + _assertReclaimClearsStaleReservation("staleslotgrant", false); + } + + /// @dev Same clean-up under Root. Root only rules out reading `msg.sender` in this frame; + /// `PopRules` still sees the controller as its caller. + function test_a_root_reclaim_clears_the_previous_holders_reservation() public { + _assertReclaimClearsStaleReservation("staleslotroot", true); + } + + /// @dev Only the previous holder's slot is garbage. A slot someone else holds is still theirs. + function test_a_reclaim_keeps_a_reservation_held_by_someone_else() public { + string memory nameLabel = "otherslotgrant"; + uint256 tokenId = _tokenIdForLabel(nameLabel); + + _grantName(nameLabel, ed); + _registerReserved(nameLabel, ed, ed); + _plantReservation(nameLabel, leonardo); + _releaseAndWaitOutRedeemWindow(ed, tokenId); + + _grantName(nameLabel, tiago); + _registerReserved(nameLabel, tiago, tiago); + + (address holder,) = popRules.getBaseNameReservation(nameLabel); + assertEq(holder, leonardo, "a reservation the previous holder did not own survives"); + } + + /// @notice Reclaims `nameLabel` through `registerReserved` while the previous holder still + /// owns a reservation on its base name, and asserts the reservation is gone. + function _assertReclaimClearsStaleReservation(string memory nameLabel, bool asRoot) private { + uint256 tokenId = _tokenIdForLabel(nameLabel); + + _grantName(nameLabel, ed); + _registerReserved(nameLabel, ed, ed); + _plantReservation(nameLabel, ed); + _releaseAndWaitOutRedeemWindow(ed, tokenId); + + (address holder,) = popRules.getBaseNameReservation(nameLabel); + assertEq(holder, ed, "the previous holder still owns a live reservation"); + + if (asRoot) { + _registerReservedAsRoot(nameLabel, tiago); + } else { + _grantName(nameLabel, tiago); + _registerReserved(nameLabel, tiago, tiago); + } + + assertEq(IERC721(address(dotnsRegistrar)).ownerOf(tokenId), tiago, "the name was reclaimed"); + (holder,) = popRules.getBaseNameReservation(nameLabel); + assertEq(holder, address(0), "the previous holder's reservation is cleared"); + } + + /// @notice Writes a base-name reservation for `user`, standing in for a sibling controller + /// such as the PoP queue head. + function _plantReservation(string memory baseName, address user) private { + _authoriseTestAsController(); + popRules.reserveBaseNameForPop(baseName, user); + } + + /// @notice Releases `tokenId` from `holder` into escrow and waits until it is reclaimable. + function _releaseAndWaitOutRedeemWindow(address holder, uint256 tokenId) private { + vm.startPrank(holder); + dotnsRegistrar.approve(address(dotnsNameEscrow), tokenId); + dotnsNameEscrow.release(tokenId); + vm.stopPrank(); + + vm.warp(dotnsNameEscrow.getReleasePosition(tokenId).redeemableUntil); + } + /// @notice Commit-reveal a reserved registration for `nameOwner`, submitted by `submitter`. function _registerReserved( string memory nameLabel,