feat: in-place upgrade for the numeric-namespace proxies - #289
Conversation
A lite username `alice.01` is registered as `alice` beneath the numeric container `01` rather than as one atomic label. The container is minted on first use, owned by and soulbound to the PoP controller, and the username is a subnode the person owns in the registry. Re-issuing an existing lite username reverts LiteNameAlreadyIssued. IDotnsRegistry.SubnodeRecord gains a persist flag so the registry writes only the ownership and resolver record and leaves the LabelStore write to the caller. A SubnodeUtils library holds the subname registration and the shared node derivation. DotnsPopLens and DotnsReverseResolver read a lite name's ownership through the registry and derive its node by splitting on the separator, so lite names enumerate and reverse-resolve. The full-person path and the public commit-reveal path are unchanged.
The lite fuzz suite resolved a username as an atomic label under the top level and read its ownership from the registrar. Derive the node under the numeric container and read ownership from the registry, matching how a lite username is now issued.
Brings the numeric-namespace change onto the upgrade-tooling branch and adds the PR-scoped upgrade scripts, pinned pre-upgrade snapshots, and fork tests that swap the registry, PoP controller, PoP rules, and reverse resolver proxies in place and redeploy the lens, so every address is kept. The fork suite issues a lite username against live state to prove it lands as a subname beneath its numeric container.
CI Summary
Labelssmartcontracts, other, scope: registration, scope: resolver, type: test, scope: pop |
github-actions
Bot
added
smartcontracts
re-gius
left a comment
re-gius
left a comment
There was a problem hiding this comment.
The swaps look safe against Paseo v0.6.0 except the PopController Old snapshot omitting _popIssued; before broadcasting, lock the script order, treat persist as a live setSubnodeOwner ABI break, and rehearse a pre-upgrade lite ERC-721 — this e2e only proves a fresh mint.
| /// the registry beneath the numeric container `01`, and the container is a real name | ||
| /// held by the controller, not an atomic registrar token. | ||
| function test_lite_username_is_issued_as_a_subname_after_the_upgrade() public { | ||
| rulesUpgrader.upgradePopRules(popRulesOwner, popRules); |
There was a problem hiding this comment.
This is the only place in the PR that encodes an upgrade order, and it is only three of the five broadcasts:
UpgradePopRulesUpgradeRegistryUpgradePopController
UpgradeReverseResolver and RedeployPopLens are not run here. upgrade.sh is on the tooling branch, not in this diff, and it still takes a single SCRIPT= so a one-off can be run in any order.
The numeric-namespace path only works in one sequence.
Options, pick one:
- Treat this test as the runbook: add the reverse and lens harnesses after the controller, and state the sequence plus the window in the test NatSpec.
- Add a one-shot script on this branch (
UpgradeNumericNamespace.s.solor a thin wrapper) that runs the five internals in that order and is what you actually broadcast.
Either way the order needs to live in a file this PR introduces, not only in upgrade.sh on the base branch.
There was a problem hiding this comment.
The mint of a fresh michael.01 is the happy path, not the v0.6.0 migration.
A pre-upgrade lite name (already an ERC-721 on v0.6.0) is untested: after the swap liteNamesOf still lists a settled one, while nameDetail and reverse do not.
Options, pick one:
- Before any swap, pick or mint a dotted lite token, snapshot owner / chat key / reverse /
isPopIssued, then after the swaps assert: the old ERC-721 is still there;liteNamesOfstill lists it if settled;nameDetail/nameOf/ lite→full link do not;reserveLiteNameof the same string succeeds at the container-then-stem node. - Do that check by hand on the fork before broadcasting, and say so in the PR body.
I would take the first.
| /// @dev The layout diff runs inside `Upgrades.upgradeProxy` before the implementation swap. No | ||
| /// `unsafeSkipAllChecks` or `unsafeAllow` override is set, so an incompatible layout | ||
| /// aborts the run rather than corrupting state. An empty upgrade call is passed because the new | ||
| /// implementation seeds no storage: `_popIssued` defaults to false for every label, which |
There was a problem hiding this comment.
This comment is wrong for Paseo v0.6.0.
v0.6.0 already has _popIssued in this slot (#275). The new impl keeps it and only shrinks __gap 50 → 49, so live true bits survive. An empty upgrade call does not reset them.
DotnsPopControllerOld omits that mapping, so the layout diff is against pre-0.6.0, not the deployed code.
Fix: copy the v0.6.0 controller as the Old snapshot (_popIssued + __gap[50]) and drop the “defaults to false” wording.
Optional: in the fork test, assert a live isPopIssued == true is still true after the swap. Today it only checks labels the test itself just minted.
| ILabelStore labelStore = ILabelStore(store); | ||
| uint256 stored = labelStore.getLabelCount(); | ||
| for (uint256 i; i < stored; ++i) { | ||
| bytes32 node = labelStore.getLabelhashAt(i); |
There was a problem hiding this comment.
A settled v0.6.0 lite name does not disappear from the new path.
liteNamesOf / _countNames walk the LabelStore by the stored node (the old ERC-721). registry.owner still delegates to the registrar, and isPopIssued stays true, so that name remains in the listing.
nameDetail (:88) and reverse go through _nodeOf (:303) → the hierarchical node, which does not exist, so they look empty.
The PR says a pre-upgrade lite name is not read and can be issued again. That is true for label→node reads, not for the listing. If anyone on Paseo has a settled alice.01 token, the lens will still list it after the swap, while nameDetail("alice.01") will not.
Options:
- Accept the split and document it (listing = store node, detail/reverse = new node).
- When the label is lite, also require
_ownedBy(_nodeOf(label))in the store walk, so old tokens drop out of the listing.
If Paseo has no settled lite names, this is dormant, but then prove that on the fork.
SubnodeUtils reuses a parent on ownership alone rather than requiring it be soulbound, so it is a general-purpose subname issuer; the container stays soulbound because the controller mints it that way. One `liteSubnodeOf` derives a lite name's node, shared by the controller, lens, and reverse resolver. The controller takes the subnode from the registry write, so the record and the chat-key and store writes land on one node. The lens binds each store row to its own text and classifies a cold-path lite name from the caller's label, so a pending subname reports its tier before it settles. The settlement invariant checks a lite name as a registry subname, and new tests cover the container lifecycle, a lite reverse claim, cold-path classification, and the store-row provenance guard. Stale atomic-label NatSpec is corrected.
Brings the generic SubnodeUtils, the unified lite-node derivation, the registry write returning the subnode, and the lens store-row and cold-path fixes onto the in-place upgrade branch, so the swapped implementations carry them.
The store write behind `persist` is gated to protocol store writers (the registrar and its controllers), not the name owner, and the registry writes it only on creation or on a reassignment to a new owner. A deferring writer backfills by deploying the owner's store and writing the label itself, which is what settlement does. Registry tests cover the no-store-write path, that a same-owner re-call does not backfill, and the authorised-writer backfill.
Brings the clarified `persist` documentation and the registry store-write tests onto the in-place upgrade branch so it stays in step with the feature branch.
2 participants
Description
Upgrades the deployed proxies in place so the numeric-namespace change reaches live state while
every address is kept. It swaps the registry, PoP controller, PoP rules, and reverse resolver
implementations behind their existing proxies and redeploys the immutable lens, repointing the
popLenskey. Each proxy swap resolves its target from the on-disk manifest and diffs the newstorage layout against a pinned pre-upgrade snapshot, failing closed if a slot moves, shrinks, or
changes type.
Fork tests run each script against live Paseo Asset Hub state through the ETH-RPC adapter. One test
per upgrade proves the swap keeps the address, the owner, and the state; an end-to-end test issues a
lite username and checks it lands as a subname beneath its numeric container, owned in the registry
rather than held as a token.
Every upgrade script, pre-upgrade snapshot, and fork test in this change is scoped to this pull
request and removed before merge, so the default branch keeps no upgrade scaffolding and the fork
suite runs zero tests between upgrades.
This branch is based on the upgrade-tooling branch and carries the numeric-namespace contract change
so the swapped implementations contain it.
Migration note: lite usernames issued before this upgrade were recorded as atomic labels under the
top level. The upgraded code addresses a lite name at the container-then-stem node, so a pre-upgrade
lite name is not read by the new path and its subname node can be issued afresh. That overwrite is
accepted.
Type
Scope
Related Issues
Fixes
Checklist
Code
forge buildpassesforge testpassesTesting
Security
selfdestructordelegatecallDocumentation
Breaking Changes
Breaking changes:
migrated. Their subname node can be issued afresh, so a pre-upgrade lite name is not preserved.
How to test
Fork tests need the local ETH-RPC adapter on
paseo_local.Notes
Upgrade scripts run through
scripts/deploy/upgrade.sh, for exampleSCRIPT=UpgradeRegistry ACCOUNT_NAME=<keystore> RPC_URL=<network> ./scripts/deploy/upgrade.sh.