dandri/simplexmq
1
0
Fork 0
mirror of https://github.com/simplex-chat/simplexmq.git synced 2026-09-14 04:27:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

resolver: reduce comments, remove REVIEW.html

Browse Source
This commit is contained in:
sh
2026-09-04 15:05:14 +00:00
parent bcc9b36e8f
commit c47aeddb18
8 changed files with 37 additions and 457 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/resolver/README.md
+13 -16
View File
@@ -139,10 +139,9 @@ usual format (EIP-55, bech32, SS58, Monero base58). Subnames work the same way
### Registration status and expiry
A response carries `status`, `expires` and `graceEnds` whenever the resolver
got far enough to read them, a successful resolve included. A client that has
just resolved a name therefore already has its expiry, and needs no second
request to warn about it. `expires` and `graceEnds` are Unix timestamps in
seconds, and both are `null` when the resolver could not read them.
read them, a successful resolve included, so a client that has just resolved a
name already knows when it expires. Both timestamps are Unix seconds, and
`null` when they could not be read.
| `status` | Meaning |
|---|---|
@@ -154,19 +153,17 @@ seconds, and both are `null` when the resolver could not read them.
| `noResolver` | registered, but points nowhere |
| `unknown` | no `SNRC_REGISTRAR_<TLD>` configured, so status could not be read |
The resolver tells `grace` and `expired` apart with the registrar's own
`available(id)` rule, `expires + GRACE_PERIOD < now`. The resolver reads
`GRACE_PERIOD` from the contract instead of assuming it, and takes `now` from
the latest block's timestamp instead of the host clock. The registrar compares
against that same block timestamp, so a machine with a wrong clock cannot
misreport a registration.
`grace` and `expired` are told apart by the registrar's own `available(id)`
rule, `expires + GRACE_PERIOD < now`. `GRACE_PERIOD` is read from the contract
rather than assumed, and `now` is the latest block's timestamp rather than the
host clock, which the registrar compares against too, so a machine with a wrong
clock cannot misreport a registration. That rule alone is not enough: it also
holds for a name nobody ever registered (`0 + GRACE_PERIOD < now`), so a zero
expiry is what separates *never registered* from *registered and since
released*.
`available(id)` on its own cannot tell the two apart, because it is also true
for a name nobody ever registered: `0 + GRACE_PERIOD < now`. The resolver uses
a zero expiry to tell *never registered* from *registered and since released*.
A subname reports the status of the 2LD above it. That is the answer a client
needs, because a subname is only as good as the name it sits under.
A subname reports the status of the 2LD above it, which is only as good as the
name it sits under.
### Querying by labelhash
scripts/resolver/REVIEW.html
-304
View File
@@ -1,304 +0,0 @@
<title>Fifteen Calls a Name</title>
<link rel="preconnect" href="https://fonts.googleapis.com">
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=Newsreader:opsz,wght@6..72,400;6..72,500;6..72,600&family=IBM+Plex+Mono:wght@400;500&family=IBM+Plex+Sans:wght@400;450;600&display=swap">
<style>
:root {
--paper: #f1f2f5; --card: #ffffff; --rule: #d9dce3; --rule-soft: #e7e9ee;
--ink: #1b1d24; --ink-2: #464b57; --ink-3: #6d7381;
--accent: #0a5fd6; --accent-bg: #e6eefc;
--crit: #b32b23; --crit-bg: #fbe9e7;
--high: #9a5c05; --high-bg: #fbf0dd;
--med: #4a3fb0; --med-bg: #ecebfa;
--ok: #196b40; --ok-bg: #e4f2ea;
--code-bg: #f4f5f8;
--shadow: 0 1px 2px rgba(27,29,36,.06), 0 8px 24px -18px rgba(27,29,36,.4);
}
@media (prefers-color-scheme: dark) {
:root:not([data-theme="light"]) {
--paper: #14161c; --card: #1c1f27; --rule: #333844; --rule-soft: #272b35;
--ink: #eceef3; --ink-2: #b7bcc8; --ink-3: #878e9e;
--accent: #6ba5ff; --accent-bg: #16233a;
--crit: #ff8b80; --crit-bg: #33191a;
--high: #e3ab4d; --high-bg: #332616;
--med: #a79cf5; --med-bg: #221f3a;
--ok: #6bcf98; --ok-bg: #162a20;
--code-bg: #171a21;
--shadow: 0 1px 2px rgba(0,0,0,.4), 0 8px 24px -18px rgba(0,0,0,.9);
}
}
:root[data-theme="dark"] {
--paper: #14161c; --card: #1c1f27; --rule: #333844; --rule-soft: #272b35;
--ink: #eceef3; --ink-2: #b7bcc8; --ink-3: #878e9e;
--accent: #6ba5ff; --accent-bg: #16233a;
--crit: #ff8b80; --crit-bg: #33191a;
--high: #e3ab4d; --high-bg: #332616;
--med: #a79cf5; --med-bg: #221f3a;
--ok: #6bcf98; --ok-bg: #162a20;
--code-bg: #171a21;
--shadow: 0 1px 2px rgba(0,0,0,.4), 0 8px 24px -18px rgba(0,0,0,.9);
}
* { box-sizing: border-box; }
body {
margin: 0; background: var(--paper); color: var(--ink);
font-family: "IBM Plex Sans", system-ui, -apple-system, "Segoe UI", sans-serif;
font-size: 16px; line-height: 1.62; -webkit-font-smoothing: antialiased;
}
.wrap { max-width: 1000px; margin: 0 auto; padding: 0 28px 96px; }
.mast { border-bottom: 1px solid var(--rule); margin-bottom: 44px; padding: 56px 0 26px; }
.eyebrow {
font-family: "IBM Plex Mono", ui-monospace, Menlo, monospace;
font-size: 11.5px; letter-spacing: .1em; text-transform: uppercase;
color: var(--ink-3); margin: 0 0 18px;
}
h1 {
font-family: Newsreader, Georgia, serif;
font-size: clamp(38px, 6.4vw, 62px); font-weight: 500; line-height: 1.04;
letter-spacing: -.015em; margin: 0 0 20px; text-wrap: balance;
}
.standfirst {
font-family: Newsreader, Georgia, serif; font-size: 20px; line-height: 1.52;
color: var(--ink-2); max-width: 62ch; margin: 0 0 14px;
}
.runline {
display: flex; flex-wrap: wrap; gap: 10px 22px; margin-top: 24px;
font-family: "IBM Plex Mono", ui-monospace, Menlo, monospace;
font-size: 11.5px; color: var(--ink-3);
}
.runline b { color: var(--ink-2); font-weight: 500; }
section { margin-top: 58px; }
h2 {
font-family: Newsreader, Georgia, serif; font-size: 30px; font-weight: 500;
letter-spacing: -.01em; margin: 0 0 8px; text-wrap: balance;
}
.lede { color: var(--ink-2); max-width: 70ch; margin: 0 0 26px; }
.f {
background: var(--card); border: 1px solid var(--rule); border-radius: 10px;
padding: 24px 26px; margin-bottom: 20px; box-shadow: var(--shadow);
}
.f.critical { border-left: 4px solid var(--crit); }
.f.high { border-left: 4px solid var(--high); }
.f.medium { border-left: 4px solid var(--med); }
.f-head { display: flex; flex-wrap: wrap; align-items: baseline; gap: 12px; }
.f-num { font-family: "IBM Plex Mono", ui-monospace, Menlo, monospace; font-size: 12px; color: var(--ink-3); }
.pill {
display: inline-block; white-space: nowrap;
font-family: "IBM Plex Mono", ui-monospace, Menlo, monospace;
font-size: 10.5px; letter-spacing: .05em; text-transform: uppercase;
padding: 3px 9px; border-radius: 999px; font-weight: 500;
}
.p-crit { color: var(--crit); background: var(--crit-bg); }
.p-high { color: var(--high); background: var(--high-bg); }
.p-med { color: var(--med); background: var(--med-bg); }
.p-mine { color: var(--card); background: var(--accent); }
h3 {
font-family: Newsreader, Georgia, serif; font-size: 23px; font-weight: 500;
line-height: 1.24; margin: 8px 0 12px; text-wrap: balance;
}
.f p { margin: 0 0 12px; color: var(--ink-2); max-width: 74ch; }
.f p:last-of-type { margin-bottom: 0; }
.f strong { color: var(--ink); font-weight: 600; }
pre {
background: var(--code-bg); border: 1px solid var(--rule-soft); border-radius: 8px;
padding: 13px 15px; overflow-x: auto; margin: 14px 0;
font-family: "IBM Plex Mono", ui-monospace, Menlo, monospace;
font-size: 12.5px; line-height: 1.6; color: var(--ink);
}
code {
font-family: "IBM Plex Mono", ui-monospace, Menlo, monospace;
font-size: 13px; background: var(--code-bg); padding: 1px 5px; border-radius: 4px;
}
.fix { background: var(--accent-bg); border-radius: 8px; padding: 13px 16px; margin-top: 14px; }
.fix b {
color: var(--accent); font-family: "IBM Plex Mono", ui-monospace, Menlo, monospace;
font-size: 10.5px; letter-spacing: .09em; text-transform: uppercase;
display: block; margin-bottom: 6px;
}
.fix p { margin: 0; color: var(--ink); }
.refs {
margin-top: 16px; padding-top: 12px; border-top: 1px solid var(--rule-soft);
font-family: "IBM Plex Mono", ui-monospace, Menlo, monospace;
font-size: 11px; color: var(--ink-3);
}
footer {
margin-top: 70px; padding-top: 22px; border-top: 1px solid var(--rule);
font-size: 13.5px; color: var(--ink-3); max-width: 78ch;
}
a { color: var(--accent); }
:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; border-radius: 3px; }
</style>
<div class="wrap">
<header class="mast">
<p class="eyebrow">SNRC resolver · adversarial + developer experience</p>
<h1>Fifteen Calls a Name</h1>
<p class="standfirst">Two reviews of the resolver as a whole, not only of the endpoint added in this branch: one assuming the caller is hostile, one assuming they are a developer trying to use it and getting no help.</p>
<p class="standfirst">The severe findings are all in the service that already shipped. The embarrassing ones are mine, in code written this session — including a field that claims to be a <code>bytes32</code> and is not.</p>
<div class="runline">
<span><b>1 critical</b></span>
<span><b>3 high</b></span>
<span><b>2 medium</b></span>
<span><b>7 DX</b></span>
<span><b>3 of them mine</b></span>
</div>
</header>
<section>
<h2>Adversarial</h2>
<p class="lede">Assume the caller is hostile and the resolver is reachable. Every finding here is in code that predates this branch except where marked.</p>
<article class="f critical">
<div class="f-head"><span class="f-num">A1</span><span class="pill p-crit">Critical</span></div>
<h3><code>/health</code> hands out the upstream RPC URL to anyone who asks</h3>
<p>The health endpoint answers, unauthenticated:</p>
<pre>{"ok": true, "rpc": "http://reth:8545", "registries": {...}}</pre>
<p>Against the documented local reth that is harmless. Against any hosted provider it is a credential: Alchemy, Infura, QuickNode and Ankr all put the API key <em>in the URL path</em>. A resolver pointed at one of those publishes its owner's node key to every caller, and node keys are metered — the finder does not need to break anything, only to spend the budget.</p>
<p>The same value leaks a second way. Both 502 handlers return <code>f"{type(e).__name__}: {e}"</code>, and <code>urlopen</code>'s exception text carries the URL it failed on. So an attacker who cannot reach <code>/health</code> gets the same string by asking for a name while the node is unreachable — which they can arrange by asking for enough of them.</p>
<div class="fix"><b>Fix</b><p>Report reachability, not configuration: <code>{"ok": true, "rpc": "reachable", "chainId": 1}</code>. If the endpoint must be identifiable, publish the host without the path. Then stop putting exception text in response bodies — log it, return a correlation id. The URL is the one string in this process that must never be echoed.</p></div>
<div class="refs">snrc-resolve.py · Handler.do_GET /health · resolve/owned-by 502 handlers</div>
</article>
<article class="f high">
<div class="f-head"><span class="f-num">A2</span><span class="pill p-high">High</span></div>
<h3>The client can authenticate; the server cannot check</h3>
<p><code>HttpResolver.hs</code> implements <code>RpcAuth</code> with bearer and basic modes, redacts the secret from <code>Show</code> so it cannot land in logs, and attaches <code>Authorization</code> to every request. The Python resolver never reads that header. There is no auth check anywhere in the service.</p>
<p>An operator who configures a token has done real work — chosen a secret, wired it through config, kept it out of logs — and has protected nothing. That is worse than having no auth feature: an absent one prompts a firewall rule, a decorative one prompts confidence.</p>
<div class="fix"><b>Fix</b><p>Verify the header when a secret is configured, with a constant-time compare, and refuse to start when a resolver binds beyond loopback without one. Until then the Haskell side's <code>RpcAuth</code> should say in its haddock that no known resolver validates it.</p></div>
<div class="refs">HttpResolver.hs · RpcAuth, authHeader · snrc-resolve.py · Handler (no auth path)</div>
</article>
<article class="f high">
<div class="f-head"><span class="f-num">A3</span><span class="pill p-high">High</span><span class="pill p-mine">Partly mine</span></div>
<h3>One request buys up to 770 upstream calls</h3>
<p>Nothing is cached and nothing is rate limited, so the amplification is the whole story:</p>
<pre>/resolve/&lt;name&gt; 15 RPC calls nameExpires, GRACE_PERIOD, resolver,
owner, 7 text records, 4 coin addresses
/owned-by/&lt;addr&gt; 770 RPC calls GRACE_PERIOD + balanceOf
+ 3 per token x MAX_OWNED (256)</pre>
<p><code>/owned-by</code> is mine, and it is the worse of the two by a factor of fifty. <code>MAX_OWNED</code> bounds a single response; it does nothing about the rate, and the expensive request is the cheap one to send. Against a metered provider this is someone else's invoice. Against self-hosted reth it is a queue nobody else gets through.</p>
<p>The address in <code>/owned-by</code> need not even hold anything — the cost is paid before the balance is known to be zero.</p>
<div class="fix"><b>Fix</b><p>Cache resolved records for a short TTL, which is safe because the underlying data changes at block cadence, not per request. Then rate-limit per client. If the resolver is meant to be public, <code>/owned-by</code> wants a lower default bound than 256, since the tail of that range is rare and the cost is linear in it.</p></div>
<div class="refs">snrc-resolve.py · resolve, owned_by, TEXT_KEYS, MAX_OWNED</div>
</article>
<article class="f high">
<div class="f-head"><span class="f-num">A4</span><span class="pill p-high">High</span></div>
<h3>The path documented for casual use is the one that listens to the world</h3>
<p><code>SNRC_BIND</code> defaults to <code>0.0.0.0</code>. The compose file publishes <code>127.0.0.1:8000:8000</code>, so the Docker path is safe — and the README offers a second path for anyone who wants a quick look:</p>
<pre>uv run scripts/resolver/service/snrc-resolve.py # defaults to local reth</pre>
<p>That inherits the default and listens on every interface, with no auth (A2) and an endpoint that publishes the node URL (A1). The safe path is the one behind a container; the casual path is the exposed one. That is the wrong way round — defaults should be safe and deployments should opt into exposure.</p>
<div class="fix"><b>Fix</b><p>Default <code>SNRC_BIND</code> to <code>127.0.0.1</code> and set <code>0.0.0.0</code> explicitly in <code>docker-compose.yml</code>, where publishing is already deliberate and already scoped to loopback on the host.</p></div>
<div class="refs">snrc-resolve.py · BIND · docker-compose.yml · README "runnable standalone"</div>
</article>
<article class="f medium">
<div class="f-head"><span class="f-num">A5</span><span class="pill p-med">Medium</span></div>
<h3>The resolver protects its caller and not itself</h3>
<p><code>HttpResolver.hs</code> is careful about exactly this: <code>brReadSome maxResponseBytes</code>, <code>redirectCount = 0</code>, an explicit timeout, with a comment explaining that adversarial endpoints must not be able to exhaust memory. The resolver then calls its own upstream with <code>urlopen(req, timeout=15).read()</code> — a timeout, and no size cap at all.</p>
<p>An RPC endpoint that is compromised, misconfigured or simply pointed at the wrong host can return a body large enough to end the process. The threat model was written down one layer up and not applied one layer down.</p>
<div class="fix"><b>Fix</b><p>Read with a cap and fail closed past it, mirroring the bound the Haskell client already uses. Consider following redirects zero times there too, for the same reason it is spelled out in <code>HttpResolver.hs</code>.</p></div>
<div class="refs">snrc-resolve.py · rpc() · HttpResolver.hs · httpGet</div>
</article>
<article class="f medium">
<div class="f-head"><span class="f-num">A6</span><span class="pill p-med">Medium</span></div>
<h3><code>http.server</code> is documented as not for production, and this is production</h3>
<p>CPython's own docs say <code>http.server</code> "is not recommended for production. It only implements basic security checks." There are no request size limits, no header count limits and no slow-read protection. It is fronted by nothing — the compose file exposes the port directly.</p>
<p>This is a reasonable choice for a script and an unreasonable one for a service an smp-server depends on for name resolution. Worth a decision rather than an accident: either it stays a dev tool and the deployment path puts a real server in front, or it becomes a service and gets one.</p>
<div class="fix"><b>Fix</b><p>Put it behind something that terminates connections properly, or move to a WSGI/ASGI server. Either way the README should say which posture is intended, because right now docker-compose implies production and the implementation implies development.</p></div>
<div class="refs">snrc-resolve.py · ThreadingHTTPServer · docker-compose.yml</div>
</article>
</section>
<section>
<h2>Developer experience</h2>
<p class="lede">Assume a developer writing a client against this, with only the README and the responses. Three of these are defects in the branch under review.</p>
<article class="f high">
<div class="f-head"><span class="f-num">D1</span><span class="pill p-high">High</span><span class="pill p-mine">Mine</span></div>
<h3><code>labelhash</code> is not a hash, it is a Python integer literal</h3>
<p>I emit it with <code>hex(token)</code>, which produces the shortest representation:</p>
<pre>"labelhash": "0xb" what it returns
"labelhash": "0x0000…000b" what a bytes32 labelhash is</pre>
<p>Every other party in this system — the contract, a block explorer, any client comparing against chain state — represents a labelhash as 32 bytes. The value returned cannot be pasted into a contract call, cannot be compared textually with an on-chain topic, and will silently mismatch rather than fail loudly. It also makes the list's own sort order wrong, since it sorts as a string.</p>
<div class="fix"><b>Fix</b><p><code>"0x" + format(token, "064x")</code>. One line, and it should carry a test, because the wrong version looks right for the common case where the leading bytes happen to be non-zero.</p></div>
<div class="refs">snrc-resolve.py · owned_by · this branch</div>
</article>
<article class="f medium">
<div class="f-head"><span class="f-num">D2</span><span class="pill p-med">Medium</span></div>
<h3>Three error shapes, so a client needs three error handlers</h3>
<p>An unhappy response is one of:</p>
<pre>{"name": …, "error": …, "configured_tlds": [...]} TLD not configured
{"error": …, "got": …} not fully qualified
{"address": …, "error": …} bad address
{"name": …, "status": …, "error": …, …} lapsed / unregistered</pre>
<p>The key naming the subject changes, <code>status</code> is present on some and not others, and <code>configured_tlds</code> appears in two of them and not the third. A client cannot write one function that turns a failure into a message.</p>
<div class="fix"><b>Fix</b><p>One envelope on every non-2xx: a stable <code>error</code> code a client can branch on, a human <code>message</code>, and the subject under a fixed key. The current strings are messages pretending to be codes.</p></div>
<div class="refs">snrc-resolve.py · resolve, owned_by, Handler.do_GET</div>
</article>
<article class="f medium">
<div class="f-head"><span class="f-num">D3</span><span class="pill p-med">Medium</span><span class="pill p-mine">Partly mine</span></div>
<h3>Two casing conventions in one document</h3>
<p><code>simplexContact</code>, <code>simplexChannel</code>, <code>checkedTlds</code>, <code>graceEnds</code> are camelCase — the file says why, so aeson can derive field names without a rewriting layer. <code>configured_tlds</code> is snake_case, in the same response body.</p>
<p>I added <code>checkedTlds</code> directly alongside the existing <code>configured_tlds</code> without noticing they disagree, which is how a convention with one exception becomes a convention with two.</p>
<div class="fix"><b>Fix</b><p>Rename <code>configured_tlds</code> to <code>configuredTlds</code>. It appears only in error bodies, so the blast radius is small — and it will only get harder once anything depends on it.</p></div>
<div class="refs">snrc-resolve.py · resolve, owned_by</div>
</article>
<article class="f medium">
<div class="f-head"><span class="f-num">D4</span><span class="pill p-med">Medium</span><span class="pill p-mine">Mine</span></div>
<h3><code>/health</code> cannot tell an operator whether the new endpoint will work</h3>
<p>Health reports <code>registries</code> and not <code>REGISTRARS</code>. Since <code>/owned-by</code> and every expiry field depend entirely on the registrar being configured, an operator who has set only the registry gets a healthy resolver, a working <code>/resolve</code>, <code>"status": "unknown"</code> on every name, and a 400 from <code>/owned-by</code> — with nothing in the health check hinting why.</p>
<p>I added the configuration and the dependency on it, and left the diagnostic reporting the older half.</p>
<div class="fix"><b>Fix</b><p>Report both maps in <code>/health</code>, subject to A1 — the addresses are public on chain, so unlike the RPC URL they are safe to publish.</p></div>
<div class="refs">snrc-resolve.py · Handler /health · REGISTRARS · this branch</div>
</article>
<article class="f medium">
<div class="f-head"><span class="f-num">D5</span><span class="pill p-med">Medium</span></div>
<h3>Nothing anywhere says which resolver you are talking to</h3>
<p>The response payload just grew three fields. A client written last month and one written today receive different documents from the same URL, and neither can ask which it is. There is no version in the path, no version in <code>/health</code>, and no capability list.</p>
<p>This matters more now than it did: <code>status</code> and <code>expires</code> are load-bearing for a renewal reminder, and a client cannot tell whether their absence means "not supported here" or "not knowable for this name".</p>
<div class="fix"><b>Fix</b><p>A version in <code>/health</code> is enough, and cheapest now. The distinction the client actually needs — unsupported versus unknowable — is otherwise impossible to make from a <code>null</code>.</p></div>
<div class="refs">snrc-resolve.py · Handler /health · README response shape</div>
</article>
<article class="f medium">
<div class="f-head"><span class="f-num">D6</span><span class="pill p-med">Medium</span><span class="pill p-mine">Mine</span></div>
<h3>Truncation is a dead end, not a page</h3>
<p><code>/owned-by</code> stops at <code>SNRC_MAX_OWNED</code> and sets <code>truncated: true</code>. There is no offset, no cursor and no ordering guarantee a caller could resume from, so an address holding more than 256 names in a TLD has no way to see the rest — the flag is honest about the problem and offers no way out of it.</p>
<p><code>truncated</code> is also a single boolean over a merged multi-TLD result, so it does not say which registrar ran out.</p>
<div class="fix"><b>Fix</b><p>Take <code>?offset=</code> and echo it back; enumeration is index-based on the registrar, so this is nearly free. Failing that, say in the response which TLD truncated, so the caller can at least narrow the query.</p></div>
<div class="refs">snrc-resolve.py · owned_by · MAX_OWNED · this branch</div>
</article>
<article class="f medium">
<div class="f-head"><span class="f-num">D7</span><span class="pill p-med">Medium</span></div>
<h3>The schema is a Haskell type in another repository</h3>
<p><code>Names/Record.hs</code> says "the Haskell type IS the schema", which serves the one consumer written in Haskell and nobody else. A TypeScript or Python client has the README and a curl example. There is no OpenAPI document, no JSON Schema, and no fixture file to test against.</p>
<p>The claim is also now slightly untrue: the resolver returns <code>status</code>, <code>expires</code> and <code>graceEnds</code>, and the record type has none of them — safely, since aeson ignores unknown fields, but the two have diverged and only a comment asserts they have not.</p>
<div class="fix"><b>Fix</b><p>Publish the response shape as a schema next to the script and generate the examples in the README from it, so drift shows up as a failing test rather than as a stale sentence.</p></div>
<div class="refs">Names/Record.hs · scripts/resolver/README.md</div>
</article>
</section>
<section>
<h2>What held</h2>
<p class="lede">Specific things I tried to break and could not.</p>
<div class="f">
<p><strong>Path handling.</strong> Names arrive percent-encoded from the Haskell client and are <code>unquote</code>d into path segments, but they only ever reach <code>keccak</code> — there is no filesystem, no database and no shell in the path, so a crafted name is a hash of a crafted name and nothing more.</p>
<p><strong>ABI decoding.</strong> <code>decode_bytes</code> checks its length before slicing and returns empty rather than throwing on a short return, so a contract answering with garbage produces an empty field rather than a crash.</p>
<p><strong>The <code>available()</code> trap.</strong> The obvious way to compute registration status is to call <code>available(id)</code>. It is true for a name nobody ever registered, since <code>0 + GRACE_PERIOD &lt; now</code>, so it silently conflates "never taken" with "released". The implementation uses <code>nameExpires</code> and applies the rule on top, and the test suite pins it.</p>
<p><strong>Address validation.</strong> <code>/owned-by</code> rejects a malformed address before any RPC call, so the validation cannot be used as an oracle or as a way to spend upstream budget.</p>
</div>
</section>
<footer>
Reviewed <code>scripts/resolver/</code> at <code>ab/snrc-resolver-owned-by</code>, together with <code>Server/Names/HttpResolver.hs</code> as its only in-tree consumer, and <code>BaseRegistrarImplementation.sol</code> for the contract behaviour the resolver mirrors. Call counts are from reading the code, not measured against a node. Findings marked <em>mine</em> are defects in this branch rather than in the service as it stands on master.
</footer>
</div>
scripts/resolver/docker-compose.yml
+2 -4
View File
@@ -150,10 +150,8 @@ services:
# only if you're deploying against a different network or contract.
# SNRC_REGISTRY_TESTING: 0x...
# SNRC_REGISTRY_SIMPLEX: 0x...
# Registrar and controller addresses, same cascade. The registrar
# drives the expiry status on /resolve; without it status reads
# "unknown". The controller drives the "reserved" status; without it a
# reserved name reads as "unregistered".
# Registrar and controller, same cascade. Without the registrar `status`
# is "unknown"; without the controller a reserved name is "unregistered".
# SNRC_REGISTRAR_TESTING: 0x...
# SNRC_REGISTRAR_SIMPLEX: 0x...
# SNRC_CONTROLLER_TESTING: 0x...
scripts/resolver/service/snrc-resolve.py
+14 -80
View File
@@ -89,33 +89,21 @@ REGISTRIES = {
"simplex": os.environ.get("SNRC_REGISTRY_SIMPLEX", ""), # not deployed yet
}
# The BaseRegistrar (ERC-721) per TLD, used for expiry and status. Separate
# from the registry above: the registry answers "who owns this node", the
# registrar holds nameExpires and GRACE_PERIOD. Not a proxy, so the address in
# deployments is the one that answers. Without one for a TLD, /resolve still
# works and reports "status": "unknown".
REGISTRARS = {
"testing": os.environ.get("SNRC_REGISTRAR_TESTING", "")
or "0xef47eb4384b46c89e4482a677c2cbcbd2a6fd85a", # mainnet .testing
"simplex": os.environ.get("SNRC_REGISTRAR_SIMPLEX", ""), # not deployed yet
}
# The SimplexController per TLD, which holds `reservedNames`. Without one for a
# TLD, `reserved` is never reported and a reserved name reads as unregistered.
CONTROLLERS = {
"testing": os.environ.get("SNRC_CONTROLLER_TESTING", "")
# The proxy, not SimplexControllerImpl: storage and events live in the
# proxy, so the implementation address answers nothing. deployments.json
# records this one under the ENS role name ETHRegistrarController;
# verification.json names it SimplexControllerProxy. Same address.
# Proxy address, not SimplexControllerImpl: storage is held by the proxy.
# Recorded in deployments.json as ETHRegistrarController.
or "0xeeb9b6bf5fb68fb726005f7ba549c2f4b32f2dad", # mainnet .testing
"simplex": os.environ.get("SNRC_CONTROLLER_SIMPLEX", ""), # not deployed yet
}
# Why a name is reserved. `reservedNames` stores only the fact, so every
# reserved name gets this same sentence; a per-name lookup (table or REST) is
# the intended replacement. Callers should render whatever this field holds
# rather than matching on its text.
# `reservedNames` stores the fact only, never a reason.
RESERVED_REASON = "reserved for a brand or public interest"
# SLIP-44 coin types (https://github.com/satoshilabs/slips/blob/master/slip-0044.md)
@@ -172,14 +160,8 @@ def is_encoded_labelhash(label: str) -> bool:
def node_of(name: str) -> bytes:
"""namehash, accepting an encoded labelhash in place of a 2LD's label.
keccak(parent || keccak(label)) reaches the same node without the label,
so a caller can check a 2LD without disclosing which one they are about to
register. Subnames are excluded - only the 2LD's owner creates them, so
there is nothing to front-run - and a bracket label there is hashed as
written.
"""
"""namehash, accepting an encoded labelhash in place of a 2LD's label. In a
subname a bracket label is hashed as written, not decoded."""
labels = name.split(".")
if len(labels) == 2 and is_encoded_labelhash(labels[0]):
return keccak(namehash(labels[1]) + bytes.fromhex(labels[0][1:-1]))
@@ -190,50 +172,29 @@ def node_of(name: str) -> bytes:
def chain_now() -> int:
"""The latest block's timestamp - the same clock the registrar reads.
Asked explicitly rather than taken from the host clock, so a skewed clock
on this machine cannot misstate a registration.
"""
"""Expiry is compared against the block timestamp, never the host clock."""
block = rpc("eth_getBlockByNumber", ["latest", False])
return decode_uint(block["timestamp"])
def grace_period(registrar: str) -> int:
"""The registrar's own GRACE_PERIOD, in seconds.
Read from the chain rather than hardcoded, so a deployment that chooses a
different window is reported correctly instead of confidently wrongly. One
call per request, not per name.
"""
"""A deployment can configure a different window, so it is read on chain."""
return decode_uint(eth_call(registrar, selector("GRACE_PERIOD()")))
def expiry_status(expires: int, grace: int, now: int) -> str:
"""Registration state from an expiry timestamp.
Mirrors the registrar's `available(id)`, which is
`expiries[id] + GRACE_PERIOD < block.timestamp`. Note that `available`
alone cannot be used for this: it is also true for a name nobody ever
registered, since `0 + GRACE_PERIOD < now`. The zero expiry is what
separates "never taken" from "lapsed and now free".
"""
"""The registrar's `available(id)` is not enough on its own: it is also
true for a name nobody registered, since 0 + GRACE_PERIOD < now."""
if expires == 0:
return "unregistered"
if expires > now:
return "registered"
if expires + grace >= now:
# Expired, but only the previous owner may renew it - nobody else can
# take it yet.
return "grace"
return "expired"
def is_reserved(tld: str, token: int) -> bool:
"""Whether the controller holds this label for a brand.
Keyed by labelhash on chain, so this answers for a hashed query too.
"""
controller = CONTROLLERS.get(tld)
if not controller:
return False
@@ -242,28 +203,14 @@ def is_reserved(tld: str, token: int) -> bool:
def name_status(name: str):
"""Registration status of the 2LD a name sits under.
Names expire lazily: the registrar keeps the record and simply stops
treating it as live, so "never registered" and "expired last Tuesday" are
both readable rather than both being absence. `nameExpires` returns 0 for a
label that was never registered, which is what separates the two.
Subnames are not registered here, so the status of `x.alice.testing` is the
status of `alice.testing` - which is the useful answer, since a subname is
only as valid as the 2LD above it.
"""
labels = name.split(".")
tld = labels[-1]
registrar = REGISTRARS.get(tld)
if not registrar or len(labels) < 2:
# No registrar configured for this TLD: say so rather than guess.
return {"status": "unknown", "expires": None, "graceEnds": None}
# The registrar keys the registration facts (nameExpires, reservedNames) on
# uint256(keccak(label)), so a 2LD queried by its encoded labelhash gets the
# same answer without the label. Decode the bracket form for a 2LD only,
# which is the rule node_of applies to the node.
# nameExpires and reservedNames are keyed on uint256(keccak(label)).
# Decoded for a 2LD only, the same rule node_of applies to the node.
label = labels[-2]
if len(labels) == 2 and is_encoded_labelhash(label):
token = int(label[1:-1], 16)
@@ -278,10 +225,6 @@ def name_status(name: str):
grace = grace_period(registrar)
status = expiry_status(expires, grace, chain_now())
# `reserved` only displaces the two states that read as "you could take
# this". A registered name is registered, and one in grace belongs to its
# owner either way - in both cases the reservation is not the answer to the
# question being asked.
if status in ("unregistered", "expired") and is_reserved(tld, token):
status = "reserved"
@@ -569,12 +512,8 @@ def split_links(value: str) -> list:
def upstream_error(subject: dict, e: Exception) -> dict:
"""A 502 body that names the failure without quoting the exception.
urlopen puts the URL it failed on into its message, and SNRC_RPC may carry
a provider key, so the text goes to the log and only a type goes to the
caller.
"""
"""urlopen puts the failing URL into its message and SNRC_RPC can carry a
provider key, so the text goes to the log and only the type to the caller."""
print(f"upstream error: {type(e).__name__}: {e}", file=sys.stderr)
return {
**subject,
@@ -598,10 +537,7 @@ def resolve(name: str):
node = node_of(name)
node_hex = node.hex()
# Registration first, because it is the fact that separates the failures a
# caller has to tell apart: a name nobody has taken, one whose registration
# lapsed and may still be renewed, one that lapsed and is now open to
# anyone, and one that is held but not pointed anywhere.
# Before the resolver lookup, so a lapsed name is not reported as noResolver.
reg = name_status(name)
if reg["status"] in ("unregistered", "reserved"):
body = {
@@ -616,8 +552,6 @@ def resolve(name: str):
else "this name has never been registered"
),
}
# Only reserved names carry a reason, so its presence is the signal
# that one is known.
if reg["status"] == "reserved":
body["reason"] = RESERVED_REASON
return 404, body
scripts/resolver/service/test_snrc_resolve.py
+6 -43
View File
@@ -86,9 +86,6 @@ class SplitLinksTests(unittest.TestCase):
class EncodedLabelhashTests(unittest.TestCase):
"""`node_of` accepts a 2LD's label as an encoded labelhash `[<64 hex>]`,
reaching the same node as the label itself."""
# keccak-256("alice"), written out in full wherever a test needs it.
# 9c0257114eb9399a2985f8e75dad7600c5d89fe3824ffa99ec1c3eb8bf3b0501
@@ -165,9 +162,6 @@ class EncodedLabelhashTests(unittest.TestCase):
self.assertEqual(snrc.node_of(name), snrc.namehash(name))
def test_status_by_hash_matches_status_by_name(self):
"""The registrar keys registration data on the labelhash too. A hashed
query therefore answers "is it free?" as well as "what does it say?",
without the label."""
future = int(time.time()) + 86400
seen = []
@@ -190,9 +184,6 @@ class EncodedLabelhashTests(unittest.TestCase):
class NameStatusTests(unittest.TestCase):
"""unresolvable has three causes and a caller has to tell them apart.
Names expire lazily, so the chain still holds the answer."""
REGISTRAR = "0xef47eb4384b46c89e4482a677c2cbcbd2a6fd85a"
GRACE = 90 * 86400
@@ -215,8 +206,7 @@ class NameStatusTests(unittest.TestCase):
snrc.rpc,
)
snrc.REGISTRARS = {"testing": self.REGISTRAR}
# These cases are about expiry alone. ReservedTests covers what a
# configured controller adds.
# Expiry alone; ReservedTests covers a configured controller.
snrc.CONTROLLERS = {"testing": ""}
snrc.chain_now = lambda: int(time.time())
@@ -230,16 +220,12 @@ class NameStatusTests(unittest.TestCase):
) = self._saved
def test_now_is_the_latest_blocks_timestamp(self):
# setUp replaces chain_now with the fixture clock; this is about the
# real one, saved as the 4th element of the setUp snapshot
# setUp replaced chain_now with the fixture clock; test the real one
real_chain_now = self._saved[3]
snrc.rpc = lambda method, params: {"timestamp": "0x65f1a2c0", "number": "0x123"}
self.assertEqual(real_chain_now(), 0x65F1A2C0)
def test_status_reads_the_chain_clock_not_the_host_clock(self):
"""The registrar compares expiry to block.timestamp, so the resolver
must too - a host clock years ahead must not turn a live name into a
claimable one."""
future = int(time.time()) + 3600
snrc.eth_call = self._expiry(future)
self.assertEqual(snrc.name_status("alice.testing")["status"], "registered")
@@ -254,8 +240,6 @@ class NameStatusTests(unittest.TestCase):
)
def test_recently_expired_is_in_grace_and_says_when_it_ends(self):
"""Only the previous owner may renew during grace - nobody else can
take the name yet, so this is a different answer from `expired`."""
past = int(time.time()) - 3600
snrc.eth_call = self._expiry(past)
self.assertEqual(
@@ -269,8 +253,7 @@ class NameStatusTests(unittest.TestCase):
self.assertEqual(snrc.name_status("alice.testing")["status"], "expired")
def test_the_boundary_belongs_to_grace(self):
"""The registrar frees a name when expires + GRACE < now, so the last
second of the window is still the owner's."""
"""The registrar frees a name only when expires + GRACE < now."""
now = int(time.time())
snrc.eth_call = self._expiry(now - self.GRACE)
self.assertEqual(snrc.name_status("alice.testing")["status"], "grace")
@@ -284,8 +267,7 @@ class NameStatusTests(unittest.TestCase):
)
def test_never_registered_is_not_confused_with_claimable(self):
"""`available(id)` is true for both, since 0 + GRACE < now. The zero
expiry is the only thing that separates them."""
"""`available(id)` is true for both, since 0 + GRACE < now."""
snrc.eth_call = self._expiry(0)
self.assertEqual(snrc.name_status("alice.testing")["status"], "unregistered")
self.assertNotEqual(snrc.name_status("alice.testing")["status"], "expired")
@@ -312,9 +294,6 @@ class NameStatusTests(unittest.TestCase):
)
def test_every_branch_returns_the_same_keys(self):
"""Callers read status/expires/graceEnds unconditionally, so a branch
that omits one is a KeyError in the caller rather than a missing field
in the JSON."""
keys = {"status", "expires", "graceEnds"}
snrc.eth_call = self._expiry(0)
self.assertEqual(set(snrc.name_status("alice.testing")), keys)
@@ -326,9 +305,6 @@ class NameStatusTests(unittest.TestCase):
class ReservedTests(unittest.TestCase):
"""A reserved name is unregistered and still unavailable, which a client
intending to register needs to know before it tries."""
REGISTRAR = "0xef47eb4384b46c89e4482a677c2cbcbd2a6fd85a"
CONTROLLER = "0x281ca41311c2aa808c917c4674639d7567b75714"
@@ -366,7 +342,6 @@ class ReservedTests(unittest.TestCase):
self.assertEqual(snrc.name_status("acme.testing")["status"], "reserved")
def test_a_live_name_is_registered_even_if_reserved(self):
"""It was handed to its brand; the reservation is no longer the answer."""
snrc.eth_call = self._chain(int(time.time()) + 86400, True)
self.assertEqual(snrc.name_status("acme.testing")["status"], "registered")
@@ -376,7 +351,7 @@ class ReservedTests(unittest.TestCase):
def test_no_controller_configured_means_reserved_is_never_reported(self):
snrc.CONTROLLERS = {"testing": ""}
snrc.eth_call = self._chain(0, True) # would say reserved if asked
snrc.eth_call = self._chain(0, True) # reserved on chain, but unread
self.assertEqual(snrc.name_status("acme.testing")["status"], "unregistered")
def test_reserved_is_asked_by_labelhash_so_a_hashed_query_works(self):
@@ -387,10 +362,6 @@ class ReservedTests(unittest.TestCase):
class ReservedReasonTests(unittest.TestCase):
"""Why a name is reserved travels in its own field, so a client can show it
without parsing the message, and so a per-name reason can replace the fixed
one without moving anything."""
REGISTRY = "0x58fc46996d975c57883564648bda5206d1a0102b"
REGISTRAR = "0xef47eb4384b46c89e4482a677c2cbcbd2a6fd85a"
CONTROLLER = "0x281ca41311c2aa808c917c4674639d7567b75714"
@@ -462,10 +433,6 @@ class ReservedReasonTests(unittest.TestCase):
class ErrorCodeTests(unittest.TestCase):
"""`error` is a fixed code a client can branch on, and `message` is the
sentence for a human. Matching on the sentence would break the moment the
wording changes, which is why the two are separate fields."""
REGISTRY = "0x58fc46996d975c57883564648bda5206d1a0102b"
REGISTRAR = "0xef47eb4384b46c89e4482a677c2cbcbd2a6fd85a"
@@ -509,8 +476,6 @@ class ErrorCodeTests(unittest.TestCase):
self.assertIn("nosuchtld", body["message"])
def test_a_registration_problem_reports_the_status_as_the_code(self):
"""For these the status is the error, so a client needs to read only
one field."""
for expires, code in (
(0, "unregistered"),
(int(time.time()) - 3600, "grace"),
@@ -539,14 +504,12 @@ class ErrorCodeTests(unittest.TestCase):
self.assertNotEqual(body["error"], body["message"])
def test_an_upstream_failure_does_not_echo_the_exception(self):
"""urlopen puts the failing URL in its message and SNRC_RPC can carry
a provider key, so only the exception type reaches the caller."""
with contextlib.redirect_stderr(io.StringIO()) as log:
body = snrc.upstream_error(
{"name": "alice.testing"},
RuntimeError("http://user:secret@rpc.example/kEy8 refused"),
)
# the operator still gets the detail, in the log
# the operator still sees the detail in the log
self.assertIn("secret", log.getvalue())
self.assertEqual(body["error"], "upstreamError")
self.assertIn("RuntimeError", body["message"])
src/Simplex/Messaging/Server/Names.hs
+2 -3
View File
@@ -76,9 +76,8 @@ fetch NamesEnv {resolverEnv} d =
mapResolverError :: ResolverError -> NameErrorType
mapResolverError = \case
HttpStatusErr 404 -> NOT_FOUND
-- 410 is a lapsed registration (past expiry, in grace or beyond): a correct
-- answer about the name, not a resolver failure, so it must not become
-- RESOLVER - that is reserved for the backing resolver/RPC breaking.
-- 410 is a lapsed registration: an answer about the name, not a resolver
-- failure, so it must not become RESOLVER.
HttpStatusErr 410 -> NOT_FOUND
HttpStatusErr 400 -> NOT_FOUND
HttpStatusErr code -> RESOLVER ("HTTP " <> T.pack (show code))
tests/RSLVTests.hs
-4
View File
@@ -92,10 +92,6 @@ testRslvBackendNotFound =
corrId `shouldBe` CorrId "rs01"
resp `shouldBe` Right (ERR (NAME NOT_FOUND))
-- The resolver answers 410 for a registration that has lapsed (in grace or
-- past it). That is a correct answer about the name, so it has to arrive as
-- NOT_FOUND; RESOLVER would make the client treat it as a broken resolver and
-- abort domain verification instead of reporting the name as unverified.
testRslvBackendGone :: IO ()
testRslvBackendGone =
withResolverServer (status410, "{}") $
tests/SMPNamesTests.hs
-3
View File
@@ -157,9 +157,6 @@ resolverSpec = do
resolveName env aliceDomain `shouldReturn` Left NOT_FOUND
it "returns NOT_FOUND on 410 (registration lapsed)" $
-- A lapsed name is a correct answer, not a resolver failure: RESOLVER
-- would make the client abort domain verification instead of reporting
-- the name as unverified.
withResolverServer (resolveResp status410 "{}") $ \port _ -> do
env <- newNamesEnv (testNamesConfig port)
resolveName env aliceDomain `shouldReturn` Left NOT_FOUND