Skip to content
Merged
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
24 changes: 12 additions & 12 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 8 additions & 8 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ members = [
]

[workspace.dependencies]
dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "4db5c36701b8f38c4aea704badb81e3103ed701d" }
dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "4db5c36701b8f38c4aea704badb81e3103ed701d" }
dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "4db5c36701b8f38c4aea704badb81e3103ed701d" }
key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "4db5c36701b8f38c4aea704badb81e3103ed701d" }
key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "4db5c36701b8f38c4aea704badb81e3103ed701d" }
key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "4db5c36701b8f38c4aea704badb81e3103ed701d" }
dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "4db5c36701b8f38c4aea704badb81e3103ed701d" }
dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "4db5c36701b8f38c4aea704badb81e3103ed701d" }
dashcore = { git = "https://github.com/dashpay/rust-dashcore", rev = "393b612269c158925451235a5d9c0ffa5e2eeed2" }
dash-network-seeds = { git = "https://github.com/dashpay/rust-dashcore", rev = "393b612269c158925451235a5d9c0ffa5e2eeed2" }
dash-spv = { git = "https://github.com/dashpay/rust-dashcore", rev = "393b612269c158925451235a5d9c0ffa5e2eeed2" }
key-wallet = { git = "https://github.com/dashpay/rust-dashcore", rev = "393b612269c158925451235a5d9c0ffa5e2eeed2" }
key-wallet-ffi = { git = "https://github.com/dashpay/rust-dashcore", rev = "393b612269c158925451235a5d9c0ffa5e2eeed2" }
key-wallet-manager = { git = "https://github.com/dashpay/rust-dashcore", rev = "393b612269c158925451235a5d9c0ffa5e2eeed2" }
dash-network = { git = "https://github.com/dashpay/rust-dashcore", rev = "393b612269c158925451235a5d9c0ffa5e2eeed2" }
dashcore-rpc = { git = "https://github.com/dashpay/rust-dashcore", rev = "393b612269c158925451235a5d9c0ffa5e2eeed2" }

tokio-metrics = "0.5"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ use std::str::FromStr;
pub struct FFITransactionBuilder {
inner: *mut c_void,
network: FFINetwork,
/// Set by `core_wallet_tx_builder_use_only_added_inputs`. key-wallet takes
/// this per funding call, which the finalizers make internally, so the
/// intent has to be carried here and read when they run.
reservation_only: bool,
}

