Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
7 changes: 7 additions & 0 deletions .github/workflows/tests-rs-workspace.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ jobs:
with:
clean: false

- name: Fail on local rust-dashcore patch override
run: |
if grep -q '^\[patch\."https://github.com/dashpay/rust-dashcore"\]' Cargo.toml; then
echo "::error::Remove the local rust-dashcore [patch] override before merging"
exit 1
fi

# target/llvm-cov-target is deliberately NOT pruned here: it holds the
# coverage-instrumented build (~10GB) that makes the test step's compile
# incremental (~2.5 min saved per run). The size/free-disk guard below
Expand Down
22 changes: 22 additions & 0 deletions packages/rs-platform-wallet-ffi/src/core_wallet/broadcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,28 @@ pub unsafe extern "C" fn core_wallet_signed_transaction_v2_fee(
PlatformWalletFFIResult::ok()
}

#[no_mangle]
pub unsafe extern "C" fn core_wallet_signed_transaction_v2_bytes(
transaction_handle: Handle,
out_bytes: *mut *mut u8,
out_len: *mut usize,
) -> PlatformWalletFFIResult {
check_ptr!(out_bytes);
check_ptr!(out_len);
*out_bytes = std::ptr::null_mut();
*out_len = 0;

let bytes = unwrap_option_or_return!(CORE_SIGNED_TRANSACTION_V2_STORAGE
.with_item(transaction_handle, |tx| dashcore::consensus::serialize(
tx.transaction.transaction()
)));
let len = bytes.len();
let boxed = bytes.into_boxed_slice();
*out_bytes = Box::into_raw(boxed) as *mut u8;
*out_len = len;
PlatformWalletFFIResult::ok()
Comment on lines +176 to +195

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.

💬 Nitpick: Declare ownership of serialized byte buffers in the C ABI

core_wallet_signed_transaction_v2_bytes returns an allocation created with Box::into_raw, but its cbindgen-visible documentation does not state that the caller takes ownership or must release the pointer using platform_wallet_bytes_free with the returned length. The current Swift wrapper correctly copies and frees it, but direct C and future binding consumers cannot infer the allocator/deallocator contract and may leak the buffer or incorrectly call the platform allocator's free().

Suggested change
#[no_mangle]
pub unsafe extern "C" fn core_wallet_signed_transaction_v2_bytes(
transaction_handle: Handle,
out_bytes: *mut *mut u8,
out_len: *mut usize,
) -> PlatformWalletFFIResult {
check_ptr!(out_bytes);
check_ptr!(out_len);
*out_bytes = std::ptr::null_mut();
*out_len = 0;
let bytes = unwrap_option_or_return!(CORE_SIGNED_TRANSACTION_V2_STORAGE
.with_item(transaction_handle, |tx| dashcore::consensus::serialize(
tx.transaction.transaction()
)));
let len = bytes.len();
let boxed = bytes.into_boxed_slice();
*out_bytes = Box::into_raw(boxed) as *mut u8;
*out_len = len;
PlatformWalletFFIResult::ok()
/// Copy the consensus-serialized finalized transaction into a newly allocated buffer.
///
/// On success, `out_bytes` receives a Rust-allocated buffer of `out_len` bytes.
/// The caller takes ownership and must release it exactly once with
/// `platform_wallet_bytes_free(*out_bytes, *out_len)`. Errors leave the outputs
/// initialized to null and zero.
///
/// # Safety
/// `out_bytes` and `out_len` must be valid writable pointers.
#[no_mangle]
pub unsafe extern "C" fn core_wallet_signed_transaction_v2_bytes(
transaction_handle: Handle,
out_bytes: *mut *mut u8,
out_len: *mut usize,
) -> PlatformWalletFFIResult {

source: ['codex']

}

/// Broadcast a transaction built by `core_wallet_tx_builder_build_signed`.
///
/// `account_type`/`account_index` identify the funding account handed to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait;
use key_wallet::managed_account::ManagedCoreFundsAccount;
use key_wallet::wallet::managed_wallet_info::coin_selection::SelectionStrategy;
use key_wallet::wallet::managed_wallet_info::fee::FeeRate;
use key_wallet::wallet::managed_wallet_info::transaction_builder::TransactionBuilder;
use key_wallet::wallet::managed_wallet_info::transaction_builder::{
TransactionBuilder, DEFAULT_MAX_OP_RETURN_BYTES,
};
use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference;
use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface;
use rs_sdk_ffi::{MnemonicResolverCoreSigner, MnemonicResolverHandle};
Expand Down Expand Up @@ -508,6 +510,57 @@ pub unsafe extern "C" fn core_wallet_tx_builder_add_output(
PlatformWalletFFIResult::ok()
}

/// Add a zero-value OP_RETURN output carrying `data`.
///
/// # Safety
/// `builder` must be a valid, non-destroyed pointer; `data` must reference a
/// readable buffer of `data_len` bytes when `data_len > 0`.
#[no_mangle]
pub unsafe extern "C" fn core_wallet_tx_builder_add_op_return(
builder: *mut FFITransactionBuilder,
data: *const u8,
data_len: usize,
) -> PlatformWalletFFIResult {
check_ptr!(builder);
if data_len > 0 {
check_ptr!(data);
}

let bytes = if data_len == 0 {
&[]
} else {
std::slice::from_raw_parts(data, data_len)
};

// `add_op_return` takes the builder by value, so a rejected payload drops it and leaves
// `take_builder`'s `mem::take` default behind — silently discarding outputs and options
// the caller already configured. Reject an over-long payload *before* taking the builder
// so the slot keeps its real state. `add_op_return` re-checks; this is the same policy
// constant, not a second opinion.
if data_len > DEFAULT_MAX_OP_RETURN_BYTES {
return PlatformWalletFFIResult::err(
PlatformWalletFFIResultCode::ErrorInvalidParameter,
format!(
"OP_RETURN payload too large: {data_len} bytes (max {DEFAULT_MAX_OP_RETURN_BYTES})"
),
);
}

let b = (*builder).take_builder();
let b = match b.add_op_return(bytes) {
Ok(b) => b,
Err(err) => {
return PlatformWalletFFIResult::err(
PlatformWalletFFIResultCode::ErrorWalletOperation,
err.to_string(),
);
}
};
(*builder).store_builder(b);

PlatformWalletFFIResult::ok()
}

/// # Safety
/// `builder` must be a valid, non-destroyed pointer; `address` a valid NUL-terminated C string.
#[no_mangle]
Expand Down Expand Up @@ -538,6 +591,40 @@ pub unsafe extern "C" fn core_wallet_tx_builder_set_change_address(
PlatformWalletFFIResult::ok()
}

/// Preserve outputs in the order they were added instead of applying BIP-69 sorting.
///
/// # Safety
/// `builder` must be a valid, non-destroyed pointer.
#[no_mangle]
pub unsafe extern "C" fn core_wallet_tx_builder_preserve_output_order(
builder: *mut FFITransactionBuilder,
) -> PlatformWalletFFIResult {
check_ptr!(builder);

let b = (*builder).take_builder();
let b = b.preserve_output_order();
(*builder).store_builder(b);

PlatformWalletFFIResult::ok()
}

/// Route change to the address of the first selected input (VIN0).
///
/// # Safety
/// `builder` must be a valid, non-destroyed pointer.
#[no_mangle]
pub unsafe extern "C" fn core_wallet_tx_builder_change_to_first_input(
builder: *mut FFITransactionBuilder,
) -> PlatformWalletFFIResult {
check_ptr!(builder);

let b = (*builder).take_builder();
let b = b.change_to_first_input();
(*builder).store_builder(b);

PlatformWalletFFIResult::ok()
}

/// # Safety
/// `builder` must be a valid, non-destroyed pointer.
#[no_mangle]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,26 @@ public final class FinalizedCoreTransaction {
}

func takeForAbandon() throws -> Handle { try takeForBroadcast() }

/// Consensus-serialized signed transaction bytes (copied out) without
/// consuming the ownership token.
public func serializedData() throws -> Data {
guard nativeHandle != 0 else {
throw PlatformWalletError.unknown("FinalizedCoreTransaction already consumed")
}

var bytesPtr: UnsafeMutablePointer<UInt8>? = nil
var bytesLen: UInt = 0
try core_wallet_signed_transaction_v2_bytes(nativeHandle, &bytesPtr, &bytesLen).check()

guard let bytesPtr, bytesLen > 0 else {
throw PlatformWalletError.unknown(
"FFI returned success but finalized transaction bytes were empty"
)
}
defer { platform_wallet_bytes_free(bytesPtr, bytesLen) }
return Data(bytes: bytesPtr, count: Int(bytesLen))
}
}

