nick/amethyst
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix: on-chain zap splits — drop sender from splits, merge duplicates, gate Send on dust

Browse Source 
Audit findings from an independent code review:

- HIGH: When the user zaps their own post (a common flow), every split
  that included the post author put the sender on the recipient list,
  and OnchainZapBuilder.buildSplit refused the whole tx with "cannot
  zap yourself". Fix: new OnchainZapSplitter.prepare() filters the
  sender's pubkey out of the splits before they reach the builder.
- HIGH: NIP-57 lets the same pubkey appear in zap-split tags more than
  once (additive weights). buildSplit rejected duplicate recipients.
  Same prepare() helper merges duplicates by summing weights, in
  first-seen order.
- HIGH: The dialog's live preview only showed amounts for recipients
  whose share was BELOW dust (because DustRecipientException only
  carries belowDust). Fix: parent composable computes shares with a
  zero dust threshold for the preview, gating the Send button on a
  separate belowDustShares check so the user can see all amounts and
  can't tap Send into a guaranteed BUILDING-stage failure.
- MEDIUM: OnchainZapSendResult.Failure didn't carry the ids of
  receipts that successfully published before a partial-publish
  failure. Added publishedReceiptEventIds: List<HexKey>.
- LOW: useSplits state was keyed by zappedEvent reference; re-emitted
  bundles would silently reset the toggle. Now keyed on the event id.

Tests added:
- splitter: prepare() drops sender, merges duplicates, filters
  non-positive weights; floating-point weights (0.1 + 0.2) sum exactly
- builder: buildSplit produces N recipient outputs + 1 change at index
  N, conserves sats, rejects duplicates and below-dust shares
- sender: sendSplit publishes one receipt per recipient sharing the
  txid with correct per-recipient amount; partial-publish failure
  carries the broadcast txid and the ids of receipts that did publish
