From f88007d7c62544cca10000583b04999c209c6a28 Mon Sep 17 00:00:00 2001 From: Siphamandla Mjoli Date: Tue, 1 Sep 2026 14:49:34 +0200 Subject: [PATCH 1/3] chore: initial commit --- KNOWN_ISSUES.md | 21 +- README.md | 9 +- contracts/registrars/DotnsRegistrar.sol | 65 +++++- contracts/registrars/IDotnsRegistrar.sol | 40 ++-- test/intergration/PopLifecycleFlow.t.sol | 58 ++--- .../DotnsRegistrarSoulboundInvariant.t.sol | 72 ++++++ .../registrar/SoulboundHandler.t.sol | 129 +++++++++++ test/unit/registrar/DotnsRegistrar.t.sol | 210 ++++++++++++++++++ 8 files changed, 530 insertions(+), 74 deletions(-) create mode 100644 test/invariant/registrar/DotnsRegistrarSoulboundInvariant.t.sol create mode 100644 test/invariant/registrar/SoulboundHandler.t.sol diff --git a/KNOWN_ISSUES.md b/KNOWN_ISSUES.md index 5fec1d070..6196c1681 100644 --- a/KNOWN_ISSUES.md +++ b/KNOWN_ISSUES.md @@ -7,9 +7,8 @@ For the security and audit status of the codebase, see [SECURITY.md](./SECURITY. | # | Issue | Type | Resolves when | | :- | :---- | :--- | :------------ | | 1 | Deferred LabelStore deployment | Runtime | Runtime allows root-origin contract deployment | -| 2 | Transfer fee is zero until the store is settled | Runtime (follows from 1) | The holder calls `claimLabelStore`, or 1 is resolved | -| 3 | Root origin is not propagated through delegatecalls | Runtime | Runtime propagates origin through delegatecalls | -| 4 | No standalone user-status mapping | Current implementation | A dedicated status mapping is added, if ever needed | +| 2 | Root origin is not propagated through delegatecalls | Runtime | Runtime propagates origin through delegatecalls | +| 3 | No standalone user-status mapping | Current implementation | A dedicated status mapping is added, if ever needed | ## 1. Deferred LabelStore deployment @@ -23,19 +22,7 @@ Substrate Root cannot deploy a contract on behalf of an account it does not cont See [README → DotnsPopController](./README.md#early-testnet-quirk-labelstore-deployment). -## 2. Transfer fee is zero until the store is settled - -**Type:** runtime limitation; a direct consequence of issue 1. - -The registrar derives the transfer-floor price by reading the label from the sender's `LabelStore`. A gateway-issued name held by a user who has not yet called `claimLabelStore` has no readable label on the sender side, so `_quoteTransferFee` returns zero regardless of the recipient's tier. Until the holder settles their store, a downward transfer (for example PopFull to NoStatus) does not charge the cross-tier friction it would otherwise owe. - -**Workaround:** clients that consume gateway-issued names should treat `claimLabelStore` as a prerequisite for accurate transfer-time pricing, not just for label discovery. - -**Resolution:** clears once issue 1 is resolved. - -See [README → DotnsPopController](./README.md#early-testnet-quirk-labelstore-deployment). - -## 3. Root origin is not propagated through delegatecalls +## 2. Root origin is not propagated through delegatecalls **Type:** runtime limitation. @@ -47,7 +34,7 @@ The substrate Root origin is not propagated through delegatecalls, so a UUPS imp See [README → RootGatewayDispatcher](./README.md#rootgatewaydispatcher). -## 4. No standalone user-status mapping +## 3. No standalone user-status mapping **Type:** current implementation. diff --git a/README.md b/README.md index 6f8f3e7dc..ba95a34f8 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,9 @@ A name someone else pays for, and a transfer, pay a non-refundable fee instead. ### Transfers re-price at the name's own length -A transfer charges the name's own price, but only in two cases: the recipient cannot clear the name's band, or the move is a personhood downgrade, where the recipient's tier is lower than the sender's. Passing a six-character name to a wallet that could never have registered it costs the name's own price, so there is no cheap way to hand a band-gated name to a party who could not have earned it. A move between two wallets that both clear the band, and a move to the same address, cost nothing. The fee, when one is owed, settles into the protocol fee pot. The deposit, when present, rides with the name: the escrow position rebinds to the new holder rather than refunding, and only releasing the name back to escrow unlocks the locked deposit. +Publicly registered names transfer freely and charge the name's own price, but only in two cases: the recipient cannot clear the name's band, or the move is a personhood downgrade, where the recipient's tier is lower than the sender's. Passing a six-character name to a wallet that could never have registered it costs the name's own price, so there is no cheap way to hand a band-gated name to a party who could not have earned it. A move between two wallets that both clear the band, and a move to the same address, cost nothing. The fee, when one is owed, settles into the protocol fee pot. The deposit, when present, rides with the name: the escrow position rebinds to the new holder rather than refunding, and only releasing the name back to escrow unlocks the locked deposit. + +Names minted through the PoP gateway are soulbound: they stay bound to the person who earned them and cannot be transferred at all. Any transfer of a gateway-issued name reverts, and quoting a transfer fee for one reverts rather than returning a price. Everything else about the name works normally, so its owner still sets records, issues subnames, and manages the name. ### Versioned pricing @@ -146,7 +148,7 @@ Each base label carries a head/tail-indexed reservation queue with a capacity of Pop-gateway issuances mint the name and persist its label, but LabelStore deployment is deferred for users who have not yet interacted with the protocol from their own address. The current pallet-revive runtime does not let substrate Root deploy contracts on behalf of an account it does not control, so the per-user LabelStore cannot be created at the moment the gateway writes. The controller stamps a pending-claim entry instead, and settlement writes the label into the owner's store, deploying the store on the first write. Settlement is permissionless via settlePendingClaims: the owner settles their own store, or after the claim window anyone settles a given owner's entry and pays the cost. Settlement always writes the label rather than dropping the entry, so a pending name is never stranded. When the runtime supports root-origin contract deployment, the deferred path collapses to a no-op and the issuance flow becomes one transaction end-to-end. This is a runtime limitation, not a protocol design choice. -Operational consequence for transfers: the registrar derives the transfer-floor price by reading the label from the sender's LabelStore. A gateway-issued name whose pending claim is not yet settled has no readable label on the sender side, so `_quoteTransferFee` returns zero regardless of the recipient's tier. Until the name is settled into a LabelStore, a downward transfer (for example PopFull to NoStatus) does not charge the cross-tier friction it would otherwise owe. Clients that consume gateway-issued names should treat settlement as a prerequisite for accurate transfer-time pricing, not just for label discovery. +Deferred settlement has no transfer-pricing consequence, because gateway-issued names are soulbound and cannot be transferred at all. The transfer-floor price is derived by reading the label from the sender's LabelStore, so a name held before its label is settled would have no readable label to price against; making gateway names non-transferable removes that path entirely rather than relying on settlement to close it. ### RootGatewayDispatcher @@ -160,6 +162,8 @@ The dispatcher exists to work around a runtime limitation: the substrate Root or ERC721-backed registrar that mints ownership of label IDs (labelhashes). Minting is restricted to every address in the controllers mapping; the mapping is owner-gated through addController and removeController. Every other contract in the system that needs to check "is this address authorised to drive name state?" consults this mapping rather than keeping a parallel list, which is what lets multiple controllers coexist on the same registrar without per-contract configuration changes. +The registrar owns transferability. A name is marked soulbound at mint when the caller is the address registered under POP_CONTROLLER, so only the PoP gateway can issue a soulbound name and no other controller can lock a public one. The marker is set once and never cleared, and isSoulbound reports it. The transfer hook rejects every transfer of a soulbound name, mint and same-address moves aside, so a gateway-issued name cannot leave its holder's wallet or enter escrow. + ### DotnsRegistry Forward registry mapping node to (owner, resolver) and supporting subnode creation. When a base name is minted on the registrar, the matching controller wires the node to the new owner through this registry. Privileged node wiring defers to the same controllers mapping on the registrar, so both controllers can write without the registry tracking controllers of its own. @@ -279,7 +283,6 @@ For Parity's security disclosure process, and Bug Bounty program, feel free to v The protocol carries a handful of constraints worth knowing before deploying or building against it. Most stem from the current pallet-revive runtime rather than from protocol design, and collapse to a no-op once the runtime gains the corresponding capability. [KNOWN_ISSUES.md](./KNOWN_ISSUES.md) is the consolidated reference; each issue is also described in full where the relevant contract is documented below. - **Deferred LabelStore deployment** (runtime). See [DotnsPopController](#early-testnet-quirk-labelstore-deployment). -- **Transfer fee is zero until the store is settled** (runtime). See [DotnsPopController](#early-testnet-quirk-labelstore-deployment). - **Root origin is not propagated through delegatecalls** (runtime). See [RootGatewayDispatcher](#rootgatewaydispatcher). - **No standalone user-status mapping** (current implementation). See [DotnsPopController](#dotnspopcontroller). diff --git a/contracts/registrars/DotnsRegistrar.sol b/contracts/registrars/DotnsRegistrar.sol index c0f40c533..3ee411007 100644 --- a/contracts/registrars/DotnsRegistrar.sol +++ b/contracts/registrars/DotnsRegistrar.sol @@ -25,10 +25,11 @@ import {DotnsConstants} from "../utils/DotnsConstants.sol"; /// @title Dotns Registrar /// @notice ERC721-backed registrar implementing permanent name ownership. -/// @dev Deliberately policy-free. Transfers are supported to allow ownership changes without -/// registry hooks, and the registrar itself does not encode pricing, reservations, or PoP -/// gating; those live in the controllers and @custom:contract IPopRules. The fee-on-transfer hook -/// in `_update` is a thin enforcement layer that consults the escrow. +/// @dev Deliberately policy-free on pricing, reservations, and PoP gating; those live in the +/// controllers and @custom:contract IPopRules. The registrar owns transferability itself: publicly +/// registered names transfer freely, while names minted through the PoP gateway are soulbound and +/// revert on transfer. The `_update` hook enforces both the soulbound gate and the fee-on-transfer +/// settlement that consults the escrow. /// @custom:security-contact admin@parity.io contract DotnsRegistrar is Initializable, @@ -53,6 +54,13 @@ contract DotnsRegistrar is /// without storing individual references. IDotnsProtocolRegistry public protocolRegistry; + /// @notice Marks a token as soulbound: minted through the PoP gateway and non-transferable. + /// @dev Set at mint by @custom:function register when the caller is the address registered + /// under `DotnsConstants.POP_CONTROLLER`. Write-once and never cleared: a name's soulbound + /// state is fixed at registration. Read by the `_update` transfer gate and by + /// @custom:function quoteTransferFee. + mapping(uint256 tokenId => bool soulbound) private _soulbound; + /// @dev Reserved storage space to allow for layout changes in the future. uint256[50] private __gap; @@ -136,6 +144,13 @@ contract DotnsRegistrar is // cannot brick the token by reverting on a malformed stem. require(bytes(label).length == 0 || label.isSingleLabel(), InvalidLabel()); _mint(owner, id); + // Provenance is verified here rather than trusted from a caller-supplied flag: only the + // canonical PoP controller mints soulbound names, so a compromised or buggy peer controller + // cannot lock a public name and the PoP controller cannot mint an unlocked one. Written + // only on the true branch to leave the public path free of a redundant zero write. + if (msg.sender == protocolRegistry.get(DotnsConstants.POP_CONTROLLER)) { + _soulbound[id] = true; + } if (bytes(label).length != 0) _writeOwnerLabel(owner, id, label); emit NameRegistered(id, owner); } @@ -158,6 +173,10 @@ contract DotnsRegistrar is returns (uint256 requiredFee) { require(to != address(0), ERC721InvalidReceiver(address(0))); + // A soulbound name cannot be transferred, so it has no transfer price. Revert rather than + // return zero: a zero here would read as "transferable, no fee" to integrators while any + // real transfer reverts in `_update`. + require(!_soulbound[tokenId], NameSoulbound(tokenId)); address from = ownerOf(tokenId); (,, requiredFee) = _quoteTransferFee(from, to, tokenId); @@ -214,6 +233,11 @@ contract DotnsRegistrar is tokenExists = _exists(tokenId); } + /// @inheritdoc IDotnsRegistrar + function isSoulbound(uint256 tokenId) external view override returns (bool soulbound) { + soulbound = _soulbound[tokenId]; + } + /// @notice Checks whether a token ID exists. function _exists(uint256 tokenId) internal view returns (bool) { return _ownerOf(tokenId) != address(0); @@ -236,10 +260,25 @@ contract DotnsRegistrar is { from = super._update(to, tokenId, auth); - // Mints and self-transfers carry no economic event. Reject any attached value on those - // paths because nothing forwards it onward, which would otherwise trap the funds in this - // contract permanently (no `receive`, no rescue path). - if (from == address(0) || from == to) { + // Mints carry no economic event and must not be blocked: the soulbound flag is written + // after `_mint`, so a mint reaches here before the flag exists. Reject any attached value + // because nothing forwards it onward (no `receive`, no rescue path). + if (from == address(0)) { + require(msg.value == 0, UnexpectedValue()); + return from; + } + + // Soulbound names are non-transferable, including a move to the sender's own address, which + // keeps this in step with @custom:function quoteTransferFee and the interface contract. It + // reverts rather than returning, unwinding the ownership move `super._update` has already + // made, and sits before any escrow or store lookup so a soulbound token is rejected even + // when the escrow is unconfigured, blocking every custody move including release into + // escrow. + require(!_soulbound[tokenId], NameSoulbound(tokenId)); + + // Self-transfers of a transferable name carry no economic event. Reject attached value for + // the same trapped-funds reason as the mint path above. + if (from == to) { require(msg.value == 0, UnexpectedValue()); return from; } @@ -302,9 +341,10 @@ contract DotnsRegistrar is { string memory fullName = _readLabelFor(factory, tokenId, from); if (bytes(fullName).length == 0) { - // Sender has no label entry for the token (typical of gateway-cold PoP mints). - // Nothing to mirror, so do not deploy a recipient store; downstream writes are - // demand-deploy through `StoreUtils.ensureLabelStore`. + // Defensive: the sender holds no label entry for the token. Gateway mints reach this + // only at mint time, and a gateway name is soulbound so it never transfers; a public + // name always carries a label. Nothing to mirror, so do not deploy a recipient store; + // downstream writes are demand-deploy through `StoreUtils.ensureLabelStore`. return; } factory.writeLabel(to, bytes32(tokenId), fullName); @@ -413,7 +453,8 @@ contract DotnsRegistrar is string memory fullName = _readLabelFor(factory, tokenId, from); // No label means there is no label-derived price to charge against; treat as a zero-fee - // move (typical of gateway-cold PoP mints that have not yet claimed a `LabelStore`). + // move. This is defensive: a gateway name is soulbound and reverts before reaching here, + // and a public name always carries a label, so no reachable transfer hits this branch. if (bytes(fullName).length == 0) return (0, 0); // A stored full name always carries the registry TLD suffix, so an empty strip means the // name is malformed for this registry (a wrong or missing suffix); fail loudly rather than diff --git a/contracts/registrars/IDotnsRegistrar.sol b/contracts/registrars/IDotnsRegistrar.sol index b0b88d9b3..5000015fd 100644 --- a/contracts/registrars/IDotnsRegistrar.sol +++ b/contracts/registrars/IDotnsRegistrar.sol @@ -41,6 +41,12 @@ interface IDotnsRegistrar is IERC721 { /// label. error InvalidLabel(); + /// @notice Thrown when a transfer or a transfer-fee quote targets a soulbound name. + /// @dev Soulbound names are minted through the PoP gateway and are permanently + /// non-transferable. Raised by the `_update` transfer gate and by + /// @custom:function quoteTransferFee. + error NameSoulbound(uint256 tokenId); + /// @notice Emitted when a name is registered. event NameRegistered(uint256 indexed id, address indexed owner); @@ -76,9 +82,22 @@ interface IDotnsRegistrar is IERC721 { /// (otherwise @custom:reverts NotController) and rejects ids that are not available /// (otherwise @custom:reverts NameNotAvailable). Emits @custom:emits NameRegistered on /// success. + /// @dev When the caller is the address registered under `DotnsConstants.POP_CONTROLLER`, the + /// name is marked soulbound and becomes permanently non-transferable (see + /// @custom:function isSoulbound). Provenance is read from the protocol registry at mint time, + /// so no other authorised controller can mint a soulbound name and the PoP controller cannot + /// mint an unlocked one. Public registrations from any other controller stay transferable. /// @param label The human-readable label string (e.g. "alice"). function register(uint256 id, address owner, string calldata label) external; + /// @notice Returns whether a token is soulbound (PoP-gateway minted and non-transferable). + /// @dev Durable on-chain marker set once at mint by @custom:function register and never + /// cleared. A `true` result means every transfer overload reverts with + /// @custom:reverts NameSoulbound and @custom:function quoteTransferFee reverts likewise. + /// @param tokenId The name's token id. + /// @return soulbound True when the name was minted through the PoP gateway. + function isSoulbound(uint256 tokenId) external view returns (bool soulbound); + /// @notice Returns whether a given token id has been minted. function exists(uint256 tokenId) external view returns (bool tokenExists); @@ -118,17 +137,9 @@ interface IDotnsRegistrar is IERC721 { /// recipient tier is strictly below the sender tier. Self-transfers and /// escrow-touching transfers (release into escrow, reclaim out of escrow) return /// zero. A token whose sender has no stored label also returns zero because there - /// is no label-derived price to charge against; this covers gateway-cold PoP mints - /// (the controller passes an empty label to @custom:function register so substrate - /// Root does not have to deploy a `LabelStore`) until the user settles via - /// @custom:function IDotnsPopController.claimLabelStore. Because settlement writes - /// the label into the original claimant's store, a transfer that happens before - /// settlement leaves the recipient with no label entry and the zero-fee branch - /// persists for that token under all future holders. A token registered with no - /// label that is moved off-chain prior to settlement therefore carries no PoP-tier - /// transfer friction. Off-chain consumers integrating PoP mints should treat - /// @custom:function claimLabelStore as a prerequisite for accurate transfer-time - /// pricing on gateway-issued names. Rejects a zero `to` with + /// is no label-derived price to charge against. Soulbound names are non-transferable + /// and have no transfer price, so a soulbound `tokenId` reverts with + /// @custom:reverts NameSoulbound rather than returning zero. Rejects a zero `to` with /// @custom:reverts ERC721InvalidReceiver, an unminted `tokenId` with /// @custom:reverts ERC721NonexistentToken via the underlying `ownerOf`, and requires /// the protocol registry to have an escrow configured (otherwise @@ -146,6 +157,7 @@ interface IDotnsRegistrar is IERC721 { /// @dev The registrar's `_update` hook consults @custom:function PopRules.transferFloor /// to compute the required transfer fee; if the caller does not forward at least that /// amount as `msg.value`, the transfer reverts with @custom:reverts TransferFeeRequired. + /// A soulbound name is non-transferable and reverts with @custom:reverts NameSoulbound. /// The `payable` modifier on every transfer overload exists so the fee can be forwarded /// in the same call. function safeTransferFrom( @@ -161,12 +173,14 @@ interface IDotnsRegistrar is IERC721 { /// @inheritdoc IERC721 /// @dev Subject to the same fee-on-transfer gate as the four-argument overload; reverts with /// @custom:reverts TransferFeeRequired when the recipient owes a non-zero transfer fee and - /// the caller has not forwarded it as `msg.value`. + /// the caller has not forwarded it as `msg.value`, and with @custom:reverts NameSoulbound when + /// the token is soulbound. function safeTransferFrom(address from, address to, uint256 tokenId) external payable override; /// @inheritdoc IERC721 /// @dev Subject to the same fee-on-transfer gate as the safe overloads; reverts with /// @custom:reverts TransferFeeRequired when the recipient owes a non-zero transfer fee and - /// the caller has not forwarded it as `msg.value`. + /// the caller has not forwarded it as `msg.value`, and with @custom:reverts NameSoulbound when + /// the token is soulbound. function transferFrom(address from, address to, uint256 tokenId) external payable override; } diff --git a/test/intergration/PopLifecycleFlow.t.sol b/test/intergration/PopLifecycleFlow.t.sol index 95ef722c7..7d08e736a 100644 --- a/test/intergration/PopLifecycleFlow.t.sol +++ b/test/intergration/PopLifecycleFlow.t.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.34; import {BaseDotns} from "../base/BaseDotns.t.sol"; import {IDotnsPopController} from "../../contracts/registrars/IDotnsPopController.sol"; +import {IDotnsRegistrar} from "../../contracts/registrars/IDotnsRegistrar.sol"; import {IDotnsRegistry} from "../../contracts/registry/IDotnsRegistry.sol"; import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; import {ILabelStore} from "../../contracts/store/ILabelStore.sol"; @@ -47,7 +48,7 @@ contract PopLifecycleFlow is BaseDotns { assertEq(ownerStore.getLabel(fullNode), string.concat(FULL_LABEL, protocolRegistry.tld())); } - function test_pop_full_name_is_first_class_erc721_name() public { + function test_pop_full_name_is_soulbound_but_fully_usable() public { _mintLiteThenClaimFull(ed); bytes32 fullNode = _nodeOf(FULL_LABEL); @@ -60,7 +61,9 @@ contract PopLifecycleFlow is BaseDotns { assertEq(dotnsPopResolver.chatKey(fullNode), CHAT_KEY); assertEq(dotnsPopResolver.liteLink(fullNode), liteLabelhash); assertEq(dotnsPopResolver.fullClaim(liteLabelhash), fullNode); + assertTrue(dotnsRegistrar.isSoulbound(fullTokenId)); + // The name is fully usable by its owner: records and subnames work. vm.prank(ed); dotnsContentResolver.setContenthash(fullNode, CONTENT_HASH_A); assertEq(dotnsContentResolver.contenthash(fullNode), CONTENT_HASH_A); @@ -68,35 +71,32 @@ contract PopLifecycleFlow is BaseDotns { bytes32 subnode = _setSubnode(ed, fullNode, SUB_LABEL, FULL_LABEL, leonardo); assertEq(dotnsRegistry.owner(subnode), leonardo); - uint256 _xferFee = dotnsRegistrar.quoteTransferFee(fullTokenId, tiago); + // It is soulbound: quoting a transfer and attempting one both revert, and ownership + // does not move. + vm.expectRevert(abi.encodeWithSelector(IDotnsRegistrar.NameSoulbound.selector, fullTokenId)); + dotnsRegistrar.quoteTransferFee(fullTokenId, tiago); + + vm.expectRevert(abi.encodeWithSelector(IDotnsRegistrar.NameSoulbound.selector, fullTokenId)); vm.prank(ed); - dotnsRegistrar.transferFrom{value: _xferFee}(ed, tiago, fullTokenId); - // Post-transfer invariants. Only ownership fields change; PoP-layer - // records are keyed by node and survive intact. - assertEq(IERC721(address(dotnsRegistrar)).ownerOf(fullTokenId), tiago); - assertEq(dotnsRegistry.owner(fullNode), tiago); - assertEq(dotnsRegistrar.labelOf(fullTokenId), FULL_LABEL); + dotnsRegistrar.transferFrom(ed, tiago, fullTokenId); + + assertEq(IERC721(address(dotnsRegistrar)).ownerOf(fullTokenId), ed); + assertEq(dotnsRegistry.owner(fullNode), ed); + // PoP-layer records and the owner's continued control are untouched by the blocked move. assertEq(dotnsPopResolver.chatKey(fullNode), CHAT_KEY); - assertEq(dotnsPopResolver.liteLink(fullNode), liteLabelhash); assertEq(dotnsPopResolver.fullClaim(liteLabelhash), fullNode); assertEq(dotnsContentResolver.contenthash(fullNode), CONTENT_HASH_A); assertEq(dotnsRegistry.owner(subnode), leonardo); - // The new owner drives node writes and subname reassignments. - vm.prank(tiago); + + vm.prank(ed); dotnsContentResolver.setContenthash(fullNode, CONTENT_HASH_B); assertEq(dotnsContentResolver.contenthash(fullNode), CONTENT_HASH_B); - bytes32 reassignedSubnode = _setSubnode(tiago, fullNode, SUB_LABEL, FULL_LABEL, ed); + bytes32 reassignedSubnode = _setSubnode(ed, fullNode, SUB_LABEL, FULL_LABEL, tiago); assertEq(reassignedSubnode, subnode); - assertEq(dotnsRegistry.owner(subnode), ed); - // The lite token is not transferred alongside the full token. - assertEq(IERC721(address(dotnsRegistrar)).ownerOf(uint256(_nodeOf(LITE_LABEL))), ed); - // Store writes are one-shot-locked at registration time, so the label - // stays under the original owner's Store even after transfer. - ILabelStore edStore = ILabelStore(storeFactory.getLabelStore(ed)); - assertEq( - edStore.getLabel(_nodeOf(FULL_LABEL)), string.concat(FULL_LABEL, protocolRegistry.tld()) - ); + assertEq(dotnsRegistry.owner(subnode), tiago); + // The lite token is also gateway-minted and equally soulbound. + assertTrue(dotnsRegistrar.isSoulbound(uint256(_nodeOf(LITE_LABEL)))); } function test_cold_gateway_reserve_then_user_settles_pending_claim() public { @@ -177,7 +177,7 @@ contract PopLifecycleFlow is BaseDotns { assertEq(dotnsPopController.pendingClaimUserCount(), 0); } - function test_transfer_of_token_with_live_pending_claim_does_not_move_claim() public { + function test_gateway_name_with_live_pending_claim_is_soulbound_and_settles_for_owner() public { _grantPopFull(ed); _gatewayReserveLiteName( IDotnsPopController.LiteRegistration({ @@ -186,20 +186,20 @@ contract PopLifecycleFlow is BaseDotns { ); uint256 tokenId = uint256(_nodeOf(LITE_LABEL)); + assertTrue(dotnsRegistrar.isSoulbound(tokenId)); + // The gateway name is soulbound while its claim is still pending, so it cannot be moved + // out of the beneficiary's wallet before settlement. This is the path the issue closes: + // a pre-claim transfer previously escaped tier pricing entirely. + vm.expectRevert(abi.encodeWithSelector(IDotnsRegistrar.NameSoulbound.selector, tokenId)); vm.prank(ed); dotnsRegistrar.transferFrom(ed, tiago, tokenId); - assertEq(IERC721(address(dotnsRegistrar)).ownerOf(tokenId), tiago); + assertEq(IERC721(address(dotnsRegistrar)).ownerOf(tokenId), ed); - // The pending claim is keyed by the original user (`ed`) and stays put - // when the token transfers. `tiago` receives an empty `LabelStore` via - // the registrar's transfer-sync path because `ed` has no store yet to - // copy the label from; this is current `DotnsRegistrar._update` - // behaviour and is independent of the pending-claim mapping. + // The pending claim is keyed by the original user and still settles into their store. IDotnsPopController.PendingClaim[] memory pending = dotnsPopController.pendingClaims(ed, 0, type(uint256).max); assertEq(pending[0].label, LITE_LABEL); assertGt(pending[0].mintedAt, 0); - assertEq(dotnsPopController.pendingClaimCountOf(tiago), 0); vm.prank(ed); dotnsPopController.settlePendingClaims(ed, type(uint256).max); diff --git a/test/invariant/registrar/DotnsRegistrarSoulboundInvariant.t.sol b/test/invariant/registrar/DotnsRegistrarSoulboundInvariant.t.sol new file mode 100644 index 000000000..014fe288f --- /dev/null +++ b/test/invariant/registrar/DotnsRegistrarSoulboundInvariant.t.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.34; + +import {BaseDotns} from "../../base/BaseDotns.t.sol"; +import {SoulboundHandler} from "./SoulboundHandler.t.sol"; + +/// @title Dotns Registrar Soulbound Invariant Suite +/// @notice Asserts that PoP-gateway (soulbound) names never change owner across any reachable +/// sequence of mints and transfer attempts, and that the campaign actually exercises +/// soulbound tokens rather than passing vacuously. +contract DotnsRegistrarSoulboundInvariantTest is BaseDotns { + /// @notice Handler driving randomised soulbound mints and transfer attempts. + SoulboundHandler public handler; + + /// @notice Deploys the handler against the registrar and the configured PoP controller, seeds + /// one soulbound token so the non-vacuity guard always has a target, and points the + /// fuzzer at the handler's selectors only. + function setUp() public override { + super.setUp(); + + handler = new SoulboundHandler( + dotnsRegistrar, address(dotnsPopController), protocolRegistry.tldNode() + ); + handler.addActor(ed); + handler.addActor(leonardo); + handler.addActor(tiago); + handler.mintSoulbound(0); + + targetContract(address(handler)); + + bytes4[] memory selectors = new bytes4[](2); + selectors[0] = handler.mintSoulbound.selector; + selectors[1] = handler.attemptSoulboundTransfer.selector; + targetSelector(FuzzSelector({addr: address(handler), selectors: selectors})); + + excludeContract(address(dotnsRegistrar)); + excludeContract(address(dotnsPopController)); + excludeContract(address(dotnsRegistrarController)); + excludeContract(address(dotnsRegistry)); + excludeContract(address(dotnsNameEscrow)); + excludeContract(address(protocolRegistry)); + excludeContract(address(storeFactory)); + } + + /// @notice The campaign must mint at least one soulbound token, otherwise the ownership + /// invariant below would hold vacuously over an empty set. + function invariant_soulbound_coverage_is_non_vacuous() public view { + assertGt(handler.soulboundCount(), 0, "no soulbound token was ever minted"); + } + + /// @notice No transfer of a soulbound token may ever succeed. This is a fixture-independent + /// signal: it trips on a wrongful success regardless of fees, receiver type, or escrow + /// configuration, so it catches a removed or weakened gate even where ownership drift alone + /// might be masked by an unrelated revert. + function invariant_no_soulbound_transfer_succeeds() public view { + assertFalse(handler.sawSuccessfulTransfer(), "a soulbound transfer succeeded"); + } + + /// @notice Every soulbound token keeps its mint-time owner and its soulbound flag no matter + /// what transfer attempts the fuzzer interleaves. + function invariant_soulbound_owner_never_changes() public view { + uint256[] memory ids = handler.soulboundIdsList(); + for (uint256 i = 0; i < ids.length; i++) { + assertTrue(dotnsRegistrar.isSoulbound(ids[i]), "token stopped being soulbound"); + assertEq( + dotnsRegistrar.ownerOf(ids[i]), + handler.soulboundOwner(ids[i]), + "soulbound token owner moved" + ); + } + } +} diff --git a/test/invariant/registrar/SoulboundHandler.t.sol b/test/invariant/registrar/SoulboundHandler.t.sol new file mode 100644 index 000000000..02e36cd5d --- /dev/null +++ b/test/invariant/registrar/SoulboundHandler.t.sol @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.34; + +import {Test} from "forge-std/Test.sol"; +import {DotnsRegistrar} from "../../../contracts/registrars/DotnsRegistrar.sol"; + +/// @title Soulbound Handler +/// @notice Bounded random-action handler that mints PoP-gateway (soulbound) names and throws every +/// transfer overload at them, so the invariant suite can assert their ownership never +/// moves across a reachable action sequence. +/// @dev Mints are driven by pranking as the address registered under `POP_CONTROLLER`, which is the +/// exact provenance the registrar checks, so the minted tokens are genuinely soulbound. The +/// handler never asserts inline: a transfer that wrongly succeeds leaves observable ownership +/// drift for the invariant view to catch, and expected reverts are swallowed so the campaign +/// keeps a low reject rate. +contract SoulboundHandler is Test { + /// @notice The base registrar under test. + DotnsRegistrar public immutable REGISTRAR; + + /// @notice Address registered under `POP_CONTROLLER`; pranked as the soulbound minter. + address public immutable POP_CONTROLLER_ADDR; + + /// @notice TLD node the suite is rooted at, used to derive token ids the way the protocol does. + bytes32 private immutable TLD_NODE; + + /// @notice Actor pool the handler mints to and transfers between. + address[] internal actors; + + /// @notice Every soulbound token id minted through the handler. + uint256[] internal soulboundIds; + + /// @notice Owner recorded at mint for each soulbound id. Ghost expectation for the invariant. + mapping(uint256 tokenId => address owner) public soulboundOwner; + + /// @notice Count of soulbound tokens minted, exposed for the non-vacuity guard. + uint256 public soulboundCount; + + /// @notice Number of transfer attempts driven against soulbound tokens, exposed so the suite + /// can prove the campaign actually exercised transfers rather than only minting. + uint256 public transferAttemptCount; + + /// @notice Trips true if any transfer of a soulbound token ever succeeds. A correct gate keeps + /// this false forever, independent of fees, receivers, or escrow configuration. + bool public sawSuccessfulTransfer; + + /// @notice Monotonic label suffix so every mint targets a fresh id. + uint64 internal nonce; + + constructor(DotnsRegistrar registrar, address popController, bytes32 tldNode) { + REGISTRAR = registrar; + POP_CONTROLLER_ADDR = popController; + TLD_NODE = tldNode; + } + + /// @notice Adds an actor to the pool. + function addActor(address actor) external { + actors.push(actor); + } + + /// @notice Mints one soulbound name to a pool actor. Callable from the suite to guarantee at + /// least one soulbound token exists before fuzzing. + function mintSoulbound(uint256 actorSeed) public { + if (actors.length == 0) return; + address actor = actors[actorSeed % actors.length]; + uint256 tokenId = _freshTokenId(); + + vm.prank(POP_CONTROLLER_ADDR); + REGISTRAR.register(tokenId, actor, ""); + + soulboundIds.push(tokenId); + soulboundOwner[tokenId] = actor; + ++soulboundCount; + } + + /// @notice Attempts to move a soulbound token through one of the transfer overloads, including + /// an operator-driven move, and swallows the expected revert. Any wrongful success is left for + /// the invariant view to detect as ownership drift. + function attemptSoulboundTransfer(uint256 tokenSeed, uint256 toSeed, uint256 variant) external { + uint256 count = soulboundIds.length; + if (count == 0) return; + + uint256 tokenId = soulboundIds[tokenSeed % count]; + address from = REGISTRAR.ownerOf(tokenId); + address to = actors[toSeed % actors.length]; + if (to == from) return; + + ++transferAttemptCount; + variant %= 4; + if (variant == 3) { + address operator = actors[(toSeed + 1) % actors.length]; + vm.prank(from); + REGISTRAR.setApprovalForAll(operator, true); + vm.prank(operator); + try REGISTRAR.transferFrom(from, to, tokenId) { + sawSuccessfulTransfer = true; + } catch {} + return; + } + + vm.startPrank(from); + if (variant == 0) { + try REGISTRAR.transferFrom(from, to, tokenId) { + sawSuccessfulTransfer = true; + } catch {} + } else if (variant == 1) { + try REGISTRAR.safeTransferFrom(from, to, tokenId) { + sawSuccessfulTransfer = true; + } catch {} + } else { + try REGISTRAR.safeTransferFrom(from, to, tokenId, "") { + sawSuccessfulTransfer = true; + } catch {} + } + vm.stopPrank(); + } + + /// @notice Returns every soulbound id minted, for the invariant view to iterate. + function soulboundIdsList() external view returns (uint256[] memory ids) { + ids = soulboundIds; + } + + /// @notice Derives a fresh token id as `namehash(tldNode, keccak256(label))` for a unique + /// label. + function _freshTokenId() internal returns (uint256 tokenId) { + ++nonce; + bytes32 labelhash = keccak256(bytes(string.concat("sb", vm.toString(nonce)))); + tokenId = uint256(keccak256(abi.encodePacked(TLD_NODE, labelhash))); + } +} diff --git a/test/unit/registrar/DotnsRegistrar.t.sol b/test/unit/registrar/DotnsRegistrar.t.sol index c4866ffda..bc986c587 100644 --- a/test/unit/registrar/DotnsRegistrar.t.sol +++ b/test/unit/registrar/DotnsRegistrar.t.sol @@ -403,4 +403,214 @@ contract DotnsRegistrarTests is BaseDotns { assertTrue(dotnsRegistrar.isApprovedForAll(nameOwner, operator)); assertTrue(dotnsRegistrar.supportsInterface(type(IERC721).interfaceId)); } + + /// @notice Mints a name as if through the PoP gateway: the caller is the address registered + /// under `POP_CONTROLLER`, and the gateway path passes an empty label. + function _mintSoulbound( + string memory label, + address nameOwner + ) + internal + returns (uint256 tokenId) + { + tokenId = _tokenIdForLabel(label); + vm.prank(address(dotnsPopController)); + dotnsRegistrar.register(tokenId, nameOwner, ""); + } + + function test_pop_gateway_mint_marks_soulbound() public { + uint256 tokenId = _mintSoulbound("soulbound01", ed); + + assertEq(dotnsRegistrar.ownerOf(tokenId), ed, "mint must not be blocked"); + assertTrue(dotnsRegistrar.isSoulbound(tokenId)); + } + + function test_public_mint_is_not_soulbound() public { + string memory label = "public01"; + uint256 tokenId = _tokenIdForLabel(label); + + vm.prank(address(dotnsRegistrarController)); + dotnsRegistrar.register(tokenId, ed, label); + + assertFalse(dotnsRegistrar.isSoulbound(tokenId)); + } + + function test_soulbound_provenance_is_only_the_pop_controller() public { + // A third authorised controller that is not the registry POP_CONTROLLER must mint + // transferable names: provenance is read from the registry, not from mere controller + // membership. + address thirdController = makeAddr("thirdController"); + vm.prank(owner); + dotnsRegistrar.addController(IDotnsController(thirdController)); + + string memory label = "thirdctrl01"; + uint256 tokenId = _tokenIdForLabel(label); + vm.prank(thirdController); + dotnsRegistrar.register(tokenId, ed, label); + + assertFalse(dotnsRegistrar.isSoulbound(tokenId)); + } + + function test_soulbound_transfer_from_reverts_and_owner_unchanged() public { + uint256 tokenId = _mintSoulbound("sbtransfer01", ed); + + vm.expectRevert(abi.encodeWithSelector(IDotnsRegistrar.NameSoulbound.selector, tokenId)); + vm.prank(ed); + dotnsRegistrar.transferFrom(ed, leonardo, tokenId); + + assertEq( + dotnsRegistrar.ownerOf(tokenId), ed, "ownership must not move on a reverted transfer" + ); + } + + function test_soulbound_safe_transfer_from_reverts_and_owner_unchanged() public { + uint256 tokenId = _mintSoulbound("sbsafe01", ed); + + vm.expectRevert(abi.encodeWithSelector(IDotnsRegistrar.NameSoulbound.selector, tokenId)); + vm.prank(ed); + dotnsRegistrar.safeTransferFrom(ed, leonardo, tokenId); + + assertEq(dotnsRegistrar.ownerOf(tokenId), ed); + } + + function test_soulbound_safe_transfer_from_with_data_reverts_and_owner_unchanged() public { + uint256 tokenId = _mintSoulbound("sbsafedata01", ed); + + vm.expectRevert(abi.encodeWithSelector(IDotnsRegistrar.NameSoulbound.selector, tokenId)); + vm.prank(ed); + dotnsRegistrar.safeTransferFrom(ed, leonardo, tokenId, bytes("payload")); + + assertEq(dotnsRegistrar.ownerOf(tokenId), ed); + } + + function test_soulbound_operator_transfer_reverts_and_owner_unchanged() public { + // An approved operator cannot move a soulbound name either: the transfer hook rejects the + // move regardless of who initiates it. + uint256 tokenId = _mintSoulbound("sboperator01", ed); + + vm.prank(ed); + dotnsRegistrar.setApprovalForAll(leonardo, true); + + vm.expectRevert(abi.encodeWithSelector(IDotnsRegistrar.NameSoulbound.selector, tokenId)); + vm.prank(leonardo); + dotnsRegistrar.transferFrom(ed, tiago, tokenId); + + assertEq(dotnsRegistrar.ownerOf(tokenId), ed); + } + + function test_soulbound_transfer_reverts_even_when_escrow_unconfigured() public { + // The gate sits before any escrow lookup, so a soulbound transfer is rejected without + // depending on the escrow being configured. + uint256 tokenId = _mintSoulbound("sbnoescrow01", ed); + + vm.mockCall( + address(protocolRegistry), + abi.encodeWithSelector(IDotnsProtocolRegistry.get.selector, DotnsConstants.NAME_ESCROW), + abi.encode(address(0)) + ); + + vm.expectRevert(abi.encodeWithSelector(IDotnsRegistrar.NameSoulbound.selector, tokenId)); + vm.prank(ed); + dotnsRegistrar.transferFrom(ed, leonardo, tokenId); + + assertEq(dotnsRegistrar.ownerOf(tokenId), ed); + } + + function test_soulbound_quote_transfer_fee_reverts() public { + uint256 tokenId = _mintSoulbound("sbquote01", ed); + + vm.expectRevert(abi.encodeWithSelector(IDotnsRegistrar.NameSoulbound.selector, tokenId)); + dotnsRegistrar.quoteTransferFee(tokenId, leonardo); + } + + function test_soulbound_cannot_enter_escrow() public { + // A soulbound name holds no escrow position, so `release` rejects it before any custody + // move is attempted. The name can never be released into escrow. + uint256 tokenId = _mintSoulbound("sbescrow01", ed); + + vm.startPrank(ed); + dotnsRegistrar.setApprovalForAll(address(dotnsNameEscrow), true); + vm.expectRevert( + abi.encodeWithSelector(IDotnsNameEscrow.DepositNotConfigured.selector, tokenId) + ); + dotnsNameEscrow.release(tokenId); + vm.stopPrank(); + + assertEq(dotnsRegistrar.ownerOf(tokenId), ed); + } + + function test_public_name_remains_transferable() public { + // Control: a publicly registered name with no fee owed still transfers normally, so the + // soulbound gate does not regress the ordinary path. + string memory label = "publicmove01"; + _register(label, ed, IPopRules.PopStatus.NoStatus); + uint256 tokenId = _tokenIdForLabel(label); + + assertFalse(dotnsRegistrar.isSoulbound(tokenId)); + + vm.prank(ed); + dotnsRegistrar.transferFrom(ed, leonardo, tokenId); + + assertEq(dotnsRegistrar.ownerOf(tokenId), leonardo); + } + + function test_soulbound_with_label_transfer_reverts() public { + // A soulbound name that carries a label (the PoP controller may register a non-empty + // label) still cannot be transferred. This exercises the labelled branch that would run + // the transfer-floor quote if the gate were absent. + string memory label = "sblabelled01"; + uint256 tokenId = _tokenIdForLabel(label); + vm.prank(address(dotnsPopController)); + dotnsRegistrar.register(tokenId, ed, label); + + assertTrue(dotnsRegistrar.isSoulbound(tokenId)); + assertEq(dotnsRegistrar.labelOf(tokenId), label); + + vm.expectRevert(abi.encodeWithSelector(IDotnsRegistrar.NameSoulbound.selector, tokenId)); + vm.prank(ed); + dotnsRegistrar.transferFrom(ed, leonardo, tokenId); + + assertEq(dotnsRegistrar.ownerOf(tokenId), ed); + } + + function test_soulbound_transfer_with_value_reverts_and_returns_value() public { + // Attaching value to a blocked soulbound transfer must revert and strand no funds: the + // sender's balance is intact and the registrar holds nothing afterwards. + uint256 tokenId = _mintSoulbound("sbvalue01", ed); + vm.deal(ed, 1 ether); + + vm.expectRevert(abi.encodeWithSelector(IDotnsRegistrar.NameSoulbound.selector, tokenId)); + vm.prank(ed); + dotnsRegistrar.transferFrom{value: 1 ether}(ed, leonardo, tokenId); + + assertEq(dotnsRegistrar.ownerOf(tokenId), ed); + assertEq(ed.balance, 1 ether, "attached value must be returned on revert"); + assertEq(address(dotnsRegistrar).balance, 0, "registrar must not retain value"); + } + + function test_soulbound_self_transfer_reverts() public { + // A soulbound name reverts even on a same-address move, keeping transferFrom in step with + // quoteTransferFee (which reverts regardless of recipient) and the interface contract. + uint256 tokenId = _mintSoulbound("sbself01", ed); + + vm.expectRevert(abi.encodeWithSelector(IDotnsRegistrar.NameSoulbound.selector, tokenId)); + vm.prank(ed); + dotnsRegistrar.transferFrom(ed, ed, tokenId); + + assertEq(dotnsRegistrar.ownerOf(tokenId), ed); + } + + function test_soulbound_single_token_approvee_transfer_reverts() public { + // A single-token approvee is blocked on the same gate as the owner and operators. + uint256 tokenId = _mintSoulbound("sbapprovee01", ed); + + vm.prank(ed); + dotnsRegistrar.approve(leonardo, tokenId); + + vm.expectRevert(abi.encodeWithSelector(IDotnsRegistrar.NameSoulbound.selector, tokenId)); + vm.prank(leonardo); + dotnsRegistrar.transferFrom(ed, tiago, tokenId); + + assertEq(dotnsRegistrar.ownerOf(tokenId), ed); + } } From 464023b25372603349c21672d012fb5b2e121665 Mon Sep 17 00:00:00 2001 From: Siphamandla Mjoli Date: Tue, 1 Sep 2026 17:02:20 +0200 Subject: [PATCH 2/3] fix: address PR comments --- README.md | 6 ++++-- contracts/registrars/DotnsRegistrar.sol | 7 +++---- contracts/registrars/IDotnsRegistrar.sol | 6 +++++- test/unit/registrar/DotnsRegistrar.t.sol | 13 ++++++++++++- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index ba95a34f8..66d1b9865 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,8 @@ Both are stamped at release time, so a governance change never moves the clocks Once `redeemableUntil` passes, `reclaim` is permissionless through the ordinary commit-reveal path, whether or not the previous holder ever withdrew. If the position still holds value, reclaim settles it: the amount is credited to the previous holder's pull-payment balance and stays claimable with no deadline. The value follows the departing holder; the name does not wait for them. Cross-paid registrations seed zero-amount positions, so those names have nothing to withdraw and nothing to settle. +Gateway-issued (soulbound) names never enter this lifecycle. Releasing a name transfers it into escrow custody, and a soulbound name reverts on any transfer, so it holds no position and has no release, redeem, or reclaim path. It stays with the person who earned it. + ### Cost-model versioning D and F are fixed for the life of a pricing model. Changing either means deploying a fresh model with the new values and registering it, at which point it becomes the current version; there is no live setter that edits the numbers in place. Governance can also point the current version back at an earlier registered model to roll a change back. An in-flight registration prices at the version it committed to, so a model change between commit and reveal leaves its cost unchanged. @@ -138,7 +140,7 @@ Dedicated controller for the Proof-of-Personhood gateway flow. Lives behind its Today the Pop gateway does not write a standalone user-status mapping. It materialises the PoP flow through gateway-issued labels, PoP resolver records, and reservation queue state; user tier checks for public pricing still come from the personhood precompile/context read. -The first, reserveBaseName, mints a lite-person username to a user. The gateway-facing input is a stem.suffix shape: a single DNS label followed by exactly one dot and a digits-only suffix of exactly two digits (for example michal.03). The controller normalises that input by stripping the dot before classification, pricing, and minting, so the on-chain label is always flat (michal.03 becomes michal03). Inputs with more than one dot, no dot, a non-digit suffix, or a suffix length other than two digits are rejected at the boundary. The stem may be any DNS-valid label of at least six characters, not only the 6 to 8 of the public PopLite tier; only governance-reserved stems (five characters or fewer) are rejected. A lite username whose stem is nine characters or longer classifies as NoStatus for public pricing and transfers, so its lite status is an issuance property rather than an economic tier. The call also persists the user's chat key on the PoP resolver and optionally enqueues a reservation for a full-person base name the user intends to claim later. +The first, reserveBaseName, mints a lite-person username to a user. The gateway-facing input is a stem.suffix shape: a single DNS label followed by exactly one dot and a digits-only suffix of exactly two digits (for example michal.03). The controller normalises that input by stripping the dot before classification, pricing, and minting, so the on-chain label is always flat (michal.03 becomes michal03). Inputs with more than one dot, no dot, a non-digit suffix, or a suffix length other than two digits are rejected at the boundary. The stem may be any DNS-valid label of at least six characters, not only the 6 to 8 of the public PopLite tier; only governance-reserved stems (five characters or fewer) are rejected. A lite username whose stem is nine characters or longer classifies as NoStatus for public pricing, so its lite status is an issuance property rather than an economic tier. The call also persists the user's chat key on the PoP resolver and optionally enqueues a reservation for a full-person base name the user intends to claim later. The second, registerBaseName, mints a full-person username. Whether the call is a claim against a prior lite reservation or a fresh standalone registration is derived from on-chain reservation state; the caller does not choose. The link argument selects the chat-key source: inherit from a prior lite label, or accept a fresh one in the payload. When inheriting, the call also writes the liteLink (full => lite) and fullClaim (lite => full) records on the PoP resolver in the same transaction so downstream consumers can resolve either direction without scanning events. @@ -162,7 +164,7 @@ The dispatcher exists to work around a runtime limitation: the substrate Root or ERC721-backed registrar that mints ownership of label IDs (labelhashes). Minting is restricted to every address in the controllers mapping; the mapping is owner-gated through addController and removeController. Every other contract in the system that needs to check "is this address authorised to drive name state?" consults this mapping rather than keeping a parallel list, which is what lets multiple controllers coexist on the same registrar without per-contract configuration changes. -The registrar owns transferability. A name is marked soulbound at mint when the caller is the address registered under POP_CONTROLLER, so only the PoP gateway can issue a soulbound name and no other controller can lock a public one. The marker is set once and never cleared, and isSoulbound reports it. The transfer hook rejects every transfer of a soulbound name, mint and same-address moves aside, so a gateway-issued name cannot leave its holder's wallet or enter escrow. +The registrar owns transferability. A name is marked soulbound at mint when the caller is the address registered under POP_CONTROLLER, so only the PoP gateway can issue a soulbound name and no other controller can lock a public one. The marker is set once and never cleared, and isSoulbound reports it. The transfer hook rejects every transfer of a soulbound name, including a move to the holder's own address and a release into escrow, so a gateway-issued name can never leave its holder's wallet. Only the mint is exempt, because the marker is set just after it. ### DotnsRegistry diff --git a/contracts/registrars/DotnsRegistrar.sol b/contracts/registrars/DotnsRegistrar.sol index 3ee411007..3f413c122 100644 --- a/contracts/registrars/DotnsRegistrar.sol +++ b/contracts/registrars/DotnsRegistrar.sol @@ -148,11 +148,10 @@ contract DotnsRegistrar is // canonical PoP controller mints soulbound names, so a compromised or buggy peer controller // cannot lock a public name and the PoP controller cannot mint an unlocked one. Written // only on the true branch to leave the public path free of a redundant zero write. - if (msg.sender == protocolRegistry.get(DotnsConstants.POP_CONTROLLER)) { - _soulbound[id] = true; - } + bool soulbound = msg.sender == protocolRegistry.get(DotnsConstants.POP_CONTROLLER); + if (soulbound) _soulbound[id] = true; if (bytes(label).length != 0) _writeOwnerLabel(owner, id, label); - emit NameRegistered(id, owner); + emit NameRegistered(id, owner, soulbound); } /// @inheritdoc IDotnsRegistrar diff --git a/contracts/registrars/IDotnsRegistrar.sol b/contracts/registrars/IDotnsRegistrar.sol index 5000015fd..04d9fbbfe 100644 --- a/contracts/registrars/IDotnsRegistrar.sol +++ b/contracts/registrars/IDotnsRegistrar.sol @@ -48,7 +48,11 @@ interface IDotnsRegistrar is IERC721 { error NameSoulbound(uint256 tokenId); /// @notice Emitted when a name is registered. - event NameRegistered(uint256 indexed id, address indexed owner); + /// @param id The token id (namehash node) that was minted. + /// @param owner The address that received the name. + /// @param soulbound True when the name is soulbound: PoP-gateway minted and non-transferable. + /// Lets indexers classify registrations without a per-token @custom:function isSoulbound read. + event NameRegistered(uint256 indexed id, address indexed owner, bool soulbound); /// @notice Emitted when a controller is added. /// @dev Typed as the shared baseline @custom:contract IDotnsController so the commit-reveal diff --git a/test/unit/registrar/DotnsRegistrar.t.sol b/test/unit/registrar/DotnsRegistrar.t.sol index bc986c587..8d1779443 100644 --- a/test/unit/registrar/DotnsRegistrar.t.sol +++ b/test/unit/registrar/DotnsRegistrar.t.sol @@ -98,7 +98,7 @@ contract DotnsRegistrarTests is BaseDotns { uint256 tokenId = _tokenIdForLabel(label); vm.expectEmit(true, true, false, true, address(dotnsRegistrar)); - emit IDotnsRegistrar.NameRegistered(tokenId, ed); + emit IDotnsRegistrar.NameRegistered(tokenId, ed, false); vm.prank(address(dotnsRegistrarController)); dotnsRegistrar.register(tokenId, ed, label); @@ -425,6 +425,17 @@ contract DotnsRegistrarTests is BaseDotns { assertTrue(dotnsRegistrar.isSoulbound(tokenId)); } + function test_pop_gateway_mint_emits_soulbound_true() public { + string memory label = "sbemit01"; + uint256 tokenId = _tokenIdForLabel(label); + + vm.expectEmit(true, true, false, true, address(dotnsRegistrar)); + emit IDotnsRegistrar.NameRegistered(tokenId, ed, true); + + vm.prank(address(dotnsPopController)); + dotnsRegistrar.register(tokenId, ed, ""); + } + function test_public_mint_is_not_soulbound() public { string memory label = "public01"; uint256 tokenId = _tokenIdForLabel(label); From 1f80792153bd5802281ca34dde3774719f8a72e2 Mon Sep 17 00:00:00 2001 From: Siphamandla Mjoli Date: Tue, 1 Sep 2026 21:41:19 +0200 Subject: [PATCH 3/3] fix: flaky test --- test/invariant/registrar/RegistrarControllerHandler.t.sol | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/invariant/registrar/RegistrarControllerHandler.t.sol b/test/invariant/registrar/RegistrarControllerHandler.t.sol index edbb9816a..bd81e23b4 100644 --- a/test/invariant/registrar/RegistrarControllerHandler.t.sol +++ b/test/invariant/registrar/RegistrarControllerHandler.t.sol @@ -144,6 +144,13 @@ contract RegistrarControllerHandler is Test { /// @param actor The actor address. /// @param status The PoP status to assign. function addActor(address actor, IPopRules.PopStatus status) external { + // Actors model real users, so reject the zero address and any contract. The fuzzer's + // address dictionary contains protocol contracts, and if the escrow were admitted as an + // actor a name could be transferred into it: an escrow-touching transfer skips the label + // mirror, leaving the escrow as a recipient with no store and breaking the recipient-store + // invariant for a reason unrelated to user transfers. + if (actor == address(0) || actor.code.length != 0) return; + if (!_isActor[actor]) { _isActor[actor] = true; actors.push(actor);