fix(Registration): root origin gates to correctly identify the caller as root

[GHkrishna]

Description

What

Four contracts gated governance on SystemUtils.originIsRoot(), which is transaction scoped: true in every frame of a Root-origin transaction, including a direct Root caller. Any contract that a Root dispatch happened to reach, inherits full DotNS governance authority while holding no DotNS key.

This PR introduces DotnsRootGateway, a non-upgradeable contract that Root dispatches to directly and moves all four gates to an address comparison against it, via a new GovernanceAuth library.

No external selector changes. No storage-layout changes. No polkadot-sdk changes. (Relation to polkadot-sdk added below ni the notes section)

Why

Root on Asset Hub is chain-wide. It is dispatched to fix a stuck contract for an unrelated project, run a migration, push a remediation — decided by voters who have never heard of DotNS. DotNS had bound its admin gate to that credential.

A referendum passes to call FooRecovery.sweep() and return locked funds to users. That contract also contains:

whitelist.grantName("polkadot", attacker);
popRules.setShortNamesEnabled(true);

Both hit require(SystemUtils.originIsRoot(), ...). The transaction was Root-dispatched, so both succeed. There is no exploit primitive, no reentrancy, no overflow, no clever calldata. The attacker calls the public function and it works; all the difficulty is in being in the stack. Nothing in the referendum shows it: the dispatch is Revive.call { dest: 0xF00…, data: sweep() } and DotNS appears nowhere. A confused access failure, not a governance failure.

Severity is medium, not worse, because no path inside DotNS reaches an attacker-controlled address (the mint is _mint, not _safeMint; the only low-level call is a refund on the priced path), so it needs a Root dispatch aimed at code governance does not control. The free-mint limb additionally needs a live commitment inside a two-sided window with a matching pricingVersion, at a moment the attacker does not schedule.

14 entry points sat on that one predicate:

Contract	Surface
DotnsRegistrarController.registerReserved	free mint, whitelist skipped, 3-to-8 band included
PopRules.setShortNamesEnabled	the short-name switch, no commitment delay
DotnsNameWhitelist	10 functions: grantName, grantNames, revokeName, setReserved, setWindow, accept, reject, setMaxClaimants, setMaxReasonBytes, setMaxGrantBatch
DotnsPopController	reserveLiteName, reserveBaseName, reserveBaseNameOnly, registerBaseName

DotnsPopController._onlyRoot was the only one that reasoned about stack scope at all, and its invariant was outbound only, "nothing reachable from an onlyRoot entrypoint may call a user controlled address". The code follows that. The inbound calls were missed.

Why simply 'callerIsRoot' cannot be used

callerIsRoot is the right primitive and does not work for these contracts. The System precompile dispatches it as env.caller_is_root(true), resolving frames().nth(2), the caller of the contract asking. A delegatecall calls push_frame, so it occupies a real frame:

Shape	nth(2)	callerIsRoot
Root → plain contract	None → falls back to Stack::origin = Root	true
Root → UUPS proxy → impl	the proxy's own non-delegate frame	false
Root → non-upgradeable forwarder	None → Root	true

Row 2 is every DotNS contract, so callerIsRoot reads false even on a direct one-hop Root dispatch. msg.sender is not a fallback either: the delegate frame's caller is Root, which has no address, so the PVM caller syscall traps. Note these are two different failure modes: one returns false, one reverts. That left originIsRoot, which answers truthfully but answers a broader question.

Upstream will not fix this. #12051 "callerIsRoot doesnt survive proxy delegatecalls" (head branch dotns/system-precompile) was an attempt by @sphamjoli to make callerIsRoot walk delegate-only frames; the PR was never merged though. #12281 added originIsRoot instead. We need to have that in a longer tem.

The fix

contracts/governance/DotnsRootGateway.sol non-upgradeable, no storage, no owner, immutable registry pointer:

function execute(address[] calldata targets, bytes[] calldata payloads) external {
    require(SystemUtils.callerIsRoot(), NotRoot());
    // each target must be isRegisteredAddress; forward by regular CALL;
}

It is the one frame shape where callerIsRoot is answerable, and it forwards by regular CALL, so downstream msg.sender is a readable address naming it exactly.

All four gates now go through one library:

require(GovernanceAuth.isGovernance(protocolRegistry, msg.sender), NotGovernance());

GovernanceAuth owns the fail closed guard for an unwired key, so there is a single place to audit rather than four copies to keep in sync.

SystemUtils.originIsRoot() is removed, not deprecated.

Consequences to review

1. A third owner-level route. DotnsProtocolRegistry.set is onlyOwner, so the owner can point ROOT_GATEWAY at a contract it controls. This changes nothing in practice. The owner authorises every UUPS upgrade and could always ship an implementation with no gate, but DEPLOYMENTS.md listed two owner-level routes and now correctly lists three.

2. The gateway must be the direct dest of the Root dispatch. Anything between Root and it (Multicall3, another forwarder) occupies the frame callerIsRoot inspects and the check reads false. That is why execute batches internally. Substrate-level Utility.batch of separate Revive.calls is fine, each starts its own stack.

3. Non-upgradeable is load-bearing, and it is still replaceable. A proxy in front of the gateway would compile, deploy, and silently brick every governance entry point. Rotation is protocolRegistry.set(ROOT_GATEWAY, newOne). The registry re-direction is the actual upgrade path.

4. Registry membership is not authority, so registering the gateway grants it nothing. LabelStore gates on StoreAuth.isStoreWriter (registrar / registrar controllers / registry), and StoreAuth.sol:11-18 explicitly rejects isRegisteredAddress as a gate, naming the registered-forwarder case.

Migration order

