Conversation
The TLD was a compile-time constant in DotnsConstants, so every network rendered names as `<label>.dot` and testnets were indistinguishable from mainnet. Move the TLD onto DotnsProtocolRegistry as the single on-chain authority. `initialize` takes a bare TLD label, derives the TLD node and suffix, and exposes `tldNode()` and `tld()`. Every consumer reads them live rather than compiling in a constant, so LabelUtils no longer folds a fixed node and the `TLD`/`DOT_NODE` constants are removed. The deploy pipeline sources the label from the `DOTNS_TLD` env var (default `dot`), keyed off the operator rather than the chain id since networks can share one chain id.
CI Summary
4naly3er AnalysisMedium (5)
Low (10)
Gas (14)
Informational (11)
Slither AnalysisHigh (3)
Medium (39)
Low (54)
Informational (61)
Contract Tests (Unit + Fuzz)BasicDotnsIntegrationReverts (test/intergration/BasicDotns.reverts.t.sol)
BasicDotnsIntegration (test/intergration/BasicDotns.t.sol)
DeployCreate3FactoryTest (test/unit/deploy/DeployCreate3Factory.t.sol)
DeterministicDeploymentTest (test/unit/deploy/DeterministicDeployment.t.sol)
DotnsContentResolverTests (test/unit/resolver/DotnsContentResolver.t.sol)
DotnsNameEscrowTest (test/unit/escrow/DotnsNameEscrow.t.sol)
DotnsNameEscrowFuzzTest (test/fuzz/escrow/DotnsNameEscrowFuzz.t.sol)
DotnsNameEscrowRefundsTest (test/unit/escrow/DotnsNameEscrowRefunds.t.sol)
DotnsPopControllerTests (test/unit/registrar/DotnsPopController.t.sol)
DotnsPopControllerFuzz (test/fuzz/registrar/DotnsPopControllerFuzz.t.sol)
DotnsPopResolverTests (test/unit/resolver/DotnsPopResolver.t.sol)
DotnsProtocolRegistryTldTests (test/unit/registry/DotnsProtocolRegistry.t.sol)
DotnsProtocolRegistryFuzzTest (test/fuzz/registry/DotnsProtocolRegistryFuzz.t.sol)
DotnsRegistrarTests (test/unit/registrar/DotnsRegistrar.t.sol)
DotnsRegistrarControllerTest (test/unit/registrar/DotnsRegistrarController.t.sol)
DotnsRegistrarControllerFuzzTest (test/fuzz/registrar/DotnsRegistrarControllerFuzz.t.sol)
DotnsRegistrarControllerLifecycleTest (test/unit/registrar/DotnsRegistrarControllerLifecycle.t.sol)
DotnsRegistryTests (test/unit/registry/DotnsRegistry.t.sol)
DotnsRegistryFuzzTest (test/fuzz/registry/DotnsRegistryFuzz.t.sol)
DotnsResolverTests (test/unit/resolver/DotnsResolver.t.sol)
DotnsReverseResolverTests (test/unit/resolver/DotnsReverseResolver.t.sol)
LabelStoreTests (test/unit/store/LabelStore.t.sol)
LabelStoreFuzzTest (test/fuzz/store/LabelStoreFuzz.t.sol)
NoStatusDepositLifecycle (test/intergration/NoStatusDepositLifecycle.t.sol)
PopRulesFuzzTest (test/fuzz/pop/PopFuzz.t.sol)
PopLifecycleFlow (test/intergration/PopLifecycleFlow.t.sol)
PopRulesTests (test/unit/pop/PopRules.t.sol)
RootGatewayDispatcherTests (test/unit/registrar/RootGatewayDispatcher.t.sol)
StoreFactoryTests (test/unit/store/StoreFactory.t.sol)
StoreIntegrationTest (test/intergration/StoreIntegration.t.sol)
StoreStressTest (test/stress/store/StoreStress.t.sol)
UserStoreTests (test/unit/store/UserStore.t.sol)
UserStoreFuzzTest (test/fuzz/store/UserStoreFuzz.t.sol)
WhitelistOperatorFlow (test/intergration/WhitelistOperatorFlow.t.sol)
Contract Tests (Invariant)DotnsNameEscrowInvariantTest (test/invariant/escrow/DotnsNameEscrowInvariant.t.sol)
DotnsPopControllerInvariant (test/invariant/registrar/DotnsPopControllerInvariant.t.sol)
DotnsRegistrarControllerInvariantTest (test/invariant/registrar/DotnsRegistrarControllerInvariant.t.sol)
DotnsRegistrarControllerRoleInvariantTest (test/invariant/registrar/DotnsRegistrarControllerRoleInvariant.t.sol)
DotnsRegistryInvariantTest (test/invariant/registry/DotnsRegistryInvariant.t.sol)
StoreInvariantTest (test/invariant/store/StoreInvariant.t.sol)
Gas ReportDotnsContentResolver
DotnsProtocolRegistry
DotnsRegistrar
DotnsRegistrarController
DotnsRegistry
DotnsResolver
DotnsReverseResolver
PopRules
Deploy ContractsDeployed addresses vs DEPLOYMENTS.mdExpected is the committed manifest; actual is this CI deployment of the same pipeline.
Labelssmartcontracts, other, scope: registration, scope: resolver, type: test, scope: pop |
|
I'm against these changes, especially in the current structure. First, this adds many cross-contract calls on the registration and transfer paths just to recover the TLD. Making it a constant, instead, trades that for different deployed bytecode per chain. Neither outcome is desirable. Second, the confusion only arises when a UI renders a name without saying which network it came from, but every client actually knows that, so I would handle this directly in UI. Finally, I think this makes testnets less faithful: having different names and different token IDs with respect to mainnet makes testing and integration slightly more error-prone. I also do not see urgency with mainnet approaching: mainnet wants .dot regardless, which is what we already have, so this only affects deployments we can redeploy at will. I would postpone the discussion until after the first mainnet deployment. |
Every consumer that reads the TLD already holds protocolRegistry and already calls Regarding the constant per-chain bytecode costs more, just not in gas. The deployed artifact then differs by network, so source verification, the OZ storage-layout baseline, and the audit surface all diverge, and every upgrade has to be validated against each chain's baseline. Which isnt ideal.
The TLD isn't only a display value, so the UI can't be where this lives. It's written into on-chain state, and the node/tokenId derivation all include it. Those get read back by consumers, not just our client. Every client knows the network only covers the clients we write. A contract reading a reverse record, or any other consumer, gets the name verbatim and has nowhere to inject a network label. Fixing it in the UI means every one of those consumers has to re-apply the same fix, instead of the name being correct at the source. There's also the collision. If every network uses .dot, then alice has the same namehash and the same tokenId everywhere. A client pointed at the wrong RPC gets a real-looking alice.dot owned by someone else, and nothing in the data says it's the wrong network. A distinct TLD makes that case not resolve at all, which is the behaviour I'd want.
I'd argue different token IDs make integration safer, not more error-prone. If a testnet and mainnet both mint alice under .dot, they share a tokenId, and nothing stops a testnet token or address book entry from being read as the mainnet one. Different TLDs mean the IDs can't coincide, so a wrong-network mixup fails instead of resolving to a plausible-but-wrong name. The point on faithfullness, tests need to mirror is the behaviour, not the literal string. The registration, commit-reveal, pricing, and PoP paths all derive from the configured TLD already, so the same code runs whatever the label is. Nothing in the suite depends on it being .dot. A testnet on its own TLD exercises the identical logic; it just doesn't produce IDs that can be confused with mainnet.
This isnt my decision as it was already requested in the Polkadot web channel |
re-gius
left a comment
There was a problem hiding this comment.
LGTM - just minor comments
@re-gius please check again |
Description
The TLD was a compile-time constant (
TLDandDOT_NODEinDotnsConstants), so every network rendered names as<label>.dotand a testnet name was indistinguishable from mainnet.This change makes the TLD per-network by moving it onto
DotnsProtocolRegistryas the single on-chain authority:DotnsProtocolRegistry.initialize(string tldLabel)takes a bare label (e.g.dot,paseo,test), validates it as a single DNS label, derives the TLD node (namehash(0, labelhash(tldLabel))) and the suffix (.<label>), and exposes them viatldNode()andtld().DotnsRegistry,DotnsRegistrar,DotnsRegistrarController,DotnsPopController,DotnsReverseResolver) reads the TLD live from the registry instead of a constant.LabelUtilsno longer folds a fixed node; its helpers take the TLD node/suffix as parameters.DotnsConstants.TLDandDotnsConstants.DOT_NODEare removed.DOTNS_TLDenv var (defaultdot) viaBaseDeployer.tldLabel(), keyed off the operator rather than the chain id because networks can share one chain id.Type
Scope
Related Issues
#201
Fixes
Fixes #201
Checklist
Code
forge buildpassesforge testpassesTesting
Security
selfdestructordelegatecallDocumentation
Breaking Changes
Breaking changes:
DotnsProtocolRegistry.initialize()now takes astring tldLabelargument.DotnsConstants.TLDandDotnsConstants.DOT_NODEare removed; callers readtldNode()/tld()from the protocol registry.LabelUtils.namehash(bytes32)andstripDotTld(string)are replaced bynamehashUnder(tldNode, labelhash),deriveNode(tldNode, label), andstripTld(tldSuffix, fullName).How to test
The whole suite runs against one TLD defined in one place (
BaseDotns.TLD_LABEL); change that constant to run every test under a different network TLD.Notes
The
DOTNS_TLDlabel never enters CREATE3 salts, so deterministic addresses and manifests are unchanged across TLD choices.