mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 03:38:23 +00:00
fix(zap): correct the pay-to probe, and stop redoing its expensive half
Audit of the previous commit. Four findings, all reachable in normal use. The probe was stricter than the hand-off it predicts. It carried CATEGORY_BROWSABLE and queried with flags=0, while the hand-off goes through startActivity, which implies CATEGORY_DEFAULT and nothing else. IntentFilter.matchCategories returns the first category on the *intent* the filter lacks, so each category added to a query narrows the match: any app declaring only DEFAULT was invisible to the probe and its chip was hidden even though tapping it would have worked. The probe now carries no category and uses MATCH_DEFAULT_ONLY, resolving exactly the set startActivity would. The <queries> entries lose the category for the same reason — there it narrows package visibility itself. The chip snapshotted the probe result with remember(target.type), so it never saw the probe finish. A web target is offered before any probe runs, since any browser opens https, so that snapshot pinned the fallback glyph and the real app icon could not appear until the picker was closed and reopened — the Venmo and PayPal case the icon exists for. It now collects the availability flow. peek() built the recipient's target list eagerly, walking the kind:10133 tag array on every call, including the one-tap zap path with the feature switched off. selectFor now takes it as a lambda behind the cheap gates, pinned by a test that counts reads. warm() runs on each picker open so resolution stays fresh when the user installs an app and comes back, but it also re-read each APK's resources and re-rasterised its icon to answer the same question. Decoded icons are now kept across warms, keyed by package and size, and the browser control probe only runs when a web target is actually present. Also: the icon failure log kept its message but dropped the throwable; it now passes it. Removes the unused clear(). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JXKZeV6FNhXF9BBjgEtfvS
This commit is contained in:
@@ -23,70 +23,60 @@
|
||||
modern device. Specific <intent> filters rather than
|
||||
QUERY_ALL_PACKAGES, which is policy-restricted on Play.
|
||||
Unknown target types all fall back to payto://<type>/<authority>,
|
||||
so the single payto entry covers the open-ended tail. -->
|
||||
so the single payto entry covers the open-ended tail.
|
||||
No <category>: a category here narrows visibility the same way it
|
||||
narrows an intent match, and would hide any app whose filter declares
|
||||
only DEFAULT - which is what our ACTION_VIEW hand-off actually uses. -->
|
||||
<intent>
|
||||
<action android:name="android.intent.action.VIEW" />
|
||||
<category android:name="android.intent.category.BROWSABLE" />
|
||||
<data android:scheme="payto" />
|
||||
</intent>
|
||||
<intent>
|
||||
<action android:name="android.intent.action.VIEW" />
|
||||
<category android:name="android.intent.category.BROWSABLE" />
|
||||
<data android:scheme="bitcoin" />
|
||||
</intent>
|
||||
<intent>
|
||||
<action android:name="android.intent.action.VIEW" />
|
||||
<category android:name="android.intent.category.BROWSABLE" />
|
||||
<data android:scheme="lightning" />
|
||||
</intent>
|
||||
<intent>
|
||||
<action android:name="android.intent.action.VIEW" />
|
||||
<category android:name="android.intent.category.BROWSABLE" />
|
||||
<data android:scheme="liquidnetwork" />
|
||||
</intent>
|
||||
<intent>
|
||||
<action android:name="android.intent.action.VIEW" />
|
||||
<category android:name="android.intent.category.BROWSABLE" />
|
||||
<data android:scheme="ethereum" />
|
||||
</intent>
|
||||
<intent>
|
||||
<action android:name="android.intent.action.VIEW" />
|
||||
<category android:name="android.intent.category.BROWSABLE" />
|
||||
<data android:scheme="monero" />
|
||||
</intent>
|
||||
<intent>
|
||||
<action android:name="android.intent.action.VIEW" />
|
||||
<category android:name="android.intent.category.BROWSABLE" />
|
||||
<data android:scheme="dash" />
|
||||
</intent>
|
||||
<intent>
|
||||
<action android:name="android.intent.action.VIEW" />
|
||||
<category android:name="android.intent.category.BROWSABLE" />
|
||||
<data android:scheme="zcash" />
|
||||
</intent>
|
||||
<intent>
|
||||
<action android:name="android.intent.action.VIEW" />
|
||||
<category android:name="android.intent.category.BROWSABLE" />
|
||||
<data android:scheme="bitcoincash" />
|
||||
</intent>
|
||||
<intent>
|
||||
<action android:name="android.intent.action.VIEW" />
|
||||
<category android:name="android.intent.category.BROWSABLE" />
|
||||
<data android:scheme="litecoin" />
|
||||
</intent>
|
||||
<intent>
|
||||
<action android:name="android.intent.action.VIEW" />
|
||||
<category android:name="android.intent.category.BROWSABLE" />
|
||||
<data android:scheme="dogecoin" />
|
||||
</intent>
|
||||
<intent>
|
||||
<action android:name="android.intent.action.VIEW" />
|
||||
<category android:name="android.intent.category.BROWSABLE" />
|
||||
<data android:scheme="solana" />
|
||||
</intent>
|
||||
<intent>
|
||||
<action android:name="android.intent.action.VIEW" />
|
||||
<category android:name="android.intent.category.BROWSABLE" />
|
||||
<data android:scheme="tron" />
|
||||
</intent>
|
||||
</queries>
|
||||
|
||||
@@ -178,12 +178,13 @@ object RailCapabilityResolver {
|
||||
hasAuthor = baseNote.author != null,
|
||||
hasZapSplit = splits.isNotEmpty(),
|
||||
senderTargets = senderTargets,
|
||||
recipientTargets = baseNote.author?.paymentTargets().orEmpty(),
|
||||
// An unresolvable URI would open nothing, so the chip is not offered.
|
||||
// Web targets always resolve; there the probe only decides the icon.
|
||||
canOpen = {
|
||||
PayToAppAvailability.peek(it.type)?.resolves == true ||
|
||||
PaymentTargetTypes.isWebTarget(it.type)
|
||||
},
|
||||
// Lazy: the tag walk only happens once the cheap gates have passed.
|
||||
recipientTargets = { baseNote.author?.paymentTargets().orEmpty() },
|
||||
)
|
||||
}
|
||||
|
||||
+37
-14
@@ -36,6 +36,7 @@ import com.vitorpamplona.quartz.utils.Log
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import java.util.concurrent.ConcurrentHashMap
|
||||
|
||||
/** What the device can do with one `payto` target type. */
|
||||
@Immutable
|
||||
@@ -74,6 +75,16 @@ object PayToAppAvailability {
|
||||
private val state = MutableStateFlow<Map<String, PayToAppInfo>>(emptyMap())
|
||||
val flow: StateFlow<Map<String, PayToAppInfo>> = state.asStateFlow()
|
||||
|
||||
/**
|
||||
* Decoded icons, kept across warms and keyed by package and size.
|
||||
*
|
||||
* [warm] runs every time the picker opens, so that resolution stays fresh when
|
||||
* the user installs an app and comes back. Re-reading the APK's resources and
|
||||
* re-rasterising the icon each of those times is the expensive half and answers
|
||||
* the same thing, so only the cheap half repeats.
|
||||
*/
|
||||
private val icons = ConcurrentHashMap<String, ImageBitmap>()
|
||||
|
||||
/** Synchronous read for `RailCapabilityResolver.peek`, which runs inside `remember {}`. */
|
||||
fun peek(rawType: String): PayToAppInfo? = state.value[PaymentTargetTypes.probeKeyFor(rawType)]
|
||||
|
||||
@@ -103,18 +114,14 @@ object PayToAppAvailability {
|
||||
return
|
||||
}
|
||||
|
||||
val browsers = browserPackages(pm)
|
||||
// Only https targets need the control probe; skip the extra query otherwise.
|
||||
val browsers = if (keys.any(PaymentTargetTypes::isWebTarget)) browserPackages(pm) else emptySet()
|
||||
state.value =
|
||||
keys.associate { type ->
|
||||
PaymentTargetTypes.probeKeyFor(type) to probe(pm, type, browsers, iconPx)
|
||||
}
|
||||
}
|
||||
|
||||
/** Drops everything, so the next [warm] re-reads a changed set of installed apps. */
|
||||
fun clear() {
|
||||
state.value = emptyMap()
|
||||
}
|
||||
|
||||
private fun probe(
|
||||
pm: PackageManager,
|
||||
rawType: String,
|
||||
@@ -173,27 +180,43 @@ object PayToAppAvailability {
|
||||
pm: PackageManager,
|
||||
info: ResolveInfo,
|
||||
px: Int,
|
||||
): ImageBitmap? =
|
||||
runCatching {
|
||||
): ImageBitmap? {
|
||||
val pkg = info.packageName() ?: return null
|
||||
icons["$pkg@$px"]?.let { return it }
|
||||
|
||||
return runCatching {
|
||||
info.loadIcon(pm).toBitmap(px, px).asImageBitmap()
|
||||
}.onSuccess {
|
||||
icons["$pkg@$px"] = it
|
||||
}.onFailure {
|
||||
Log.w("PayToAppAvailability") { "Could not load icon for ${info.packageName()}: ${it.message}" }
|
||||
Log.w("PayToAppAvailability", "Could not load icon for $pkg", it)
|
||||
}.getOrNull()
|
||||
}
|
||||
|
||||
@SuppressLint("QueryPermissionsNeeded")
|
||||
private fun queryActivities(
|
||||
pm: PackageManager,
|
||||
intent: Intent,
|
||||
): List<ResolveInfo> = runCatching { pm.queryIntentActivities(intent, 0) }.getOrDefault(emptyList())
|
||||
): List<ResolveInfo> =
|
||||
runCatching {
|
||||
// MATCH_DEFAULT_ONLY mirrors startActivity, which implies CATEGORY_DEFAULT.
|
||||
// Without it we would list activities the hand-off could never launch.
|
||||
pm.queryIntentActivities(intent, PackageManager.MATCH_DEFAULT_ONLY)
|
||||
}.getOrDefault(emptyList())
|
||||
|
||||
/** Packages that answer a URL nobody can own — i.e. general-purpose browsers. */
|
||||
@SuppressLint("QueryPermissionsNeeded")
|
||||
private fun browserPackages(pm: PackageManager): Set<String> = queryActivities(pm, viewIntent(CONTROL_URL)).mapNotNull { it.packageName() }.toSet()
|
||||
|
||||
private fun viewIntent(uri: String) =
|
||||
Intent(Intent.ACTION_VIEW, uri.toUri()).apply {
|
||||
addCategory(Intent.CATEGORY_BROWSABLE)
|
||||
}
|
||||
/**
|
||||
* Deliberately carries **no** category. `IntentFilter.matchCategories` returns
|
||||
* the first category on the *intent* that the filter lacks, so every category
|
||||
* added here narrows the match — a probe carrying `BROWSABLE` would miss any
|
||||
* app whose filter declares only `DEFAULT`, and hide a chip that would have
|
||||
* opened fine. Paired with `MATCH_DEFAULT_ONLY` in [queryActivities], this
|
||||
* resolves exactly the set `startActivity` would.
|
||||
*/
|
||||
private fun viewIntent(uri: String) = Intent(Intent.ACTION_VIEW, uri.toUri())
|
||||
|
||||
private fun ResolveInfo.packageName(): String? = activityInfo?.packageName
|
||||
}
|
||||
|
||||
@@ -2427,9 +2427,15 @@ private fun PayToHandoffChip(
|
||||
onHandedOff: () -> Unit,
|
||||
) {
|
||||
val style = remember(target.type) { paymentTargetStyleFor(target.type) }
|
||||
val app = remember(target.type) { PayToAppAvailability.peek(target.type) }
|
||||
val uri = remember(target) { PaymentTargetTypes.uriFor(target.type, target.authority) }
|
||||
|
||||
// Collected, not peeked once: a web target is offered before the probe has run
|
||||
// (any browser opens https), so a snapshot taken at first composition would pin
|
||||
// the fallback glyph and the real app icon would never arrive until the picker
|
||||
// was closed and reopened.
|
||||
val apps by PayToAppAvailability.flow.collectAsStateWithLifecycle()
|
||||
val app = remember(apps, target.type) { apps[PaymentTargetTypes.probeKeyFor(target.type)] }
|
||||
|
||||
val uriHandler = LocalUriHandler.current
|
||||
val context = LocalContext.current
|
||||
val clipboard = LocalClipboard.current
|
||||
|
||||
+7
-3
@@ -84,16 +84,20 @@ object PayToRailMatcher {
|
||||
* @param hasZapSplit a `payto` hand-off leaves with one authority and returns
|
||||
* no receipt, so it cannot honour a note that asks to divide the zap.
|
||||
* @param canOpen whether an installed app resolves this target's URI.
|
||||
* @param recipientTargets read lazily. Parsing the recipient's kind:10133 walks
|
||||
* its tag array and allocates, and the common case is that a gate has already
|
||||
* failed — the setting is off, or the note carries a split — so the cheap
|
||||
* checks must run first. `peek` is on the one-tap zap path too.
|
||||
*/
|
||||
fun selectFor(
|
||||
enabled: Boolean,
|
||||
hasAuthor: Boolean,
|
||||
hasZapSplit: Boolean,
|
||||
senderTargets: List<PaymentTarget>,
|
||||
recipientTargets: List<PaymentTarget>,
|
||||
canOpen: (PaymentTarget) -> Boolean,
|
||||
recipientTargets: () -> List<PaymentTarget>,
|
||||
): List<PaymentTarget> {
|
||||
if (!enabled || !hasAuthor || hasZapSplit) return emptyList()
|
||||
return match(senderTargets, recipientTargets).filter(canOpen).take(MAX_CHIPS)
|
||||
if (!enabled || !hasAuthor || hasZapSplit || senderTargets.isEmpty()) return emptyList()
|
||||
return match(senderTargets, recipientTargets()).filter(canOpen).take(MAX_CHIPS)
|
||||
}
|
||||
}
|
||||
|
||||
+21
-1
@@ -173,7 +173,7 @@ class PayToRailGateTest {
|
||||
sender: List<PaymentTarget> = mine,
|
||||
recipient: List<PaymentTarget> = theirs,
|
||||
canOpen: (PaymentTarget) -> Boolean = anyAppOpens,
|
||||
) = PayToRailMatcher.selectFor(enabled, hasAuthor, hasZapSplit, sender, recipient, canOpen)
|
||||
) = PayToRailMatcher.selectFor(enabled, hasAuthor, hasZapSplit, sender, canOpen) { recipient }
|
||||
|
||||
@Test
|
||||
fun offeredWhenEveryGatePasses() {
|
||||
@@ -214,6 +214,26 @@ class PayToRailGateTest {
|
||||
assertEquals(PayToRailMatcher.MAX_CHIPS, select(sender = many, recipient = many).size)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun recipientTargetsAreNotReadWhenACheapGateAlreadyFailed() {
|
||||
// peek() runs on the one-tap zap path too, and reading the recipient's
|
||||
// kind:10133 walks its tag array. Nothing should touch it once a gate fails.
|
||||
var reads = 0
|
||||
val counted = {
|
||||
reads++
|
||||
theirs
|
||||
}
|
||||
|
||||
PayToRailMatcher.selectFor(false, true, false, mine, anyAppOpens, counted)
|
||||
PayToRailMatcher.selectFor(true, false, false, mine, anyAppOpens, counted)
|
||||
PayToRailMatcher.selectFor(true, true, true, mine, anyAppOpens, counted)
|
||||
PayToRailMatcher.selectFor(true, true, false, emptyList(), anyAppOpens, counted)
|
||||
assertEquals(0, reads)
|
||||
|
||||
PayToRailMatcher.selectFor(true, true, false, mine, anyAppOpens, counted)
|
||||
assertEquals(1, reads)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun lightningAndBitcoinStayWithTheirOwnRails() {
|
||||
val wallets = listOf(t("lightning", "a@b.c"), t("btc", "bc1q"))
|
||||
|
||||
Reference in New Issue
Block a user