1. Deploy DotnsRootGateway
2. Add ROOT_GATEWAY in the protocol registry
3. Upgrade the four implementations

Reversed, the governance surface reverts until the key is added. Recoverable; the zero-address guard in GovernanceAuth makes it fail loudly rather than silently comparing against address(0).

Type

• Bug fix
• Feature
• Breaking change
• Documentation
• Chore
• Refactor
• Security

Scope

• Registration
• Resolver
• Store
• Proof of Personhood
• Deployment scripts
• Tests

Related Issues

Fixes

#300

Checklist

Code

• Follows project style
• forge build passes
• forge test passes
• No new compiler warnings

Testing

• New tests added for changed behavior
• Fuzz tests added where applicable
• Invariant tests verified

Security

• No new selfdestruct or delegatecall
• Access control reviewed
• No storage layout conflicts (for upgradeable contracts)

Documentation

• NatSpec updated on changed interfaces
• [ ]x README updated if needed

Governance dispatch section added in DEPLOYMENTS.md with the wrapped cast recipe, and two corrections that described a guarantee the Root branch never delivered: registerReserved "requires a grant naming registration.owner", and "no single key can grant a name".

CONTRIBUTING.md records the deploy-stage requirement (new contracts must be deployed from a stage in scripts/deploy/run.sh, not DotnsDeployer.s.sol, which build-genesis.sh never runs) and the gateway as the one sanctioned exception to the no-immutable-peers rule.

Breaking Changes

• No breaking changes
• Breaking changes documented below

Breaking changes:

Operational, not ABI. Every external selector, event, storage slot and deployed address on existing contracts is unchanged, and the public paths (commit, register, registerReserved as a grantee) are exactly identical.

how a governance call is dispatched changes as below:

before

Revive.call { dest: <DotnsNameWhitelist>, data: grantName(string,address) }

After (Root calls the gateway, which forwards that same inner call):

  Revive.call {
    dest: <DotnsRootGateway>,
    data: execute(
      targets:  [<DotnsNameWhitelist>],
      payloads: [grantName("alice", 0xBeneficiary)]
    )
  }

Anything that builds a Root dispatch must be updated: governance runbooks, referendum-prep scripts, and any sudo helper in dotns-cli. Tooling that only reads, registers, or resolves names needs no change. The deployment manifest gains a DotnsRootGateway entry at deploy time.

How to test

# the new contract, end to end
forge test --mp 'test/unit/governance/*'

# the regression for the finding: a contract reached mid-transaction holds no authority
forge test --mt test_governance_rejects_an_intermediate_contract

# the gate rejects a third party on each surface
forge test --mt test_governance_rejects_a_third_party_caller

# full suite as CI shards it
forge test --no-match-contract Invariant --no-match-path 'test/fork/**'
FOUNDRY_PROFILE=ci forge test --match-contract Invariant

Notes

Naming: we already use "gateway" for the PoP path. The README calls DotnsPopController "the personhood gateway". DotnsRootGateway overloads that. The old RootGatewayDispatcher used "gateway" for the Root path too, so there's precedent either way. If anyone feels strongly, DotnsRootDispatcher works and it's a cheap rename now. Once it's deployed the address is in the manifest and renaming means a redeploy.

Left out on purpose: requiring a grant on the governance branch of registerReserved. Worth doing at some point, since it clears the stranded-grant case at DotnsRegistrarController.sol:309-315, but it doesn't fix this and on its own it would have made things worse. grantName sits behind the same gate and does more: an attacker inside a governance transaction just grants themselves the name and registers it later through the normal path, whenever they like, with no commitment window to line up. Patching only registerReserved makes the attack easier, not harder.

Separate upstream change: We can file callerIsRootThroughDelegate() against polkadot-sdk, a new precompile method with #12051, Spha's semantics. If that lands and Asset Hub picks it up, the gateway goes away and SystemUtils changes by one line. That'might take some time though.

Before deploy: commit the updated deployments//.json once the gateway is deployed, same as any new contract.

[github-actions]

CI Summary

Check	Result
4naly3er Analysis	Found 39 issues: 1 high, 5 medium, 9 low, 13 gas, 11 informational - View Report
Slither Analysis	Found 195 issues: 4 high, 40 medium, 92 low, 59 informational - View Report
Contract Tests (Unit + Fuzz)	All tests passed (665 total) - View Report
Contract Tests (Invariant)	All tests passed (58 total) - View Report
Gas Report	9 contracts analyzed - View Report
Coverage	Failed - Tests failed, cannot compute coverage
Documentation	Passed - 70 pages generated - View Docs
Format & Lint	Passed - Code formatted correctly
File Validation	Passed - All tracked files valid
Deploy Contracts	Failed - addresses differ from the committed manifest
PR Title	PR Title Valid
Labels	Unknown
Secret Scan	Passed - No secrets detected

[re-gius]

I believe the architecture makes sense, so I am generally in favor of this change. However, I would postpone it to R2 for the following reasons:

1. In this R1 setting where the owner still retains significant power, a single owner-signed call can make any new contract trusted as governance. This does not extend significantly the vulnerability surface, since the owner already has unlimited upgrading power, but a registry set operations is definitely less-watched than a proxy upgrade, which is itself more silent than a root referendum (however shady the referendum call may be). This makes a critical change to the root gateway far less visible than a dangerous root call.
2. It makes sense to have this audited and tested end-to-end for R2, once we move all the main power to root. Now we still have significant ownership powers to potentially address a mischievous root call (which is unlikely)
3. R2 may let us build a better final solution if any change to the polkadot-sdk lands in the meantime

[GHkrishna]

Closing as not required, refer #300 comments for the alternate fix