[pwltr]

Related: synonymdev/bitkit-ios#485

Companion: iOS #879.

This PR preserves an outgoing RBF replacement's selected fee rate when activity arrives or is refreshed, so consumer apps can show the correct confirmation estimate.

Description

• Adds atomic RBF recording so the replacement's rate is retained as pending metadata or applied immediately when its activity already exists, without resurrecting a removed original.
• Leaves the original transaction's own fee and fee rate unchanged, including when it confirms instead of the replacement; only the replacement receives the selected boost rate.
• Adds wallet-scoped, fee-preserving onchain snapshot upserts so late sync and transaction events cannot overwrite the recorded rate or create a duplicate activity under a different payment ID.
• Preserves existing tags and metadata, rolls back partial database changes on failure, and leaves explicit generic fee-rate corrections available.
• Exposes both operations through UniFFI and includes the generated Swift interface and FFI declarations.

Out of Scope

• Activity history: correcting fee rates already stored incorrectly before this fix.
• Platform SDK publication, version/checksum updates, Android consumer adoption, and incoming CPFP behavior.

Preview

N/A — shared persistence APIs; no UI changes.

QA Notes

Manual Tests

• Consumer app → send an unconfirmed onchain transaction at a low custom fee, boost via RBF, and open its replacement → the confirmation estimate matches the boosted rate and the actual replacement fee is retained.
• Consumer app → refresh, relaunch, and confirm the replacement → its recorded fee rate remains correct.

The initial implementation was verified in the iOS simulator using a local SDK built on the apps' existing 0.5.18-compatible base. That historical simulator result predates the latest original-confirmation correction. Native app verification against the updated SDK remains pending publication/dependency adoption.

Automated Checks

• added rbf_tests.rs — eleven regressions cover replacement arrival order, persistence across database reopen, stale snapshots, wallet isolation, invalid inputs, metadata preservation, and atomic rollback/retry.
• ran the activity suite on the 0.5.18-compatible fix snapshot — 206 passed, zero failures; focused RBF regressions also passed.
• ran Clippy on that snapshot — existing warnings only; none in the changed activity implementation or RBF test file.
• ran a simulator-only SDK build and the iOS consumer regression suites — all 32 iOS tests passed, including eight RBF tests and existing activity/hardware-wallet/transfer coverage.
• ran Rust formatting and generated Swift interface typechecking after rebasing onto master — passed.

Latest review follow-up verification

Verified on macOS at ae0193272bd965a297a2546121af4d2e5b60e37e (2026-10-22):

• The new original-confirmation regression failed before the fix: recording a 25 sat/vB boost changed the original's 1 sat/vB rate to 25. With the fix, the original keeps its fee and rate while pending, on confirmation, and after reopening the database.
• CARGO_INCREMENTAL=0 cargo test --locked --offline --lib modules::activity — 207 passed, zero failures, including all eleven RBF regressions.
• CARGO_INCREMENTAL=0 cargo clippy --locked --offline --lib --tests --message-format=short — passed with pre-existing warnings; no warnings in the changed implementation or RBF tests.
• cargo fmt --check and git diff --check — passed.
• Documented the outgoing RBF replacement-to-original linkage and the snapshot's absolute-fee contract. Replaced the 16-element SQL parameter tuple with rusqlite::params!.

No public signatures, binding declarations, schema, or dependency versions changed in this follow-up. A new native app/device run was not performed for it.

[talosmachina]

No findings. Adds record_rbf_boost and a fee-rate-preserving onchain snapshot upsert, so an RBF replacement keeps the selected rate while the original keeps its own. Reviewed ae01932, full tier.

What I checked, and 7 candidates I ruled out

Read: the diff in src/lib.rs, activity/mod.rs, activity/README.md, all of activity/rbf_tests.rs, and in activity/implementation.rs record_rbf_boost, upsert_onchain_activities_with_fee_rate_policy, get_pre_activity_metadata_in_connection, transfer_pre_activity_metadata_in_transaction, apply_pre_activity_metadata_for_onchain, add_pre_activity_metadata, the schema and the activities_update_trigger.
Consumers traced: ldk-node bump_fee_by_rbf and update_payment_store_inner (master 0527e43), bitkit-ios processOnchainPayment and boostOnchainTransaction (master), and the call sites in synonymdev/bitkit-ios#879.
Ran: cargo test --locked --lib modules::activity at ae01932 on Linux: 207 passed, 0 failed, all 11 rbf_tests included. cargo fmt --check clean.
CI: green (Android and iOS bindgen, manifests, usdt-tests).

