From 458802495f7e03cb48b38e70fe59e80c9f008ce9 Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 20 Aug 2026 17:07:37 +0530 Subject: [PATCH 01/12] feat: add redeemUntil functionality Signed-off-by: GHkrishna --- contracts/escrow/DotnsNameEscrow.sol | 99 ++++++++++++++----- contracts/escrow/IDotnsNameEscrow.sol | 43 +++++++- scripts/deploy/DeployPolicy.s.sol | 7 +- scripts/deploy/DotnsDeployer.s.sol | 14 ++- test/base/BaseDotns.t.sol | 10 +- test/unit/escrow/DotnsNameEscrowRefunds.t.sol | 7 +- 6 files changed, 152 insertions(+), 28 deletions(-) diff --git a/contracts/escrow/DotnsNameEscrow.sol b/contracts/escrow/DotnsNameEscrow.sol index cc9e5102..1892a925 100644 --- a/contracts/escrow/DotnsNameEscrow.sol +++ b/contracts/escrow/DotnsNameEscrow.sol @@ -35,18 +35,27 @@ contract DotnsNameEscrow is uint256 public constant MAX_REFUND_PAGE_SIZE = 200; /// @notice Upper bound on the configurable release-cooldown. - /// @dev The cooldown gates only the release-to-reclaim window, not the long-lived deposit lock, - /// so it is intentionally kept short. Capping at one hour also keeps the cast to `uint64` - /// well below the saturation point at every plausible block timestamp. + /// @dev The cooldown gates only the release-to-withdraw delay, not the long-lived deposit lock + /// and not the reclaim boundary (see `redeemWindow`), so it is intentionally kept short. + /// Capping at one hour also keeps the cast to `uint64` well below the saturation point at + /// every plausible block timestamp. uint256 public constant MAX_COOLDOWN = 1 hours; + /// @notice Upper bound on the configurable redeem window. + /// @dev The redeem window is a different quantity from the cooldown: it is the period after + /// release in which only the previous holder may act, and it gates reclaim rather than + /// withdrawal. The bound limits how long policy can hold a released name out of + /// circulation, and keeps the cast to `uint64` in release well below saturation. + uint256 public constant MAX_REDEEM_WINDOW = 30 days; + /// @notice The protocol registry for resolving sibling contract addresses. IDotnsProtocolRegistry public protocolRegistry; - /// @notice Cooldown period after release during which refunds can be - /// claimed but not yet reclaimed. - /// @dev Forces a delay between `release` and `reclaim` so the original payer has an - /// uncontested window to pull their refund before the controller hands the name out again. + /// @notice Delay after release before the deposit withdrawal may be credited. + /// @dev Forces a delay between `release` and `withdraw`. It does not bound reclaim: the + /// release-to-reclaim boundary is `redeemWindow`, a separate and longer quantity. Also + /// supplies the per-entry clock for time-locked refund credits, which is why raising it + /// would slow every refund path and not just the deposit one. uint256 public cooldown; /// @notice Total amount of a specific asset reserved across all positions. @@ -90,8 +99,18 @@ contract DotnsNameEscrow is /// @notice Monotonic counter assigning entryIds to new refund credits. uint256 private _nextEntryId; + /// @notice Period after release during which only the previous holder may act. + /// @dev Distinct from `cooldown`. Inside this window the holder may `redeem` the name back + /// and nobody else may take it (`available` reports false); once it elapses `reclaim` + /// becomes permissionless and any unwithdrawn deposit is credited to the recipient + /// rather than stranded. Appended after the pre-existing variables and paid for out of + /// `__gap`, so the layout of everything above is untouched. + uint256 public redeemWindow; + /// @dev Reserved storage space to allow for layout changes in the future. - uint256[50] private __gap; + /// @dev Reduced from 50 to 49 when `redeemWindow` was appended above, keeping the total + /// storage footprint of this contract unchanged. + uint256[49] private __gap; /// @notice Restricts calls to the configured registrar controller. modifier onlyController() { @@ -117,16 +136,18 @@ contract DotnsNameEscrow is /// @custom:function updateCooldown, which rejects a zero value (@custom:reverts /// InvalidCooldown) and any value above @custom:constant MAX_COOLDOWN (@custom:reverts /// CooldownTooLong), and emits @custom:emits CooldownUpdated as part of seeding the - /// initial cooldown. + /// initial cooldown. `redeemWindowSeconds` is forwarded to @custom:function + /// updateRedeemWindow, which rejects a zero value (@custom:reverts InvalidRedeemWindow) + /// and any value above @custom:constant MAX_REDEEM_WINDOW (@custom:reverts + /// RedeemWindowTooLong), and emits @custom:emits RedeemWindowUpdated. /// @param registry Protocol registry used to resolve registrar and controller addresses. /// @param cooldownSeconds Refund cooldown after release. + /// @param redeemWindowSeconds Period after release in which only the previous holder may act. function initialize( IDotnsProtocolRegistry registry, - uint256 cooldownSeconds - ) - external - initializer - { + uint256 cooldownSeconds, + uint256 redeemWindowSeconds + ) external initializer { require(address(registry) != address(0), InvalidAsset()); __Ownable_init(msg.sender); @@ -134,6 +155,7 @@ contract DotnsNameEscrow is protocolRegistry = registry; updateCooldown(cooldownSeconds); + updateRedeemWindow(redeemWindowSeconds); } /// @inheritdoc IDotnsNameEscrow @@ -148,12 +170,25 @@ contract DotnsNameEscrow is } /// @inheritdoc IDotnsNameEscrow - function getReleasePosition(uint256 tokenId) - external - view - override - returns (ReleasePosition memory position) - { + function updateRedeemWindow( + uint256 newRedeemWindow + ) public override onlyOwner { + require(newRedeemWindow != 0, InvalidRedeemWindow()); + require( + newRedeemWindow <= MAX_REDEEM_WINDOW, + RedeemWindowTooLong(newRedeemWindow, MAX_REDEEM_WINDOW) + ); + + uint256 currentRedeemWindow = redeemWindow; + redeemWindow = newRedeemWindow; + + emit RedeemWindowUpdated(currentRedeemWindow, newRedeemWindow); + } + + /// @inheritdoc IDotnsNameEscrow + function getReleasePosition( + uint256 tokenId + ) external view override returns (ReleasePosition memory position) { position = _positions[tokenId]; } @@ -324,22 +359,40 @@ contract DotnsNameEscrow is require(approvedForEscrow, EscrowNotApproved(tokenId)); + // Fail closed on an unseeded window rather than stamping `redeemableUntil` at the current + // timestamp, which would collapse the holder's exclusive redeem phase to zero length and + // open permissionless reclaim the instant the name is released. Only reachable on a proxy + // upgraded without pairing the upgrade with `updateRedeemWindow`. + uint256 currentRedeemWindow = redeemWindow; + require(currentRedeemWindow != 0, InvalidRedeemWindow()); + // Snapshot the position fields once into stack locals so the trailing event emit reuses - // them without three extra warm SLOADs after the state mutation. The cast to `uint64` is - // safe because `cooldown` is bounded by @custom:constant MAX_COOLDOWN. + // them without three extra warm SLOADs after the state mutation. Both casts to `uint64` are + // safe because `cooldown` and `redeemWindow` are bounded by @custom:constant MAX_COOLDOWN + // and @custom:constant MAX_REDEEM_WINDOW respectively. address asset = position.asset; uint256 amount = position.amount; // forge-lint: disable-next-line(unsafe-typecast) uint64 availableAt = uint64(block.timestamp + cooldown); + // forge-lint: disable-next-line(unsafe-typecast) + uint64 redeemUntil = uint64(block.timestamp + currentRedeemWindow); position.withdrawAvailableAt = availableAt; + position.redeemableUntil = redeemUntil; position.released = true; registrar.safeTransferFrom(currentOwner, address(this), tokenId); _addReleasedToken(tokenId); - emit NameReleased(tokenId, msg.sender, asset, amount, availableAt); + emit NameReleased( + tokenId, + msg.sender, + asset, + amount, + availableAt, + redeemUntil + ); } /// @inheritdoc IDotnsNameEscrow diff --git a/contracts/escrow/IDotnsNameEscrow.sol b/contracts/escrow/IDotnsNameEscrow.sol index 69623dc2..f35570ac 100644 --- a/contracts/escrow/IDotnsNameEscrow.sol +++ b/contracts/escrow/IDotnsNameEscrow.sol @@ -53,6 +53,10 @@ interface IDotnsNameEscrow { /// ledger). The position is deleted on `reclaim`, freeing the slot for re-registration. /// @param asset Deposit asset. `address(0)` denotes native token. /// @param withdrawAvailableAt Earliest timestamp at which withdrawal is permitted. + /// @param redeemableUntil Timestamp at which the holder's exclusive redeem window closes and + /// permissionless reclaim opens. Appended last so every pre-existing field keeps its + /// byte offset across the upgrade; it packs into the trailing slot alongside + /// `withdrawAvailableAt`, `released` and `claimed` without consuming a new one. struct ReleasePosition { address recipient; address asset; @@ -60,6 +64,7 @@ interface IDotnsNameEscrow { uint64 withdrawAvailableAt; bool released; bool claimed; + uint64 redeemableUntil; } /// @notice Time-locked refund entry produced when the protocol owes a recipient value @@ -84,12 +89,14 @@ interface IDotnsNameEscrow { /// @param recipient Refund recipient snapshotted at release time. /// @param asset Deposit asset. `address(0)` denotes native token. /// @param withdrawAvailableAt Earliest withdrawal timestamp. + /// @param redeemableUntil Timestamp at which the redeem window closes and reclaim opens. event NameReleased( uint256 indexed tokenId, address indexed recipient, address indexed asset, uint256 amount, - uint256 withdrawAvailableAt + uint256 withdrawAvailableAt, + uint256 redeemableUntil ); /// @notice Emitted when a refund is credited to the recipient's pending balance. @@ -130,6 +137,12 @@ interface IDotnsNameEscrow { /// @notice Emitted when the cooldown duration for future releases is updated. event CooldownUpdated(uint256 indexed currentCooldown, uint256 indexed newCooldown); + /// @notice Emitted when the redeem window for future releases is updated. + event RedeemWindowUpdated( + uint256 indexed currentRedeemWindow, + uint256 indexed newRedeemWindow + ); + /// @notice Emitted when a cross-tier fee is paid into the insurance fund. /// @param payer Original `msg.sender` whose value funded the fee. /// @param isRegistration True when emitted from `depositInsurance`; false from @@ -179,6 +192,17 @@ interface IDotnsNameEscrow { /// @param maxAllowed Upper bound enforced by the contract. error CooldownTooLong(uint256 supplied, uint256 maxAllowed); + /// @notice Thrown when the configured redeem window is invalid. + /// @dev Also thrown by `release` when the window has never been seeded, which fails the release + /// closed rather than collapsing the holder's exclusive redeem phase to zero length. + error InvalidRedeemWindow(); + + /// @notice Thrown when the supplied redeem window exceeds the contract's configured upper + /// bound. + /// @param supplied Redeem window value the caller asked for. + /// @param maxAllowed Upper bound enforced by the contract. + error RedeemWindowTooLong(uint256 supplied, uint256 maxAllowed); + /// @notice Thrown when the supplied amount is invalid. error InvalidAmount(); @@ -326,6 +350,11 @@ interface IDotnsNameEscrow { /// minted name has a reachable lifecycle. The escrow must additionally be approved to /// move the NFT, otherwise @custom:reverts EscrowNotApproved. Emits @custom:emits /// NameReleased once the NFT is moved into custody. + /// Release stamps two independent clocks. `withdrawAvailableAt` (release + `cooldown`) + /// opens the deposit withdrawal; `redeemableUntil` (release + `redeemWindow`) closes the + /// holder's exclusive redeem phase and opens permissionless reclaim. Both are snapshots + /// so later policy changes never move an in-flight position. A release attempted while + /// `redeemWindow` is unseeded triggers @custom:reverts InvalidRedeemWindow. function release(uint256 tokenId) external; /// @notice Credits the refundable deposit for a released token to the recipient's pending @@ -376,6 +405,18 @@ interface IDotnsNameEscrow { /// values. function updateCooldown(uint256 newCooldown) external; + /// @notice Updates the redeem window for future releases. + /// @dev Owner-only. Affects only releases recorded after this call; positions already released + /// keep the `redeemableUntil` snapshot taken at their release time. `newRedeemWindow` must + /// be non-zero, otherwise @custom:reverts InvalidRedeemWindow, and must not exceed the + /// contract's `MAX_REDEEM_WINDOW` upper bound, otherwise @custom:reverts + /// RedeemWindowTooLong; the bound limits how long policy can hold a released name out of + /// circulation and protects the `uint64` cast in release from truncation. Emits + /// @custom:emits RedeemWindowUpdated with the prior and new values. + /// This is also the post-upgrade seeding hook: pair it with `upgradeToAndCall` so an + /// upgraded proxy never runs with an unseeded window. + function updateRedeemWindow(uint256 newRedeemWindow) external; + /// @notice Pulls a single time-locked refund entry. /// @dev Caller must be the entry's recipient (@custom:reverts NotRefundRecipient otherwise), /// the entry must exist (@custom:reverts NoSuchRefundEntry on a deleted or unknown id), diff --git a/scripts/deploy/DeployPolicy.s.sol b/scripts/deploy/DeployPolicy.s.sol index d1aba6c7..9c938032 100644 --- a/scripts/deploy/DeployPolicy.s.sol +++ b/scripts/deploy/DeployPolicy.s.sol @@ -17,6 +17,7 @@ contract DeployPolicy is BaseDeployer { uint64 public constant MIN_COMMITMENT_AGE = 6 seconds; uint64 public constant MAX_COMMITMENT_AGE = 1 days; uint256 public constant ESCROW_COOLDOWN = 15 minutes; + uint256 public constant ESCROW_REDEEM_WINDOW = 1 days; function run() external { address owner = msg.sender; @@ -63,7 +64,11 @@ contract DeployPolicy is BaseDeployer { "DotnsNameEscrow.sol:DotnsNameEscrow", abi.encodeCall( DotnsNameEscrow.initialize, - (IDotnsProtocolRegistry(protocolRegistry), ESCROW_COOLDOWN) + ( + IDotnsProtocolRegistry(protocolRegistry), + ESCROW_COOLDOWN, + ESCROW_REDEEM_WINDOW + ) ), "DotnsNameEscrow" ); diff --git a/scripts/deploy/DotnsDeployer.s.sol b/scripts/deploy/DotnsDeployer.s.sol index 7cc9442d..08074344 100644 --- a/scripts/deploy/DotnsDeployer.s.sol +++ b/scripts/deploy/DotnsDeployer.s.sol @@ -42,6 +42,14 @@ contract DotnsDeployer is BaseDeployer { /// post-deploy via @custom:function DotnsNameEscrow.updateCooldown. uint256 public constant ESCROW_COOLDOWN = 15 minutes; + /// @notice Default redeem window for the freshly-deployed name escrow. + /// @dev The period after a release in which only the previous holder may act: they alone may + /// `redeem` the name back, and `available` reports false so nobody wastes a commitment on + /// it. Once it elapses, reclaim is permissionless. Well below the escrow's + /// @custom:constant MAX_REDEEM_WINDOW ceiling. The protocol owner rotates this post-deploy + /// via @custom:function DotnsNameEscrow.updateRedeemWindow. + uint256 public constant ESCROW_REDEEM_WINDOW = 1 days; + /// @notice Operator address granted `WHITELIST_OPERATOR_ROLE` on the /// registrar controller at fresh-deploy time. /// @dev Permits managing the public-controller whitelist via @@ -309,7 +317,11 @@ contract DotnsDeployer is BaseDeployer { "DotnsNameEscrow.sol:DotnsNameEscrow", abi.encodeCall( DotnsNameEscrow.initialize, - (IDotnsProtocolRegistry(protocolRegistryProxy), ESCROW_COOLDOWN) + ( + IDotnsProtocolRegistry(protocolRegistryProxy), + ESCROW_COOLDOWN, + ESCROW_REDEEM_WINDOW + ) ), "DotnsNameEscrow" ); diff --git a/test/base/BaseDotns.t.sol b/test/base/BaseDotns.t.sol index 105ebfa9..c88d958c 100644 --- a/test/base/BaseDotns.t.sol +++ b/test/base/BaseDotns.t.sol @@ -146,6 +146,10 @@ abstract contract BaseDotns is Test { /// @custom:constant MAX_COOLDOWN ceiling. uint256 public constant ESCROW_COOLDOWN = 15 minutes; + /// @notice Default redeem window for the freshly-deployed name escrow. + /// @custom:constant MAX_REDEEM_WINDOW ceiling. + uint256 public constant ESCROW_REDEEM_WINDOW = 1 days; + /// @notice Zero hash constant. bytes32 public constant ZERO_HASH = bytes32(0); @@ -295,7 +299,11 @@ abstract contract BaseDotns is Test { "DotnsNameEscrow.sol:DotnsNameEscrow", abi.encodeCall( DotnsNameEscrow.initialize, - (IDotnsProtocolRegistry(protocolRegistryAddress), ESCROW_COOLDOWN) + ( + IDotnsProtocolRegistry(protocolRegistryAddress), + ESCROW_COOLDOWN, + ESCROW_REDEEM_WINDOW + ) ) ); dotnsNameEscrow = DotnsNameEscrow(payable(dotnsNameEscrowAddress)); diff --git a/test/unit/escrow/DotnsNameEscrowRefunds.t.sol b/test/unit/escrow/DotnsNameEscrowRefunds.t.sol index 9985ed82..c91719f0 100644 --- a/test/unit/escrow/DotnsNameEscrowRefunds.t.sol +++ b/test/unit/escrow/DotnsNameEscrowRefunds.t.sol @@ -66,7 +66,12 @@ contract DotnsNameEscrowRefundsTest is BaseDotns { // constructor disables initialisers. DotnsNameEscrowRefundHarness impl = new DotnsNameEscrowRefundHarness(); bytes memory initData = abi.encodeCall( - DotnsNameEscrow.initialize, (IDotnsProtocolRegistry(address(protocolRegistry)), 1 hours) + DotnsNameEscrow.initialize, + ( + IDotnsProtocolRegistry(address(protocolRegistry)), + 1 hours, + ESCROW_REDEEM_WINDOW + ) ); address proxy = address(new ERC1967Proxy(address(impl), initData)); harness = DotnsNameEscrowRefundHarness(payable(proxy)); From 5dbc16cc2ee19ae428f79cdf36aff9183ce1af9d Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 20 Aug 2026 17:08:27 +0530 Subject: [PATCH 02/12] chore: forge format Signed-off-by: GHkrishna --- contracts/escrow/DotnsNameEscrow.sol | 29 +++++++++---------- contracts/escrow/IDotnsNameEscrow.sol | 5 +--- contracts/registrars/DotnsRegistrar.sol | 4 ++- contracts/utils/Multicall3.sol | 4 ++- contracts/utils/StringUtils.sol | 4 ++- scripts/deploy/DeployPolicy.s.sol | 6 +--- test/base/BaseDotns.t.sol | 2 +- .../registrar/DotnsPopControllerFuzz.t.sol | 4 ++- test/unit/escrow/DotnsNameEscrowRefunds.t.sol | 6 +--- 9 files changed, 29 insertions(+), 35 deletions(-) diff --git a/contracts/escrow/DotnsNameEscrow.sol b/contracts/escrow/DotnsNameEscrow.sol index 1892a925..f7c0c019 100644 --- a/contracts/escrow/DotnsNameEscrow.sol +++ b/contracts/escrow/DotnsNameEscrow.sol @@ -141,13 +141,16 @@ contract DotnsNameEscrow is /// and any value above @custom:constant MAX_REDEEM_WINDOW (@custom:reverts /// RedeemWindowTooLong), and emits @custom:emits RedeemWindowUpdated. /// @param registry Protocol registry used to resolve registrar and controller addresses. - /// @param cooldownSeconds Refund cooldown after release. + /// @param cooldownSeconds Delay after release before the deposit withdrawal may be credited. /// @param redeemWindowSeconds Period after release in which only the previous holder may act. function initialize( IDotnsProtocolRegistry registry, uint256 cooldownSeconds, uint256 redeemWindowSeconds - ) external initializer { + ) + external + initializer + { require(address(registry) != address(0), InvalidAsset()); __Ownable_init(msg.sender); @@ -170,9 +173,7 @@ contract DotnsNameEscrow is } /// @inheritdoc IDotnsNameEscrow - function updateRedeemWindow( - uint256 newRedeemWindow - ) public override onlyOwner { + function updateRedeemWindow(uint256 newRedeemWindow) public override onlyOwner { require(newRedeemWindow != 0, InvalidRedeemWindow()); require( newRedeemWindow <= MAX_REDEEM_WINDOW, @@ -186,9 +187,12 @@ contract DotnsNameEscrow is } /// @inheritdoc IDotnsNameEscrow - function getReleasePosition( - uint256 tokenId - ) external view override returns (ReleasePosition memory position) { + function getReleasePosition(uint256 tokenId) + external + view + override + returns (ReleasePosition memory position) + { position = _positions[tokenId]; } @@ -385,14 +389,7 @@ contract DotnsNameEscrow is _addReleasedToken(tokenId); - emit NameReleased( - tokenId, - msg.sender, - asset, - amount, - availableAt, - redeemUntil - ); + emit NameReleased(tokenId, msg.sender, asset, amount, availableAt, redeemUntil); } /// @inheritdoc IDotnsNameEscrow diff --git a/contracts/escrow/IDotnsNameEscrow.sol b/contracts/escrow/IDotnsNameEscrow.sol index f35570ac..90fa46be 100644 --- a/contracts/escrow/IDotnsNameEscrow.sol +++ b/contracts/escrow/IDotnsNameEscrow.sol @@ -138,10 +138,7 @@ interface IDotnsNameEscrow { event CooldownUpdated(uint256 indexed currentCooldown, uint256 indexed newCooldown); /// @notice Emitted when the redeem window for future releases is updated. - event RedeemWindowUpdated( - uint256 indexed currentRedeemWindow, - uint256 indexed newRedeemWindow - ); + event RedeemWindowUpdated(uint256 indexed currentRedeemWindow, uint256 indexed newRedeemWindow); /// @notice Emitted when a cross-tier fee is paid into the insurance fund. /// @param payer Original `msg.sender` whose value funded the fee. diff --git a/contracts/registrars/DotnsRegistrar.sol b/contracts/registrars/DotnsRegistrar.sol index 98a80791..7a10b3f9 100644 --- a/contracts/registrars/DotnsRegistrar.sol +++ b/contracts/registrars/DotnsRegistrar.sol @@ -270,7 +270,9 @@ contract DotnsRegistrar is positionSyncNeeded = position.recipient != address(0) && to != position.recipient; } - if (requiredFee == 0 && msg.value == 0 && !positionSyncNeeded) return from; + if (requiredFee == 0 && msg.value == 0 && !positionSyncNeeded) { + return from; + } IDotnsNameEscrow(payable(escrow)).chargeTransferFee{value: msg.value}( IDotnsNameEscrow.ChargeTransferFeeParams({ diff --git a/contracts/utils/Multicall3.sol b/contracts/utils/Multicall3.sol index b95ea19c..273be430 100644 --- a/contracts/utils/Multicall3.sol +++ b/contracts/utils/Multicall3.sol @@ -79,7 +79,9 @@ contract Multicall3 { Result memory result = returnData[i]; call = calls[i]; (result.success, result.returnData) = call.target.call(call.callData); - if (requireSuccess) require(result.success, "Multicall3: call failed"); + if (requireSuccess) { + require(result.success, "Multicall3: call failed"); + } unchecked { ++i; } diff --git a/contracts/utils/StringUtils.sol b/contracts/utils/StringUtils.sol index 30741c2a..80ce03d7 100644 --- a/contracts/utils/StringUtils.sol +++ b/contracts/utils/StringUtils.sol @@ -215,7 +215,9 @@ library StringUtils { { if (end <= start) return false; if (end - start > MAX_DNS_LABEL_OCTETS) return false; - if (label[start] == bytes1(0x2d) || label[end - 1] == bytes1(0x2d)) return false; + if (label[start] == bytes1(0x2d) || label[end - 1] == bytes1(0x2d)) { + return false; + } for (uint256 i = start; i < end; ++i) { bytes1 char = label[i]; diff --git a/scripts/deploy/DeployPolicy.s.sol b/scripts/deploy/DeployPolicy.s.sol index 9c938032..b2251d21 100644 --- a/scripts/deploy/DeployPolicy.s.sol +++ b/scripts/deploy/DeployPolicy.s.sol @@ -64,11 +64,7 @@ contract DeployPolicy is BaseDeployer { "DotnsNameEscrow.sol:DotnsNameEscrow", abi.encodeCall( DotnsNameEscrow.initialize, - ( - IDotnsProtocolRegistry(protocolRegistry), - ESCROW_COOLDOWN, - ESCROW_REDEEM_WINDOW - ) + (IDotnsProtocolRegistry(protocolRegistry), ESCROW_COOLDOWN, ESCROW_REDEEM_WINDOW) ), "DotnsNameEscrow" ); diff --git a/test/base/BaseDotns.t.sol b/test/base/BaseDotns.t.sol index c88d958c..f948683c 100644 --- a/test/base/BaseDotns.t.sol +++ b/test/base/BaseDotns.t.sol @@ -146,7 +146,7 @@ abstract contract BaseDotns is Test { /// @custom:constant MAX_COOLDOWN ceiling. uint256 public constant ESCROW_COOLDOWN = 15 minutes; - /// @notice Default redeem window for the freshly-deployed name escrow. + /// @notice Default escrow redeem window used in tests. Bounded by the escrow's /// @custom:constant MAX_REDEEM_WINDOW ceiling. uint256 public constant ESCROW_REDEEM_WINDOW = 1 days; diff --git a/test/fuzz/registrar/DotnsPopControllerFuzz.t.sol b/test/fuzz/registrar/DotnsPopControllerFuzz.t.sol index 55801745..52c74d47 100644 --- a/test/fuzz/registrar/DotnsPopControllerFuzz.t.sol +++ b/test/fuzz/registrar/DotnsPopControllerFuzz.t.sol @@ -21,7 +21,9 @@ contract DotnsPopControllerFuzz is BaseDotns { // `value` to `[0, 99]` so the resulting suffix matches the `NAMEXX` contract used // throughout the PoP controller tests. function _twoDigitDecimal(uint256 value) internal pure returns (string memory s) { - if (value < 10) return string.concat("0", StringUtils.uintToString(value)); + if (value < 10) { + return string.concat("0", StringUtils.uintToString(value)); + } return StringUtils.uintToString(value); } diff --git a/test/unit/escrow/DotnsNameEscrowRefunds.t.sol b/test/unit/escrow/DotnsNameEscrowRefunds.t.sol index c91719f0..8b353a30 100644 --- a/test/unit/escrow/DotnsNameEscrowRefunds.t.sol +++ b/test/unit/escrow/DotnsNameEscrowRefunds.t.sol @@ -67,11 +67,7 @@ contract DotnsNameEscrowRefundsTest is BaseDotns { DotnsNameEscrowRefundHarness impl = new DotnsNameEscrowRefundHarness(); bytes memory initData = abi.encodeCall( DotnsNameEscrow.initialize, - ( - IDotnsProtocolRegistry(address(protocolRegistry)), - 1 hours, - ESCROW_REDEEM_WINDOW - ) + (IDotnsProtocolRegistry(address(protocolRegistry)), 1 hours, ESCROW_REDEEM_WINDOW) ); address proxy = address(new ERC1967Proxy(address(impl), initData)); harness = DotnsNameEscrowRefundHarness(payable(proxy)); From c3af33d43f05bff13bbeec1e6f680939b6aaebed Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 20 Aug 2026 19:21:17 +0530 Subject: [PATCH 03/12] fix: update deposit settling, reedem func and reclaim Signed-off-by: GHkrishna --- contracts/escrow/DotnsNameEscrow.sol | 79 +++++++++++++++++++++++++-- contracts/escrow/IDotnsNameEscrow.sol | 48 ++++++++++++++-- 2 files changed, 117 insertions(+), 10 deletions(-) diff --git a/contracts/escrow/DotnsNameEscrow.sol b/contracts/escrow/DotnsNameEscrow.sol index f7c0c019..ce0179ad 100644 --- a/contracts/escrow/DotnsNameEscrow.sol +++ b/contracts/escrow/DotnsNameEscrow.sol @@ -404,11 +404,40 @@ contract DotnsNameEscrow is WithdrawalTooEarly(tokenId, position.withdrawAvailableAt, block.timestamp) ); - uint256 owed = position.amount; - address asset = position.asset; // `position.recipient == msg.sender` was just enforced above, so reuse the local in place // of an extra warm SLOAD. - address recipient = msg.sender; + _settleDeposit(position, tokenId, msg.sender); + } + + /// @notice Moves a position's outstanding deposit onto the recipient's pull-payment balance. + /// @dev Shared by @custom:function withdraw, where the recipient pulls the deposit themselves, + /// and by @custom:function reclaim, where a third party takes the name and the deposit is + /// settled on the departing holder's behalf. Both credit the same ledger and neither + /// transfers value, so the accounting is identical and lives here once. Draws from the + /// per-asset `tokenReserved` pool first and tops up from `insuranceFund` on shortfall; + /// @custom:reverts InsufficientFunds when even the combined balance cannot cover the + /// amount owed. Emits @custom:emits RefundWithdrawn, and @custom:emits InsuranceDraw + /// whenever the insurance fund contributes. + /// A zero-amount position is a no-op: it writes nothing and emits nothing, which keeps the + /// free-registration lifecycle free of meaningless ledger entries and events. + /// @param position Storage pointer to the position being settled. + /// @param recipient Address credited with the deposit. Always the position recipient. + function _settleDeposit( + ReleasePosition storage position, + uint256 tokenId, + address recipient + ) + private + { + uint256 owed = position.amount; + address asset = position.asset; + + // Effects: flag the position settled regardless of amount so `claimed` remains a faithful + // record of "the deposit for this position has been dealt with". + position.claimed = true; + + if (owed == 0) return; + uint256 reserved = tokenReserved[asset]; uint256 fromRefundable; @@ -425,8 +454,6 @@ contract DotnsNameEscrow is ); } - // Effects: mutate state only after all checks have passed. - position.claimed = true; position.amount = 0; tokenReserved[asset] -= fromRefundable; if (fromInsurance > 0) { @@ -647,10 +674,23 @@ contract DotnsNameEscrow is { ReleasePosition storage position = _positions[tokenId]; - require(position.released && position.claimed, NotReclaimable(tokenId)); + // The gate is the elapsed redeem window, not the `claimed` flag. Gating on `claimed` made + // recyclability depend on the previous holder choosing to withdraw, which strands the name + // forever whenever they have no reason to: a zero-amount position has nothing to collect, + // so "never withdraws" is the default rather than the exception. The window bounds the + // wait instead, and any unwithdrawn value is settled below rather than held hostage. + require( + position.released && block.timestamp >= position.redeemableUntil, + NotReclaimable(tokenId) + ); address previousRecipient = position.recipient; + // Settle before deleting: the departing holder keeps their claim on the deposit even though + // they are losing the name. `_settleDeposit` is a no-op for a zero-amount position and for + // one already withdrawn, so the common paths cost nothing extra. + _settleDeposit(position, tokenId, previousRecipient); + delete _positions[tokenId]; _removeReleasedToken(tokenId); @@ -659,6 +699,33 @@ contract DotnsNameEscrow is emit NameReclaimed(tokenId, previousRecipient, newOwner); } + /// @inheritdoc IDotnsNameEscrow + function redeem(uint256 tokenId) external override nonReentrant { + ReleasePosition storage position = _positions[tokenId]; + + require(position.recipient == msg.sender, NotRefundRecipient(msg.sender, tokenId)); + // One error for the whole state predicate: unreleased, already withdrawn, or past the + // window are all simply "not redeemable" from the caller's point of view, and collapsing + // them avoids leaking a three-way state machine into the revert surface. + require( + position.released && !position.claimed && block.timestamp < position.redeemableUntil, + NotRedeemable(tokenId) + ); + + // Restore the pre-release state and nothing more. Recipient, asset and amount are left + // untouched so the deposit stays locked against the name; clearing the clocks means a later + // release starts a fresh pair rather than inheriting stale deadlines. + position.released = false; + position.withdrawAvailableAt = 0; + position.redeemableUntil = 0; + + _removeReleasedToken(tokenId); + + _registrar().safeTransferFrom(address(this), msg.sender, tokenId); + + emit NameRedeemed(tokenId, msg.sender); + } + /// @inheritdoc IERC721Receiver function onERC721Received( address, diff --git a/contracts/escrow/IDotnsNameEscrow.sol b/contracts/escrow/IDotnsNameEscrow.sol index 90fa46be..798e3e78 100644 --- a/contracts/escrow/IDotnsNameEscrow.sol +++ b/contracts/escrow/IDotnsNameEscrow.sol @@ -140,6 +140,12 @@ interface IDotnsNameEscrow { /// @notice Emitted when the redeem window for future releases is updated. event RedeemWindowUpdated(uint256 indexed currentRedeemWindow, uint256 indexed newRedeemWindow); + /// @notice Emitted when a released token is redeemed by its previous holder. + /// @dev The counterpart to @custom:emits NameReleased: custody returns to `recipient` and the + /// deposit stays locked, so no value event accompanies this. + /// @param recipient Address the NFT was returned to, which is also the position recipient. + event NameRedeemed(uint256 indexed tokenId, address indexed recipient); + /// @notice Emitted when a cross-tier fee is paid into the insurance fund. /// @param payer Original `msg.sender` whose value funded the fee. /// @param isRegistration True when emitted from `depositInsurance`; false from @@ -221,9 +227,19 @@ interface IDotnsNameEscrow { /// @notice Thrown when the refund has already been claimed. error AlreadyClaimed(uint256 tokenId); - /// @notice Thrown when a token is not in a reclaimable state (released + claimed). + /// @notice Thrown when a token is not in a reclaimable state. + /// @dev Reclaimable means released with the redeem window elapsed. A released token still + /// inside its window is deliberately not reclaimable: that window belongs to the previous + /// holder. Whether the deposit was withdrawn is irrelevant, because reclaim settles any + /// unwithdrawn amount itself. error NotReclaimable(uint256 tokenId); + /// @notice Thrown when a token is not in a redeemable state. + /// @dev Redeemable means released, not yet withdrawn, and still inside the redeem window. + /// A withdrawn position is excluded on purpose: the holder has already taken the deposit + /// value out, so returning the name as well would leave it unbacked. + error NotRedeemable(uint256 tokenId); + /// @notice Thrown when escrow is not approved to transfer the token. error EscrowNotApproved(uint256 tokenId); @@ -384,14 +400,38 @@ interface IDotnsNameEscrow { /// `claimWithdrawal`. function pendingWithdrawal(address recipient) external view returns (uint256 amount); - /// @notice Transfers a released-and-claimed token from escrow custody to a new owner. + /// @notice Transfers a released token whose redeem window has elapsed to a new owner. /// @dev Hands the NFT back to the controller for re-registration. Only the configured /// controller may call this, otherwise @custom:reverts NotController, and the position - /// must be both released and claimed, otherwise @custom:reverts NotReclaimable. Emits - /// @custom:emits NameReclaimed once custody is transferred. + /// must be released with `redeemableUntil` reached, otherwise @custom:reverts + /// NotReclaimable. Emits @custom:emits NameReclaimed once custody is transferred. + /// Reclaim does not require the deposit to have been withdrawn first. If the position + /// still holds value, this call settles it: the amount is debited from `tokenReserved` + /// (topping up from the insurance fund on shortfall, @custom:reverts InsufficientFunds if + /// even the combined balance is short) and credited to the previous recipient's + /// pull-payment balance, claimable through @custom:function claimWithdrawal with no + /// deadline. That is what keeps a name recyclable when its previous holder never returns: + /// the value follows them, the name does not wait for them. Emits @custom:emits + /// RefundWithdrawn on settlement, and @custom:emits InsuranceDraw when the insurance fund + /// tops up a shortfall. /// @param newOwner Address of the new registrant taking over the name. function reclaim(uint256 tokenId, address newOwner) external; + /// @notice Returns a released token to its previous holder during the redeem window. + /// @dev The undo for an accidental release, and the reason the redeem window exists. Only the + /// position recipient may call this (@custom:reverts NotRefundRecipient otherwise), the + /// position must be released and not yet withdrawn, and `block.timestamp` must still be + /// below `redeemableUntil`; a position failing any of those is not redeemable and + /// @custom:reverts NotRedeemable. + /// No value moves. The position keeps its recipient, asset and amount, so the deposit + /// stays locked exactly as it was before the release and the name returns to its + /// pre-release state, releasable again later on a fresh pair of clocks. Excluding + /// withdrawn positions is deliberate: a holder who has already pulled the deposit would + /// otherwise recover the name without it being deposit-backed, breaking the one-deposit- + /// per-live-name bound. The choice is therefore exclusive — take the value back, or take + /// the name back. Emits @custom:emits NameRedeemed once custody returns. + function redeem(uint256 tokenId) external; + /// @notice Updates the cooldown duration for future releases. /// @dev Owner-only. Affects only releases recorded after this call; positions already released /// keep the `withdrawAvailableAt` snapshot taken at their release time. `newCooldown` From a3339559095a6f84ba130ebb7561b72efec28a7c Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 20 Aug 2026 19:22:01 +0530 Subject: [PATCH 04/12] fix: name availability Signed-off-by: GHkrishna --- contracts/registrars/DotnsRegistrar.sol | 14 +++++++++++++- contracts/registrars/IDotnsRegistrar.sol | 14 ++++++++++---- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/contracts/registrars/DotnsRegistrar.sol b/contracts/registrars/DotnsRegistrar.sol index 7a10b3f9..7f83dcf8 100644 --- a/contracts/registrars/DotnsRegistrar.sol +++ b/contracts/registrars/DotnsRegistrar.sol @@ -102,7 +102,19 @@ contract DotnsRegistrar is function available(uint256 id) public view override returns (bool isAvailable) { address holder = _ownerOf(id); if (holder == address(0)) return true; - return holder == protocolRegistry.get(DotnsConstants.NAME_ESCROW); + + address escrow = protocolRegistry.get(DotnsConstants.NAME_ESCROW); + if (holder != escrow) return false; + + // Escrow custody on its own no longer means registrable. While a released position is + // inside its redeem window the name still belongs to its previous holder, and reclaim + // would revert with NotReclaimable. Reporting it available there would advertise the name + // as free and send registrants through an entire commit-reveal cycle that cannot succeed, + // so availability tracks the window rather than custody. + IDotnsNameEscrow.ReleasePosition memory position = + IDotnsNameEscrow(payable(escrow)).getReleasePosition(id); + + return block.timestamp >= position.redeemableUntil; } /// @inheritdoc IDotnsRegistrar diff --git a/contracts/registrars/IDotnsRegistrar.sol b/contracts/registrars/IDotnsRegistrar.sol index dab246f0..92b978f9 100644 --- a/contracts/registrars/IDotnsRegistrar.sol +++ b/contracts/registrars/IDotnsRegistrar.sol @@ -57,10 +57,16 @@ interface IDotnsRegistrar is IERC721 { /// @notice Returns whether a registration call may proceed for `id`. /// @dev Signals two distinct paths to the controller. Returns `true` when the owner slot is /// empty (a fresh @custom:function register call may mint) AND when the current owner is - /// the configured escrow (the controller must then route through - /// @custom:function IDotnsNameEscrow.reclaim instead of @custom:function register, because - /// `register` calls `_mint` which rejects existing tokens). All other holders return - /// `false`. The controller distinguishes the two `true` cases via @custom:function exists. + /// the configured escrow and the released position's redeem window has elapsed (the + /// controller must then route through @custom:function IDotnsNameEscrow.reclaim instead of + /// @custom:function register, because `register` calls `_mint` which rejects existing + /// tokens). All other holders return `false`. The controller distinguishes the two `true` + /// cases via @custom:function exists. + /// Escrow custody inside the redeem window returns `false`: that window belongs to the + /// previous holder, who may still @custom:function IDotnsNameEscrow.redeem the name, and + /// reclaim would revert until it elapses. Clients wanting the exact moment a released name + /// becomes registrable should read `redeemableUntil` from + /// @custom:function IDotnsNameEscrow.getReleasePosition. function available(uint256 id) external view returns (bool isAvailable); /// @notice Registers a name permanently. From 1f713a6f2f90a741d4a18db1cb97478af94ff1dc Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 20 Aug 2026 19:23:46 +0530 Subject: [PATCH 05/12] fix: tests Signed-off-by: GHkrishna --- .../escrow/DotnsNameEscrowInvariant.t.sol | 55 ++- test/invariant/escrow/EscrowHandler.t.sol | 103 ++++- .../DotnsRegistrarControllerInvariant.t.sol | 26 +- .../RegistrarControllerHandler.t.sol | 7 + test/unit/escrow/DotnsNameEscrow.t.sol | 4 + test/unit/escrow/DotnsNameEscrowRedeem.t.sol | 432 ++++++++++++++++++ test/unit/registrar/DotnsRegistrar.t.sol | 40 +- .../DotnsRegistrarControllerLifecycle.t.sol | 9 + 8 files changed, 652 insertions(+), 24 deletions(-) create mode 100644 test/unit/escrow/DotnsNameEscrowRedeem.t.sol diff --git a/test/invariant/escrow/DotnsNameEscrowInvariant.t.sol b/test/invariant/escrow/DotnsNameEscrowInvariant.t.sol index 1b2ca363..d2545e6a 100644 --- a/test/invariant/escrow/DotnsNameEscrowInvariant.t.sol +++ b/test/invariant/escrow/DotnsNameEscrowInvariant.t.sol @@ -29,7 +29,7 @@ contract DotnsNameEscrowInvariantTest is BaseDotns { targetContract(address(handler)); - bytes4[] memory selectors = new bytes4[](10); + bytes4[] memory selectors = new bytes4[](12); selectors[0] = handler.commitRegisterAndDeposit.selector; selectors[1] = handler.registerCrossTier.selector; selectors[2] = handler.releaseToken.selector; @@ -40,6 +40,10 @@ contract DotnsNameEscrowInvariantTest is BaseDotns { selectors[7] = handler.transferDeposited.selector; selectors[8] = handler.transferPayable.selector; selectors[9] = handler.advanceTime.selector; + // The two halves of the redeem window. Without both, the fuzzer can only reach reclaim by + // way of a withdrawal, which is precisely the assumption the reclaim deadlock rested on. + selectors[10] = handler.redeemReleased.selector; + selectors[11] = handler.reRegisterReleased.selector; targetSelector(FuzzSelector({addr: address(handler), selectors: selectors})); excludeContract(address(dotnsRegistrarController)); @@ -158,9 +162,12 @@ contract DotnsNameEscrowInvariantTest is BaseDotns { } } - /// @notice Every withdrawn-but-not-reclaimed token must be held by escrow and available. - /// @dev Under the custody model, withdrawn tokens stay in escrow custody until a new - /// registrant reclaims them. They must remain `available()` for re-registration. + /// @notice Every withdrawn-but-not-reclaimed token must be held by escrow, and available + /// exactly when its redeem window has elapsed. + /// @dev Withdrawn tokens stay in escrow custody until a new registrant reclaims them. Custody + /// alone no longer implies availability: withdrawing does not shorten the previous + /// holder's redeem window, so a withdrawn position can still be inside it. Availability + /// is therefore asserted against the window rather than unconditionally. function invariant_withdrawn_tokens_are_in_escrow_custody_and_available() public view { uint256[] memory withdrawn = handler.getWithdrawnTokenIds(); @@ -172,9 +179,47 @@ contract DotnsNameEscrowInvariantTest is BaseDotns { address(dotnsNameEscrow), "Withdrawn token must be held by escrow" ); + + IDotnsNameEscrow.ReleasePosition memory position = + dotnsNameEscrow.getReleasePosition(tokenId); + + assertEq( + dotnsRegistrar.available(tokenId), + block.timestamp >= position.redeemableUntil, + "Withdrawn token is available exactly once its redeem window has elapsed" + ); + } + } + + /// @notice No released token can ever be stuck: it is always either redeemable or reclaimable. + /// @dev This is the property the bug violated, stated directly. Under the old + /// `released && claimed` reclaim gate a released position whose holder never withdrew was + /// neither redeemable (no such call existed) nor reclaimable (the flag was never set), so + /// the name left circulation permanently. The two phases must tile the whole timeline with + /// no gap, and must not overlap -- an overlap would mean the previous holder and a new + /// registrant could both act on the same name. + function invariant_released_tokens_are_never_stuck() public view { + uint256[] memory released = handler.getReleasedTokenIds(); + + for (uint256 i; i < released.length; ++i) { + uint256 tokenId = released[i]; + + IDotnsNameEscrow.ReleasePosition memory position = + dotnsNameEscrow.getReleasePosition(tokenId); + + if (!position.released) continue; + + bool insideWindow = block.timestamp < position.redeemableUntil; + // Withdrawing forfeits the redeem right, but it cannot strand the name: reclaim opens + // on the same boundary regardless. + bool redeemable = insideWindow && !position.claimed; + bool reclaimable = !insideWindow; + assertTrue( - dotnsRegistrar.available(tokenId), "Withdrawn token must be available for reclaim" + redeemable || reclaimable, + "A released position must always be redeemable or reclaimable, never neither" ); + assertFalse(redeemable && reclaimable, "The redeem and reclaim phases must not overlap"); } } } diff --git a/test/invariant/escrow/EscrowHandler.t.sol b/test/invariant/escrow/EscrowHandler.t.sol index b8459fa6..1604ba7a 100644 --- a/test/invariant/escrow/EscrowHandler.t.sol +++ b/test/invariant/escrow/EscrowHandler.t.sol @@ -269,13 +269,9 @@ contract EscrowHandler is Test { uint256 index = tokenSeed % _depositedTokenIds.length; uint256 tokenId = _depositedTokenIds[index]; - // Only release if the deposit has a non-zero amount (NoStatus names with a refundable - // position) - if (depositAmounts[tokenId] == 0) { - _removeDeposited(index); - return; - } - + // Zero-amount positions are released too. They used to be skipped here, which meant the + // fuzzer never explored the exact state the reclaim deadlock lived in: a released position + // with nothing to withdraw, and therefore no reason for its holder ever to call `withdraw`. address tokenOwner = registrar.ownerOf(tokenId); IDotnsNameEscrow.ReleasePosition memory positionBefore = escrow.getReleasePosition(tokenId); @@ -338,6 +334,91 @@ contract EscrowHandler is Test { _accountInsuranceDraws(logs); } + /// @notice Redeems a released token back to its previous holder inside the redeem window. + /// @dev Picks from `_releasedTokenIds` and only acts while the position is still redeemable + /// (released, unwithdrawn, inside the window). Moves the token back to + /// `_depositedTokenIds` because a redeem restores the pre-release state exactly: the + /// deposit is still locked and the name is releasable again. No value moves, so no ghost + /// accounting changes. + /// @param tokenSeed Seed for selecting which released token to redeem. + function redeemReleased(uint256 tokenSeed) external { + if (_releasedTokenIds.length == 0) return; + + uint256 index = tokenSeed % _releasedTokenIds.length; + uint256 tokenId = _releasedTokenIds[index]; + + IDotnsNameEscrow.ReleasePosition memory position = escrow.getReleasePosition(tokenId); + if (!position.released || position.claimed) return; + if (block.timestamp >= position.redeemableUntil) return; + + vm.prank(position.recipient); + escrow.redeem(tokenId); + + _depositedTokenIds.push(tokenId); + _removeReleased(index); + } + + /// @notice Re-registers a released token whose window elapsed, without any prior withdrawal. + /// @dev The path the old `released && claimed` gate made unreachable, and the reason this + /// action exists separately from `reRegisterReclaimed`: that one draws from + /// `_withdrawnTokenIds`, so nothing ever exercised reclaim against an unwithdrawn + /// position. Reclaim settles any outstanding deposit onto the previous recipient's + /// pull-payment balance, so the credit is mirrored into `ghost_pendingCredits`. + /// @param tokenSeed Seed for selecting which released token to re-register. + /// @param actorSeed Seed selecting the new registrant. + function reRegisterReleased(uint256 tokenSeed, uint256 actorSeed) external { + if (_releasedTokenIds.length == 0 || actors.length == 0) return; + + uint256 index = tokenSeed % _releasedTokenIds.length; + uint256 tokenId = _releasedTokenIds[index]; + string memory label = labelByTokenId[tokenId]; + address actor = actors[actorSeed % actors.length]; + + IDotnsNameEscrow.ReleasePosition memory position = escrow.getReleasePosition(tokenId); + if (!position.released) return; + if (block.timestamp < position.redeemableUntil) { + vm.warp(position.redeemableUntil); + } + + // Outstanding value on the position is what reclaim will settle. A position already + // withdrawn carries a zero amount, so this is naturally zero for those. + uint256 outstanding = position.amount; + + bytes32 secret = keccak256(abi.encodePacked(label, actor, block.timestamp, labelNonce)); + + IDotnsRegistrarController.Registration memory registration = + IDotnsRegistrarController.Registration({ + label: label, owner: actor, secret: secret, reserved: true + }); + + bytes32 commitment = controller.makeCommitment(registration); + + vm.prank(actor); + controller.commit(commitment); + + uint256 minAge = controller.minCommitmentAge(); + vm.warp(block.timestamp + minAge + 1); + + uint256 price = popRules.priceWithCheck(label, actor).price; + + vm.recordLogs(); + vm.prank(actor); + try controller.register{value: price}(registration) { + Vm.Log[] memory logs = vm.getRecordedLogs(); + + _removeReleased(index); + _depositedTokenIds.push(tokenId); + depositAmounts[tokenId] = price; + depositRecipients[tokenId] = address(0); + labelByTokenId[tokenId] = label; + + ghost_pendingCredits += outstanding; + _accountInsuranceDraws(logs); + } catch { + return; + } + } + /// @notice Pulls the caller's accumulated pending refund balance. /// @dev Walks the actor list, picks one with a non-zero pending balance, and calls /// `claimWithdrawal()` from that actor. Revert-safe: returns early when there is @@ -412,6 +493,14 @@ contract EscrowHandler is Test { string memory label = labelByTokenId[tokenId]; address actor = actors[actorSeed % actors.length]; + // Reclaim is gated on the redeem window, not on the withdrawal. Without this warp every + // attempt would revert NotReclaimable and be swallowed by the try/catch below, so the + // action would look like it was running while covering nothing. + IDotnsNameEscrow.ReleasePosition memory position = escrow.getReleasePosition(tokenId); + if (block.timestamp < position.redeemableUntil) { + vm.warp(position.redeemableUntil); + } + bytes32 secret = keccak256(abi.encodePacked(label, actor, block.timestamp, labelNonce)); IDotnsRegistrarController.Registration memory registration = diff --git a/test/invariant/registrar/DotnsRegistrarControllerInvariant.t.sol b/test/invariant/registrar/DotnsRegistrarControllerInvariant.t.sol index 7252fe01..3607d9b5 100644 --- a/test/invariant/registrar/DotnsRegistrarControllerInvariant.t.sol +++ b/test/invariant/registrar/DotnsRegistrarControllerInvariant.t.sol @@ -101,25 +101,33 @@ contract DotnsRegistrarControllerInvariantTest is BaseDotns { assertEq(address(dotnsRegistrarController).balance, 0, "Controller must not hold funds"); } + /// @notice The escrow always holds at least the value it owes across every ledger. + /// @dev Previously asserted strict equality against reserves + insurance + pending withdrawals + /// while iterating only the first five actors and omitting the time-locked refund ledger + /// entirely. Both were latent: any refund entry, or a sixth actor holding a pending + /// balance, made the equality wrong for reasons unrelated to solvency. Now that reclaim + /// settles unwithdrawn deposits onto the pull-payment ledger, more paths reach that state, + /// so the assertion is stated as the property that actually matters -- the escrow is never + /// short -- over every actor and every ledger. + /// Balance may legitimately exceed the sum: force-sent value (`selfdestruct`) is + /// unaccounted-for surplus the escrow has no ledger for, which is why this is `assertGe`. + /// The time-locked refund ledger is not summed here because this handler does not drive + /// it; `DotnsNameEscrowInvariant.invariant_solvency` covers all four ledgers together. function invariant_value_conservation() public view { - // Escrow balance equals reserves + insurance + pending withdrawals. uint256 reservedAmount = dotnsNameEscrow.reserves(address(0)); uint256 insurance = dotnsNameEscrow.insuranceFund(); + address[] memory actorList = handler.getActors(); uint256 pendingTotal; - for (uint256 i; i < 5; ++i) { - try handler.actors(i) returns (address actor) { - pendingTotal += dotnsNameEscrow.pendingWithdrawal(actor); - } catch { - break; - } + for (uint256 i; i < actorList.length; ++i) { + pendingTotal += dotnsNameEscrow.pendingWithdrawal(actorList[i]); } uint256 escrowBalance = address(dotnsNameEscrow).balance; - assertEq( + assertGe( escrowBalance, reservedAmount + insurance + pendingTotal, - "Escrow balance must equal reserves + insurance + pending withdrawals" + "Escrow balance must cover reserves + insurance + pending withdrawals" ); } diff --git a/test/invariant/registrar/RegistrarControllerHandler.t.sol b/test/invariant/registrar/RegistrarControllerHandler.t.sol index 3f5822d2..5516ea85 100644 --- a/test/invariant/registrar/RegistrarControllerHandler.t.sol +++ b/test/invariant/registrar/RegistrarControllerHandler.t.sol @@ -47,6 +47,13 @@ contract RegistrarControllerHandler is Test { /// @notice Actor pool the handler cycles through. address[] public actors; + /// @notice Returns the full actor list. + /// @dev Exists so invariants can sum a per-actor ledger across every actor rather than probing + /// `actors(i)` up to a hardcoded index and silently ignoring the rest. + function getActors() external view returns (address[] memory list) { + list = actors; + } + /// @notice Tracks the PoP status assigned to each registered actor. mapping(address actor => IPopRules.PopStatus status) public actorStatus; diff --git a/test/unit/escrow/DotnsNameEscrow.t.sol b/test/unit/escrow/DotnsNameEscrow.t.sol index c6f2740a..06496ea1 100644 --- a/test/unit/escrow/DotnsNameEscrow.t.sol +++ b/test/unit/escrow/DotnsNameEscrow.t.sol @@ -137,6 +137,10 @@ contract DotnsNameEscrowTest is BaseDotns { "escrow holds token before reclaim" ); + // Withdrawing no longer opens reclaim on its own: the redeem window has to elapse, because + // that window is the previous holder's exclusive claim on the name. + vm.warp(block.timestamp + ESCROW_REDEEM_WINDOW + 1); + vm.prank(address(dotnsRegistrarController)); dotnsNameEscrow.reclaim(tokenId, leonardo); diff --git a/test/unit/escrow/DotnsNameEscrowRedeem.t.sol b/test/unit/escrow/DotnsNameEscrowRedeem.t.sol new file mode 100644 index 00000000..f872459c --- /dev/null +++ b/test/unit/escrow/DotnsNameEscrowRedeem.t.sol @@ -0,0 +1,432 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.34; + +import {BaseDotns} from "../../base/BaseDotns.t.sol"; +import {IDotnsNameEscrow} from "../../../contracts/escrow/IDotnsNameEscrow.sol"; +import {IPopRules} from "../../../contracts/pop/IPopRules.sol"; + +/// @title DotnsNameEscrowRedeemTest +/// @notice Unit tests for the two-phase release lifecycle on @custom:contract DotnsNameEscrow: +/// the previous holder's exclusive redeem window, permissionless reclaim once it elapses, +/// and the deposit settlement that makes the second possible without stranding value. +/// @dev The defect these cover: reclaim used to gate on the `claimed` flag, which is only set by +/// `withdraw`. A holder who released a name and never withdrew removed the label from +/// circulation permanently. For the zero-amount positions seeded by free registrations there +/// is nothing to withdraw, so that was the default outcome rather than an edge case. +contract DotnsNameEscrowRedeemTest is BaseDotns { + /// @notice 14-char label classifying as NoStatus, so registration seeds a funded position. + string internal constant FUNDED_LABEL = "redeemlabela01"; + + /// @notice 6-char digit-free label classifying as PopFull: registration is free, so the + /// position it seeds carries a zero amount. + string internal constant FREE_LABEL = "redeem"; + + /// @notice Register `label` for `nameOwner` at `status` and return its tokenId. + function _registerAt( + string memory label, + address nameOwner, + IPopRules.PopStatus status + ) + internal + returns (uint256 tokenId) + { + _register(label, nameOwner, status); + tokenId = _tokenIdForLabel(label); + } + + /// @notice Approve the escrow for `tokenId` and release it as `caller`. + function _approveAndRelease(uint256 tokenId, address caller) internal { + vm.startPrank(caller); + dotnsRegistrar.approve(address(dotnsNameEscrow), tokenId); + dotnsNameEscrow.release(tokenId); + vm.stopPrank(); + } + + function _positionOf(uint256 tokenId) + internal + view + returns (IDotnsNameEscrow.ReleasePosition memory position) + { + position = dotnsNameEscrow.getReleasePosition(tokenId); + } + + // -------------------------------------------------------------------------------------- + // release stamps both clocks + // -------------------------------------------------------------------------------------- + + function test_release_stamps_independent_withdraw_and_redeem_clocks() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + + uint256 releasedAt = block.timestamp; + _approveAndRelease(tokenId, ed); + + IDotnsNameEscrow.ReleasePosition memory position = _positionOf(tokenId); + + assertEq( + position.withdrawAvailableAt, + releasedAt + ESCROW_COOLDOWN, + "withdraw clock is release + cooldown" + ); + assertEq( + position.redeemableUntil, + releasedAt + ESCROW_REDEEM_WINDOW, + "redeem clock is release + redeem window" + ); + assertGt( + position.redeemableUntil, + position.withdrawAvailableAt, + "the redeem window must outlast the withdraw cooldown for the phases to be distinct" + ); + } + + function test_release_reverts_when_redeem_window_is_unseeded() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + + // Simulate a proxy upgraded without pairing the upgrade with `updateRedeemWindow`. The + // release must fail closed rather than stamp `redeemableUntil` at the current timestamp, + // which would open permissionless reclaim the instant the name was released. + vm.store(address(dotnsNameEscrow), _redeemWindowSlot(), bytes32(0)); + assertEq(dotnsNameEscrow.redeemWindow(), 0, "redeem window is unseeded for this case"); + + vm.startPrank(ed); + dotnsRegistrar.approve(address(dotnsNameEscrow), tokenId); + vm.expectRevert(IDotnsNameEscrow.InvalidRedeemWindow.selector); + dotnsNameEscrow.release(tokenId); + vm.stopPrank(); + } + + /// @dev `redeemWindow` sits immediately after `_nextEntryId` in the layout. Located by scanning + /// rather than hardcoded so the test fails loudly on a layout change instead of silently + /// poking an unrelated slot. + function _redeemWindowSlot() internal view returns (bytes32 slot) { + uint256 expected = dotnsNameEscrow.redeemWindow(); + for (uint256 i = 0; i < 64; ++i) { + if (uint256(vm.load(address(dotnsNameEscrow), bytes32(i))) == expected) { + return bytes32(i); + } + } + revert("redeemWindow slot not found; storage layout changed"); + } + + // -------------------------------------------------------------------------------------- + // redeem: the previous holder's undo + // -------------------------------------------------------------------------------------- + + function test_redeem_returns_the_name_and_moves_no_value() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + _approveAndRelease(tokenId, ed); + + IDotnsNameEscrow.ReleasePosition memory before = _positionOf(tokenId); + uint256 reservedBefore = dotnsNameEscrow.reserves(address(0)); + uint256 escrowBalanceBefore = address(dotnsNameEscrow).balance; + uint256 edBalanceBefore = ed.balance; + + vm.expectEmit(true, true, true, true, address(dotnsNameEscrow)); + emit IDotnsNameEscrow.NameRedeemed(tokenId, ed); + vm.prank(ed); + dotnsNameEscrow.redeem(tokenId); + + assertEq(dotnsRegistrar.ownerOf(tokenId), ed, "custody returns to the previous holder"); + assertEq(ed.balance, edBalanceBefore, "redeem must not pay the holder anything"); + assertEq( + address(dotnsNameEscrow).balance, + escrowBalanceBefore, + "redeem must not move value out of escrow" + ); + assertEq( + dotnsNameEscrow.reserves(address(0)), + reservedBefore, + "the deposit stays reserved against the name" + ); + assertEq( + dotnsNameEscrow.pendingWithdrawal(ed), + 0, + "redeem must not credit the pull-payment ledger" + ); + + IDotnsNameEscrow.ReleasePosition memory position = _positionOf(tokenId); + assertFalse(position.released, "released flag cleared"); + assertEq(position.withdrawAvailableAt, 0, "withdraw clock cleared"); + assertEq(position.redeemableUntil, 0, "redeem clock cleared"); + assertEq(position.recipient, before.recipient, "recipient preserved"); + assertEq(position.asset, before.asset, "asset preserved"); + assertEq(position.amount, before.amount, "deposit still locked against the name"); + assertFalse(position.claimed, "position is not marked settled by a redeem"); + } + + function test_redeem_removes_the_token_from_released_enumeration() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + _approveAndRelease(tokenId, ed); + + assertEq(dotnsNameEscrow.releasedTokenCount(), 1, "released set holds the token"); + + vm.prank(ed); + dotnsNameEscrow.redeem(tokenId); + + assertEq(dotnsNameEscrow.releasedTokenCount(), 0, "released set drops the redeemed token"); + } + + function test_redeem_then_release_again_starts_fresh_clocks() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + _approveAndRelease(tokenId, ed); + + vm.prank(ed); + dotnsNameEscrow.redeem(tokenId); + + vm.warp(block.timestamp + 5 days); + uint256 secondReleaseAt = block.timestamp; + _approveAndRelease(tokenId, ed); + + IDotnsNameEscrow.ReleasePosition memory position = _positionOf(tokenId); + assertTrue(position.released, "the name is releasable again after a redeem"); + assertEq( + position.withdrawAvailableAt, + secondReleaseAt + ESCROW_COOLDOWN, + "the second release recomputes the withdraw clock rather than inheriting a stale one" + ); + assertEq( + position.redeemableUntil, + secondReleaseAt + ESCROW_REDEEM_WINDOW, + "the second release recomputes the redeem clock" + ); + } + + function test_revert_redeem_after_the_window_closes() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + _approveAndRelease(tokenId, ed); + + vm.warp(_positionOf(tokenId).redeemableUntil); + + vm.prank(ed); + vm.expectRevert(abi.encodeWithSelector(IDotnsNameEscrow.NotRedeemable.selector, tokenId)); + dotnsNameEscrow.redeem(tokenId); + } + + function test_revert_redeem_by_someone_other_than_the_recipient() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + _approveAndRelease(tokenId, ed); + + vm.prank(leonardo); + vm.expectRevert( + abi.encodeWithSelector(IDotnsNameEscrow.NotRefundRecipient.selector, leonardo, tokenId) + ); + dotnsNameEscrow.redeem(tokenId); + } + + function test_revert_redeem_on_an_unreleased_position() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + + vm.prank(ed); + vm.expectRevert(abi.encodeWithSelector(IDotnsNameEscrow.NotRedeemable.selector, tokenId)); + dotnsNameEscrow.redeem(tokenId); + } + + /// @dev The load-bearing exclusion. Withdraw opens at +cooldown while redeem stays open to + /// +redeemWindow, so without this guard a holder could pull the deposit early and then + /// take the name back, ending up with a NoStatus name that no deposit backs. That would + /// break the one-deposit-per-live-name bound the deposit exists to enforce. + function test_revert_redeem_after_withdrawing_the_deposit() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + _approveAndRelease(tokenId, ed); + + vm.warp(block.timestamp + ESCROW_COOLDOWN + 1); + vm.prank(ed); + dotnsNameEscrow.withdraw(tokenId); + + // Still inside the redeem window, so only the `claimed` flag stands between the holder and + // a name they have already been paid for. + assertLt(block.timestamp, _positionOf(tokenId).redeemableUntil, "still inside the window"); + + vm.prank(ed); + vm.expectRevert(abi.encodeWithSelector(IDotnsNameEscrow.NotRedeemable.selector, tokenId)); + dotnsNameEscrow.redeem(tokenId); + } + + // -------------------------------------------------------------------------------------- + // reclaim: permissionless once the window elapses + // -------------------------------------------------------------------------------------- + + function test_revert_reclaim_while_inside_the_redeem_window() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + _approveAndRelease(tokenId, ed); + + vm.warp(block.timestamp + ESCROW_COOLDOWN + 1); + vm.prank(ed); + dotnsNameEscrow.withdraw(tokenId); + + // Withdrawing is no longer what opens reclaim, so even a settled position stays locked + // until the window elapses. + vm.prank(address(dotnsRegistrarController)); + vm.expectRevert(abi.encodeWithSelector(IDotnsNameEscrow.NotReclaimable.selector, tokenId)); + dotnsNameEscrow.reclaim(tokenId, leonardo); + } + + function test_reclaim_settles_an_unwithdrawn_deposit_to_the_previous_holder() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + _approveAndRelease(tokenId, ed); + + uint256 deposit = _positionOf(tokenId).amount; + assertGt(deposit, 0, "this case needs a funded position"); + + vm.warp(_positionOf(tokenId).redeemableUntil); + + // ed never withdrew. Reclaim must not strand their deposit. + vm.prank(address(dotnsRegistrarController)); + dotnsNameEscrow.reclaim(tokenId, leonardo); + + assertEq(dotnsRegistrar.ownerOf(tokenId), leonardo, "the name goes to the new registrant"); + assertEq( + dotnsNameEscrow.pendingWithdrawal(ed), + deposit, + "the deposit follows the departing holder onto the pull-payment ledger" + ); + assertEq(_positionOf(tokenId).recipient, address(0), "position cleared for re-registration"); + } + + /// @dev Acceptance criterion: the credit has no deadline, so a holder who reappears much later + /// is still made whole. + function test_settled_deposit_stays_claimable_long_after_reclaim() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + _approveAndRelease(tokenId, ed); + + uint256 deposit = _positionOf(tokenId).amount; + vm.warp(_positionOf(tokenId).redeemableUntil); + + vm.prank(address(dotnsRegistrarController)); + dotnsNameEscrow.reclaim(tokenId, leonardo); + + vm.warp(block.timestamp + 365 days); + + uint256 balanceBefore = ed.balance; + vm.prank(ed); + uint256 claimed = dotnsNameEscrow.claimWithdrawal(); + + assertEq(claimed, deposit, "the full deposit is claimable a year later"); + assertEq(ed.balance - balanceBefore, deposit, "and actually lands with the holder"); + } + + function test_reclaim_after_withdrawal_credits_nothing_twice() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + _approveAndRelease(tokenId, ed); + + uint256 deposit = _positionOf(tokenId).amount; + + vm.warp(block.timestamp + ESCROW_COOLDOWN + 1); + vm.prank(ed); + dotnsNameEscrow.withdraw(tokenId); + + assertEq(dotnsNameEscrow.pendingWithdrawal(ed), deposit, "withdraw credited once"); + + vm.warp(_positionOf(tokenId).redeemableUntil); + vm.prank(address(dotnsRegistrarController)); + dotnsNameEscrow.reclaim(tokenId, leonardo); + + assertEq( + dotnsNameEscrow.pendingWithdrawal(ed), + deposit, + "an already-settled position must not be credited a second time on reclaim" + ); + } + + // -------------------------------------------------------------------------------------- + // the zero-amount case: what the bug actually was + // -------------------------------------------------------------------------------------- + + /// @dev The headline regression test. A free registration seeds a zero-amount position, so its + /// holder has nothing to withdraw and therefore no reason ever to call `withdraw`. Under + /// the old `released && claimed` gate that made the name permanently unregisterable. + function test_zero_amount_release_becomes_reclaimable_without_any_withdrawal() public { + uint256 tokenId = _registerAt(FREE_LABEL, ed, IPopRules.PopStatus.PopFull); + + assertEq(_positionOf(tokenId).amount, 0, "a free registration seeds a zero-amount position"); + + _approveAndRelease(tokenId, ed); + + // ed never withdraws: there is nothing to withdraw. + assertFalse(_positionOf(tokenId).claimed, "nothing was ever withdrawn"); + assertFalse( + dotnsRegistrar.available(tokenId), + "the name is not advertised as free while ed can still redeem it" + ); + + vm.warp(_positionOf(tokenId).redeemableUntil); + + assertTrue( + dotnsRegistrar.available(tokenId), + "once the window elapses the name reports registrable" + ); + + uint256 escrowBalanceBefore = address(dotnsNameEscrow).balance; + + vm.prank(address(dotnsRegistrarController)); + dotnsNameEscrow.reclaim(tokenId, leonardo); + + assertEq(dotnsRegistrar.ownerOf(tokenId), leonardo, "a third party takes over the name"); + assertEq( + dotnsNameEscrow.pendingWithdrawal(ed), + 0, + "a zero-amount position writes no ledger entry" + ); + assertEq(address(dotnsNameEscrow).balance, escrowBalanceBefore, "and moves no value"); + } + + // -------------------------------------------------------------------------------------- + // governance: updateRedeemWindow + // -------------------------------------------------------------------------------------- + + function test_updateRedeemWindow_sets_the_value_and_emits() public { + uint256 current = dotnsNameEscrow.redeemWindow(); + uint256 next = 3 days; + + vm.expectEmit(true, true, true, true, address(dotnsNameEscrow)); + emit IDotnsNameEscrow.RedeemWindowUpdated(current, next); + vm.prank(owner); + dotnsNameEscrow.updateRedeemWindow(next); + + assertEq(dotnsNameEscrow.redeemWindow(), next, "the new window is stored"); + } + + function test_revert_updateRedeemWindow_on_zero() public { + vm.prank(owner); + vm.expectRevert(IDotnsNameEscrow.InvalidRedeemWindow.selector); + dotnsNameEscrow.updateRedeemWindow(0); + } + + function test_revert_updateRedeemWindow_above_the_ceiling() public { + uint256 max = dotnsNameEscrow.MAX_REDEEM_WINDOW(); + + vm.prank(owner); + vm.expectRevert( + abi.encodeWithSelector(IDotnsNameEscrow.RedeemWindowTooLong.selector, max + 1, max) + ); + dotnsNameEscrow.updateRedeemWindow(max + 1); + } + + function test_revert_updateRedeemWindow_from_a_non_owner() public { + vm.prank(ed); + vm.expectRevert(); + dotnsNameEscrow.updateRedeemWindow(3 days); + } + + /// @dev The window is snapshotted per position at release time, so retuning policy must never + /// move the goalposts for a name already in flight. + function test_updateRedeemWindow_does_not_move_an_in_flight_position() public { + uint256 tokenId = _registerAt(FUNDED_LABEL, ed, IPopRules.PopStatus.NoStatus); + _approveAndRelease(tokenId, ed); + + uint64 stamped = _positionOf(tokenId).redeemableUntil; + + // Read the ceiling before pranking: an external call in the argument list would consume + // the prank and the update would arrive from the test contract instead of the owner. + uint256 max = dotnsNameEscrow.MAX_REDEEM_WINDOW(); + + vm.prank(owner); + dotnsNameEscrow.updateRedeemWindow(max); + + assertEq( + _positionOf(tokenId).redeemableUntil, + stamped, + "an in-flight position keeps the window it was released under" + ); + } +} diff --git a/test/unit/registrar/DotnsRegistrar.t.sol b/test/unit/registrar/DotnsRegistrar.t.sol index e49ae6b3..c4866ffd 100644 --- a/test/unit/registrar/DotnsRegistrar.t.sol +++ b/test/unit/registrar/DotnsRegistrar.t.sol @@ -166,9 +166,10 @@ contract DotnsRegistrarTests is BaseDotns { assertFalse(dotnsRegistrar.available(tokenId)); } - function test_available_when_token_held_by_escrow_returns_true() public { + function test_available_is_false_while_released_token_is_inside_redeem_window() public { // Drive a registration through the public controller so a deposit position seeds, - // approve the escrow, release the token, then assert availability flips back to true. + // approve the escrow, release the token, then assert the name is NOT advertised as free + // while its previous holder can still redeem it. string memory label = "availreclaim01"; _register(label, ed, IPopRules.PopStatus.NoStatus); uint256 tokenId = _tokenIdForLabel(label); @@ -178,10 +179,43 @@ contract DotnsRegistrarTests is BaseDotns { dotnsNameEscrow.release(tokenId); vm.stopPrank(); + assertEq(dotnsRegistrar.ownerOf(tokenId), address(dotnsNameEscrow)); + assertFalse( + dotnsRegistrar.available(tokenId), + "a released token inside its redeem window must not report available" + ); + + // Still false one second before the boundary: the window is inclusive of its final second. + IDotnsNameEscrow.ReleasePosition memory position = + dotnsNameEscrow.getReleasePosition(tokenId); + vm.warp(position.redeemableUntil - 1); + assertFalse( + dotnsRegistrar.available(tokenId), + "availability must not open before redeemableUntil is reached" + ); + } + + function test_available_when_redeem_window_elapsed_returns_true() public { + string memory label = "availreclaim02"; + _register(label, ed, IPopRules.PopStatus.NoStatus); + uint256 tokenId = _tokenIdForLabel(label); + + vm.startPrank(ed); + dotnsRegistrar.setApprovalForAll(address(dotnsNameEscrow), true); + dotnsNameEscrow.release(tokenId); + vm.stopPrank(); + + IDotnsNameEscrow.ReleasePosition memory position = + dotnsNameEscrow.getReleasePosition(tokenId); + + // Exactly at the boundary the name is registrable, matching reclaim's `>=` gate so the + // two views can never disagree about whether a registration would succeed. + vm.warp(position.redeemableUntil); + assertEq(dotnsRegistrar.ownerOf(tokenId), address(dotnsNameEscrow)); assertTrue( dotnsRegistrar.available(tokenId), - "escrow-held tokens must be available for re-registration" + "escrow-held tokens must be available once the redeem window has elapsed" ); } diff --git a/test/unit/registrar/DotnsRegistrarControllerLifecycle.t.sol b/test/unit/registrar/DotnsRegistrarControllerLifecycle.t.sol index 73154de7..3312c2b4 100644 --- a/test/unit/registrar/DotnsRegistrarControllerLifecycle.t.sol +++ b/test/unit/registrar/DotnsRegistrarControllerLifecycle.t.sol @@ -41,6 +41,9 @@ contract DotnsRegistrarControllerLifecycleTest is BaseDotns { vm.prank(originalOwner); dotnsNameEscrow.withdraw(tokenId); + // Reclaim opens on the redeem window elapsing, not on the withdrawal landing. + vm.warp(block.timestamp + ESCROW_REDEEM_WINDOW + 1); + // Inline the new owner's commit-reveal because BaseDotns helpers quote // priceWithCheck up-front, which reverts against the stale reservation. // The controller's reclaim path is what garbage-collects the slot. @@ -233,6 +236,9 @@ contract DotnsRegistrarControllerLifecycleTest is BaseDotns { vm.prank(ed); dotnsNameEscrow.withdraw(tokenId); + // Reclaim opens on the redeem window elapsing, not on the withdrawal landing. + vm.warp(block.timestamp + ESCROW_REDEEM_WINDOW + 1); + RegistrationProbe probe = new RegistrationProbe(address(dotnsRegistry), address(dotnsReverseResolver)); vm.deal(address(probe), DEFAULT_BALANCE); @@ -513,6 +519,9 @@ contract DotnsRegistrarControllerLifecycleTest is BaseDotns { vm.prank(ed); dotnsNameEscrow.withdraw(tokenId); + // Reclaim opens on the redeem window elapsing, not on the withdrawal landing. + vm.warp(block.timestamp + ESCROW_REDEEM_WINDOW + 1); + ReentrantOwner attacker = new ReentrantOwner(dotnsRegistrarController); vm.deal(address(attacker), DEFAULT_BALANCE); From 152597343ddb4cf3d5813d310fc5e9d90c8dccdd Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Thu, 20 Aug 2026 19:24:05 +0530 Subject: [PATCH 06/12] fix: readme Signed-off-by: GHkrishna --- README.md | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index dbc770b2..02accbf2 100644 --- a/README.md +++ b/README.md @@ -18,18 +18,6 @@ DotNS is a naming system for Polkadot. An account can register a .dot name, rece Current network addresses and deployment notes are listed in [DEPLOYMENTS.md](./DEPLOYMENTS.md). -### Cutting a release - -A release publishes the contract ABIs as GitHub release assets. It does not deploy anything; deploying contracts to a network is a separate process, described in [DEPLOYMENTS.md](./DEPLOYMENTS.md). - -Run **Publish Release Package** from the Actions tab, pick the branch to release from, and enter the version (`v0.5.5`). The workflow does the rest: it builds, tests, extracts the ABIs listed in [.github/abi-contracts.txt](./.github/abi-contracts.txt), creates the release as a draft with every asset attached, verifies the set against what the build produced, and only then publishes. Pushing a matching tag runs the same workflow, so `git tag v0.5.5 && git push origin v0.5.5` remains equivalent. - -Pre-releases use **Publish Beta Package** with a suffixed version, `v0.5.5-rc1`. The version is the release identity; the `version` field in `package.json` is unrelated and nothing reads it. - -Do not create releases through the GitHub UI's release form, or with `gh release create`. Both publish immediately, and because this repository has immutable releases enabled, a published release can no longer accept assets: only its title and notes stay editable. A release made that way carries no ABIs at all. The workflow rejects an already-published version before building, so the mistake fails in seconds rather than silently shipping an empty release. - -If a run fails partway, re-run it from the Actions tab; the draft is updated rather than duplicated. One case needs a manual step: the upload replaces an asset of the same name but never removes others, so if the contract list changed since the failed run, the draft still carries the assets it no longer expects and the verification step will keep refusing to publish. Delete the draft and re-run. If the version has already been published, use a different one, since its assets cannot be changed. - ## Economics dotNS uses a single tunable constant, written **D** throughout the protocol. D is the starting price used by PopRules and equals ten DOT at launch; governance can adjust it under the same gate as the upgrade authority. D is the only money quantity the protocol charges; everything else is a composition of D with zero. @@ -73,12 +61,34 @@ The friction is constant and additive across downward hops. Every step that cros The escrow maintains two separate pull-payment ledgers. The split is deliberate: one ledger is for immediate overpayment withdrawals, and the other is for refunds that must wait behind a cooldown. - **Overpayment ledger.** No cooldown. Used only as the fallback when a direct registration overpayment cannot be returned to the sender inline. -- **Refund ledger.** Every refund has its own cooldown clock. Used for the deposit unlocked when a holder releases a funded name back to escrow, and for transfer-fee overpayments. Transfers never credit the refund ledger because the position rides with the name; only release-and-withdraw does. +- **Refund ledger.** Every refund has its own cooldown clock. Used for transfer-fee overpayments. Transfers never credit the refund ledger because the position rides with the name. -Only registrations try to return surplus immediately. Every other refund path waits behind its own cooldown. The cooldown is bounded to minutes: it is the window between release and reclaim during which the original payer has an uncontested chance to pull their refund before the controller hands the name out again, not a long-lived lock. Governance can tune it within that band. +Only registrations try to return surplus immediately. Every other refund path waits behind a clock. The deposit unlocked by releasing a funded name is credited to the overpayment ledger rather than the refund ledger: the delay comes from the position's own `withdrawAvailableAt` stamp, so once `withdraw` lands the credit is immediately pullable. The cooldown is bounded to minutes and governance can tune it within that band. Clients can enumerate pending refunds through the escrow's public refund views. Pagination is capped so refund discovery remains bounded. +### Release lifecycle + +Releasing a name starts two independent clocks, and the distinction between them is what makes a released name both recoverable and recyclable. + +| Clock | Length | What it gates | +|---|---|---| +| `withdrawAvailableAt` | release + `cooldown` (15 minutes at launch, ≤ 1 hour) | When the holder may credit the deposit to themselves via `withdraw` | +| `redeemableUntil` | release + `redeemWindow` (1 day at launch, ≤ 30 days) | When the holder's exclusive claim on the name ends and `reclaim` opens to anyone | + +Both are snapshotted at release time, so a governance change never moves the goalposts on a name already in flight. `redeemWindow` is tuned through `updateRedeemWindow` under the same gate as the upgrade authority. + +Inside the redeem window the name belongs to its previous holder. They alone may act on it, and `DotnsRegistrar.available` reports **false** so no client advertises the name as free and no registrant burns a commit-reveal cycle on a registration that cannot succeed. Their options are exclusive: + +- **`redeem`** returns the NFT 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 who has been paid for the name cannot also take it back; otherwise they would hold a NoStatus name that no deposit backs, and the Sybil bound of one D per live NoStatus name would not hold. + +Once `redeemableUntil` is reached, `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 through `claimWithdrawal` with no deadline. The value follows the departing holder; the name does not wait for them. + +That last point is the whole reason the window exists. Reclaim used to require the previous holder to have withdrawn first, which meant a holder who released a name and never came back removed the label from circulation permanently. For the zero-amount positions seeded by free PopFull and PopLite registrations there is nothing to withdraw, so never withdrawing was the default rather than the exception. Bounding the wait with a clock replaces a dependency on someone else's action with one that elapses on its own. + +Clients wanting the exact moment a released name becomes registrable should read `redeemableUntil` from `getReleasePosition` rather than polling `available`. + ## Contracts Two controllers sit on top of a single registrar and a single protocol registry. The registrar holds the ERC721 token per name; the registry holds the forward node => (owner, resolver) mapping and subname hierarchy; the resolvers hold per-name records; the protocol registry is the indirection layer through which every contract resolves its siblings at runtime. Controllers are the entry points: they mint names and drive the side effects. Neither controller imports the other. The layers underneath arbitrate collision handling: ERC721 uniqueness on the registrar, and a single reservation table on PopRules that both flows read through. From 707d369a9fc953f9148542e90141203915062285 Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Mon, 24 Aug 2026 12:22:58 +0530 Subject: [PATCH 07/12] fix: zero withdrawal amount handling during redeem Signed-off-by: GHkrishna --- contracts/escrow/DotnsNameEscrow.sol | 14 ++++++--- test/unit/escrow/DotnsNameEscrowRedeem.t.sol | 33 ++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/contracts/escrow/DotnsNameEscrow.sol b/contracts/escrow/DotnsNameEscrow.sol index ce0179ad..f67304d5 100644 --- a/contracts/escrow/DotnsNameEscrow.sol +++ b/contracts/escrow/DotnsNameEscrow.sol @@ -432,12 +432,18 @@ contract DotnsNameEscrow is uint256 owed = position.amount; address asset = position.asset; - // Effects: flag the position settled regardless of amount so `claimed` remains a faithful - // record of "the deposit for this position has been dealt with". - position.claimed = true; - + // Nothing to settle: return before touching `claimed`. That flag is what `redeem` reads to + // decide whether the holder has already been paid for the name, so setting it here would + // make a zero-amount `withdraw`, which pays nothing and emits nothing, silently forfeit + // the holder's right to recover their own name for no consideration at all. Free PopFull + // and PopLite registrations seed exactly these positions, and `withdraw` is the step the + // old contract required before a name could be recycled, so that is a path holders will + // take. if (owed == 0) return; + // Effects: from here the deposit really is being handed over, so the flag is set. + position.claimed = true; + uint256 reserved = tokenReserved[asset]; uint256 fromRefundable; diff --git a/test/unit/escrow/DotnsNameEscrowRedeem.t.sol b/test/unit/escrow/DotnsNameEscrowRedeem.t.sol index f872459c..3b751aaf 100644 --- a/test/unit/escrow/DotnsNameEscrowRedeem.t.sol +++ b/test/unit/escrow/DotnsNameEscrowRedeem.t.sol @@ -242,6 +242,39 @@ contract DotnsNameEscrowRedeemTest is BaseDotns { dotnsNameEscrow.redeem(tokenId); } + /// @dev The mirror of the case above, and the reason `claimed` is only set when value actually + /// moves. A free registration has nothing to withdraw, so `withdraw` pays the holder + /// nothing, if it still flagged the position claimed it would silently forfeit their + /// right to recover their own name for no consideration whatsoever. `withdraw` is also the + /// step the old contract required before a name could be recycled, so it is a call holders + /// have every reason to make. + function test_zero_amount_withdrawal_does_not_forfeit_the_redeem_right() public { + uint256 tokenId = _registerAt(FREE_LABEL, ed, IPopRules.PopStatus.PopFull); + assertEq(_positionOf(tokenId).amount, 0, "this case needs a zero-amount position"); + + _approveAndRelease(tokenId, ed); + + vm.warp(block.timestamp + ESCROW_COOLDOWN + 1); + uint256 balanceBefore = ed.balance; + vm.prank(ed); + dotnsNameEscrow.withdraw(tokenId); + + assertEq(ed.balance, balanceBefore, "there was nothing to pay out"); + assertEq(dotnsNameEscrow.pendingWithdrawal(ed), 0, "and nothing to credit"); + assertFalse( + _positionOf(tokenId).claimed, + "a settlement that moved no value must not flag the position claimed" + ); + + // Still inside the window, and still ed's name to recover. + vm.prank(ed); + dotnsNameEscrow.redeem(tokenId); + + assertEq( + dotnsRegistrar.ownerOf(tokenId), ed, "ed recovers the name they were never paid for" + ); + } + // -------------------------------------------------------------------------------------- // reclaim: permissionless once the window elapses // -------------------------------------------------------------------------------------- From 07e796d7e579b6e98e9614d49ba0d1a15071875a Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Mon, 24 Aug 2026 12:29:17 +0530 Subject: [PATCH 08/12] fix: name availability considering both released and reedemable period Signed-off-by: GHkrishna --- contracts/registrars/DotnsRegistrar.sol | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/contracts/registrars/DotnsRegistrar.sol b/contracts/registrars/DotnsRegistrar.sol index 7f83dcf8..dbc9925e 100644 --- a/contracts/registrars/DotnsRegistrar.sol +++ b/contracts/registrars/DotnsRegistrar.sol @@ -114,7 +114,12 @@ contract DotnsRegistrar is IDotnsNameEscrow.ReleasePosition memory position = IDotnsNameEscrow(payable(escrow)).getReleasePosition(id); - return block.timestamp >= position.redeemableUntil; + // `released` is part of the predicate, not a redundant check. Availability here means + // "reclaim would succeed", and reclaim requires the position to be released as well as out + // of its window. The escrow's `onERC721Received` rejects unsolicited transfers so custody + // without a released position should be unreachable, but reporting a name registrable on + // the strength of a zero `redeemableUntil` alone would be wrong. + return position.released && block.timestamp >= position.redeemableUntil; } /// @inheritdoc IDotnsRegistrar From e185606ebac7856aa4c80ee94d61460c97d3474b Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Mon, 24 Aug 2026 12:30:08 +0530 Subject: [PATCH 09/12] fix: token getting stucked Signed-off-by: GHkrishna --- .../escrow/DotnsNameEscrowInvariant.t.sol | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/test/invariant/escrow/DotnsNameEscrowInvariant.t.sol b/test/invariant/escrow/DotnsNameEscrowInvariant.t.sol index d2545e6a..4342f168 100644 --- a/test/invariant/escrow/DotnsNameEscrowInvariant.t.sol +++ b/test/invariant/escrow/DotnsNameEscrowInvariant.t.sol @@ -199,10 +199,19 @@ contract DotnsNameEscrowInvariantTest is BaseDotns { /// no gap, and must not overlap -- an overlap would mean the previous holder and a new /// registrant could both act on the same name. function invariant_released_tokens_are_never_stuck() public view { - uint256[] memory released = handler.getReleasedTokenIds(); + // Withdrawn tokens are still released positions, and a position settled while inside its + // window is the only state with no action available right now. Iterating released tokens + // alone would skip exactly that state, because the handler moves a token out of + // `_releasedTokenIds` the moment it is withdrawn, so the invariant meant to prove nothing + // gets stuck would never evaluate the one state that pauses. + _assertNotStuck(handler.getReleasedTokenIds()); + _assertNotStuck(handler.getWithdrawnTokenIds()); + } - for (uint256 i; i < released.length; ++i) { - uint256 tokenId = released[i]; + /// @notice Asserts the never-stuck property across a set of token ids. + function _assertNotStuck(uint256[] memory tokenIds) private view { + for (uint256 i; i < tokenIds.length; ++i) { + uint256 tokenId = tokenIds[i]; IDotnsNameEscrow.ReleasePosition memory position = dotnsNameEscrow.getReleasePosition(tokenId); @@ -210,15 +219,26 @@ contract DotnsNameEscrowInvariantTest is BaseDotns { if (!position.released) continue; bool insideWindow = block.timestamp < position.redeemableUntil; - // Withdrawing forfeits the redeem right, but it cannot strand the name: reclaim opens - // on the same boundary regardless. bool redeemable = insideWindow && !position.claimed; bool reclaimable = !insideWindow; - assertTrue( - redeemable || reclaimable, - "A released position must always be redeemable or reclaimable, never neither" - ); + // Withdrawing forfeits the redeem right, so a settled position waits out the rest of + // its window with nothing to do. That is a pause, not a deadlock, and what makes it a + // pause is that the deadline is finite and still ahead. Assert that rather than + // exempting the state from the invariant. + if (!redeemable && !reclaimable) { + assertTrue( + position.claimed, + "the only actionless state is a position whose deposit is already settled" + ); + assertGt( + position.redeemableUntil, + block.timestamp, + "and it must be waiting on a deadline that actually arrives" + ); + continue; + } + assertFalse(redeemable && reclaimable, "The redeem and reclaim phases must not overlap"); } } From 67168cd790aa10cae6b876b44dc6ec204f736864 Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Mon, 24 Aug 2026 13:18:15 +0530 Subject: [PATCH 10/12] fix: equality of escrow balance while considering pending withdrawls Signed-off-by: GHkrishna --- .../DotnsRegistrarControllerInvariant.t.sol | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/invariant/registrar/DotnsRegistrarControllerInvariant.t.sol b/test/invariant/registrar/DotnsRegistrarControllerInvariant.t.sol index 3607d9b5..25936386 100644 --- a/test/invariant/registrar/DotnsRegistrarControllerInvariant.t.sol +++ b/test/invariant/registrar/DotnsRegistrarControllerInvariant.t.sol @@ -109,8 +109,12 @@ contract DotnsRegistrarControllerInvariantTest is BaseDotns { /// settles unwithdrawn deposits onto the pull-payment ledger, more paths reach that state, /// so the assertion is stated as the property that actually matters -- the escrow is never /// short -- over every actor and every ledger. - /// Balance may legitimately exceed the sum: force-sent value (`selfdestruct`) is - /// unaccounted-for surplus the escrow has no ledger for, which is why this is `assertGe`. + /// Kept as strict equality deliberately. Relaxing it to `assertGe` would state only that + /// the escrow is never short, which no longer bites: now that reclaim moves value, a bug + /// that debits `tokenReserved` and forgets to credit `_pendingWithdrawals` leaves the + /// balance "above" the sum and sails past a `>=` assertion. Equality is what catches value + /// going missing. It holds because no handler action force-sends value to the escrow; if + /// one is ever added, account for the surplus in a ghost rather than weakening this. /// The time-locked refund ledger is not summed here because this handler does not drive /// it; `DotnsNameEscrowInvariant.invariant_solvency` covers all four ledgers together. function invariant_value_conservation() public view { @@ -124,10 +128,10 @@ contract DotnsRegistrarControllerInvariantTest is BaseDotns { } uint256 escrowBalance = address(dotnsNameEscrow).balance; - assertGe( + assertEq( escrowBalance, reservedAmount + insurance + pendingTotal, - "Escrow balance must cover reserves + insurance + pending withdrawals" + "Escrow balance must equal reserves + insurance + pending withdrawals" ); } From f2fdbd52815dc71dfe600d307f738512f16f8dbe Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Mon, 24 Aug 2026 13:27:35 +0530 Subject: [PATCH 11/12] chore: add todo for min redeem window Signed-off-by: GHkrishna --- contracts/escrow/DotnsNameEscrow.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contracts/escrow/DotnsNameEscrow.sol b/contracts/escrow/DotnsNameEscrow.sol index f67304d5..01ca03e2 100644 --- a/contracts/escrow/DotnsNameEscrow.sol +++ b/contracts/escrow/DotnsNameEscrow.sol @@ -48,6 +48,9 @@ contract DotnsNameEscrow is /// circulation, and keeps the cast to `uint64` in release well below saturation. uint256 public constant MAX_REDEEM_WINDOW = 30 days; + // TODO: Consider adding a MIN_REDEEM_WINDOW to prevent a malicious owner from setting it to 0 + // and allowing immediate reclaim. + /// @notice The protocol registry for resolving sibling contract addresses. IDotnsProtocolRegistry public protocolRegistry; From 327ea160ece8dc231acbe431cce1919e67e6c7c7 Mon Sep 17 00:00:00 2001 From: GHkrishna Date: Mon, 24 Aug 2026 14:15:36 +0530 Subject: [PATCH 12/12] fix: deployment.md for updated configs Signed-off-by: GHkrishna --- DEPLOYMENTS.md | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/DEPLOYMENTS.md b/DEPLOYMENTS.md index e5f68f11..e7f0fcef 100644 --- a/DEPLOYMENTS.md +++ b/DEPLOYMENTS.md @@ -181,6 +181,44 @@ bun run deploy:testnet If ACCOUNT_PASSWORD is not set and the process has a TTY, the runner prompts once for the keystore password and passes it to every stage. +## Upgrading a proxy that gained a new configuration value + +An upgrade that adds a governance-tunable storage value must seed it in the **same transaction** as the implementation swap. A bare `upgradeTo` leaves the new slot at zero, and a proxy running with an unseeded policy value is a live misconfiguration, not a pending chore. + +UUPS supports this directly: `upgradeToAndCall` performs the post-upgrade call as a delegatecall from the proxy context, so `msg.sender` is preserved and an `onlyOwner` setter is callable as part of the upgrade. + +### DotnsNameEscrow: `redeemWindow` + +The escrow's redeem window is the period after a `release` in which only the previous holder may act — they alone may `redeem` the name back, and `available` reports `false` so nobody wastes a commitment on it. Once it elapses, `reclaim` is permissionless. It is a separate value from `cooldown` and defaults to `ESCROW_REDEEM_WINDOW` (1 day) on a fresh deploy. + +Upgrade an existing escrow proxy like this, not with a bare `upgradeTo`: + +```bash +# 1 day, matching the ESCROW_REDEEM_WINDOW deploy constant +cast send "$ESCROW_PROXY" \ + 'upgradeToAndCall(address,bytes)' \ + "$NEW_IMPL" \ + "$(cast calldata 'updateRedeemWindow(uint256)' 86400)" \ + --account "$DEPLOYER" --rpc-url "$RPC_URL" +``` + +Verify before considering the upgrade done: + +```bash +cast call "$ESCROW_PROXY" 'redeemWindow()(uint256)' --rpc-url "$RPC_URL" # expect 86400 +``` + +If the window is left at zero, `release` reverts with `InvalidRedeemWindow` for **every** name on that deployment. That is deliberate: the alternative would be stamping `redeemableUntil` at the current timestamp, which silently opens permissionless reclaim the instant a name is released and hands the name to whoever is watching. A loud failure on `release` is recoverable with one owner transaction; a silent one is not. + +To recover a proxy already upgraded without seeding, call the setter directly — no second upgrade is needed: + +```bash +cast send "$ESCROW_PROXY" 'updateRedeemWindow(uint256)' 86400 \ + --account "$DEPLOYER" --rpc-url "$RPC_URL" +``` + +Bounds: non-zero and at most `MAX_REDEEM_WINDOW` (30 days). Changing the window later affects only releases recorded after the change; positions already released keep the `redeemableUntil` snapshot taken at their release time. + ## Deployment pipeline The fresh-deploy pipeline is split across five stages: