fix(server): resolve the dotNS controller whether the gateway stores a dispatcher or the controller - #564
fix(server): resolve the dotNS controller whether the gateway stores a dispatcher or the controller#564re-gius wants to merge 3 commits into
Conversation
| decode_address(&output) | ||
| .map(Some) | ||
| .map_err(|err| format!("RootGatewayDispatcher.TARGET(): {err}")) | ||
| match transport.view(&stored, call_no_args("TARGET()")).await { |
There was a problem hiding this comment.
The CLI caches the controller for the run, but the core doesnt. lookup_dotns_identity builds a fresh DotnsLookup and calls discover_pop_controller on every lookup, so once chains are repointed every username resolution pays the reverting TARGET() first.
Each step carries OPERATION_TIMEOUT (10s) inside a 45s LOOKUP_BUDGET, so on a slow node that revert can eat a fifth of the budget for no information. Worth caching the controller in the core, or flipping the probe order when the repoint lands?
There was a problem hiding this comment.
With e8ea25a we have protocolRegistry() first now, so we moved the delay to the outdated path. Caching would be ideal, but this is just a temporary change and we should move to the controller-only path by the end of the week
| /// The stored address is either a `RootGatewayDispatcher`, whose `TARGET()` is the | ||
| /// controller, or the controller itself. Both are in service: a chain keeps its | ||
| /// dispatcher until the gateway pallet is repointed. `TARGET()` decides which, and a | ||
| /// revert means the address is not a dispatcher, since the contract executed and had no |
There was a problem hiding this comment.
The whole scheme depends on TARGET() existing only on the dispatcher. Everywhere else here fails closed, but this one is different: if the controller ever gains a TARGET(), discovery takes the dispatcher branch and decode_address hands back whatever it returned. Thats a wrong address, not an error.
Does dotns#258 guarantee the controller has no TARGET(), and is that guaranteed to hold after it, or is it just true today?
There was a problem hiding this comment.
It was just true today and we cannot say anything about the controller's future form, so I fixed it in e8ea25a by inverting the probe: protocolRegistry() goes first now, so discovery keys on a function the controller has rather than one it lacks.
2 participants
Description
This is a transitional step, deliberately shaped to avoid breaking changes after paritytech/dotns#258 .
DotnsGateway.DispatcherAddressis about to change meaning. Today it holds aRootGatewayDispatcher, anddiscover_pop_controllercallsTARGET()on it to reach theDotnsPopController. paritytech/dotns#258 moves the substrate Root check into the controller itself, after which the dispatcher has no purpose and the gateway pallet gets repointed at the controller proxy viaset_dispatcher_address. From that moment the stored address has noTARGET(), discovery errors, and host username resolution stops working.Switching outright to "the stored address is the controller" would have to land in the same instant as that extrinsic, on every chain at once. Instead discovery now works out which contract it is looking at, so one binary serves both states.
TARGET()exists only on the dispatcher andprotocolRegistry()only on the controller, so a revert from one identifies the other. A revert is an answer about the contract; a node failure is not an answer at all.TARGET()protocolRegistry()Not a breaking change. On every chain as it stands the stored address is still a dispatcher,
TARGET()still answers, and the resolved controller is identical so the new branch is never reached. No configuration changes are needed, andHOST_CLI_DOTNS_POP_CONTROLLERstill overrides discovery unchanged. Nothing here waits on dotns#258 either.Temporary by design. Once every chain stores the controller, the
TARGET()hop and theprotocolRegistry()confirmation both become dead weight and discovery collapses to reading the address and using it. That cleanup is a follow-up, not part of this PR, because until then the dispatcher path is the live one on previewnet and paseo-next-v2. The cost of carrying both in the meantime is one extra reverting view per resolution after a repoint,.The identity.rs test mock still scripts the dispatcher shape (:341, :440), matching today's chains; the new unit tests cover the repointed shape. Point it at a controller later and it panics unscripted view rather than returning a wrong address.