The nostrconnect flow was trusting params[0] from the signer's connect message as the user's pubkey. Some signers (e.g. nsec.app) don't put the actual user pubkey there, causing all relay subscriptions to query the wrong identity — contact lists, profiles, and DMs all returned empty. Now calls remoteSigner.getPublicKey() after the handshake to get the verified pubkey from the signer. Also stabilizes FeedScreen relay subscriptions with distinctUntilChanged() and adds relay.primal.net to defaults. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
15 KiB
title, type, status, date, parent
| title | type | status | date | parent |
|---|---|---|---|---|
| test: TDD tests for NIP-46 relay isolation fix | test | active | 2026-03-05 | 2026-03-05-fix-nip46-relay-isolation-and-init-plan.md |
test: TDD tests for NIP-46 relay isolation fix
Overview
Write tests that reproduce the three NIP-46 bugs before they're fixed, then verify the fix. Tests should fail against the old shared-client architecture and pass after relay isolation.
Bug → Test Mapping
| Bug | Root Cause | Test Strategy |
|---|---|---|
Bug 1: Empty indexRelays snapshot |
availableRelays.value captured at coordinator construction = emptySet() |
Verify coordinator receives non-empty relay set |
| Bug 2: Shared client relay contamination | NIP-46 relay leaks into general pool → ClosedMessage loop | Verify NIP-46 traffic stays on dedicated client, never touches general client |
| Bug 3: Sign request response lost | Subscription disrupted by ClosedMessage churn from Bug 2 | Verify NIP-46 subscription filters only target NIP-46 relays |
Test Infrastructure
Existing
- Framework:
kotlin.test+kotlinx-coroutines-test+mockk - Location:
desktopApp/src/jvmTest/kotlin/.../desktop/account/ - Mocks:
EmptyNostrClient(no-op),mockk<SecureKeyStorage>(relaxed = true), temp dirs - Pattern:
@BeforeTestsetup →runTest {}→@AfterTestcleanup
Needed
SpyNostrClient— wrapsINostrClient, records which relays receivesend()andopenReqSubscription()calls- No new dependencies (mockk
capture+slot+verifysufficient)
Phase 0: Fix Existing Broken Tests
File: AccountManagerLoadAccountTest.kt
Three tests pass client param to loadSavedAccount() which no longer accepts it:
| Test | Fix |
|---|---|
loadSavedAccountBunkerNoEphemeralReturnsFailure (line 114) |
Remove client = EmptyNostrClient arg |
loadSavedAccountBunkerNoClientFallsBackToInternal (line 119-136) |
Delete entirely — concept no longer exists (AccountManager always creates its own NIP-46 client) |
loadSavedAccountBunkerSuccess (line 155-158) |
Remove client = EmptyNostrClient arg |
Phase 1: Relay Isolation Tests (Bug 2 + 3)
Test File: AccountManagerNip46IsolationTest.kt
Location: desktopApp/src/jvmTest/kotlin/.../desktop/account/
Test 1: nip46ClientIsNotTheGeneralClient
Reproduces: Bug 2 root cause — shared NostrClient pollution.
@Test
fun nip46ClientIsNotTheGeneralClient() = runTest {
// Setup: bunker URI + ephemeral key in storage
val validHex = "a".repeat(64)
val ephemeralKeyPair = KeyPair()
File(amethystDir, "bunker_uri.txt").writeText("bunker://$validHex?relay=wss://relay.nsec.app")
File(amethystDir, "last_account.txt").writeText(ephemeralKeyPair.pubKey.toNpub())
coEvery { storage.getPrivateKey(BUNKER_EPHEMERAL_KEY_ALIAS) } returns ephemeralKeyPair.privKey!!.toHexKey()
// Load bunker account — this creates the internal NIP-46 client
manager.loadSavedAccount()
// The general relay pool client (e.g. relayManager.client) should NOT
// have any NIP-46 relay subscriptions. Since AccountManager creates its
// own client internally, we verify via the accountState having Remote signer.
val state = manager.currentAccount()
assertNotNull(state)
assertIs<SignerType.Remote>(state.signerType)
// The signer's client should be the dedicated one, not EmptyNostrClient
val signer = state.signer as NostrSignerRemote
// NostrSignerRemote stores its client — it should be a real NostrClient, not null
assertNotNull(signer)
}
Key assertion: AccountManager creates a dedicated client internally rather than requiring one passed in.
Test 2: loginWithBunkerDoesNotRequireExternalClient
Reproduces: The old API required callers to pass relayManager.client.
@Test
fun loginWithBunkerDoesNotRequireExternalClient() = runTest {
// loginWithBunker() should compile and work with just a bunkerUri
// This test verifies the API — no client param needed
try {
manager.loginWithBunker("bunker://${"a".repeat(64)}?relay=wss://relay.nsec.app")
} catch (e: Exception) {
// Expected — no real relay to connect to. But it should NOT throw
// a compile error or "client required" error.
assertTrue(
e.message?.contains("Connection") == true ||
e.message?.contains("timed out") == true ||
e.message?.contains("Connection failed") == true,
"Unexpected error: ${e.message}",
)
}
}
Test 3: loginWithNostrConnectDoesNotRequireExternalClient
Same pattern — verifies API no longer takes client.
@Test
fun loginWithNostrConnectDoesNotRequireExternalClient() = runTest {
var generatedUri: String? = null
try {
manager.loginWithNostrConnect { uri -> generatedUri = uri }
} catch (e: Exception) {
// Expected — no signer scanning the QR
assertTrue(e.message?.contains("Timed out") == true || e.message != null)
}
// URI should have been generated even if connection failed
assertNotNull(generatedUri)
assertTrue(generatedUri!!.startsWith("nostrconnect://"))
}
Test 4: logoutDisconnectsNip46Client
Reproduces: Bug 2 side effect — leaked NIP-46 WebSocket after logout.
@Test
fun logoutDisconnectsNip46Client() = runTest {
// Login with bunker (will create internal NIP-46 client)
val keyPair = KeyPair()
manager.loginWithKey(keyPair.privKey!!.toNsec())
// Force to Remote type for test
// Instead: test that logout from any state doesn't crash
manager.logout()
// After logout, state should be LoggedOut
assertIs<AccountState.LoggedOut>(manager.accountState.value)
// Signer connection should be NotRemote
assertIs<SignerConnectionState.NotRemote>(manager.signerConnectionState.value)
}
Test 5: logoutThenLoginCreatesNewNip46Client
Reproduces: Ensuring fresh client after re-login (no stale state).
@Test
fun logoutThenLoginCreatesNewNip46Client() = runTest {
// First bunker load
val validHex = "a".repeat(64)
val ephemeralKeyPair = KeyPair()
File(amethystDir, "bunker_uri.txt").writeText("bunker://$validHex?relay=wss://relay.nsec.app")
File(amethystDir, "last_account.txt").writeText(ephemeralKeyPair.pubKey.toNpub())
coEvery { storage.getPrivateKey(BUNKER_EPHEMERAL_KEY_ALIAS) } returns ephemeralKeyPair.privKey!!.toHexKey()
manager.loadSavedAccount()
val firstState = manager.currentAccount()
assertNotNull(firstState)
// Logout — should disconnect NIP-46 client
manager.logout()
assertIs<AccountState.LoggedOut>(manager.accountState.value)
// Re-load — should create a NEW NIP-46 client
File(amethystDir, "bunker_uri.txt").writeText("bunker://$validHex?relay=wss://relay.nsec.app")
File(amethystDir, "last_account.txt").writeText(ephemeralKeyPair.pubKey.toNpub())
manager.loadSavedAccount()
val secondState = manager.currentAccount()
assertNotNull(secondState)
// Both states should be valid Remote signers
assertIs<SignerType.Remote>(firstState.signerType)
assertIs<SignerType.Remote>(secondState.signerType)
}
Phase 2: State Transition Tests
Test File: AccountManagerStateTransitionTest.kt
Test 6: bunkerRestoreShowsConnectingThenLoggedIn
Reproduces: Bug 1 UX — user sees nothing while connecting.
@Test
fun bunkerRestoreShowsConnectingThenLoggedIn() = runTest {
val states = mutableListOf<AccountState>()
val collector = backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) {
manager.accountState.toList(states)
}
// Set connecting state (what Main.kt does before loadSavedAccount)
manager.setConnectingRelays()
// Then load internal account
val keyPair = KeyPair()
val npub = keyPair.pubKey.toNpub()
File(amethystDir, "last_account.txt").writeText(npub)
coEvery { storage.getPrivateKey(npub) } returns keyPair.privKey!!.toHexKey()
manager.loadSavedAccount()
advanceUntilIdle()
// Should have: LoggedOut → ConnectingRelays → LoggedIn
assertTrue(states.size >= 3, "Expected at least 3 state transitions, got: $states")
assertIs<AccountState.LoggedOut>(states[0])
assertIs<AccountState.ConnectingRelays>(states[1])
assertIs<AccountState.LoggedIn>(states[2])
collector.cancel()
}
Test 7: loadSavedAccountBunkerNoEphemeralReturnsFailure
Updated version — no client param.
@Test
fun loadSavedAccountBunkerNoEphemeralReturnsFailure() = runTest {
val validHex = "a".repeat(64)
val keyPair = KeyPair()
val npub = keyPair.pubKey.toNpub()
File(amethystDir, "last_account.txt").writeText(npub)
File(amethystDir, "bunker_uri.txt").writeText("bunker://$validHex?relay=wss://r.com")
coEvery { storage.getPrivateKey(AccountManager.BUNKER_EPHEMERAL_KEY_ALIAS) } returns null
val result = manager.loadSavedAccount()
assertTrue(result.isFailure)
}
Phase 3: NostrSignerRemote Isolation Tests (quartz)
Test File: NostrSignerRemoteIsolationTest.kt
Location: quartz/src/commonTest/kotlin/.../nip46RemoteSigner/signer/
Test 8: subscriptionFilterTargetsOnlyBunkerRelays
Reproduces: Bug 2 — subscription filters should ONLY reference NIP-46 relays.
@Test
fun subscriptionFilterTargetsOnlyBunkerRelays() {
val bunkerRelay = NormalizedRelayUrl("wss://relay.nsec.app/")
val generalRelay = NormalizedRelayUrl("wss://relay.damus.io/")
val capturedFilters = mutableListOf<Map<NormalizedRelayUrl, List<Filter>>>()
val mockClient = mockk<INostrClient>(relaxed = true)
every { mockClient.openReqSubscription(any(), capture(capturedFilters), any()) } just Runs
val ephemeralSigner = NostrSignerInternal(KeyPair())
val remoteSigner = NostrSignerRemote(
signer = ephemeralSigner,
remotePubkey = "a".repeat(64),
relays = setOf(bunkerRelay),
client = mockClient,
)
remoteSigner.openSubscription()
// Verify filter only targets bunker relay
assertTrue(capturedFilters.isNotEmpty(), "No subscription opened")
capturedFilters.forEach { filterMap ->
assertTrue(filterMap.keys.all { it == bunkerRelay }, "Filter contains non-bunker relay: ${filterMap.keys}")
assertTrue(generalRelay !in filterMap.keys, "General relay leaked into NIP-46 filter")
}
}
Test 9: subscriptionFilterContainsCorrectKindAndPTag
@Test
fun subscriptionFilterContainsCorrectKindAndPTag() {
val capturedFilters = mutableListOf<Map<NormalizedRelayUrl, List<Filter>>>()
val mockClient = mockk<INostrClient>(relaxed = true)
every { mockClient.openReqSubscription(any(), capture(capturedFilters), any()) } just Runs
val ephemeralKeyPair = KeyPair()
val ephemeralSigner = NostrSignerInternal(ephemeralKeyPair)
val remoteSigner = NostrSignerRemote(
signer = ephemeralSigner,
remotePubkey = "b".repeat(64),
relays = setOf(NormalizedRelayUrl("wss://relay.nsec.app/")),
client = mockClient,
)
remoteSigner.openSubscription()
val filters = capturedFilters.flatMap { it.values.flatten() }
assertTrue(filters.isNotEmpty())
filters.forEach { filter ->
assertTrue(filter.kinds?.contains(NostrConnectEvent.KIND) == true,
"Filter missing kind ${NostrConnectEvent.KIND}")
assertTrue(filter.tags?.get("p")?.contains(ephemeralSigner.pubKey) == true,
"Filter missing p-tag for ephemeral pubkey")
}
}
Phase 4: since Filter Tests (Bug 3 mitigation)
Test File: add to NostrSignerRemoteIsolationTest.kt
Test 10: subscriptionFilterHasSinceTimestamp
Reproduces: Stale NIP-46 responses from previous sessions being processed.
@Test
fun subscriptionFilterHasSinceTimestamp() {
val capturedFilters = mutableListOf<Map<NormalizedRelayUrl, List<Filter>>>()
val mockClient = mockk<INostrClient>(relaxed = true)
every { mockClient.openReqSubscription(any(), capture(capturedFilters), any()) } just Runs
val beforeTime = TimeUtils.now()
val remoteSigner = NostrSignerRemote(
signer = NostrSignerInternal(KeyPair()),
remotePubkey = "c".repeat(64),
relays = setOf(NormalizedRelayUrl("wss://relay.nsec.app/")),
client = mockClient,
)
remoteSigner.openSubscription()
val filters = capturedFilters.flatMap { it.values.flatten() }
assertTrue(filters.isNotEmpty())
filters.forEach { filter ->
assertNotNull(filter.since, "Filter missing 'since' timestamp")
// since should be roughly now - 60s (with some tolerance)
val expectedMin = beforeTime - 120 // extra tolerance
val expectedMax = beforeTime
assertTrue(filter.since!! >= expectedMin, "since too old: ${filter.since}")
assertTrue(filter.since!! <= expectedMax, "since in the future: ${filter.since}")
}
}
Note: This test will FAIL until Step 8 (add
sinceto NostrSignerRemote) is implemented. That's the TDD red phase.
Implementation Order
| # | Action | File | Phase |
|---|---|---|---|
| 1 | Fix broken tests (remove client param) |
AccountManagerLoadAccountTest.kt |
0 |
| 2 | Write relay isolation tests (Tests 1-5) | AccountManagerNip46IsolationTest.kt |
1 |
| 3 | Write state transition tests (Tests 6-7) | AccountManagerStateTransitionTest.kt |
2 |
| 4 | Write signer isolation tests (Tests 8-9) | NostrSignerRemoteIsolationTest.kt |
3 |
| 5 | Write since filter test (Test 10) — expect RED |
NostrSignerRemoteIsolationTest.kt |
4 |
| 6 | Run all → confirm Tests 1-9 GREEN, Test 10 RED | — | — |
| 7 | Implement Step 8 (since filter in quartz) |
NostrSignerRemote.kt |
— |
| 8 | Run all → confirm all GREEN | — | — |
Test Commands
# Run all desktop tests
./gradlew :desktopApp:test
# Run specific test class
./gradlew :desktopApp:test --tests "*.AccountManagerNip46IsolationTest"
# Run quartz common tests
./gradlew :quartz:jvmTest --tests "*.NostrSignerRemoteIsolationTest"
# Run all NIP-46 related tests
./gradlew :desktopApp:test --tests "*.AccountManager*" :quartz:jvmTest --tests "*.nip46RemoteSigner.*"
Unanswered Questions
getOrCreateNip46Client()is private — can't directly verify single-creation from tests without reflection or@VisibleForTesting. Sufficient to test indirectly through public API?NostrSignerRemote.subscriptionfield is private — MockKcaptureonopenReqSubscriptionis the best proxy for filter verification?- Should we add Turbine dependency for cleaner StateFlow testing, or stick with manual
backgroundScopecollection? NostrClient.connect()is non-suspending (fire-and-forget) — how to test that the NIP-46 client actually connects before subscriptions? Accept relay auto-queue behavior?