Add Step 3 to the audit: catch-block Log.w/e calls that interpolate
${e.message} but don't pass `e` lose the stack trace and log "...: null"
when the exception's message is null. Document the throwable-overload
fix alongside the existing lambda-overload conversion.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously the catch-block warnings interpolated ${e.message} but
dropped the throwable, losing the stack trace and showing nothing for
exceptions whose message is null. Switch to the (tag, msg, throwable)
overload so the cause and stack trace are logged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Avoid eager string interpolation when the log level is filtered out at
runtime. Covers Log.d/i calls and Log.w/e calls that did not pass a
throwable.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two issues fixed:
1. BunkerResponseDeserializer produces generic BunkerResponse for decrypt/encrypt
results (plain strings that aren't pubkeys or JSON). The response parsers only
matched typed subtypes (BunkerResponseDecrypt/Encrypt), causing all decrypt and
encrypt operations through NIP-46 bunker to fail with ReceivedButCouldNotPerform.
Fix: parsers now fall back to extracting response.result directly.
2. RemoteSignerManager timeout was 30s, shorter than Amber's 60s approval window.
Fix: increased to 65s with safe retry (republishes same request, preserving UUID
so bunker responses always match).
3. Desktop ChatPane showed raw ciphertext on decrypt failure instead of an error
message. Fix: shows "Could not decrypt the message" in italic.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merge upstream main into feat/desktop-multi-account, resolving:
- Main.kt: Surface wrapper + CompositionLocalProvider + nip11Fetcher from upstream, multi-account DeckSidebar/LoginScreen from HEAD
- FeedScreen.kt: viewport-aware scroll from HEAD + contentPadding from upstream
- DeckSidebar.kt: merged imports (multi-account + titleBarInsetTop + MaterialSymbols)
- Migrated AccountSwitcherDropdown + AddAccountDialog from material.icons to MaterialSymbols
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
"AmethystDns" identified the owner; "SurgeDns" identifies the
behavior. The class is built to absorb sudden bursts of concurrent
DNS work — 700 relays reconnecting, a feed scrolling through dozens
of media hosts, a profile screen requesting a NIP-05 — without
falling over: lock-free reads, single-flight coalescing, and
stale-while-revalidate so recurring hosts never block.
File renames:
- AmethystDns.kt -> SurgeDns.kt
- AmethystDnsStore.kt -> SurgeDnsStore.kt
- AmethystDnsTest.kt -> SurgeDnsTest.kt
Symbol renames in every touchpoint (AppModules, factories,
managers, event listeners): AmethystDns -> SurgeDns,
amethystDns -> surgeDns.
- evictIfOverCap -> purgeExpiredIfOverCap. The method only purges
expired entries; "evict" implied LRU behavior.
- triggerBackgroundRefresh -> scheduleBackgroundRefresh. Better
reflects that we hand the work to an executor that may queue it.
- fresh -> freshEntry in resolveAsLeader. The bare adjective read as
a boolean.
- KEY_CACHE -> KEY_SNAPSHOT in AmethystDnsStore. The constant
identifier now matches what we actually store via dns.snapshot();
the string value is unchanged.
Two correctness fixes from the audit:
1. Save race. The previous order (snapshot -> write -> clearDirty)
could lose a putPositive that landed between the write and the
clearDirty: clearDirty unconditionally wiped the flag, so the
just-written entry would never be persisted. Replace clearDirty
with tryClearDirty (compareAndSet), called BEFORE the snapshot —
any concurrent put then re-marks dirty and is captured by the next
save. Add markDirty so the store can re-flag on write failure.
Also drop the buggy clearDirty in load(): if puts happened between
AmethystDns construction and load completion, we used to silently
discard their dirty signal. restore() never marks dirty, so the
manual clear was both unnecessary and incorrect.
2. Empty positive results. A misbehaving Dns delegate returning
emptyList() would land in the cache as a positive entry — harmless
in lookup semantics (unwrap throws on empty) but wrongly persisted
to disk and dirty-flagged. Treat empty as UnknownHostException so
it goes through putNegative.
Add three tests:
- failed refresh demotes a stale positive entry to negative
- failed lookup does not mark cache dirty
- refresh executor rejection cleans up the inflight slot
Drop the AmethystDns.shared lazy companion. Construct one
amethystDns in AppModules and thread it through:
- DualHttpClientManager / OkHttpClientFactory
- DualHttpClientManagerForRelays / OkHttpClientFactoryForRelays
- MediaCallEventListenerFactory / MediaCallEventListener
- DnsInvalidatingEventListener.Factory
- AmethystDnsStore
This matches the dependency-injection style the rest of AppModules
uses and lets tests inject a mock Dns where needed. Behavior is
unchanged — every consumer still shares the same single instance,
just by construction rather than by a static singleton.
Mechanical cleanups from a final review pass. No behavior change.
- Drop the dead `private val xxxMillis = xxxMs` aliasing — just use
the constructor params directly.
- Extract positiveExpiry()/negativeExpiry() and evictIfOverCap() so
putPositive and putNegative read as one line each.
- Extract lookupAndCache() from resolveAsLeader so the leader's
bookkeeping (re-check, complete future, remove inflight) is
separate from the upstream/cache write logic.
- Tighten awaitFollower's cause-handling.
- Use runCatching in the refresh executor task.
- Drop DnsCacheRecord's no-arg constructor — jacksonObjectMapper()
registers the Kotlin module, which uses the primary constructor
reflectively.
- Rename `hostname` -> `host` in private methods that receive the
already-normalized lowercase key.
Translate Nests, PDF, and duration strings/plurals that were missing
from the four target locales.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two follow-ups from the resolver audit:
1. Lowercase the hostname at the public boundary (lookup, invalidate,
restore). DNS is case-insensitive; OkHttp normally hands us
lowercase but custom callers and persisted records can mix case.
This prevents "Example.com" and "example.com" from creating
separate cache entries.
2. Drop the cache entry when an OkHttp call to a host fails outright.
Without this, a stale-cached IP that no longer works would be
served for up to 24h before the soft TTL ran out. Hooking
callFailed (final-stage signal after OkHttp tried every address)
instead of per-attempt connectFailed avoids over-invalidating
multi-A-record hosts where one IP is dead and OkHttp recovers via
the next.
- Media path: invalidation folded into MediaCallEventListener.finish
alongside its existing timing logging.
- Relay path: new DnsInvalidatingEventListener wired into
OkHttpClientFactoryForRelays via eventListenerFactory.
The isAllHls guard already forces imetas.isNotEmpty(), so the
?: imetas.first() fallback could never fire. Switch to the throwing
minBy variant — same runtime semantics, no elvis noise, no nullable
intermediate.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Use the existing HlsContentTypes.HLS_PLAYLIST constant for the
canonical HLS playlist mime in GalleryThumb's isHlsMimeType so the
read-side stays in sync with the publish path (HlsVideoEventBuilder,
HlsPublishOrchestrator). Trim two block comments down to the
non-obvious WHY only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The "Add Media to Gallery" action in the zoomable view passed only
blurhash, dim, hash, and mime type — losing the poster URL that the
underlying NIP-71 imeta carries on its `image` field. The resulting
ProfileGalleryEntryEvent therefore had no `image` tag, and the
gallery card fell back to the play-icon placeholder for any HLS URL
because the .m3u8 playlist itself can't be decoded as an image.
Thread the poster through Account.addToGallery and
AccountViewModel.addMediaToGallery as an optional `image` parameter,
and pull it from the MediaUrlVideo's artworkUri at the call site.
Non-video content (MediaUrlImage, etc.) still passes null.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cold starts were the resolver's worst case: every host paid a sync
getaddrinfo. With a persisted snapshot, every previously-seen host
falls into the stale-while-revalidate path on first lookup — the
cached IP is served immediately and a background refresh updates it.
~700 blocking system calls at app start become zero.
Switch entry expiries from System.nanoTime to System.currentTimeMillis
so timestamps survive process death (nanoTime is monotonic per
process, undefined across restarts). Add snapshot()/restore() that
serialize only fresh positive entries — negative entries are skipped
and re-resolved synchronously, and restore() uses putIfAbsent so a
fresh in-memory entry is never clobbered by a stale on-disk one.
Add AmethystDnsStore as a thin SharedPreferences + Jackson wrapper.
The blob is plain (not encrypted): hostnames are already exposed in
the user's signed relay list, Coil's image cache, and the system
resolver's own state.
Wire load + save into AppModules:
- load() runs once at app start on the IO scope.
- save() runs every 5 minutes (skipped when nothing has changed via
the dirty flag), on trim() when the app backgrounds, and once on
terminate() as a best-effort flush.
A NIP-71 HLS publish writes one imeta tag per rendition (master + each
variant), all on the same event. The gallery card was expanding that
into one sub-tile per imeta inside AutoNonlazyGrid, producing a 3x6
sub-grid of black play-icon placeholders for a single video — the
.m3u8 playlist is a text manifest Coil can't decode.
When every imeta on a VideoEvent is an HLS playlist, pick a single
imeta to render: prefer one carrying a poster image, breaking ties
by smallest dimensions to favor the lowest-resolution variant. Fall
back to the first imeta if none carry a poster. Non-HLS multi-imeta
videos keep their existing per-imeta layout to avoid regressions for
any author publishing alternate-format renditions side by side.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After the first lookup of a host, recurring connections never block on
getaddrinfo again. Soft-expired positive entries are returned
immediately while a background refresh updates the cache through a
small fixed thread pool (8 daemon threads). The refresh is coalesced
through the same inflight map, so a fan-out of stale reads to the same
host still triggers exactly one upstream call.
Negative entries deliberately do NOT serve stale — a transient DNS
failure must recover quickly, not keep returning UnknownHostException.
A failed refresh demotes the entry to negative so the next caller sees
a fresh failure rather than forever-stale wrong IPs.
Add TTL jitter on positive writes so a burst of co-written entries
(e.g. ~700 relay reconnects at app start) doesn't all expire at the
same instant 24h later. Default jitter equals the base TTL, so entries
spread their expiries uniformly across [24h, 48h]. Combined with the
bounded refresh executor, even a daily-app-open user with 1000 stale
hosts generates a few seconds of background work and zero blocking
waits in the foreground.
HLS publishes built NIP-71 video events with imetas pointing only at
.m3u8 playlists, leaving HLS-unaware UI surfaces (gallery thumbnails,
previews) with nothing to render. This change extracts a still frame
from the picked source video, encodes it as JPEG, uploads it
alongside the segments, and threads the URL onto every imeta's
`image` property — exactly where the gallery already reads it.
- HlsVideoPublishInput gains a posterUrl field; HlsVideoEventBuilder
applies it to every VideoMeta.image (master + renditions).
- HlsPublishOrchestrator gains an injected uploadPoster closure
(default no-op for tests). Failures here log + continue rather
than aborting — the user has already paid the cost of the long
segment uploads, so a missing poster shouldn't block publish.
- HlsPublishOrchestratorFactory wires the production closure:
MediaMetadataRetriever frame extraction (reusing
service.uploads.getThumbnail), JPEG encode at quality 85,
upload via the same HlsBlobUploader, temp-file cleanup.
- New tests: poster URL propagation (builder), poster URL flow
through orchestrator, and graceful failure when the poster
closure throws.
https://claude.ai/code/session_011e5CNmGU5Sbzqb9w9hhd66
Profile-gallery thumbnails routed every MediaUrlContent through
SubcomposeAsyncImage, which works for .mp4 frames via Coil's
VideoFrameDecoder but fails on .m3u8 playlists (text manifests, not
decodable as image frames). The result was a grid of identical
PlayCircleOutline error icons for HLS gallery entries.
- Plumb artworkUri through MediaUrlVideo construction in
ProfileGalleryEntryEvent (image()/thumb() tags) and VideoEvent
(imeta image property), so a published poster is used when present.
- Prefer artworkUri over content.url as the SubcomposeAsyncImage
source for videos.
- For HLS videos with no artwork, skip the doomed image fetch and
render blurhash + centered play overlay directly. Extracted into a
VideoPlaceholder helper reused by the error fallback.
- Overlay the play icon on successfully decoded video frames so
videos read as videos in the grid.
https://claude.ai/code/session_011e5CNmGU5Sbzqb9w9hhd66
The previous cache was a synchronizedMap(LinkedHashMap(access-order=true)).
Access-order LRU rewrites the linked list on get(), so every cache hit
took the global monitor — at 700 concurrent relay reconnects, the lock
serialized every dispatcher worker through a single critical section.
Switch to ConcurrentHashMap so reads are lock-free. Amethyst's steady
state is well under maxEntries (~750 distinct hosts vs cap of 2000), so
strict LRU was never going to evict anything and the access-order
machinery was pure contention.
Bump positive TTL from 5min to 24h: relay and CDN IPs change on the
order of days, and we are not a recursive resolver — there's no
correctness reason to honor authoritative TTLs. A 5min cycle made us
re-resolve all 700 relays every 5 minutes.
Also add a leader-side cache re-check after acquiring inflight
ownership. If a peer leader refreshed the entry while we were claiming
the slot, we skip getaddrinfo entirely.
Eviction is now a cheap on-demand sweep of expired entries when the
map exceeds maxEntries; in normal use it never runs.
A Nostr feed can touch hundreds of distinct hosts (Blossom servers,
relays, NIP-05 domains, image proxies). The 256-entry cap was small
enough that active users would churn the LRU and pay extra getaddrinfo
calls. 2000 still costs <100KB of heap.
Avoids paying the getaddrinfo tax on every HTTP call to the same
host. Adds a single-flight, LRU+TTL DNS resolver wired into both the
media and relay OkHttp clients via a process-wide shared instance, so
resolutions cross-cut images, relays, and NIP-05 lookups.
- Per-host coalescing: N concurrent lookups for the same host share one
upstream call.
- Different hosts proceed in parallel — no global lock around the
upstream resolver.
- Negative cache (10s) prevents hammering on typos / dead hosts.
- Positive cache (5m) survives a feed scroll.
Adds the underlying IP literal as a separate ElectrumX entry in both
the clearnet default list and the Tor list, mirroring how
46.229.238.187:57002 sits beside nmc2.bitcoins.sk:57002.
Why a bare IP entry alongside a hostname entry?
- Robustness against unhealthy ICANN DNS paths. The relay.testls.bit
name resolves through Namecoin (.bit), which most resolvers ship
via a NIP-05 lookup against ElectrumX itself \u2014 chicken-and-egg if
the only listed servers ARE .bit-resolved. A bare IP gives the
resolver pool a completely-DNS-free anchor.
- Same physical box, same self-signed cert, same SHA-256 (DER) pin
(bb0b35e6\u20266cba) added in the prior commit, so no new cert pin is
needed.
The .onion entry added in the prior commit already serves as the
DNS-free Tor anchor; this commit handles the equivalent for clearnet.
Verified end-to-end:
node tls direct connect 23.158.233.10:50002 (no SNI) -> server.version
reply ElectrumX 1.16.0; cert SHA-256 matches the pinned hash exactly.
Adds the relay.testls.bit ElectrumX-NMC deployment as a second public
Namecoin name resolver. The same box also runs the Namecoin-anchored
Nostr relay at wss://relay.testls.bit/, so a single TLSA record on
d/testls anchors the trust chain for both services.
Default server set (clearnet):
+ relay.testls.bit:50002 (TLS, self-signed cert pinned)
Tor server set (onion-first):
+ 6cbn4rsk...onion:50001 (plaintext over Tor; re-uses the
relay's existing v3 hidden service,
onion key already authenticates)
+ relay.testls.bit:50002 (clearnet TLS fallback)
Cert pin (SHA-256 of DER):
bb0b35e64235a794e157b69acd72da4ccc16a4d81d0ff6b1d8f7fdfc6cca6cba
The new self-signed cert is appended to PINNED_ELECTRUMX_CERTS so
strict-TLS Android devices (Samsung One UI 7, GrapheneOS) accept it
without a trust-all fallback.
Reference deployment also exposes wss://relay.testls.bit/electrumx
(WebSocket transport for browser-based Nostr clients), which inherits
the relay's existing TLSA-pinned ECDSA cert from d/testls. The current
JVM ElectrumXClient is TCP+TLS only; adding a WS variant of the client
is a larger change and tracked separately.
Adds a new `NamecoinImportResolver` that follows the `import` field of a
Namecoin Domain Name Object value, recursively merging the imported
values into the importing object before `NamecoinNameResolver` reads
the `nostr` field.
Why
---
The 520-byte per-name limit on Namecoin is tight when a single record
needs `ip`, NIP-05 names, TLS, and per-subdomain map entries. A common
workaround (used by `testls.bit` and others) is to keep the apex record
small by delegating shared blocks into a sibling name via
`"import":"dd/<name>"` per ifa-0001 §"import". Without import support,
NIP-05 search for `testls.bit` (and the like) fails: the resolver sees
no `nostr` field at `d/testls` and gives up before consulting the
imported `dd/testls` that actually carries the names block.
What
----
Per ifa-0001 §"import":
* Importer items take precedence over imported items (including `null`,
which suppresses the imported counterpart).
* Recursion is supported up to 4 levels (the spec minimum) by default.
Cycles are broken by a visited-set; over-budget chains are silently
truncated, with the importer's own items still taking effect.
* Multiple imports in one array merge later-wins (the array's own
order), with the importing object on top of all of them.
* Subdomain selectors (the optional 2nd element of each inner array)
are resolved through a `map` walk inside the imported value, with
the standard exact-label > `*` wildcard > `""` default rules.
* Three short-hand value forms are accepted alongside the canonical
array-of-arrays: `"import":"d/foo"`, `"import":["d/foo"]`, and
`"import":["d/foo","selector"]`. They map onto the canonical
representation losslessly.
* A failed import lookup (name not found, malformed JSON, network
error) is treated as `{}` rather than failing the whole record, so
transient ElectrumX hiccups don't kill resolution. The importing
object's own items still apply.
Wires the expander into `NamecoinNameResolver.performLookup` and
`performLookupDetailed`. Records without an `import` key skip the
resolver entirely (zero extra I/O), so non-import names pay no cost.
Tests
-----
20 new tests in `NamecoinImportTest`:
* Unit (resolver in isolation): no-import passthrough, all four
shorthand value forms, importer precedence, null suppression,
depth-4 recursion happy path, over-budget truncation, lookup
failure, malformed JSON, malformed `import` value, cycle
protection, multi-label selector descending in DNS order, wildcard
fallback.
* Integration with `NamecoinNameResolver`: bare and named NIP-05 across
an import (mirroring the real-world `testls.bit -> dd/testls`
deployment); regression guard that no-import records issue exactly
one ElectrumX query; importer-wins on `nostr.names`; lenient
failure when the imported boilerplate is unreachable;
`resolveDetailed` returns `Success` and `NoNostrField` correctly
across imports.
All 15 existing `NamecoinNameResolverTest` cases continue to pass.
Scope note
----------
This PR is intentionally scoped to the NIP-05 / search path. The
relay-resolution path (#2595) also benefits from import support; that
wiring is left to land alongside #2595 so the two PRs can move
independently.
David Kaspar