This commit is contained in:
Claude
2026-05-20 20:14:44 +00:00
parent 3ed2245d8c
commit 45aa6044b7
6 changed files with 303 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/wallet/OnchainZapSendDialog.kt
+66 -44
View File
@@ -70,6 +70,7 @@ import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols
import com.vitorpamplona.amethyst.commons.onchain.DustRecipientException
import com.vitorpamplona.amethyst.commons.onchain.OnchainZapSendResult
import com.vitorpamplona.amethyst.commons.onchain.OnchainZapSendStage
import com.vitorpamplona.amethyst.commons.onchain.OnchainZapShare
import com.vitorpamplona.amethyst.commons.onchain.OnchainZapSplitter
import com.vitorpamplona.amethyst.model.LocalCache
import com.vitorpamplona.amethyst.model.User
@@ -161,24 +162,25 @@ fun OnchainZapSendDialog(
// Pull pubkey-based zap splits off the zapped event. Lightning-address-only
// splits are dropped because we can't derive a Taproot output from an
// lnAddress — they appear in [skippedLnSplits] so the UI can warn the user
// that those recipients won't be paid on-chain.
// that those recipients won't be paid on-chain. The sender's own pubkey is
// also dropped (zapping your own post is common; the on-chain builder
// refuses self-pays) and duplicate pubkeys are merged.
val senderPubKey = accountViewModel.account.signer.pubKey
val zappedEventId = zappedEvent?.event?.id
val rawSplits =
remember(zappedEventId) {
zappedEvent?.event?.zapSplitSetup().orEmpty()
}
val onchainSplits =
remember(zappedEvent) {
zappedEvent
?.event
?.zapSplitSetup()
.orEmpty()
.filterIsInstance<ZapSplitSetup>()
remember(zappedEventId, senderPubKey) {
val raw = rawSplits.filterIsInstance<ZapSplitSetup>().map { it.pubKeyHex to it.weight }
OnchainZapSplitter.prepare(raw, senderPubKey)
}
val skippedLnSplits =
remember(zappedEvent) {
zappedEvent
?.event
?.zapSplitSetup()
.orEmpty()
.filterIsInstance<ZapSplitSetupLnAddress>()
remember(zappedEventId) {
rawSplits.filterIsInstance<ZapSplitSetupLnAddress>()
}
var useSplits by remember(zappedEvent) { mutableStateOf(onchainSplits.isNotEmpty()) }
var useSplits by remember(zappedEventId) { mutableStateOf(onchainSplits.isNotEmpty()) }
val splitMode = useSplits && onchainSplits.isNotEmpty()
LaunchedEffect(Unit) {
@@ -202,13 +204,51 @@ fun OnchainZapSendDialog(
?: searchInput.trim().takeIf { it.isNotEmpty() }?.let { decodePublicKeyAsHexOrNull(it) }
?: nip05Resolved?.pubkeyHex
val amountSats = amountInput.trim().toLongOrNull()
// Preview the per-recipient share allocation. Always compute the full
// list (even when some shares would land below dust) so the UI can show
// every recipient's amount; the dust offenders are flagged separately
// and gate the Send button.
val previewShares =
remember(splitMode, onchainSplits, amountSats) {
if (!splitMode || amountSats == null || amountSats <= 0) {
null
} else {
runCatching {
OnchainZapSplitter.distribute(
totalSats = amountSats,
splits = onchainSplits,
dustThresholdSats = OnchainZapBuilder.DUST_THRESHOLD_SATS,
)
}.getOrElse { e ->
if (e is DustRecipientException) {
// Re-run with a 0 dust threshold to get the full shape
// for the preview; the real send still uses the proper
// dust check via [DustRecipientException].
runCatching {
OnchainZapSplitter.distribute(
totalSats = amountSats,
splits = onchainSplits,
dustThresholdSats = 0L,
)
}.getOrNull()
} else {
null
}
}
}
}
val belowDustShares =
previewShares.orEmpty().filter { it.sats < OnchainZapBuilder.DUST_THRESHOLD_SATS }
val canSend =
!sending &&
result == null &&
(splitMode || resolvedRecipient != null) &&
amountSats != null &&
amountSats > 0 &&
fees != null
fees != null &&
(!splitMode || (previewShares != null && belowDustShares.isEmpty()))
ModalBottomSheet(
onDismissRequest = { if (!sending) onDismiss() },
@@ -261,8 +301,8 @@ fun OnchainZapSendDialog(
if (splitMode) {
SplitsRecipientSection(
splits = onchainSplits,
previewShares = previewShares,
skippedLnSplits = skippedLnSplits,
amountSats = amountSats,
onDisable = { useSplits = false },
accountViewModel = accountViewModel,
)
@@ -345,7 +385,7 @@ fun OnchainZapSendDialog(
try {
OnchainZapSplitter.distribute(
totalSats = amount,
splits = onchainSplits.map { it.pubKeyHex to it.weight },
splits = onchainSplits,
dustThresholdSats = OnchainZapBuilder.DUST_THRESHOLD_SATS,
)
} catch (e: DustRecipientException) {
@@ -698,32 +738,15 @@ private fun SendButton(
@Composable
private fun SplitsRecipientSection(
splits: List<ZapSplitSetup>,
splits: List<Pair<HexKey, Double>>,
previewShares: List<OnchainZapShare>?,
skippedLnSplits: List<ZapSplitSetupLnAddress>,
amountSats: Long?,
onDisable: () -> Unit,
accountViewModel: AccountViewModel,
) {
SectionLabel("Splits among ${splits.size} recipients")
// Compute per-recipient shares for the live preview. Below-dust splits are
// surfaced inline so the user sees why Send might fail before they tap it.
val shares =
remember(splits, amountSats) {
if (amountSats == null || amountSats <= 0) {
null
} else {
runCatching {
OnchainZapSplitter.distribute(
totalSats = amountSats,
splits = splits.map { it.pubKeyHex to it.weight },
dustThresholdSats = OnchainZapBuilder.DUST_THRESHOLD_SATS,
)
}.getOrElse { e ->
if (e is DustRecipientException) e.belowDust else null
}
}
}
val totalWeight = splits.sumOf { it.second }
Surface(
shape = MaterialTheme.shapes.medium,
@@ -731,21 +754,21 @@ private fun SplitsRecipientSection(
modifier = Modifier.fillMaxWidth(),
) {
Column(modifier = Modifier.padding(vertical = 4.dp)) {
splits.forEach { split ->
val share = shares?.firstOrNull { it.recipientPubKey == split.pubKeyHex }
splits.forEach { (pubKey, weight) ->
val share = previewShares?.firstOrNull { it.recipientPubKey == pubKey }
Row(
modifier = Modifier.padding(horizontal = 12.dp, vertical = 6.dp),
verticalAlignment = Alignment.CenterVertically,
) {
UserPicture(
userHex = split.pubKeyHex,
userHex = pubKey,
size = 28.dp,
accountViewModel = accountViewModel,
nav = EmptyNav(),
)
Spacer(Modifier.size(8.dp))
Text(
text = "${formatWeight(split.weight, splits)}",
text = formatWeight(weight, totalWeight),
style = MaterialTheme.typography.bodySmall,
color = MaterialTheme.colorScheme.onSurfaceVariant,
modifier = Modifier.weight(1f),
@@ -789,10 +812,9 @@ private fun SplitsRecipientSection(
private fun formatWeight(
weight: Double,
all: List<ZapSplitSetup>,
totalWeight: Double,
): String {
val total = all.sumOf { it.weight }
val pct = (weight / total) * 100.0
val pct = (weight / totalWeight) * 100.0
return if (pct >= 99.95) {
"100%"
} else {
commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/onchain/OnchainZapSender.kt
+7
View File
@@ -87,6 +87,12 @@ sealed interface OnchainZapSendResult {
val cause: Throwable? = null,
/** Non-null when the payment was broadcast but a later stage failed. */
val broadcastTxid: String? = null,
/**
* Receipt ids that DID publish before the failure, in the order they
* were sent. Empty for non-publishing failures and for single-recipient
* publishes that fail on the first receipt.
*/
val publishedReceiptEventIds: List<HexKey> = emptyList(),
) : OnchainZapSendResult
}
@@ -359,6 +365,7 @@ object OnchainZapSender {
"but the next receipt could not be published",
cause = e,
broadcastTxid = txid,
publishedReceiptEventIds = publishedIds.toList(),
)
}
}
commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/onchain/OnchainZapSplitter.kt
+25
View File
@@ -50,6 +50,31 @@ class DustRecipientException(
* - any share that lands below dust is reported via [DustRecipientException]
*/
object OnchainZapSplitter {
/**
* Clean raw `["zap", pubkey, relay, weight]` splits for the on-chain path:
*
* 1. drop the sender's own pubkey (the on-chain builder refuses self-pays,
* and a self-share would otherwise abort the whole zap when the user
* zaps their own post — common, since most splits include the author)
* 2. merge duplicates by pubkey, summing their weights (Lightning splits
* can repeat a pubkey; on-chain we want exactly one output per pubkey
* so each recipient gets one receipt with one consolidated amount)
*
* The returned list preserves first-seen input order.
*/
fun prepare(
rawSplits: List<Pair<HexKey, Double>>,
senderPubKey: HexKey,
): List<Pair<HexKey, Double>> {
val merged = linkedMapOf<HexKey, Double>()
for ((pubKey, weight) in rawSplits) {
if (pubKey == senderPubKey) continue
if (weight <= 0.0) continue
merged[pubKey] = (merged[pubKey] ?: 0.0) + weight
}
return merged.entries.map { it.key to it.value }
}
fun distribute(
totalSats: Long,
splits: List<Pair<HexKey, Double>>,
commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/onchain/OnchainZapSenderTest.kt
+82
View File
@@ -272,4 +272,86 @@ class OnchainZapSenderTest {
assertEquals(OnchainZapSendStage.SIGNING, result.stage)
assertEquals(null, backend.broadcastedHex)
}
@Test
fun sendSplitProducesOneReceiptPerRecipient() =
runTest {
val r1 = xOnly("000000000000000000000000000000000000000000000000000000000000000d")
val r2 = xOnly("000000000000000000000000000000000000000000000000000000000000000e")
val r3 = xOnly("000000000000000000000000000000000000000000000000000000000000000f")
val backend = FakeBackend(listOf(Utxo("1".repeat(64), 0, 1_000_000L, 6)))
val publishedReceipts = mutableListOf<OnchainZapEvent>()
val result =
OnchainZapSender.sendSplit(
backend = backend,
signer = senderSigner,
senderPubKey = senderPubKey,
recipients =
listOf(
OnchainZapShare(r1, 50_000L, 5.0),
OnchainZapShare(r2, 30_000L, 3.0),
OnchainZapShare(r3, 20_000L, 2.0),
),
feeRateSatPerVByte = 5.0,
comment = "thanks all",
zappedEvent = null,
) { template ->
val event = senderSigner.sign<OnchainZapEvent>(template)
publishedReceipts += event
event
}
assertIs<OnchainZapSendResult.Success>(result)
assertEquals(3, publishedReceipts.size)
assertEquals(2, result.extraReceiptEventIds.size)
// All receipts must reference the SAME txid.
val broadcastTxid = BitcoinTransaction.parse(backend.broadcastedHex!!).txid()
assertEquals(broadcastTxid, result.txid)
assertTrue(publishedReceipts.all { it.txid() == broadcastTxid })
// Per-recipient receipts carry the right pubkey + sat amount.
val byRecipient = publishedReceipts.associateBy { it.recipient() }
assertEquals(50_000L, byRecipient[r1]?.claimedAmountInSats())
assertEquals(30_000L, byRecipient[r2]?.claimedAmountInSats())
assertEquals(20_000L, byRecipient[r3]?.claimedAmountInSats())
}
@Test
fun sendSplitPartialPublishFailureKeepsBroadcastedReceiptIds() =
runTest {
val r1 = xOnly("000000000000000000000000000000000000000000000000000000000000000d")
val r2 = xOnly("000000000000000000000000000000000000000000000000000000000000000e")
val r3 = xOnly("000000000000000000000000000000000000000000000000000000000000000f")
val backend = FakeBackend(listOf(Utxo("1".repeat(64), 0, 1_000_000L, 6)))
var calls = 0
val result =
OnchainZapSender.sendSplit(
backend = backend,
signer = senderSigner,
senderPubKey = senderPubKey,
recipients =
listOf(
OnchainZapShare(r1, 50_000L, 1.0),
OnchainZapShare(r2, 30_000L, 1.0),
OnchainZapShare(r3, 20_000L, 1.0),
),
feeRateSatPerVByte = 5.0,
comment = "",
zappedEvent = null,
) { template ->
calls++
if (calls == 2) throw RuntimeException("relay rejected receipt")
senderSigner.sign(template)
}
assertIs<OnchainZapSendResult.Failure>(result)
assertEquals(OnchainZapSendStage.PUBLISHING, result.stage)
// Tx is on-chain.
assertTrue(result.broadcastTxid != null)
// Exactly one receipt was published before the failure.
assertEquals(1, result.publishedReceiptEventIds.size)
}
}
commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/onchain/OnchainZapSplitterTest.kt
+40
View File
@@ -111,6 +111,33 @@ class OnchainZapSplitterTest {
assertEquals(50_000L, shares[0].sats)
}
@Test
fun prepareDropsSenderAndMergesDuplicates() {
val cleaned =
OnchainZapSplitter.prepare(
rawSplits = listOf(a to 1.0, b to 2.0, a to 1.0, c to 0.0, b to 1.0),
senderPubKey = c,
)
// sender c is dropped, a is merged (1+1=2), b is merged (2+1=3), c=0 filtered too.
assertEquals(listOf(a to 2.0, b to 3.0), cleaned)
}
@Test
fun prepareDropsSenderEvenIfOnlyEntry() {
val cleaned = OnchainZapSplitter.prepare(listOf(a to 1.0), senderPubKey = a)
assertTrue(cleaned.isEmpty())
}
@Test
fun prepareSkipsNegativeOrZeroWeights() {
val cleaned =
OnchainZapSplitter.prepare(
rawSplits = listOf(a to 1.0, b to 0.0, c to -1.0),
senderPubKey = "deadbeef".repeat(8),
)
assertEquals(listOf(a to 1.0), cleaned)
}
@Test
fun fractionalWeightsWork() {
val shares =
@@ -125,4 +152,17 @@ class OnchainZapSplitterTest {
assertTrue(shares[1].sats in 29_900..30_100)
assertTrue(shares[2].sats in 19_900..20_100)
}
@Test
fun floatingPointWeightsSumExactly() {
// 0.1 + 0.2 = 0.30000000000000004 in IEEE-754. Make sure that doesn't
// leak a missing or extra sat.
val shares =
OnchainZapSplitter.distribute(
totalSats = 1_000_000L,
splits = listOf(a to 0.1, b to 0.2),
dustThresholdSats = 330L,
)
assertEquals(1_000_000L, shares.sumOf { it.sats })
}
}
quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nipBCOnchainZaps/builder/OnchainZapBuilderTest.kt
+83
View File
@@ -196,4 +196,87 @@ class OnchainZapBuilderTest {
val result = OnchainZapBuilder.build(senderPubKey, recipientPubKey, 25_000L, 2.0, utxos)
assertTrue(result.selectedUtxos.all { it.confirmations > 0 }, "must not select the 0-conf UTXO")
}
@Test
fun buildSplitProducesOneOutputPerRecipientPlusChange() {
// Three distinct recipients derived from low-entropy private keys; not
// for production use but fine for shape assertions.
val r1 =
Secp256k1Instance
.compressedPubKeyFor("000000000000000000000000000000000000000000000000000000000000000b".hexToByteArray())
.copyOfRange(1, 33)
.toHexKey()
val r2 =
Secp256k1Instance
.compressedPubKeyFor("000000000000000000000000000000000000000000000000000000000000000c".hexToByteArray())
.copyOfRange(1, 33)
.toHexKey()
val r3 =
Secp256k1Instance
.compressedPubKeyFor("000000000000000000000000000000000000000000000000000000000000000d".hexToByteArray())
.copyOfRange(1, 33)
.toHexKey()
val utxos = listOf(utxo(1_000_000L, 1))
val result =
OnchainZapBuilder.buildSplit(
senderPubKey = senderPubKey,
recipients = listOf(r1 to 50_000L, r2 to 30_000L, r3 to 20_000L),
feeRateSatPerVByte = 5.0,
availableUtxos = utxos,
)
assertEquals(100_000L, result.recipientSats)
assertTrue(result.changeSats > 0, "should have a change output")
// Conservation: inputs == sum(outputs) + fee.
val inputSum = result.selectedUtxos.sumOf { it.valueSats }
assertEquals(inputSum, result.recipientSats + result.changeSats + result.feeSats)
// 3 recipient outputs + 1 change.
val tx = result.psbt.unsignedTx
assertEquals(4, tx.outputs.size)
assertEquals(50_000L, tx.outputs[0].valueSats)
assertEquals(TaprootAddress.scriptPubKeyHexForRecipient(r1).lowercase(), tx.outputs[0].scriptPubKey.toHexKey())
assertEquals(30_000L, tx.outputs[1].valueSats)
assertEquals(TaprootAddress.scriptPubKeyHexForRecipient(r2).lowercase(), tx.outputs[1].scriptPubKey.toHexKey())
assertEquals(20_000L, tx.outputs[2].valueSats)
assertEquals(TaprootAddress.scriptPubKeyHexForRecipient(r3).lowercase(), tx.outputs[2].scriptPubKey.toHexKey())
// Change is always the last output.
assertEquals(result.changeSats, tx.outputs[3].valueSats)
assertEquals(senderScriptHex, tx.outputs[3].scriptPubKey.toHexKey())
}
@Test
fun buildSplitRejectsDuplicateRecipients() {
val utxos = listOf(utxo(1_000_000L, 1))
val ex =
assertFailsWith<IllegalArgumentException> {
OnchainZapBuilder.buildSplit(
senderPubKey = senderPubKey,
recipients = listOf(recipientPubKey to 10_000L, recipientPubKey to 5_000L),
feeRateSatPerVByte = 5.0,
availableUtxos = utxos,
)
}
assertTrue(ex.message!!.contains("distinct"))
}
@Test
fun buildSplitRejectsBelowDustRecipient() {
val r2 =
Secp256k1Instance
.compressedPubKeyFor("000000000000000000000000000000000000000000000000000000000000000c".hexToByteArray())
.copyOfRange(1, 33)
.toHexKey()
val utxos = listOf(utxo(1_000_000L, 1))
assertFailsWith<IllegalArgumentException> {
OnchainZapBuilder.buildSplit(
senderPubKey = senderPubKey,
recipients = listOf(recipientPubKey to 50_000L, r2 to 100L),
feeRateSatPerVByte = 5.0,
availableUtxos = utxos,
)
}
}
}