Skip to content
Open
Show file tree
Hide file tree
Changes from all 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 @@ -265,13 +265,20 @@ sealed class DashSdkError(
PlatformWallet(message, cause)

/**
* `ErrorStaleReservationToken` (native code 34). A deferred
* (BIP70/BIP270) [broadcastSigned][org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.broadcastSigned]
* token has outlived its funding reservation's lifetime: key-wallet's
* TTL may already have swept and re-selected the inputs, so acting on it
* could touch a newer, unrelated reservation. The call did NOT touch the
* network. NOT retryable in place — rebuild the payment with
* [buildSignedPayment][org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.buildSignedPayment].
* `ErrorStaleReservationToken` (native code 34). A payment's funding
* reservation has outlived its lifetime: key-wallet's TTL may already
* have swept and re-selected the inputs, so acting on it could touch a
* newer, unrelated reservation. The call did NOT touch the network.
* NOT retryable in place — rebuild the payment.
*
* The code is shared by BOTH deferred-payment surfaces (the messages
* distinguish them): a deferred (BIP70/BIP270)
* [broadcastSigned][org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.broadcastSigned]
* token, rebuilt with
* [buildSignedPayment][org.dashfoundation.dashsdk.wallet.ManagedPlatformWallet.buildSignedPayment];
* and a token-less V2 finalized handle whose
* [broadcastTransaction][org.dashfoundation.dashsdk.wallet.ManagedCoreWallet.broadcastTransaction]
* aged past the same reservation bound (abandon still works at any age).
*
* Sibling of the other two deferred-token failures this code used to
* conflate: [ReservationTokenConsumed] (unknown / already broadcast /
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,34 @@ class ManagedCoreWallet internal constructor(handle: Long) : AutoCloseable {
tx.accountIndex,
)

/** Consume and broadcast a V2 finalized transaction. */
/**
* Consume and broadcast a V2 finalized transaction. A handle held past the
* reservation age bound throws the typed
* [StaleReservationToken][org.dashfoundation.dashsdk.errors.DashSdkError.PlatformWallet.StaleReservationToken]
* (native code 34, shared with the deferred-token surface) instead of
* broadcasting against inputs key-wallet's TTL may have re-selected.
*
* On that refusal the handle has **already been consumed** by this call, so
* a follow-up [abandonTransaction] is an invalid-handle error, not a recovery
* path — there is nothing left to release, and the aged reservation is left
* for key-wallet's TTL to reclaim (releasing it by outpoint could free a
* newer build's reservation). Recover by rebuilding the transaction.
*/
fun broadcastTransaction(tx: FinalizedCoreTransaction): String =
WalletManagerNative.coreWalletBroadcastSignedTransactionV2(
handle,
tx.takeForBroadcast(),
)

/** Consume without sending and release the selected inputs immediately. */
/**
* Consume a finalized transaction without sending. Below the reservation age
* bound this releases the selected inputs immediately so a rebuild can
* reselect them. If the handle has aged past the bound the by-outpoint
* release is skipped — key-wallet's TTL may already have swept and
* re-reserved the outpoint, so releasing it could free a newer build's
* reservation — and the aged reservation is left for the TTL to reclaim; the
* handle is torn down either way.
*/
fun abandonTransaction(tx: FinalizedCoreTransaction) {
WalletManagerNative.coreWalletAbandonSignedTransactionV2(
handle,
Expand Down
71 changes: 71 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 @@ -327,6 +327,26 @@ mod tests {
runtime().block_on(core.abandon_transaction(&retry));
}

/// Prove the funding reservation was released owner-guarded: a fresh
/// finalize of the same size reselects the single fixture UTXO. An aged
/// abandon/free with the build's owner token present releases via
/// `release_reservation_if_owner` (safe at any age — no-op once ownership
/// transferred), so the input must be immediately reselectable.
fn assert_released_for_rebuild(core: &TestCore, signer: &WalletSigner, tag: u8) {
let rebuild = runtime().block_on(core.finalize_transaction(
TransactionBuilder::new().add_output(
&Address::dummy(Network::Testnet, usize::from(tag)),
1_000_000,
),
AccountTypePreference::BIP44,
0,
signer,
));
let rebuilt = rebuild
.expect("aged abandon/free must release the still-owned reservation for a rebuild");
runtime().block_on(core.abandon_transaction(&rebuilt));
}

#[test]
fn double_free_is_safe_and_releases_reservation() {
let (core, signer) =
Expand Down Expand Up @@ -366,6 +386,57 @@ mod tests {
CORE_WALLET_STORAGE.remove(other_handle);
}

/// The deinit/GC backstop (`core_wallet_signed_transaction_v2_free`) is the
/// exact path shumkov flagged: a `FinalizedCoreTransaction` never broadcast
/// or abandoned, freed by the host GC long after finalize. If the reservation
/// has aged past the guard bound the free must **skip** the by-outpoint
/// release — key-wallet's TTL may already have swept and re-reserved the
/// outpoint, and releasing it would free that newer build's reservation. The
/// handle is still torn down (the storage entry is removed) so a re-free is a
/// safe no-op.
#[test]
fn aged_v2_free_releases_owner_guarded() {
let (core, signer) =
runtime().block_on(funded_spv_core_wallet(StandardAccountType::BIP44Account));
let transaction_handle = insert(&core, finalize(&core, &signer, 48));

// Age the pinned handle past the guard bound (still below the TTL, so the
// reservation is provably still held — only the software guard trips).
runtime().block_on(platform_wallet::test_support::age_core_past_reservation_guard(&core));

core_wallet_signed_transaction_v2_free(transaction_handle);

// The aged free released owner-guarded: the input is reselectable.
assert_released_for_rebuild(&core, &signer, 49);
// Handle is gone regardless — a re-free is a harmless no-op.
core_wallet_signed_transaction_v2_free(transaction_handle);
}

/// The FFI broadcast/abandon *failure* paths (invalid or wrong-generation
/// wallet handle) route their cleanup through `abandon_transaction`, so they
/// inherit the same policy: an aged handle with the build's owner token
/// still releases owner-guarded (safe at any age), so the failure-path
/// cleanup frees the still-owned input instead of stranding it.
#[test]
fn aged_failure_path_abandon_releases_owner_guarded() {
let (origin, signer) =
runtime().block_on(funded_spv_core_wallet(StandardAccountType::BIP44Account));
let transaction_handle = insert(&origin, finalize(&origin, &signer, 50));

runtime().block_on(platform_wallet::test_support::age_core_past_reservation_guard(&origin));

// Invalid wallet handle → routes through abandon_transaction, then returns
// ErrorInvalidHandle. The embedded aged reservation is released
// owner-guarded on the way out.
let invalid =
unsafe { core_wallet_abandon_signed_transaction_v2(u64::MAX, transaction_handle) };
assert_eq!(
invalid.code,
PlatformWalletFFIResultCode::ErrorInvalidHandle
);
assert_released_for_rebuild(&origin, &signer, 51);
}

#[test]
fn abandon_then_free_or_broadcast_cannot_reconsume_handle() {
let (core, signer) =
Expand Down
50 changes: 50 additions & 0 deletions packages/rs-platform-wallet-ffi/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,20 @@ pub enum PlatformWalletFFIResultCode {
/// [`Self::ErrorReservationWalletMismatch`] (36, minted against a different
/// wallet generation). All three are non-retryable-in-place and none touched
/// the network; they are distinct codes so a host can message each precisely.
///
/// Also maps `PlatformWalletError::StaleReservation` from the atomic V2
/// finalized-transaction handle path
/// (`core_wallet_broadcast_signed_transaction_v2`): a pinned handle whose
/// funding reservation aged past the SAME `RESERVATION_MAX_AGE_BLOCKS` bound
/// carries the identical "may already have been swept — rebuild" meaning, so
/// the two surfaces intentionally share this one code. The V2 handle carries
/// no numeric reservation token, hence a distinct (token-less) wallet-error
/// variant behind the same FFI code. Abandon/free of a V2 handle never
/// surfaces this — abandon returns no result code, and past the age bound it
/// deliberately skips the by-outpoint release (dropping only the handle and
/// leaving the aged outpoint to key-wallet's TTL) precisely because
/// releasing an aged reservation could free an unrelated newer build's
/// reservation.
ErrorStaleReservationToken = 34,

/// Maps `SignedPaymentError::StaleToken`. The deferred reservation token is
Expand Down Expand Up @@ -442,6 +456,14 @@ impl From<PlatformWalletError> for PlatformWalletFFIResult {
PlatformWalletError::TransactionBroadcast(..) => {
PlatformWalletFFIResultCode::ErrorTransactionBroadcastRejected
}
// The V2 finalized-transaction handle path's age guard. Shares the
// `ErrorStaleReservationToken` code with the deferred registry-token
// sibling (`SignedPaymentError::StaleReservationToken`): both mean
// "the funding reservation may already have been swept — rebuild",
// and neither touched the network. See the code's doc note.
PlatformWalletError::StaleReservation => {
PlatformWalletFFIResultCode::ErrorStaleReservationToken
}
// A definitively-failed address-nonce race (reaches the blanket impl
// via identity `top_up_from_addresses` → `?`/`.into()`). Exposing
// provided/expected nonce as structured out-fields is INTENTIONALLY
Expand Down Expand Up @@ -1040,6 +1062,34 @@ mod tests {
assert_eq!(msg, rendered, "Display payload must survive verbatim");
}

/// The V2 finalized-transaction handle age guard
/// (`core_wallet_broadcast_signed_transaction_v2` → `broadcast_finalized_transaction`)
/// surfaces `PlatformWalletError::StaleReservation` through the blanket
/// `From` impl, which must reuse the deferred registry-token path's
/// `ErrorStaleReservationToken` (34) code rather than flattening to
/// `ErrorUnknown` — the two surfaces share the "reservation may have been
/// swept; rebuild" meaning and this one code. The typed Display rendering
/// survives across the boundary as the message.
#[test]
fn stale_reservation_maps_to_shared_stale_reservation_code() {
let err = PlatformWalletError::StaleReservation;
let rendered = err.to_string();
let result: PlatformWalletFFIResult = err.into();
assert_eq!(
result.code,
PlatformWalletFFIResultCode::ErrorStaleReservationToken,
"StaleReservation must reuse the registry-token stale code (rendered: {rendered})"
);
assert!(!result.message.is_null());
let msg = unsafe { std::ffi::CStr::from_ptr(result.message) }
.to_string_lossy()
.into_owned();
assert_eq!(
msg, rendered,
"Display payload must survive the FFI boundary verbatim"
);
}

/// `AddressNonceMismatch` maps to the dedicated `ErrorAddressNonceMismatch`
/// FFI code through the blanket `From` impl (the path identity
/// `top_up_from_addresses` takes via `?`/`.into()`) rather than flattening
Expand Down
25 changes: 25 additions & 0 deletions packages/rs-platform-wallet/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,31 @@ pub enum PlatformWalletError {
)]
TransactionBroadcastUnconfirmed(String),

/// A finalized V2 transaction handle
/// (`core_wallet_tx_builder_finalize` → `broadcast_finalized_transaction`)
/// was held long enough that its funding reservation may already have been
/// swept and re-selected by key-wallet's TTL: the wallet's
/// `last_processed_height` advanced at least
/// [`RESERVATION_MAX_AGE_BLOCKS`](crate::wallet::reservations::RESERVATION_MAX_AGE_BLOCKS)
/// blocks past the height the reservation was stamped at
/// ([`SignedCoreTransaction::reservation_height`](crate::SignedCoreTransaction::reservation_height)).
/// Broadcasting it could spend against a newer, unrelated reservation, so it
/// is refused **before** touching the network — NOT retryable in place, the
/// caller must rebuild the payment. Abandoning/freeing the handle stays
/// allowed at any age, but past the bound `abandon_transaction` drops only
/// the handle and deliberately **skips** the by-outpoint reservation
/// release: the outpoint may already have been swept and re-reserved by an
/// unrelated build, so releasing it could free that newer reservation. The
/// aged outpoint is left for key-wallet's TTL to reclaim.
Comment on lines +101 to +108

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.

🟡 Suggestion: StaleReservation docs describe the old abandon behavior

These comments say aged abandon/free always skips reservation release, but CoreWallet::abandon_transaction now skips only for token-less transactions. A normal funded finalized handle carries an owner token and attempts release_reservation_if_owner at every age, releasing inputs only while this build still owns them and safely doing nothing after ownership transfers. The same obsolete policy appears in wallet/reservations.rs:57-68, wallet/signed_payment_registry.rs:163-168, test_support.rs:364-366, wallet/core/broadcast.rs:403-406, rs-platform-wallet-ffi/src/error.rs:269-281, the FFI test comment at rs-platform-wallet-ffi/src/core_wallet/broadcast.rs:389-396, and Kotlin's ManagedCoreWallet.kt:64-71. Update these mirrors to distinguish owner-guarded cleanup from the token-less by-outpoint fallback.

source: ['codex', 'coderabbit']

///
/// This is the V2 handle-path sibling of the deferred registry-token
/// [`SignedPaymentError::StaleReservationToken`](crate::SignedPaymentError::StaleReservationToken);
/// both share the same age bound and the FFI `ErrorStaleReservationToken`
/// code. Carries no token — the handle path is keyed by an opaque handle,
/// not a numeric reservation token.
#[error("finalized transaction reservation has outlived its lifetime; rebuild the payment")]
StaleReservation,

#[error("Transaction building failed: {0}")]
TransactionBuild(String),

Expand Down
31 changes: 31 additions & 0 deletions packages/rs-platform-wallet/src/test_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,37 @@ pub async fn funded_spv_core_wallet(
)
}

/// Advance `core`'s `last_processed_height` to just past the reservation age
/// guard bound ([`RESERVATION_MAX_AGE_BLOCKS`](crate::wallet::reservations::RESERVATION_MAX_AGE_BLOCKS))
/// but below key-wallet's `ReservationSet` TTL, so a handle finalized at the
/// current height ages enough to trip the software guard while its underlying
/// reservation is provably still held (no key-wallet sweep yet). Returns the new
/// height.
///
/// FFI lifecycle tests use this to exercise the aged abandon/free skip-release
/// path — the deinit/GC backstop and the broadcast/abandon failure paths that
/// route their cleanup through `abandon_transaction`.
pub async fn age_core_past_reservation_guard<B>(core: &crate::CoreWallet<B>) -> u32
where
B: crate::broadcaster::TransactionBroadcaster + ?Sized,
{
use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface;

let stamped = core
.last_processed_height()
.await
.expect("wallet present in manager");
let target = stamped + crate::wallet::reservations::RESERVATION_MAX_AGE_BLOCKS + 2;
{
let mut wm = core.wallet_manager.write().await;
let (_, info) = wm
.get_wallet_and_info_mut(&core.wallet_id())
.expect("wallet present in manager");
info.core_wallet.update_last_processed_height(target);
}
target
}

/// No-op persister satisfying [`PlatformWalletManager`] construction for tests
/// that need a full [`PlatformWallet`] but no real persistence pipeline.
pub struct NoopTestPersister;
Expand Down
Loading
Loading