From a593ec35f43f659112d5f583cbf7927730d8154d Mon Sep 17 00:00:00 2001 From: davotoula Date: Fri, 12 Jun 2026 13:54:49 +0200 Subject: [PATCH] Code review: - Account.kt: collapse three identical backend-not-configured Failure constructions into one helper - OnchainZapSendError: declare causeIsUserFacing on the enum so the sender owns which failures carry a human-readable cause, instead of the UI mapper hardcoding the list - SendPaymentScreen: fee chip label is now a single format resource instead of manual string concatenation --- .../vitorpamplona/amethyst/model/Account.kt | 25 ++++++++----------- .../profile/payment/SendPaymentScreen.kt | 8 ++++-- .../loggedIn/wallet/OnchainZapErrorStrings.kt | 16 +++--------- amethyst/src/main/res/values/strings.xml | 1 + .../commons/onchain/OnchainZapSender.kt | 24 ++++++++++++------ 5 files changed, 37 insertions(+), 37 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index 05ab907604..b11b13d01b 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -892,6 +892,13 @@ class Account( return zapRequest } + private fun onchainBackendNotConfigured() = + OnchainZapSendResult.Failure( + OnchainZapSendStage.LOADING_UTXOS, + OnchainZapSendError.BACKEND_NOT_CONFIGURED, + ONCHAIN_BACKEND_NOT_CONFIGURED, + ) + /** * Send a NIP-BC onchain zap: build a Bitcoin transaction paying the recipient's * derived Taproot address, sign it, broadcast it, and publish the kind:8333 @@ -907,11 +914,7 @@ class Account( ): OnchainZapSendResult { val backend = cache.onchainBackend - ?: return OnchainZapSendResult.Failure( - OnchainZapSendStage.LOADING_UTXOS, - OnchainZapSendError.BACKEND_NOT_CONFIGURED, - ONCHAIN_BACKEND_NOT_CONFIGURED, - ) + ?: return onchainBackendNotConfigured() return OnchainZapSender.send( backend = backend, signer = signer, @@ -936,11 +939,7 @@ class Account( ): OnchainZapSendResult { val backend = cache.onchainBackend - ?: return OnchainZapSendResult.Failure( - OnchainZapSendStage.LOADING_UTXOS, - OnchainZapSendError.BACKEND_NOT_CONFIGURED, - ONCHAIN_BACKEND_NOT_CONFIGURED, - ) + ?: return onchainBackendNotConfigured() return OnchainZapSender.sendToAddress( backend = backend, signer = signer, @@ -964,11 +963,7 @@ class Account( ): OnchainZapSendResult { val backend = cache.onchainBackend - ?: return OnchainZapSendResult.Failure( - OnchainZapSendStage.LOADING_UTXOS, - OnchainZapSendError.BACKEND_NOT_CONFIGURED, - ONCHAIN_BACKEND_NOT_CONFIGURED, - ) + ?: return onchainBackendNotConfigured() return OnchainZapSender.sendSplit( backend = backend, signer = signer, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/payment/SendPaymentScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/payment/SendPaymentScreen.kt index 796f0324ad..9363aa9567 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/payment/SendPaymentScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/payment/SendPaymentScreen.kt @@ -749,8 +749,12 @@ private fun OnchainFeeSection( label = { Text( if (rate != null) { - "${stringRes(tier.labelRes)} · " + - stringRes(R.string.onchain_send_fee_rate_eta, "%.1f".format(rate), stringRes(tier.etaLabelRes)) + stringRes( + R.string.onchain_send_fee_tier_label_rate_eta, + stringRes(tier.labelRes), + "%.1f".format(rate), + stringRes(tier.etaLabelRes), + ) } else { stringRes(tier.labelRes) }, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/OnchainZapErrorStrings.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/OnchainZapErrorStrings.kt index ef9b98ca6b..54610c98da 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/OnchainZapErrorStrings.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/OnchainZapErrorStrings.kt @@ -49,19 +49,11 @@ fun OnchainZapSendResult.Failure.userMessage(context: Context): String { } /** - * Untranslated diagnostic detail worth showing under the localized headline: - * build/sign/dust failures carry a crafted, specific reason (e.g. insufficient - * funds, signer tampering, which share is below dust). Other stages only carry - * low-level exception text, which we keep out of the UI. + * Untranslated diagnostic detail worth showing under the localized headline. + * Which errors carry a human-readable cause is a fact about the sender, + * declared on [OnchainZapSendError.causeIsUserFacing]. */ -fun OnchainZapSendResult.Failure.technicalDetail(): String? = - when (error) { - OnchainZapSendError.BUILD_FAILED, - OnchainZapSendError.SIGN_FAILED, - OnchainZapSendError.RECIPIENT_BELOW_DUST, - -> cause?.message - else -> null - } +fun OnchainZapSendResult.Failure.technicalDetail(): String? = if (error.causeIsUserFacing) cause?.message else null @StringRes private fun OnchainZapSendError.messageRes(): Int = diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index 3e37af6f24..7b9b5f4abc 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -2240,6 +2240,7 @@ ~30 min ~10 min %1$s sat/vB · %2$s + %1$s · %2$s sat/vB · %3$s Loading fee estimates… Send %1$s sats diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/onchain/OnchainZapSender.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/onchain/OnchainZapSender.kt index 6f9975dc7d..ad244bd623 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/onchain/OnchainZapSender.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/onchain/OnchainZapSender.kt @@ -61,27 +61,35 @@ enum class OnchainZapSendStage { * [OnchainZapSendResult.Failure.message] stays available for logs, tests, and * the CLI. */ -enum class OnchainZapSendError { +enum class OnchainZapSendError( + /** + * Whether [OnchainZapSendResult.Failure.cause] carries a message crafted + * for humans (insufficient funds, signer tampering, which share is below + * dust) as opposed to low-level exception text. UIs may surface the cause + * message as a diagnostic detail when this is true. + */ + val causeIsUserFacing: Boolean, +) { /** No [OnchainBackend] is configured for the account. */ - BACKEND_NOT_CONFIGURED, + BACKEND_NOT_CONFIGURED(causeIsUserFacing = false), /** The chain backend could not return the sender's UTXOs. */ - LOAD_UTXOS_FAILED, + LOAD_UTXOS_FAILED(causeIsUserFacing = false), /** Coin selection / PSBT assembly failed (e.g. insufficient funds). */ - BUILD_FAILED, + BUILD_FAILED(causeIsUserFacing = true), /** A recipient's split share is below the dust threshold. */ - RECIPIENT_BELOW_DUST, + RECIPIENT_BELOW_DUST(causeIsUserFacing = true), /** Signing, signature verification, or finalization failed. */ - SIGN_FAILED, + SIGN_FAILED(causeIsUserFacing = true), /** The signed transaction could not be broadcast. */ - BROADCAST_FAILED, + BROADCAST_FAILED(causeIsUserFacing = false), /** The payment broadcast, but a kind:8333 receipt could not be published. */ - RECEIPT_PUBLISH_FAILED, + RECEIPT_PUBLISH_FAILED(causeIsUserFacing = false), } /** Outcome of an [OnchainZapSender.send] attempt. */