Ruled out

• Duplicate activity for the replacement: ldk-node builds PaymentId(txid.to_byte_array()), so the payment id is the internal byte order, not the displayed txid, and a snapshot's id and tx_id differ. The preserving path resolves the row by (wallet_id, tx_id) before falling back to activity.id, so the replacement lands on one row whichever arrives first.
• Replacement arrives before the boost is recorded: record_rbf_boost finds the row by txid and applies the just-written metadata in the same transaction (recording_after_replacement_arrives_preserves_other_fields_and_tags).
• Original confirms instead of the replacement: the original's fee and fee_rate are never written, only is_boosted while pending (original_fee_rate_survives_boost_recording_confirmation_and_reopen). The pending row keyed by the losing replacement txid stays behind with empty tags, which nothing ever matches; the same holds today for a send whose tx never shows up.
• Pending metadata vs the preserved rate: the metadata transfer runs after the upsert inside the transaction, so a recorded boost rate wins over a stored fallback, and the row is deleted on first use, so a later stale snapshot cannot re-apply it.
• Behaviour change on errors: metadata transfer failures on the preserving path now roll back the upsert instead of being swallowed after commit. The rollback test covers it, and the iOS sync loop catches per payment, so one failure does not stop a sync.
• created_at = strftime('%s','now') is text: the column has INTEGER affinity, so SQLite stores an integer; the existing update trigger uses the same expression.
• Chained boost: a replacement is itself a sent activity with a txid different from the next one, so it passes the original check, and the generic upsert/update APIs still overwrite the rate for deliberate corrections (full_updates_and_upserts_still_allow_deliberate_fee_corrections).

Merge confidence: 4/5, the Rust side is covered by rbf_tests.rs, but the new UniFFI calls are exercised only through bitkit-ios#879, which I did not build here (no iOS toolchain on Linux), and I read implementation.rs around the change rather than whole.

[coreyphillips]

Two independent reviews, nothing blocking a merge.

worth doing, does not block

• Pending replacement fee metadata is orphaned when the original confirms (src/modules/activity/implementation.rs:795). If the original transaction wins the race and confirms, the pending pre_activity_metadata row that record_rbf_boost wrote under the replacement txid stays in the table forever. The row is written at src/modules/activity/implementation.rs:795. It is only deleted when an activity with the replacement txid shows up. That can never happen once the original confirms, because the replacement conflicts with it. The row carries no link back to the original, so nothing can find it to clean it up. It will also be included in every get_all_pre_activity_metadata backup. I reproduced this in a scratch copy: rust db.insert_onchain_activity(&base("o", "tx-o"))?; db.record_rbf_boost(DEFAULT_WALLET_ID, "o", "tx-r", 25)?; let mut c = db.get_activity_by_tx_id(DEFAULT_WALLET_ID, "tx-o")?.unwrap(); c.confirmed = true; c.confirm_timestamp = Some(200); db.upsert_onchain_activity_preserving_fee_rate(&c)?; // get_all_pre_activity_metadata() -> [("tx-r", 25)] It does no functional harm, because the row can never be applied to anything. But this is new state the old consumer flow never created, and the PR explicitly covers the case where the original confirms instead. Either record enough information to drop the row when the original confirms, or document that it is expected to be left behind.
• Snapshots leave unknown fee rates stuck at zero (src/modules/activity/implementation.rs:1093). A stored zero rate prevents later snapshots from filling missing fee information. watch_only_activity_from_detail emits zero when the rate is unavailable. I reproduced two snapshot upserts for the same wallet and txid, first at zero and then at 25: the absolute fee updated but the rate stayed zero. Preserve positive stored rates while allowing an unknown zero rate to be filled.

[coreyphillips]

Rechecked after replies, with no new commit since ae01932. Nothing blocking remains.

still open