/// Owned signed-transaction bytes handed across the C ABI as the `out_tx`
Expand Down Expand Up @@ -142,11 +146,13 @@ pub unsafe extern "C" fn core_wallet_tx_builder_finalize(

let signer =
MnemonicResolverCoreSigner::new(core_signer_handle, wallet.wallet_id(), wallet.network());
let finalized = runtime().block_on(wallet.core().finalize_transaction(
let reservation_only = (*builder).reservation_only;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This reads through builder after Box::from_raw(builder) on line 134 handed the allocation to ffi. Not a use-after-free — the box outlives the read — but it is an aliasing violation: Box is noalias, so under Stacked Borrows the raw-pointer read invalidates ffi's tag, and the deallocation when ffi drops at the end of the function then runs on an invalidated tag. Miri flags this shape.

It is also gratuitous, since the correct source is already in scope and used two lines up (ffi.network):

let reservation_only = ffi.reservation_only;

core_wallet_signed_payment_finalize reads everything off ffi and never touches builder after the reclaim — worth matching.

let finalized = runtime().block_on(wallet.core().finalize_transaction_with_options(
inner,
account_type.funding_sources(),
account_index,
&signer,
reservation_only,
));
let finalized = unwrap_result_or_return!(finalized);

Expand Down Expand Up @@ -451,7 +457,11 @@ pub unsafe extern "C" fn core_wallet_tx_builder_new(
network: FFINetwork,
) -> *mut FFITransactionBuilder {
let inner = Box::into_raw(Box::new(TransactionBuilder::new())) as *mut c_void;
Box::into_raw(Box::new(FFITransactionBuilder { inner, network }))
Box::into_raw(Box::new(FFITransactionBuilder {
inner,
network,
reservation_only: false,
}))
}

/// # Safety
Expand Down Expand Up @@ -616,6 +626,26 @@ pub unsafe extern "C" fn core_wallet_tx_builder_set_fee_rate(
PlatformWalletFFIResult::ok()
}

/// Fund the build from the inputs `core_wallet_tx_builder_add_inputs_from_outpoints`
/// supplied, and nothing else.
///
/// Without this, the wallet-aware finalizers offer every unreserved UTXO of the
/// funding account alongside the seeded ones, so seeding a subset does not
/// restrict what gets selected. A caller draining an account in batches that
/// each stay under the standard-transaction input limit needs this, or every
/// batch sees the whole account and fails with a too-many-inputs error.
///
/// # Safety
/// `builder` must be a valid, non-destroyed pointer.
#[no_mangle]
pub unsafe extern "C" fn core_wallet_tx_builder_use_only_added_inputs(
builder: *mut FFITransactionBuilder,
) -> PlatformWalletFFIResult {
check_ptr!(builder);
(*builder).reservation_only = true;
PlatformWalletFFIResult::ok()
}

/// # Safety
/// `builder` must be a valid, non-destroyed pointer.
#[no_mangle]
Expand Down
27 changes: 26 additions & 1 deletion packages/rs-platform-wallet/src/wallet/core/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,26 @@ impl<B: TransactionBroadcaster + ?Sized> CoreWallet<B> {
/// Consume a configured builder, atomically fund and reserve its selected
/// inputs, then sign without holding the wallet-manager lock.
pub async fn finalize_transaction<S: TransactionSigner + ?Sized + Sync>(
&self,
builder: TransactionBuilder,
sources: &[AccountTypePreference],
source_index: u32,
signer: &S,
) -> Result<SignedCoreTransaction, PlatformWalletError> {
self.finalize_transaction_with_options(builder, sources, source_index, signer, false)
.await
}

/// `reservation_only` funds through
/// [`TransactionBuilder::add_funding_reservation_only`]: the sources take on
/// their reservation bookkeeping but offer no candidates, so the build spends
/// only the inputs already seeded on the builder.
///
/// A caller draining an account in batches under the standard-transaction
/// input limit needs it — ordinary funding offers the whole account on top of
/// the batch, so every batch trips the cap and an account above it can never
/// be drained.
pub async fn finalize_transaction_with_options<S: TransactionSigner + ?Sized + Sync>(
&self,
builder: TransactionBuilder,
// The funding sources to POOL, in order — the first supplies the
Expand All @@ -311,6 +331,7 @@ impl<B: TransactionBroadcaster + ?Sized> CoreWallet<B> {
sources: &[AccountTypePreference],
source_index: u32,
signer: &S,
reservation_only: bool,
) -> Result<SignedCoreTransaction, PlatformWalletError> {
let primary = *sources.first().ok_or_else(|| {
PlatformWalletError::TransactionBuild("no funding sources named".into())
Expand Down Expand Up @@ -375,7 +396,11 @@ impl<B: TransactionBroadcaster + ?Sized> CoreWallet<B> {
paths.insert(utxo.address.clone(), path);
}
}
builder = builder.add_funding(managed, account);
builder = if reservation_only {
builder.add_funding_reservation_only(managed, account)
} else {
builder.add_funding(managed, account)
};
offered_accounts.push(at);
}
// A strict single-source SET selector (a DashPay preference
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,19 @@ public final class CoreTransactionBuilder {
return self
}

/// Fund the build from the inputs `addInputs` supplied, and nothing else.
///
/// Without this, `finalizeAtomic` adds every unreserved UTXO of the funding
/// account to the candidate pool, so seeding a subset does not restrict what
/// gets selected. A caller draining an account in batches that each stay
/// under the standard-transaction input limit needs this, or every batch
/// sees the whole account and fails with a too-many-inputs error.
@discardableResult
public func useOnlyAddedInputs() throws -> CoreTransactionBuilder {
try core_wallet_tx_builder_use_only_added_inputs(handle).check()
return self
}

@discardableResult
public func setCurrentHeight(_ height: UInt32) throws -> CoreTransactionBuilder {
try core_wallet_tx_builder_set_current_height(handle, height).check()
Expand Down
Loading