mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-10 08:27:04 +00:00
feat(nip46): batched consent via concurrent request dispatch
Third refinement from the Primal comparison — and the one that needed an architecture change, not just UI. Quartz: NostrConnectSignerService now fans each request into a child coroutine under a Semaphore(maxConcurrentHandles=16) instead of handling them inline, so a request awaiting a consent prompt no longer blocks other clients' auto-allowed traffic and several prompts can be pending at once. Intake (dedup, staleness, rate-limit, seen-id persistence) stays on the single consumer. Two guards keep it safe: BunkerRequestProcessor serializes the actual crypto with a Mutex (authorization — the prompt — runs unlocked, only sign/encrypt/decrypt holds the lock) so an external NIP-55 signer never sees concurrent IPC ops; and Nip46PermissionAuthorizer serializes first-connect consent so two connects can't stack dialogs. Covered by BunkerRequestProcessorConcurrencyTest (crypto never overlaps; a blocked prompt doesn't stall another client's signing). Amethyst: SignerConsentCoordinator is now a shared pending StateFlow; one SignerConsentActivity observes it and shows the rich single-request dialog (1 pending) or a batched checkbox list with select-all + a Remember toggle + Allow/Deny selected (>1). Dismissing the sheet denies every still-open request (fail closed). Needs on-device validation (burst batching, no concurrent external-signer IPC, fail-closed on dismiss) — see the device checklist. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FHr2mu5SiHwYNR7evYUuF
This commit is contained in:
@@ -109,13 +109,29 @@ that loop via `collectLatest`.
|
||||
- **FIXED — unbounded first-connect prompt.** `Nip46ConsentBridge.requestConnect`
|
||||
now has the same 120s `withTimeoutOrNull` as `requestOp`, so an ignored
|
||||
first-connect dialog can no longer wedge the loop forever.
|
||||
- **Consent blocks other clients (bounded).** While one prompt is open, other
|
||||
clients' requests queue in the 256-deep DROP_LATEST channel and, past that,
|
||||
drop. Bounded by the 120s timeouts. A proper fix is to dispatch prompt-needing
|
||||
requests to child jobs (keeping dedup/rate-limit/`decide()` on the loop
|
||||
thread, serializing only the dialogs) so auto-allowed traffic keeps flowing —
|
||||
deferred because it risks stacked dialogs + concurrent external-signer ops and
|
||||
needs on-device validation.
|
||||
- **FIXED — consent no longer blocks other clients (needs on-device
|
||||
validation).** The service now fans each request out into a child coroutine
|
||||
under a `Semaphore(maxConcurrentHandles=16)`; dedup/staleness/rate-limit stay on
|
||||
the single consumer, only `handle()` runs concurrently. So a request awaiting a
|
||||
prompt no longer stalls auto-allowed traffic, and several prompts can be pending
|
||||
at once. Two guards keep this safe: (1) the identity signer's crypto is
|
||||
serialized by `BunkerRequestProcessor.cryptoLock` — authorization (the prompt)
|
||||
runs UNLOCKED, only the sign/encrypt/decrypt holds the lock — so an external
|
||||
NIP-55 app never sees concurrent IPC ops; (2) first-connect consent is
|
||||
serialized by `Nip46PermissionAuthorizer.connectLock` so two connects can't stack
|
||||
dialogs. Per-op prompts batch: the shared `SignerConsentCoordinator.pending`
|
||||
flow drives one dialog (1 pending) or a checkbox list (>1). Covered by
|
||||
`BunkerRequestProcessorConcurrencyTest`, but the on-device paths below still need
|
||||
a real run:
|
||||
- [ ] **Burst batching:** a client fires several dangerous-kind requests at once
|
||||
→ one batched sheet with checkboxes + select-all, Allow/Deny selected,
|
||||
"Remember" toggle. Approving a subset leaves the rest pending.
|
||||
- [ ] **Auto-allowed keeps flowing:** while a prompt sits open, a REASONABLE
|
||||
auto-allowed request from another app still gets signed and answered.
|
||||
- [ ] **No concurrent external-signer ops:** with a NIP-55 external signer, two
|
||||
approved requests do not drive overlapping IPC (they serialize).
|
||||
- [ ] **Fail-closed on dismiss:** backing out of the batched sheet denies every
|
||||
still-open request (not just the selected ones).
|
||||
- **Relay-set change cancels in-flight work.** A `logout` (or a new nostrconnect
|
||||
pairing) mutates the listen set → `collectLatest` restarts the service →
|
||||
cancels the in-flight `handle()`. Practical impact is low (a logout ACK is lost
|
||||
|
||||
+164
-27
@@ -40,13 +40,16 @@ import androidx.compose.foundation.text.selection.SelectionContainer
|
||||
import androidx.compose.foundation.verticalScroll
|
||||
import androidx.compose.material3.Button
|
||||
import androidx.compose.material3.ButtonDefaults
|
||||
import androidx.compose.material3.Checkbox
|
||||
import androidx.compose.material3.HorizontalDivider
|
||||
import androidx.compose.material3.MaterialTheme
|
||||
import androidx.compose.material3.OutlinedButton
|
||||
import androidx.compose.material3.Surface
|
||||
import androidx.compose.material3.Switch
|
||||
import androidx.compose.material3.Text
|
||||
import androidx.compose.material3.TextButton
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.LaunchedEffect
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.remember
|
||||
@@ -54,12 +57,14 @@ import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.platform.LocalConfiguration
|
||||
import androidx.compose.ui.res.pluralStringResource
|
||||
import androidx.compose.ui.res.stringResource
|
||||
import androidx.compose.ui.text.font.FontFamily
|
||||
import androidx.compose.ui.text.style.TextAlign
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.compose.ui.window.Dialog
|
||||
import androidx.compose.ui.window.DialogProperties
|
||||
import androidx.lifecycle.compose.collectAsStateWithLifecycle
|
||||
import com.vitorpamplona.amethyst.R
|
||||
import com.vitorpamplona.amethyst.commons.connectedApps.signers.SignerOpGrant
|
||||
import com.vitorpamplona.amethyst.commons.favorites.FavoriteApp
|
||||
@@ -76,41 +81,40 @@ import com.vitorpamplona.quartz.nip59Giftwrap.rumors.RumorAssembler
|
||||
import com.vitorpamplona.quartz.utils.TimeUtils
|
||||
|
||||
class SignerConsentActivity : ComponentActivity() {
|
||||
private var token: String? = null
|
||||
private var decided = false
|
||||
|
||||
override fun onCreate(savedInstanceState: Bundle?) {
|
||||
super.onCreate(savedInstanceState)
|
||||
val token = intent.getStringExtra(SignerConsentCoordinator.EXTRA_TOKEN)
|
||||
this.token = token
|
||||
val info = token?.let { SignerConsentCoordinator.infoFor(it) }
|
||||
if (token == null || info == null) {
|
||||
finish()
|
||||
return
|
||||
}
|
||||
|
||||
setContent {
|
||||
AmethystTheme {
|
||||
SignerConsentDialog(
|
||||
info = info,
|
||||
onGrant = { grant ->
|
||||
decided = true
|
||||
SignerConsentCoordinator.complete(token, grant)
|
||||
finish()
|
||||
},
|
||||
onDismiss = {
|
||||
decided = true
|
||||
SignerConsentCoordinator.cancel(token)
|
||||
finish()
|
||||
},
|
||||
)
|
||||
// The signer services requests concurrently, so more than one may await consent. Observe
|
||||
// the shared queue: one request shows the rich dialog, several show a batched list. When
|
||||
// the queue empties (all decided), close.
|
||||
val pending by SignerConsentCoordinator.pending.collectAsStateWithLifecycle()
|
||||
LaunchedEffect(pending.isEmpty()) { if (pending.isEmpty()) finish() }
|
||||
when {
|
||||
pending.isEmpty() -> Unit
|
||||
pending.size == 1 -> {
|
||||
val p = pending.first()
|
||||
SignerConsentDialog(
|
||||
info = p.info,
|
||||
onGrant = { SignerConsentCoordinator.complete(p.token, it) },
|
||||
onDismiss = { SignerConsentCoordinator.complete(p.token, SignerOpGrant.DenyOnce) },
|
||||
)
|
||||
}
|
||||
else ->
|
||||
BatchedConsentDialog(
|
||||
pending = pending,
|
||||
onResolve = { tokens, grant -> SignerConsentCoordinator.completeAll(tokens, grant) },
|
||||
onDismiss = { SignerConsentCoordinator.denyAllPending() },
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
override fun finish() {
|
||||
if (!decided) token?.let { SignerConsentCoordinator.cancel(it) }
|
||||
super.finish()
|
||||
override fun onDestroy() {
|
||||
super.onDestroy()
|
||||
// Torn down for good (back / dismiss), not a config change: fail every still-open request closed.
|
||||
if (isFinishing) SignerConsentCoordinator.denyAllPending()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -355,3 +359,136 @@ private fun SignerConsentDialog(
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Shown when more than one request is awaiting consent at once (the signer services requests
|
||||
* concurrently). Lists each with a checkbox — all selected by default — and resolves the selected
|
||||
* ones together as Allow or Deny. "Remember" makes an Allow persist per-op ([SignerOpGrant.AllowForOp]);
|
||||
* off is a one-time [SignerOpGrant.AllowOnce]. Requests left unselected stay pending and re-render
|
||||
* (as this list, or the single-request dialog once one remains).
|
||||
*/
|
||||
@Composable
|
||||
private fun BatchedConsentDialog(
|
||||
pending: List<PendingConsent>,
|
||||
onResolve: (tokens: List<String>, grant: SignerOpGrant) -> Unit,
|
||||
onDismiss: () -> Unit,
|
||||
) {
|
||||
val maxHeight = LocalConfiguration.current.screenHeightDp.dp * 0.85f
|
||||
var selected by remember(pending.size) { mutableStateOf(pending.map { it.token }.toSet()) }
|
||||
var rememberChoice by remember { mutableStateOf(true) }
|
||||
|
||||
Dialog(
|
||||
onDismissRequest = onDismiss,
|
||||
properties = DialogProperties(usePlatformDefaultWidth = false),
|
||||
) {
|
||||
Surface(
|
||||
modifier =
|
||||
Modifier
|
||||
.fillMaxWidth()
|
||||
.padding(horizontal = 16.dp)
|
||||
.heightIn(max = maxHeight),
|
||||
shape = MaterialTheme.shapes.extraLarge,
|
||||
color = MaterialTheme.colorScheme.surface,
|
||||
tonalElevation = 6.dp,
|
||||
) {
|
||||
Column(modifier = Modifier.padding(vertical = 20.dp)) {
|
||||
Text(
|
||||
pluralStringResource(R.plurals.nip46_signer_batch_title, pending.size, pending.size),
|
||||
style = MaterialTheme.typography.titleLarge,
|
||||
modifier = Modifier.padding(horizontal = 24.dp),
|
||||
)
|
||||
TextButton(
|
||||
onClick = {
|
||||
selected = if (selected.size == pending.size) emptySet() else pending.map { it.token }.toSet()
|
||||
},
|
||||
contentPadding = PaddingValues(horizontal = 20.dp, vertical = 2.dp),
|
||||
) {
|
||||
Text(
|
||||
stringResource(
|
||||
if (selected.size == pending.size) R.string.nip46_signer_batch_select_none else R.string.nip46_signer_batch_select_all,
|
||||
),
|
||||
style = MaterialTheme.typography.labelLarge,
|
||||
)
|
||||
}
|
||||
|
||||
Column(
|
||||
modifier =
|
||||
Modifier
|
||||
.weight(1f, fill = false)
|
||||
.verticalScroll(rememberScrollState()),
|
||||
) {
|
||||
pending.forEach { p ->
|
||||
Row(
|
||||
modifier =
|
||||
Modifier
|
||||
.fillMaxWidth()
|
||||
.padding(horizontal = 12.dp, vertical = 2.dp),
|
||||
verticalAlignment = Alignment.CenterVertically,
|
||||
horizontalArrangement = Arrangement.spacedBy(4.dp),
|
||||
) {
|
||||
Checkbox(
|
||||
checked = p.token in selected,
|
||||
onCheckedChange = { on -> selected = if (on) selected + p.token else selected - p.token },
|
||||
)
|
||||
Column(modifier = Modifier.weight(1f)) {
|
||||
Text(
|
||||
"${p.info.appletTitle} · ${p.info.operationSummary}",
|
||||
style = MaterialTheme.typography.bodyMedium,
|
||||
maxLines = 1,
|
||||
)
|
||||
if (p.info.contentPreview.isNotBlank()) {
|
||||
Text(
|
||||
p.info.contentPreview,
|
||||
style = MaterialTheme.typography.bodySmall,
|
||||
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||
maxLines = 1,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Row(
|
||||
modifier = Modifier.fillMaxWidth().padding(horizontal = 24.dp, vertical = 4.dp),
|
||||
verticalAlignment = Alignment.CenterVertically,
|
||||
horizontalArrangement = Arrangement.spacedBy(8.dp),
|
||||
) {
|
||||
Switch(checked = rememberChoice, onCheckedChange = { rememberChoice = it })
|
||||
Text(
|
||||
stringResource(R.string.nip46_signer_batch_remember),
|
||||
style = MaterialTheme.typography.bodyMedium,
|
||||
)
|
||||
}
|
||||
|
||||
Spacer(Modifier.height(8.dp))
|
||||
HorizontalDivider()
|
||||
Spacer(Modifier.height(8.dp))
|
||||
|
||||
Button(
|
||||
onClick = {
|
||||
val tokens = pending.filter { it.token in selected }
|
||||
// Per-op remember uses each request's own op; one-time is a single AllowOnce.
|
||||
if (rememberChoice) {
|
||||
tokens.forEach { onResolve(listOf(it.token), SignerOpGrant.AllowForOp(it.info.op)) }
|
||||
} else {
|
||||
onResolve(tokens.map { it.token }, SignerOpGrant.AllowOnce)
|
||||
}
|
||||
},
|
||||
enabled = selected.isNotEmpty(),
|
||||
modifier = Modifier.fillMaxWidth().padding(horizontal = 24.dp),
|
||||
) {
|
||||
Text(stringResource(R.string.nip46_signer_batch_allow, selected.size))
|
||||
}
|
||||
OutlinedButton(
|
||||
onClick = { onResolve(pending.filter { it.token in selected }.map { it.token }, SignerOpGrant.DenyOnce) },
|
||||
enabled = selected.isNotEmpty(),
|
||||
modifier = Modifier.fillMaxWidth().padding(horizontal = 24.dp),
|
||||
colors = ButtonDefaults.outlinedButtonColors(contentColor = MaterialTheme.colorScheme.error),
|
||||
) {
|
||||
Text(stringResource(R.string.nip46_signer_batch_deny, selected.size))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+49
-28
@@ -28,6 +28,9 @@ import com.vitorpamplona.amethyst.commons.connectedApps.signers.SignerOpGrant
|
||||
import com.vitorpamplona.quartz.nip01Core.core.Event
|
||||
import com.vitorpamplona.quartz.nip01Core.signers.EventTemplate
|
||||
import kotlinx.coroutines.CompletableDeferred
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.update
|
||||
import java.util.UUID
|
||||
import java.util.concurrent.ConcurrentHashMap
|
||||
|
||||
@@ -61,17 +64,29 @@ data class SignerConsentInfo(
|
||||
val previewTemplate: EventTemplate<Event>? = null,
|
||||
)
|
||||
|
||||
/** One pending per-operation consent request, as the batched sheet renders it. */
|
||||
data class PendingConsent(
|
||||
val token: String,
|
||||
val info: SignerConsentInfo,
|
||||
)
|
||||
|
||||
/**
|
||||
* Bridges the broker to the per-operation signer consent UI.
|
||||
* A dismissed dialog resolves to [SignerOpGrant.DenyOnce] — fails closed.
|
||||
* Bridges the broker to the per-operation signer consent UI. The signer services requests
|
||||
* concurrently (so their prompts can batch), so several requests can await consent at once: they all
|
||||
* land in [pending], one [SignerConsentActivity] observes that list and shows a single-request dialog
|
||||
* or a batched list, and each resolved token completes its own deferred. A dismissed/ignored request
|
||||
* resolves to [SignerOpGrant.DenyOnce] — fails closed.
|
||||
*/
|
||||
object SignerConsentCoordinator {
|
||||
private class Pending(
|
||||
val info: SignerConsentInfo,
|
||||
val deferred: CompletableDeferred<SignerOpGrant>,
|
||||
)
|
||||
private val deferreds = ConcurrentHashMap<String, CompletableDeferred<SignerOpGrant>>()
|
||||
private val _pending = MutableStateFlow<List<PendingConsent>>(emptyList())
|
||||
|
||||
private val pending = ConcurrentHashMap<String, Pending>()
|
||||
/** The live set of requests awaiting the user's decision; the Activity renders this. */
|
||||
val pending: StateFlow<List<PendingConsent>> = _pending
|
||||
|
||||
// A stable notification id (one prompt notification for the whole batch, updated as requests
|
||||
// arrive) so concurrent requests don't each post their own.
|
||||
private val batchNotificationId = "nip46-signer-consent".hashCode()
|
||||
|
||||
suspend fun requestConsent(
|
||||
context: Context,
|
||||
@@ -79,48 +94,54 @@ object SignerConsentCoordinator {
|
||||
): SignerOpGrant {
|
||||
val token = UUID.randomUUID().toString()
|
||||
val deferred = CompletableDeferred<SignerOpGrant>()
|
||||
pending[token] = Pending(info, deferred)
|
||||
deferreds[token] = deferred
|
||||
_pending.update { it + PendingConsent(token, info) }
|
||||
|
||||
// Fast path when Amethyst already owns the foreground: open the dialog directly. When the app
|
||||
// is backgrounded this is silently dropped by Android 12+ BAL, so the full-screen-intent
|
||||
// notification below is what actually surfaces the prompt. Wrapped because a blocked launch
|
||||
// can throw on some OEMs rather than no-op.
|
||||
// notification is what surfaces the prompt. Both are idempotent — the Activity is singleTop and
|
||||
// observes [pending], and the notification uses a stable id, so concurrent requests just refresh
|
||||
// the one prompt. Wrapped because a BAL-blocked launch can throw on some OEMs rather than no-op.
|
||||
runCatching {
|
||||
context.startActivity(
|
||||
Intent(context, SignerConsentActivity::class.java)
|
||||
.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
|
||||
.putExtra(EXTRA_TOKEN, token),
|
||||
.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_SINGLE_TOP),
|
||||
)
|
||||
}
|
||||
|
||||
val notificationId =
|
||||
SignerConsentNotifier.show(
|
||||
context = context,
|
||||
activityClass = SignerConsentActivity::class.java,
|
||||
extraKey = EXTRA_TOKEN,
|
||||
token = token,
|
||||
titleRes = R.string.nip46_signer_notif_sign_title,
|
||||
)
|
||||
SignerConsentNotifier.show(
|
||||
context = context,
|
||||
activityClass = SignerConsentActivity::class.java,
|
||||
extraKey = EXTRA_TOKEN,
|
||||
token = "nip46-signer-consent",
|
||||
titleRes = R.string.nip46_signer_notif_sign_title,
|
||||
)
|
||||
|
||||
return try {
|
||||
deferred.await()
|
||||
} finally {
|
||||
pending.remove(token)
|
||||
SignerConsentNotifier.cancel(context, notificationId)
|
||||
deferreds.remove(token)
|
||||
_pending.update { list -> list.filterNot { it.token == token } }
|
||||
if (_pending.value.isEmpty()) SignerConsentNotifier.cancel(context, batchNotificationId)
|
||||
}
|
||||
}
|
||||
|
||||
fun infoFor(token: String): SignerConsentInfo? = pending[token]?.info
|
||||
|
||||
fun complete(
|
||||
token: String,
|
||||
grant: SignerOpGrant,
|
||||
) {
|
||||
pending[token]?.deferred?.complete(grant)
|
||||
deferreds[token]?.complete(grant)
|
||||
}
|
||||
|
||||
fun cancel(token: String) {
|
||||
pending[token]?.deferred?.complete(SignerOpGrant.DenyOnce)
|
||||
fun completeAll(
|
||||
tokens: Collection<String>,
|
||||
grant: SignerOpGrant,
|
||||
) {
|
||||
tokens.forEach { complete(it, grant) }
|
||||
}
|
||||
|
||||
/** Deny every still-open request — used when the user dismisses the whole sheet. Fails closed. */
|
||||
fun denyAllPending() {
|
||||
deferreds.values.forEach { it.complete(SignerOpGrant.DenyOnce) }
|
||||
}
|
||||
|
||||
const val EXTRA_TOKEN = "napplet_signer_consent_token"
|
||||
|
||||
@@ -952,6 +952,15 @@
|
||||
<string name="nip46_signer_hero_title">Sign for other apps</string>
|
||||
<string name="nip46_signer_turn_on">Turn on signer</string>
|
||||
<string name="nip46_signer_live">Live</string>
|
||||
<plurals name="nip46_signer_batch_title">
|
||||
<item quantity="one">%1$d signing request</item>
|
||||
<item quantity="other">%1$d signing requests</item>
|
||||
</plurals>
|
||||
<string name="nip46_signer_batch_select_all">Select all</string>
|
||||
<string name="nip46_signer_batch_select_none">Select none</string>
|
||||
<string name="nip46_signer_batch_remember">Remember these for each app</string>
|
||||
<string name="nip46_signer_batch_allow">Allow %1$d</string>
|
||||
<string name="nip46_signer_batch_deny">Deny %1$d</string>
|
||||
<string name="nip46_signer_activity_title">Recent activity</string>
|
||||
<string name="nip46_signer_activity_empty">No requests serviced yet.</string>
|
||||
<string name="nip46_signer_activity_denied">denied</string>
|
||||
|
||||
+32
-9
@@ -39,6 +39,8 @@ import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestSign
|
||||
import com.vitorpamplona.quartz.nip46RemoteSigner.server.Nip46ConnectDecision
|
||||
import com.vitorpamplona.quartz.nip46RemoteSigner.server.Nip46RequestAuthorizer
|
||||
import com.vitorpamplona.quartz.utils.TimeUtils
|
||||
import kotlinx.coroutines.sync.Mutex
|
||||
import kotlinx.coroutines.sync.withLock
|
||||
|
||||
/**
|
||||
* Bridges the NIP-46 signer core to Amethyst's shared "Connected Apps" trust
|
||||
@@ -110,6 +112,12 @@ class Nip46PermissionAuthorizer(
|
||||
private val lastUsedThrottle = mutableMapOf<String, Long>()
|
||||
private val throttleLock = KmpLock()
|
||||
|
||||
// Serializes first-connect consent. The service now handles requests concurrently so per-op prompts
|
||||
// can batch, but the connect prompt is a separate single dialog — this keeps two clients connecting
|
||||
// at once from stacking two connect dialogs; the second waits for the first to resolve. A coroutine
|
||||
// Mutex (not KmpLock) because the guarded region awaits a user dialog and must suspend, not block.
|
||||
private val connectLock = Mutex()
|
||||
|
||||
/** The ledger coordinate for [clientPubKey] under this account. */
|
||||
fun coordinateFor(clientPubKey: HexKey): String = coordinateFor(signerPubKey, clientPubKey)
|
||||
|
||||
@@ -137,16 +145,31 @@ class Nip46PermissionAuthorizer(
|
||||
}
|
||||
|
||||
val coordinate = coordinateFor(clientPubKey)
|
||||
if (!ledger.hasPolicy(coordinate)) {
|
||||
// First contact: ask the user (if a prompt is wired) which trust level to grant; a
|
||||
// headless signer with no prompt falls back to the non-interactive default.
|
||||
when (val consent = connectConsent?.invoke(coordinate, clientPubKey, request)) {
|
||||
null -> ledger.setPolicy(coordinate, defaultPolicyOnConnect)
|
||||
is AppConnectResult.Connected -> ledger.setPolicy(coordinate, consent.policy)
|
||||
AppConnectResult.Blocked -> return Nip46ConnectDecision.Reject("blocked by user")
|
||||
AppConnectResult.Cancelled -> return Nip46ConnectDecision.Reject("connection declined")
|
||||
// Serialize first-contact consent so two concurrent connects don't stack dialogs. The
|
||||
// hasPolicy re-check inside the lock also means a client that connected on another in-flight
|
||||
// request isn't prompted twice.
|
||||
val rejection =
|
||||
connectLock.withLock {
|
||||
if (ledger.hasPolicy(coordinate)) {
|
||||
null
|
||||
} else {
|
||||
// First contact: ask the user (if a prompt is wired) which trust level to grant; a
|
||||
// headless signer with no prompt falls back to the non-interactive default.
|
||||
when (val consent = connectConsent?.invoke(coordinate, clientPubKey, request)) {
|
||||
null -> {
|
||||
ledger.setPolicy(coordinate, defaultPolicyOnConnect)
|
||||
null
|
||||
}
|
||||
is AppConnectResult.Connected -> {
|
||||
ledger.setPolicy(coordinate, consent.policy)
|
||||
null
|
||||
}
|
||||
AppConnectResult.Blocked -> "blocked by user"
|
||||
AppConnectResult.Cancelled -> "connection declined"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if (rejection != null) return Nip46ConnectDecision.Reject(rejection)
|
||||
touchLastUsed(coordinate)
|
||||
onConnected?.invoke(clientPubKey, request)
|
||||
|
||||
|
||||
+14
-1
@@ -45,6 +45,8 @@ import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerResponsePong
|
||||
import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerResponsePublicKey
|
||||
import com.vitorpamplona.quartz.nip46RemoteSigner.ReadWrite
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import kotlinx.coroutines.sync.Mutex
|
||||
import kotlinx.coroutines.sync.withLock
|
||||
|
||||
/**
|
||||
* The signer/bunker side of NIP-46: turns a decrypted [BunkerRequest] from a
|
||||
@@ -72,6 +74,15 @@ class BunkerRequestProcessor(
|
||||
val relays: suspend () -> Set<NormalizedRelayUrl>,
|
||||
val authorizer: Nip46RequestAuthorizer,
|
||||
) {
|
||||
/**
|
||||
* Serializes the actual crypto ([signer] `sign`/`nip04|44_*`) across concurrent [process] calls.
|
||||
* The service may run several requests at once so their consent prompts can batch, but the identity
|
||||
* signer — especially an external NIP-55 app reached over IPC — must not see concurrent operations,
|
||||
* so only the crypto runs under this lock. Authorization (which may open a user prompt and block for
|
||||
* a long time) runs OUTSIDE the lock, so a pending prompt never stalls other clients' signing.
|
||||
*/
|
||||
private val cryptoLock = Mutex()
|
||||
|
||||
/**
|
||||
* Fulfils a single decrypted [request] sent by [clientPubKey], returning the
|
||||
* response to encrypt and send back. Never throws — signer/authorizer errors
|
||||
@@ -147,7 +158,9 @@ class BunkerRequestProcessor(
|
||||
// external signer is gone — so refuse rather than prompt or hang on a key we can't use.
|
||||
BunkerResponseError(request.id, ERROR_ACCOUNT_UNAVAILABLE)
|
||||
} else if (authorizer.authorize(clientPubKey, request)) {
|
||||
block()
|
||||
// Authorization ran unlocked (it may have blocked on a user prompt); the crypto itself runs
|
||||
// under [cryptoLock] so concurrent authorized requests don't hit the signer at the same time.
|
||||
cryptoLock.withLock { block() }
|
||||
} else {
|
||||
BunkerResponseError(request.id, ERROR_UNAUTHORIZED)
|
||||
}
|
||||
|
||||
+36
-1
@@ -33,8 +33,12 @@ import com.vitorpamplona.quartz.nip46RemoteSigner.NostrConnectEvent
|
||||
import com.vitorpamplona.quartz.utils.Log
|
||||
import com.vitorpamplona.quartz.utils.TimeUtils
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.channels.BufferOverflow
|
||||
import kotlinx.coroutines.channels.Channel
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.supervisorScope
|
||||
import kotlinx.coroutines.sync.Semaphore
|
||||
|
||||
/**
|
||||
* Runs a NIP-46 remote signer ("bunker") for one account: subscribes to the
|
||||
@@ -97,6 +101,15 @@ class NostrConnectSignerService(
|
||||
* kept small so an app restart re-signs as little as possible (relays replay only this far back).
|
||||
*/
|
||||
val maxRequestAgeSeconds: Long = 30,
|
||||
/**
|
||||
* How many requests may be in-flight (past dedup/rate-limit) at once. Each request is handled in
|
||||
* its own child coroutine so that a request awaiting a user consent prompt does NOT block other
|
||||
* clients' auto-allowed traffic — and so several prompts can be pending together and be approved in
|
||||
* one batch. Intake (dedup, staleness, rate-limit) stays on the single consumer; only [handle] fans
|
||||
* out. The actual crypto is still serialized inside [BunkerRequestProcessor]. This bounds how many
|
||||
* child coroutines (and pending prompts) can accumulate under a flood.
|
||||
*/
|
||||
val maxConcurrentHandles: Int = 16,
|
||||
/**
|
||||
* Event ids serviced in a previous run, used to seed the in-memory dedup set so a relay replaying
|
||||
* stored requests across an app restart is caught by EXACT event id — immune to client clock skew,
|
||||
@@ -155,6 +168,14 @@ class NostrConnectSignerService(
|
||||
return
|
||||
}
|
||||
|
||||
// supervisorScope: each request is handled in a child coroutine so a prompt awaiting the user
|
||||
// doesn't block the loop or other clients; a failing child never tears down the loop or siblings.
|
||||
supervisorScope {
|
||||
runLoop()
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun kotlinx.coroutines.CoroutineScope.runLoop() {
|
||||
val self = transportSigner.pubKey
|
||||
// Bounded + DROP_LATEST so a flood bounds memory instead of growing an unlimited queue.
|
||||
val events = Channel<NostrConnectEvent>(capacity = maxQueue, onBufferOverflow = BufferOverflow.DROP_LATEST)
|
||||
@@ -182,6 +203,8 @@ class NostrConnectSignerService(
|
||||
// Seed the dedup set with ids serviced in a prior run so a relay replaying stored requests after
|
||||
// a restart is caught by exact id (see [initialSeen]).
|
||||
val seen = LinkedHashSet(initialSeen)
|
||||
// Bounds how many requests can be in-flight (and how many prompts can be pending) at once.
|
||||
val handleGate = Semaphore(maxConcurrentHandles)
|
||||
// Only ask relays for recent requests: kind-24133 is ephemeral, but relays that store it would
|
||||
// otherwise replay every old request each time we (re)subscribe. See [maxRequestAgeSeconds].
|
||||
val filter = Filter(kinds = listOf(NostrConnectEvent.KIND), tags = mapOf("p" to listOf(self)), since = TimeUtils.now() - maxRequestAgeSeconds)
|
||||
@@ -209,9 +232,21 @@ class NostrConnectSignerService(
|
||||
Log.w("NIP46Signer") { "rate-limited request from ${event.pubKey.take(8)}…" }
|
||||
continue
|
||||
}
|
||||
handle(event)
|
||||
// Remember this id (persisted by the host) so a later restart won't re-service the replay.
|
||||
// Done on the single consumer — BEFORE fanning out — because the host's seen-id store is
|
||||
// not synchronized; a request we decided to service here should not be re-prompted after a
|
||||
// restart even if it is ultimately denied (the in-memory `seen` already covers this run).
|
||||
onHandledId?.invoke(event.id)
|
||||
// Acquire BEFORE launching so intake applies backpressure at the cap instead of spawning
|
||||
// unbounded child coroutines; dedup/rate-limit above already ran on this single consumer.
|
||||
handleGate.acquire()
|
||||
launch {
|
||||
try {
|
||||
handle(event)
|
||||
} finally {
|
||||
handleGate.release()
|
||||
}
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
client.unsubscribe(subId)
|
||||
|
||||
+172
@@ -0,0 +1,172 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.quartz.nip46RemoteSigner.server
|
||||
|
||||
import com.vitorpamplona.quartz.nip01Core.core.Event
|
||||
import com.vitorpamplona.quartz.nip01Core.core.HexKey
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer
|
||||
import com.vitorpamplona.quartz.nip01Core.signers.EventTemplate
|
||||
import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner
|
||||
import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequest
|
||||
import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestConnect
|
||||
import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestSign
|
||||
import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerResponse
|
||||
import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerResponseEvent
|
||||
import com.vitorpamplona.quartz.nip57Zaps.LnZapPrivateEvent
|
||||
import com.vitorpamplona.quartz.nip57Zaps.LnZapRequestEvent
|
||||
import kotlinx.coroutines.CompletableDeferred
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* Concurrency guarantees the service relies on when it fans requests out into child coroutines so
|
||||
* their consent prompts can batch: the identity signer's crypto must never run concurrently (an
|
||||
* external NIP-55 app can't take overlapping IPC ops), while authorization — which may block on a
|
||||
* user prompt for a long time — must NOT hold that lock, so a pending prompt can't stall other
|
||||
* clients' signing.
|
||||
*/
|
||||
class BunkerRequestProcessorConcurrencyTest {
|
||||
private val userPubKey = "a".repeat(64)
|
||||
private val clientPubKey = "c".repeat(64)
|
||||
private val relay = RelayUrlNormalizer.normalizeOrNull("wss://relay.example.com")!!
|
||||
|
||||
private fun signTemplate(id: String) = BunkerRequestSign(id, EventTemplate<Event>(createdAt = 1L, kind = 1, tags = emptyArray(), content = "hi"))
|
||||
|
||||
/** A signer whose sign() parks on [signGate] so tests can observe how many run at once. */
|
||||
private class GatedSigner(
|
||||
pubKey: HexKey,
|
||||
val signGate: CompletableDeferred<Unit>,
|
||||
) : NostrSigner(pubKey) {
|
||||
var inFlight = 0
|
||||
var maxConcurrent = 0
|
||||
var signCount = 0
|
||||
|
||||
val canned = Event(id = "e".repeat(64), pubKey = pubKey, createdAt = 1L, kind = 1, tags = emptyArray(), content = "s", sig = "f".repeat(128))
|
||||
|
||||
override fun isWriteable() = true
|
||||
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
override suspend fun <T : Event> sign(
|
||||
createdAt: Long,
|
||||
kind: Int,
|
||||
tags: Array<Array<String>>,
|
||||
content: String,
|
||||
): T {
|
||||
inFlight++
|
||||
maxConcurrent = maxOf(maxConcurrent, inFlight)
|
||||
signGate.await()
|
||||
inFlight--
|
||||
signCount++
|
||||
return canned as T
|
||||
}
|
||||
|
||||
override suspend fun nip04Encrypt(
|
||||
plaintext: String,
|
||||
toPublicKey: HexKey,
|
||||
) = ""
|
||||
|
||||
override suspend fun nip04Decrypt(
|
||||
ciphertext: String,
|
||||
fromPublicKey: HexKey,
|
||||
) = ""
|
||||
|
||||
override suspend fun nip44Encrypt(
|
||||
plaintext: String,
|
||||
toPublicKey: HexKey,
|
||||
) = ""
|
||||
|
||||
override suspend fun nip44Decrypt(
|
||||
ciphertext: String,
|
||||
fromPublicKey: HexKey,
|
||||
) = ""
|
||||
|
||||
override suspend fun decryptZapEvent(event: LnZapRequestEvent): LnZapPrivateEvent = throw NotImplementedError()
|
||||
|
||||
override suspend fun deriveKey(nonce: HexKey): HexKey = throw NotImplementedError()
|
||||
|
||||
override suspend fun signPsbt(psbtHex: String): String = throw NotImplementedError()
|
||||
|
||||
override fun hasForegroundSupport() = false
|
||||
}
|
||||
|
||||
/** authorize() parks on the deferred returned by [gateFor] (null = allow immediately). */
|
||||
private class GatedAuthorizer(
|
||||
val gateFor: (BunkerRequest) -> CompletableDeferred<Boolean>?,
|
||||
) : Nip46RequestAuthorizer {
|
||||
override suspend fun onConnect(
|
||||
clientPubKey: HexKey,
|
||||
request: BunkerRequestConnect,
|
||||
) = Nip46ConnectDecision.Accept("ack")
|
||||
|
||||
override suspend fun authorize(
|
||||
clientPubKey: HexKey,
|
||||
request: BunkerRequest,
|
||||
): Boolean = gateFor(request)?.await() ?: true
|
||||
}
|
||||
|
||||
@Test
|
||||
fun cryptoIsSerializedAcrossConcurrentAuthorizedRequests() =
|
||||
runTest {
|
||||
val signGate = CompletableDeferred<Unit>()
|
||||
val signer = GatedSigner(userPubKey, signGate)
|
||||
val processor = BunkerRequestProcessor(signer, { setOf(relay) }, GatedAuthorizer { null })
|
||||
|
||||
launch { processor.process(clientPubKey, signTemplate("1")) }
|
||||
launch { processor.process(clientPubKey, signTemplate("2")) }
|
||||
testScheduler.advanceUntilIdle()
|
||||
|
||||
// Both were authorized instantly, but only one may be inside the signer at a time.
|
||||
assertEquals(1, signer.inFlight, "only one sign holds the crypto lock")
|
||||
assertEquals(1, signer.maxConcurrent, "crypto never overlapped")
|
||||
|
||||
signGate.complete(Unit)
|
||||
testScheduler.advanceUntilIdle()
|
||||
assertEquals(2, signer.signCount, "both eventually signed, one after the other")
|
||||
assertEquals(1, signer.maxConcurrent, "still never overlapped")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun aBlockedPromptDoesNotStallAnotherClientsSigning() =
|
||||
runTest {
|
||||
val signGate = CompletableDeferred<Unit>().apply { complete(Unit) } // signing itself never blocks here
|
||||
val signer = GatedSigner(userPubKey, signGate)
|
||||
val prompt = CompletableDeferred<Boolean>() // stands in for a user consent dialog left open
|
||||
val blocked = signTemplate("blocked")
|
||||
val processor =
|
||||
BunkerRequestProcessor(signer, { setOf(relay) }, GatedAuthorizer { if (it === blocked) prompt else null })
|
||||
|
||||
launch { processor.process(clientPubKey, blocked) } // parks in authorize(), never touching the lock
|
||||
var fastResult: BunkerResponse? = null
|
||||
launch { fastResult = processor.process(clientPubKey, signTemplate("fast")) }
|
||||
testScheduler.advanceUntilIdle()
|
||||
|
||||
// The auto-allowed request signed and returned while the prompt is still open.
|
||||
assertTrue(fastResult is BunkerResponseEvent, "auto-allowed request completed while a prompt was pending")
|
||||
assertEquals(1, signer.signCount)
|
||||
|
||||
prompt.complete(true)
|
||||
testScheduler.advanceUntilIdle()
|
||||
assertEquals(2, signer.signCount, "the prompted request signs once approved")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user