Skip to content

Conversation

@Mshehu5
Copy link

@Mshehu5 Mshehu5 commented Dec 21, 2025

Description

This PR addresses issues encountered while implementing persistence for Payjoin specifically around the BlockchainClient only being an owned variable rather than being able to be borrowed/referenced as &Blockchainclient.
While working on persistence I ran into problems while working on resume command which needs a blockchain client to resume states such as monitor_payjoin_proposal (receiver) and process_payjoin_proposal (sender)
Because BlockchainClient can only be owned the current design will require a function signature of passing two separate clients to resume sender and receiver states. I initially considered splitting the command into resume_send and resume_receive but this does not fully solve the issue. In particular the sender’s process_payjoin_proposal may call broadcast_transaction and potentially broadcast multiple transactions for persisted send entries stored in the database which still requires reusable access to the client.

This Ownership issue was previously mentioned in #200 and is also noted in a comment at the top of monitor_payjoin_proposal. It prevents the function from being able to resync multiple times and reliably detect when a transaction appears in the mempool. The root cause is that the Kyoto client Box is destructured and spawned into other tasks when passed through sync_kyoto_client making it unusable afterward.

What this PR changes
This PR fixes the issue by refactoring sync_kyoto_client

  • The logic responsible for running the Kyoto node and logger is moved into new_blockchain_client.

  • Instead of returning a Box the function now returns a KyotoClientHandle. Previously the boxed client takes ownership when destructured inside sync_kyoto_client, preventing reuse/reference.
    With the new design sync_kyoto_client takes &mut KyotoClientHandle, allowing the client to be Refrenced which can be used syncing and broadcasting transactions without consumption

  • Additionally monitor_payjoin_proposal is refactored to support resyncing demonstrating that the Kyoto client refactor successfully resolves the original limitations

Notes to the reviewers

After refactor I tested the kyoto client on regtest (Cause I do not have access to a signet) I had to set a trusted peer in the code to connect with a cbf count of 1 this worked and I also made transaction using the steps below:
N.B Payjoin was also tested for the monitor_payjoin_proposal refactor using steps in project readme

bitcoin.conf 

regtest=1
server=1
rpcuser=user
rpcpassword=password
rpcallowip=127.0.0.1
blockfilterindex=1
listen=1
fallbackfee=0.001

[regtest]
bind=127.0.0.1
port=18444
peerblockfilters=1

Step 1: Create transaction

PSBT=$(cargo run --features cbf,sqlite -- \
  --network $NETWORK \
  wallet \
  --wallet sender_wallet \
  --ext-descriptor "$SENDER_EXT_DESC" \
  --int-descriptor "$SENDER_INT_DESC" \
  --database-type $DATABASE_TYPE \
  create_tx --to $RECEIVER_ADDR:50000 --fee_rate 1.0 | jq -r '.psbt')

Step 2: Sign transaction

SIGNED_PSBT=$(cargo run --features cbf,sqlite -- \
  --network $NETWORK \
  wallet \
  --wallet sender_wallet \
  --ext-descriptor "$SENDER_EXT_DESC" \
  --int-descriptor "$SENDER_INT_DESC" \
  --database-type $DATABASE_TYPE \
  sign "$PSBT" | jq -r '.psbt')

Step 3: Broadcast transaction

cargo run --features cbf,sqlite -- \
  --network $NETWORK \
  wallet \
  --wallet sender_wallet \
  --ext-descriptor "$SENDER_EXT_DESC" \
  --int-descriptor "$SENDER_INT_DESC" \
  --database-type $DATABASE_TYPE \
  --client-type $CLIENT_TYPE \
  --cbf-peer $CBF_PEER \
  --cbf-conn-count $CBF_CONN_COUNT \
  broadcast --psbt "$SIGNED_PSBT"

Mine a block to confirm
bitcoin-cli -regtest generatetoaddress 1 $(bitcoin-cli -regtest getnewaddress)

Checking Transaction Status
After broadcasting, wait a moment and sync your wallet:
Sync wallet

cargo run --features cbf,sqlite -- \
  --network $NETWORK \
  wallet \
  --wallet receiver_wallet \
  --ext-descriptor "$RECEIVER_EXT_DESC" \
  --int-descriptor "$RECEIVER_INT_DESC" \
  --database-type $DATABASE_TYPE \
  --client-type $CLIENT_TYPE \
  --cbf-peer $CBF_PEER \
  --cbf-conn-count $CBF_CONN_COUNT \
  sync

Check balance

cargo run --features cbf,sqlite -- \
  --network $NETWORK \
  wallet \
  --wallet receiver_wallet \
  --ext-descriptor "$RECEIVER_EXT_DESC" \
  --int-descriptor "$RECEIVER_INT_DESC" \
  --database-type $DATABASE_TYPE \
  balance

List recent transactions

cargo run --features cbf,sqlite -- \
  --network $NETWORK \
  wallet \
  --wallet sender_wallet \
  --ext-descriptor "$SENDER_EXT_DESC" \
  --int-descriptor "$SENDER_INT_DESC" \
  --database-type $DATABASE_TYPE \
  transactions

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

Refactor KyotoClient to use a KyotoClientHandle struct instead of
Box<LightClient>. Previously, the Box<LightClient> was consumed
when destructured in sync_kyoto_client, preventing the function
from being called multiple times or borrowed. With the handle
pattern, sync_kyoto_client now takes &mut KyotoClientHandle,
allowing it to be borrowed and reused.

This also allows the node to be started at creation time in
new_blockchain_client rather than during sync, making the client
ready for use immediately after creation.

- Add KyotoClientHandle struct containing requester and
  update_subscriber
- Move node startup and logger spawning to new_blockchain_client
- Update sync_kyoto_client to accept &mut KyotoClientHandle
  instead of consuming Box<LightClient>
Update function signatures to accept &mut BlockchainClient instead
of taking ownership. This is required after the Kyoto client
refactoring to allow the client to be used mutably across multiple
operations, including repeated calls to sync_kyoto_client.

- Update handle_online_wallet_subcommand signature
- Update all PayjoinManager methods to use &mut BlockchainClient
- Fix parameter dereferencing in full_scan calls
- Update all call sites to pass mutable references
Replace single sync-and-check with periodic polling loop.
This allows multiple sync operations since sync_wallet now accepts
a reference to BlockchainClient, enabling proper long-running
monitoring instead of a one-time check.
@coveralls
Copy link

Pull Request Test Coverage Report for Build 20412193828

Details

  • 0 of 113 (0.0%) changed or added relevant lines in 3 files are covered.
  • 3 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.01%) to 7.89%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/handlers.rs 0 22 0.0%
src/utils.rs 0 22 0.0%
src/payjoin/mod.rs 0 69 0.0%
Files with Coverage Reduction New Missed Lines %
src/handlers.rs 1 15.28%
src/payjoin/mod.rs 1 0.0%
src/utils.rs 1 0.0%
Totals Coverage Status
Change from base Build 20377635666: -0.01%
Covered Lines: 172
Relevant Lines: 2180

💛 - Coveralls

@notmandatory notmandatory moved this to In Progress in BDK-CLI Dec 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants