Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,26 @@ class ManagedPlatformWallet internal constructor(
val rawTxBytes: ByteArray,
val feeDuffs: Long,
val reservationToken: Long,
/**
* Value in duffs of the sole non-OP_RETURN output of the REGISTERED
* transaction — the one [broadcastSigned] will send.
*
* Computed Rust-side during finalization and carried in the
* registration result, NOT re-derived here from [rawTxBytes]: those
* bytes are a mutable copy the host owns, while the broadcast uses the
* registered transaction referenced by [reservationToken]. Deriving it
* here could report a value the broadcast does not pay.
*
* Needed for a DRAIN ([CoreTransactionBuilder.SelectionStrategy.ALL]),
* where the ENGINE sets this output to `total inputs − fee` and the
* caller therefore never supplied it. A swap must quote from this and
* then broadcast THIS payment, so quote and payment cannot disagree.
*
* 0 when the payment has no single destination (multi-recipient, or an
* OP_RETURN-only build) — read that as "not applicable", not "pays
* nothing".
*/
val deliverableAmountDuffs: Long = 0,
) : AutoCloseable {

// GC backstop: releases the token if it was neither broadcast nor
Expand All @@ -233,6 +253,7 @@ class ManagedPlatformWallet internal constructor(
*/
override fun close() = cleanable.clean()


override fun equals(other: Any?): Boolean =
other is SignedCoreTransaction &&
txidHex == other.txidHex &&
Expand Down Expand Up @@ -263,12 +284,16 @@ class ManagedPlatformWallet internal constructor(
/**
* Decode the big-endian native BLOB the atomic
* finalize-and-register FFI returns: `u64 token, u64 feeDuffs,
* u32 txidLen, txid utf8, u32 txBytesLen, txBytes`.
* u64 deliverableDuffs, u32 txidLen, txid utf8, u32 txBytesLen,
* txBytes`. `deliverableDuffs` is computed from the REGISTERED
* transaction Rust-side (see
* [SignedCoreTransaction.deliverableAmountDuffs]).
*/
internal fun fromRegisterBlob(blob: ByteArray): SignedCoreTransaction {
val buffer = java.nio.ByteBuffer.wrap(blob) // big-endian by default
val token = buffer.long
val feeDuffs = buffer.long
val deliverableDuffs = buffer.long
val txidLen = buffer.int
val txidBytes = ByteArray(txidLen)
buffer.get(txidBytes)
Expand All @@ -280,6 +305,7 @@ class ManagedPlatformWallet internal constructor(
rawTxBytes = rawTxBytes,
feeDuffs = feeDuffs,
reservationToken = token,
deliverableAmountDuffs = deliverableDuffs,
)
}
}
Expand Down Expand Up @@ -346,6 +372,18 @@ class ManagedPlatformWallet internal constructor(
* output indices, as MAYAChain does.
* @param changeToFirstInput route change back to the first selected
* input's address (VIN0) instead of a fresh change address.
* @param selectionStrategy coin-selection strategy, or null to leave the
* builder's default. Pass
* [CoreTransactionBuilder.SelectionStrategy.ALL] to DRAIN the funding
* account: every spendable UTXO is selected, there is no change, and the
* engine sets the single value-carrying output to `total inputs − fee`
* — so the `amount` given in [recipients] is IGNORED (pass 0). A
* zero-value [opReturnData] carrier may accompany the destination (the
* MAYACHAIN "swap my whole balance" case); its bytes are priced into
* the fee. Read what the drain will actually pay from
* [SignedCoreTransaction.deliverableAmountDuffs] BEFORE broadcasting —
* that is the only way to learn the engine-computed amount, and it is
* what a swap quote must be taken from.
*/
suspend fun buildSignedPayment(
recipients: List<Pair<String, Long>>,
Expand All @@ -356,6 +394,7 @@ class ManagedPlatformWallet internal constructor(
opReturnData: ByteArray? = null,
preserveOutputOrder: Boolean = false,
changeToFirstInput: Boolean = false,
selectionStrategy: CoreTransactionBuilder.SelectionStrategy? = null,
): SignedCoreTransaction = gate.opWithCleanupOnCancellation(
// Native finalization mints the token and transfers reservation ownership
// to it before the blocking JNI call returns, so the token already exists
Expand All @@ -369,8 +408,15 @@ class ManagedPlatformWallet internal constructor(
) {
require(accountIndex >= 0) { "accountIndex must be non-negative, got $accountIndex" }
require(recipients.isNotEmpty()) { "recipients must not be empty" }
require(recipients.all { it.second > 0 }) {
"every recipient amount must be positive"
// A DRAIN has the engine set the destination output to
// (total inputs − fee), so the caller's amount is ignored and 0 is the
// honest value to pass. Requiring a positive one here would make
// "send my whole balance" inexpressible through this API — the caller
// would have to invent a placeholder the engine then discards.
val draining = selectionStrategy == CoreTransactionBuilder.SelectionStrategy.ALL
require(draining || recipients.all { it.second > 0 }) {
"every recipient amount must be positive (except under " +
"SelectionStrategy.ALL, where the engine computes it)"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
val builderAccountType = when (accountType) {
AccountType.BIP44 -> CoreTransactionBuilder.AccountType.BIP44
Expand Down Expand Up @@ -400,6 +446,12 @@ class ManagedPlatformWallet internal constructor(
if (changeToFirstInput) {
builder.changeToFirstInput()
}
// Set LAST so it applies to the fully-composed output set: a
// drain (SelectionStrategy.ALL) requires exactly one
// value-carrying output, and the engine rejects the build here
// — before anything is reserved — if the OP_RETURN above
// carries a value or a second spendable output was added.
selectionStrategy?.let { builder.setSelectionStrategy(it) }
Comment on lines 446 to +454

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Memo-bearing drains are rejected by the pinned engine

This builds a destination output plus a zero-value OP_RETURN and then selects ALL, but Cargo.toml and Cargo.lock still pin rust-dashcore to dca5b05b83a9a2d6fd7b650e7cce4356d1c2ec29. At that exact revision, key-wallet's assemble_unsigned uses let [out] = tx_outputs.as_mut_slice() and returns SelectionStrategy::All requires exactly one output (the destination) whenever the memo creates a second output. rust-dashcore PR #928 changes this block to count exactly one non-OP_RETURN value carrier while allowing zero-value OP_RETURN outputs, but this PR changes only the Kotlin/JNI files and does not update the dependency. Therefore the documented MAYACHAIN drain path always fails; pin a revision containing #928 or include equivalent engine support before exposing this behavior.

source: ['codex']

builder.finalizeSignedPayment(
this@ManagedPlatformWallet,
builderAccountType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,18 @@ import java.util.concurrent.atomic.AtomicInteger
*/
class SignedCoreTransactionTest {

private fun registerBlob(token: Long, fee: Long, txid: String, txBytes: ByteArray): ByteArray {
private fun registerBlob(
token: Long,
fee: Long,
txid: String,
txBytes: ByteArray,
deliverable: Long = 0,
): ByteArray {
val txidBytes = txid.toByteArray(Charsets.UTF_8)
val buf = ByteBuffer.allocate(8 + 8 + 4 + txidBytes.size + 4 + txBytes.size)
val buf = ByteBuffer.allocate(8 + 8 + 8 + 4 + txidBytes.size + 4 + txBytes.size)
buf.putLong(token)
buf.putLong(fee)
buf.putLong(deliverable)
buf.putInt(txidBytes.size)
buf.put(txidBytes)
buf.putInt(txBytes.size)
Expand Down Expand Up @@ -73,4 +80,42 @@ class SignedCoreTransactionTest {

assertEquals(1, runs.get())
}

// --- deliverableAmountDuffs -------------------------------------------
//
// Carried in the registration blob, computed Rust-side from the REGISTERED
// transaction. It must NOT be re-derived from rawTxBytes: those are a
// mutable copy the host owns, while the broadcast sends the registered
// transaction referenced by the token.

@Test
fun deliverableAmountComesFromTheBlobNotTheBytes() {
val signed = ManagedPlatformWallet.SignedCoreTransaction.fromRegisterBlob(
registerBlob(token = 7L, fee = 432L, txid = "ab", txBytes = byteArrayOf(9, 9, 9),
deliverable = 27_442_985L)
)
assertEquals(27_442_985L, signed.deliverableAmountDuffs)
}

@Test
fun mutatingRawBytesCannotChangeTheDeliverableAmount() {
// The guarantee the drain quote rests on: what was quoted is what the
// registered transaction pays, whatever happens to the host's copy.
val signed = ManagedPlatformWallet.SignedCoreTransaction.fromRegisterBlob(
registerBlob(token = 1L, fee = 1L, txid = "cd", txBytes = byteArrayOf(1, 2, 3, 4),
deliverable = 500_000L)
)
signed.rawTxBytes.fill(0xFF.toByte())
assertEquals(500_000L, signed.deliverableAmountDuffs)
}

@Test
fun deliverableAmountIsZeroWhenTheEngineReportsNoSingleDestination() {
// Multi-recipient or OP_RETURN-only builds have no single deliverable
// output; Rust reports 0 and the host reads that as "not applicable".
val signed = ManagedPlatformWallet.SignedCoreTransaction.fromRegisterBlob(
registerBlob(token = 2L, fee = 10L, txid = "ef", txBytes = ByteArray(0))
)
assertEquals(0L, signed.deliverableAmountDuffs)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::types::{FFINetwork, Network};
use crate::{check_ptr, unwrap_option_or_return, unwrap_result_or_return};
use dashcore::blockdata::transaction::special_transaction::TransactionPayload;
use dashcore::hashes::Hash;
use dashcore::{Address as DashAddress, OutPoint, Txid};
use dashcore::{Address as DashAddress, OutPoint, TxOut, Txid};
use key_wallet::account::ManagedAccountCollection;
use key_wallet::managed_account::ManagedCoreFundsAccount;
use key_wallet::wallet::managed_wallet_info::coin_selection::SelectionStrategy;
Expand Down Expand Up @@ -215,6 +215,32 @@ pub unsafe extern "C" fn core_wallet_tx_builder_finalize(
/// `core_wallet_transaction_free`). `out_bytes_ptr`/`out_bytes_len` borrow
/// `out_tx`'s buffer — copy them out before freeing `out_tx`.
///
/// Value of the sole non-OP_RETURN output: what a broadcast of this
/// transaction actually pays out.
///
/// Returns 0 when there is no single such output. A multi-recipient build has
/// no one deliverable amount, and an OP_RETURN-only build pays no one — hosts
/// read the 0 as "not applicable" rather than "pays nothing", so the two cases
/// need not be told apart here.
///
/// Output ORDER is deliberately irrelevant: a MAYAChain deposit carries its
/// memo at VOUT1, while other layouts put the data carrier first.
fn sole_deliverable_value(outputs: &[TxOut]) -> u64 {
let mut carriers = outputs.iter().filter(|out| !out.script_pubkey.is_op_return());
match (carriers.next(), carriers.next()) {
(Some(only), None) => only.value,
_ => 0,
}
}

/// Also writes `out_deliverable_duffs`: the value of the sole non-OP_RETURN
/// output of the REGISTERED transaction — what a later broadcast actually
/// pays out. Hosts need it for a drain (`SelectionStrategy::All`), where the
/// engine, not the caller, sets that output to `total inputs - fee`; reading it
/// from the registered transaction here keeps a quote and its payment from
/// disagreeing. Writes 0 when there is no single such output (multi-recipient,
/// or an OP_RETURN-only build) — "not applicable", not "pays nothing".
///
/// # Safety
/// `builder` must be a valid, non-destroyed pointer; `wallet` a valid
/// platform-wallet handle; `core_signer_handle` a valid resolver handle; every
Expand All @@ -234,6 +260,7 @@ pub unsafe extern "C" fn core_wallet_signed_payment_finalize(
out_tx: *mut FFICoreTransaction,
out_bytes_ptr: *mut *const u8,
out_bytes_len: *mut usize,
out_deliverable_duffs: *mut u64,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
) -> PlatformWalletFFIResult {
check_ptr!(builder);
check_ptr!(core_signer_handle);
Expand All @@ -243,6 +270,7 @@ pub unsafe extern "C" fn core_wallet_signed_payment_finalize(
check_ptr!(out_tx);
check_ptr!(out_bytes_ptr);
check_ptr!(out_bytes_len);
check_ptr!(out_deliverable_duffs);
// Publish sentinels into EVERY output before any fallible step (wallet
// resolution, network validation, signing, registration), so an error
// return never leaves caller-supplied garbage in an out param that a host
Expand All @@ -257,6 +285,7 @@ pub unsafe extern "C" fn core_wallet_signed_payment_finalize(
};
*out_bytes_ptr = std::ptr::null();
*out_bytes_len = 0;
*out_deliverable_duffs = 0;

// `finalize_transaction` consumes the builder: reclaim both heap boxes up
// front so they are freed on every return path below.
Expand Down Expand Up @@ -339,6 +368,17 @@ pub unsafe extern "C" fn core_wallet_signed_payment_finalize(
}
};

// The deliverable amount, taken from the transaction that will actually be
// broadcast — not re-derived by the host from a copy of the bytes. Under a
// drain the ENGINE sets this output (total inputs - fee), so the caller
// never supplied it and has no other authoritative source; a host that
// re-parsed its own byte array could quote a value the broadcast does not
// pay. Defined only for a single-destination payment: exactly one output
// that is not an OP_RETURN data carrier. Anything else reports 0, which the
// host reads as "not applicable" rather than "pays nothing".
let deliverable_duffs = sole_deliverable_value(&finalized.transaction().output);
unsafe { *out_deliverable_duffs = deliverable_duffs };
Comment thread
coderabbitai[bot] marked this conversation as resolved.

let serialized = dashcore::consensus::serialize(finalized.transaction());
let len = serialized.len();

Expand Down Expand Up @@ -820,3 +860,83 @@ pub unsafe extern "C" fn core_wallet_transaction_free(tx: *mut FFICoreTransactio
tx.tx_bytes = std::ptr::null_mut();
tx.tx_len = 0;
}


#[cfg(test)]
mod tests {
use super::sole_deliverable_value;
use dashcore::blockdata::script::ScriptBuf;
use dashcore::TxOut;

/// A spendable output. The script only has to NOT be an OP_RETURN.
fn destination(value: u64) -> TxOut {
TxOut {
value,
script_pubkey: ScriptBuf::from(vec![0x76, 0xa9, 0x14]),
}
}

fn op_return(payload: &[u8]) -> TxOut {
let data = dashcore::script::PushBytesBuf::try_from(payload.to_vec())
.expect("test payload is within push limits");
TxOut {
value: 0,
script_pubkey: ScriptBuf::new_op_return(&data),
}
}

#[test]
fn a_lone_destination_is_the_deliverable_amount() {
assert_eq!(sole_deliverable_value(&[destination(27_442_985)]), 27_442_985);
}

/// The MAYAChain shape: vault output plus a zero-value memo. The memo must
/// not be mistaken for a second recipient, in EITHER order — Maya puts the
/// memo at VOUT1, but nothing in the calculation may depend on that.
#[test]
fn a_data_carrier_beside_the_destination_is_ignored_in_both_orders() {
let memo = op_return(b"=:MAYA.CACAO:maya1abc");
assert_eq!(
sole_deliverable_value(&[destination(27_442_985), memo.clone()]),
27_442_985,
"memo after the destination (the Maya layout)"
);
assert_eq!(
sole_deliverable_value(&[memo, destination(27_442_985)]),
27_442_985,
"memo before the destination"
);
}

/// Two recipients have no single deliverable amount. Reporting either one
/// would let a host quote a number the payment does not pay.
#[test]
fn two_spendable_outputs_report_zero() {
assert_eq!(sole_deliverable_value(&[destination(1_000), destination(2_000)]), 0);
}

#[test]
fn two_spendable_outputs_report_zero_even_beside_a_data_carrier() {
assert_eq!(
sole_deliverable_value(&[destination(1_000), op_return(b"x"), destination(2_000)]),
0
);
}

/// An OP_RETURN-only build pays no one; so does an empty output set.
#[test]
fn a_transaction_with_no_spendable_output_reports_zero() {
assert_eq!(sole_deliverable_value(&[op_return(b"data only")]), 0);
assert_eq!(sole_deliverable_value(&[]), 0);
}

/// An asset lock's single output IS an OP_RETURN, so it reports 0 rather
/// than its burn value. That is the intended reading: the credits go to an
/// identity, not to a payee a host would quote.
#[test]
fn an_op_return_carrying_value_still_reports_zero() {
let mut burn = op_return(b"credits");
burn.value = 500_000;
assert_eq!(sole_deliverable_value(&[burn]), 0);
}
}
Loading
Loading