/// A built, signed Core transaction whose funding UTXOs are reserved, awaiting
Expand Down Expand Up @@ -266,6 +286,20 @@ public final class CoreTransactionBuilder {
return self
}

/// Add a zero-value OP_RETURN output carrying `data` for a MAYACHAIN-style
/// deposit. See https://docs.mayaprotocol.com/mayachain-dev-docs/concepts/sending-transactions
@discardableResult
public func addOpReturn(_ data: Data) throws -> CoreTransactionBuilder {
try data.withUnsafeBytes { buf in
try core_wallet_tx_builder_add_op_return(
handle,
buf.baseAddress?.assumingMemoryBound(to: UInt8.self),
UInt(data.count)
).check()
}
return self
}

@discardableResult
public func setChangeAddress(_ address: String) throws -> CoreTransactionBuilder {
let c = strdup(address)
Expand All @@ -274,6 +308,22 @@ public final class CoreTransactionBuilder {
return self
}

/// Preserve outputs in insertion order for a MAYACHAIN-style deposit.
/// See https://docs.mayaprotocol.com/mayachain-dev-docs/concepts/sending-transactions
@discardableResult
public func preserveOutputOrder() throws -> CoreTransactionBuilder {
try core_wallet_tx_builder_preserve_output_order(handle).check()
return self
}

/// Route change to the first selected input address (VIN0) for a MAYACHAIN-style deposit.
/// See https://docs.mayaprotocol.com/mayachain-dev-docs/concepts/sending-transactions
@discardableResult
public func changeToFirstInput() throws -> CoreTransactionBuilder {
try core_wallet_tx_builder_change_to_first_input(handle).check()
return self
Comment on lines 292 to +324

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: Guard the new Swift setters after Rust consumes the builder

finalizeAtomic, buildSigned, and finalizeSignedPayment set consumed after Rust reclaims the FFITransactionBuilder and its inner builder with Box::from_raw, but the new addOpReturn, preserveOutputOrder, and changeToFirstInput methods do not check that state. Calling one of these public Swift methods after any consuming finalizer passes the retained non-null but dangling handle through the C ABI; check_ptr! only rejects null and the FFI then dereferences freed memory. Add the same consumed guard used by the finalizers before each new method crosses the FFI boundary.

source: ['codex']

}

@discardableResult
public func setFeeRate(satPerKb: UInt64) throws -> CoreTransactionBuilder {
try core_wallet_tx_builder_set_fee_rate(handle, satPerKb).check()
Expand Down
Loading
Loading