• Pending replacement fee metadata is orphaned when the original confirms (src/modules/activity/implementation.rs:795). If the original transaction wins the race and confirms, the pending pre_activity_metadata row that record_rbf_boost wrote under the replacement txid stays in the table forever. The row is written at src/modules/activity/implementation.rs:795. It is only deleted when an activity with the replacement txid shows up. That can never happen once the original confirms, because the replacement conflicts with it. The row carries no link back to the original, so nothing can find it to clean it up. It will also be included in every get_all_pre_activity_metadata backup. I reproduced this in a scratch copy: rust db.insert_onchain_activity(&base("o", "tx-o"))?; db.record_rbf_boost(DEFAULT_WALLET_ID, "o", "tx-r", 25)?; let mut c = db.get_activity_by_tx_id(DEFAULT_WALLET_ID, "tx-o")?.unwrap(); c.confirmed = true; c.confirm_timestamp = Some(200); db.upsert_onchain_activity_preserving_fee_rate(&c)?; // get_all_pre_activity_metadata() -> [("tx-r", 25)] It does no functional harm, because the row can never be applied to anything. But this is new state the old consumer flow never created, and the PR explicitly covers the case where the original confirms instead. Either record enough information to drop the row when the original confirms, or document that it is expected to be left behind. HEAD is still ae01932, and git diff ae01932 HEAD is empty. The insert at implementation.rs:795 is unchanged, and the new README section on RBF linkage does not mention the leftover row. @talosmachina's review agrees the row "stays behind", so the point is confirmed rather than shown wrong. The comparison to a send whose tx never shows up is only partly fair: bitkit-ios deletes its pre-activity metadata when a send fails (SendConfirmationView.swift:1454). To close this, add a cleanup path or one README sentence saying the row is expected to remain when the original confirms.
• Snapshots leave unknown fee rates stuck at zero (src/modules/activity/implementation.rs:1093). A stored zero rate prevents later snapshots from filling missing fee information. watch_only_activity_from_detail emits zero when the rate is unavailable. I reproduced two snapshot upserts for the same wallet and txid, first at zero and then at 25: the absolute fee updated but the rate stayed zero. Preserve positive stored rates while allowing an unknown zero rate to be filled. Implementation.rs:1093 still reads CASE WHEN ?16 THEN onchain_activity.fee_rate ELSE excluded.fee_rate END, while fee on the line above is still overwritten. A stored zero (unknown) rate therefore still blocks a later snapshot from filling it in. None of the responses addresses this. Adding AND onchain_activity.fee_rate > 0 to the WHEN condition would fix it.

[ovitrif]

Approved and green, ready for you to merge and release.

[coreyphillips]

Rechecked after replies, with no new commit since ae01932. Nothing blocking remains.

still open

• Pending replacement fee metadata is orphaned when the original confirms (src/modules/activity/implementation.rs:795). If the original transaction wins the race and confirms, the pending pre_activity_metadata row that record_rbf_boost wrote under the replacement txid stays in the table forever. The row is written at src/modules/activity/implementation.rs:795. It is only deleted when an activity with the replacement txid shows up. That can never happen once the original confirms, because the replacement conflicts with it. The row carries no link back to the original, so nothing can find it to clean it up. It will also be included in every get_all_pre_activity_metadata backup. I reproduced this in a scratch copy: rust db.insert_onchain_activity(&base("o", "tx-o"))?; db.record_rbf_boost(DEFAULT_WALLET_ID, "o", "tx-r", 25)?; let mut c = db.get_activity_by_tx_id(DEFAULT_WALLET_ID, "tx-o")?.unwrap(); c.confirmed = true; c.confirm_timestamp = Some(200); db.upsert_onchain_activity_preserving_fee_rate(&c)?; // get_all_pre_activity_metadata() -> [("tx-r", 25)] It does no functional harm, because the row can never be applied to anything. But this is new state the old consumer flow never created, and the PR explicitly covers the case where the original confirms instead. Either record enough information to drop the row when the original confirms, or document that it is expected to be left behind. HEAD is still ae01932, and the insert at implementation.rs:793-800 is unchanged. Nothing removes the replacement-txid pre_activity_metadata row when the original confirms. The new README section on RBF linkage does not say the row is expected to be left behind. The only response is an approval comment, and it does not engage with this point.
• Snapshots leave unknown fee rates stuck at zero (src/modules/activity/implementation.rs:1093). A stored zero rate prevents later snapshots from filling missing fee information. watch_only_activity_from_detail emits zero when the rate is unavailable. I reproduced two snapshot upserts for the same wallet and txid, first at zero and then at 25: the absolute fee updated but the rate stayed zero. Preserve positive stored rates while allowing an unknown zero rate to be filled. Implementation.rs:1093 still reads CASE WHEN ?16 THEN onchain_activity.fee_rate ELSE excluded.fee_rate END. A stored zero rate is still never filled by a later snapshot, while fee is still overwritten. The approval comment does not rebut this. Adding a fee_rate > 0 guard to the WHEN would settle it.

[talosmachina]

@coreyphillips you are right about my comparison, so I am correcting it. bitkit-ios deletes the send's pre-activity metadata when the send fails (SendConfirmationView.swift, deletePreActivityMetadata(paymentId:) in the failure path). So a failed send does not leave a row behind, and nothing in today's flow leaves one the way record_rbf_boost can. Checked again at ae01932:

• Leftover replacement row. I agree it is new state. The row is written at implementation.rs:793-800 with only the replacement txid. The original's boost_tx_ids does not link back to it (the README says that link is one-way), so when the original confirms there is nothing to match the row on and nothing deletes it. Two ways to close it, and @pwltr can pick: store the original activity id on the row, and delete rows carrying it when a preserving upsert confirms that original; or add the README sentence that says the row stays behind. The first is the real fix. The second is enough if the row is accepted as harmless, since it has empty tags and no activity can ever match it.
• Zero rate stuck. The guard is correct, but no current caller hits this. The only place a zero rate comes from is watch_only_activity_from_detail. Its activities reach the database through upsert_activities, which calls upsert_onchain_activities(.., false) (lib.rs:1038), the overwriting path. The LDK path in bitkit-ios#879 falls back to existingOnchain?.feeRate ?? 1, never to 0. Writing WHEN ?16 AND onchain_activity.fee_rate > 0 at implementation.rs:1093 would still be cheap insurance, so a future watch-only caller of the preserving API does not lock in an unknown rate.

Neither point blocks the merge.

[coreyphillips]

Rechecked after replies, with no new commit since ae01932. Nothing blocking remains.

still open

• Pending replacement fee metadata is orphaned when the original confirms (src/modules/activity/implementation.rs:795). If the original transaction wins the race and confirms, the pending pre_activity_metadata row that record_rbf_boost wrote under the replacement txid stays in the table forever. The row is written at src/modules/activity/implementation.rs:795. It is only deleted when an activity with the replacement txid shows up. That can never happen once the original confirms, because the replacement conflicts with it. The row carries no link back to the original, so nothing can find it to clean it up. It will also be included in every get_all_pre_activity_metadata backup. I reproduced this in a scratch copy: rust db.insert_onchain_activity(&base("o", "tx-o"))?; db.record_rbf_boost(DEFAULT_WALLET_ID, "o", "tx-r", 25)?; let mut c = db.get_activity_by_tx_id(DEFAULT_WALLET_ID, "tx-o")?.unwrap(); c.confirmed = true; c.confirm_timestamp = Some(200); db.upsert_onchain_activity_preserving_fee_rate(&c)?; // get_all_pre_activity_metadata() -> [("tx-r", 25)] It does no functional harm, because the row can never be applied to anything. But this is new state the old consumer flow never created, and the PR explicitly covers the case where the original confirms instead. Either record enough information to drop the row when the original confirms, or document that it is expected to be left behind. The insert at src/modules/activity/implementation.rs:794-801 still writes the replacement row with only the replacement txid and nothing that links it to the original. The README (lines 116-131) still does not say the row is left behind. The reply agrees this is new state and lists two ways to close it, but neither has landed. Either store the original id on the row and delete it when that original confirms, or add the README sentence.
• Snapshots leave unknown fee rates stuck at zero (src/modules/activity/implementation.rs:1093). A stored zero rate prevents later snapshots from filling missing fee information. watch_only_activity_from_detail emits zero when the rate is unavailable. I reproduced two snapshot upserts for the same wallet and txid, first at zero and then at 25: the absolute fee updated but the rate stayed zero. Preserve positive stored rates while allowing an unknown zero rate to be filled. Line 1093 still reads CASE WHEN ?16 THEN onchain_activity.fee_rate ELSE excluded.fee_rate END, so a stored zero is still kept over a later positive rate. The reply says no current caller hits this, and that checks out in this repo: upsert_activities uses the overwriting path at src/lib.rs:1038. But that does not rebut the point, which was already scoped to the public API, and the reply calls the AND onchain_activity.fee_rate > 0 guard cheap insurance. The guard has not been added.