Skip to content

refactor: remove BoardingWallet trait #166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
GaneshPatil7517 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

refactor: remove BoardingWallet trait #166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
42f5343
refactor: remove BoardingWallet trait
GaneshPatil7517 Feb 3, 2026
d7f0a60
Update ark-client/src/batch.rs
GaneshPatil7517 Feb 3, 2026
89ea0da
Update ark-client/src/batch.rs
GaneshPatil7517 Feb 3, 2026
dd5fac9
Update ark-client/src/unilateral_exit.rs
GaneshPatil7517 Feb 3, 2026
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
47 changes: 10 additions & 37 deletions ark-bdk-wallet/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use anyhow::Result;
use ark_client::error::Error;
use ark_client::error::ErrorContext;
use ark_client::wallet::Balance;
use ark_client::wallet::BoardingWallet;
use ark_client::wallet::OnchainWallet;
use ark_client::wallet::Persistence;
use ark_core::BoardingOutput;
Expand All @@ -16,9 +15,7 @@ use bdk_wallet::Wallet as BdkWallet;
use bitcoin::bip32::Xpriv;
use bitcoin::key::Keypair;
use bitcoin::key::Secp256k1;
use bitcoin::secp256k1::schnorr::Signature;
use bitcoin::secp256k1::All;
use bitcoin::secp256k1::Message;
use bitcoin::Address;
use bitcoin::Amount;
use bitcoin::FeeRate;
Expand All @@ -38,8 +35,6 @@ pub struct Wallet<DB>
where
DB: Persistence,
{
kp: Keypair,
secp: Secp256k1<All>,
inner: Arc<RwLock<BdkWallet>>,
#[cfg(not(all(target_arch = "wasm32", target_os = "unknown")))]
client: esplora_client::AsyncClient,
Expand All @@ -54,7 +49,7 @@ where
{
pub fn new(
kp: Keypair,
secp: Secp256k1<All>,
_secp: Secp256k1<All>,
network: Network,
esplora_url: &str,
db: DB,
Expand All @@ -75,8 +70,6 @@ where
esplora_client::Builder::new(esplora_url).build_async_with_sleeper::<WebSleeper>()?;

Ok(Self {
kp,
secp,
inner: Arc::new(RwLock::new(wallet)),
client,
db,
Expand Down Expand Up @@ -237,44 +230,24 @@ where
}
}

impl<DB> BoardingWallet for Wallet<DB>
impl<DB> Persistence for Wallet<DB>
where
DB: Persistence,
{
fn new_boarding_output(
fn save_boarding_output(
&self,
server_pk: XOnlyPublicKey,
exit_delay: bitcoin::Sequence,
network: Network,
) -> Result<BoardingOutput, Error> {
let sk = self.kp.secret_key();
let (owner_pk, _) = sk.public_key(&self.secp).x_only_public_key();

let boarding_output =
BoardingOutput::new(&self.secp, server_pk, owner_pk, exit_delay, network)?;

self.db
.save_boarding_output(sk, boarding_output.clone())
.context("Failed saving boarding output")?;

Ok(boarding_output)
sk: bitcoin::secp256k1::SecretKey,
boarding_output: BoardingOutput,
) -> Result<(), Error> {
self.db.save_boarding_output(sk, boarding_output)
}

fn get_boarding_outputs(&self) -> Result<Vec<BoardingOutput>, Error> {
fn load_boarding_outputs(&self) -> Result<Vec<BoardingOutput>, Error> {
self.db.load_boarding_outputs()
}

fn sign_for_pk(&self, pk: &XOnlyPublicKey, msg: &Message) -> Result<Signature, Error> {
let key = self
.db
.sk_for_pk(pk)
.with_context(|| format!("Failed retrieving SK for PK {pk}"))?;

let sig = self
.secp
.sign_schnorr_no_aux_rand(msg, &key.keypair(&self.secp));

Ok(sig)
fn sk_for_pk(&self, pk: &XOnlyPublicKey) -> Result<bitcoin::secp256k1::SecretKey, Error> {
self.db.sk_for_pk(pk)
}
}

Expand Down
20 changes: 12 additions & 8 deletions ark-client/src/batch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use crate::error::ErrorContext as _;
use crate::swap_storage::SwapStorage;
use crate::utils::sleep;
use crate::utils::timeout_op;
use crate::wallet::BoardingWallet;
use crate::wallet::OnchainWallet;
use crate::wallet::Persistence;
use crate::Blockchain;
use crate::Client;
use crate::Error;
Expand Down Expand Up @@ -52,7 +52,7 @@ use std::collections::HashMap;
impl<B, W, S, K> Client<B, W, S, K>
where
B: Blockchain,
W: BoardingWallet + OnchainWallet,
W: OnchainWallet + Persistence,
S: SwapStorage + 'static,
K: crate::KeyProvider,
{
Expand Down Expand Up @@ -927,7 +927,7 @@ where
&self,
) -> Result<(Vec<batch::OnChainInput>, Vec<intent::Input>, Amount), Error> {
// Get all known boarding outputs.
let boarding_outputs = self.inner.wallet.get_boarding_outputs()?;
let boarding_outputs = self.inner.wallet.load_boarding_outputs()?;

let mut boarding_inputs: Vec<batch::OnChainInput> = Vec::new();
let mut total_amount = Amount::ZERO;
Expand Down Expand Up @@ -1162,11 +1162,12 @@ where
})?;

let owner_pk = onchain_input.boarding_output().owner_pk();
let sig = self
let sk = self
.inner
.wallet
.sign_for_pk(&owner_pk, &msg)
.sk_for_pk(&owner_pk)
.map_err(|e| ark_core::Error::ad_hoc(e.to_string()))?;
let sig = secp.sign_schnorr_no_aux_rand(&msg, &sk.keypair(&secp));

Ok((sig, owner_pk))
};
Expand Down Expand Up @@ -1635,10 +1636,13 @@ where
schnorr::Signature,
ark_core::Error,
> {
self.inner
let sk = self
.inner
.wallet
.sign_for_pk(pk, msg)
.map_err(|e| ark_core::Error::ad_hoc(e.to_string()))
.sk_for_pk(pk)
.map_err(|e| ark_core::Error::ad_hoc(e.to_string()))?;
let secp = self.secp();
Ok(secp.sign_schnorr_no_aux_rand(msg, &sk.keypair(secp)))
};

sign_commitment_psbt(
Expand Down
4 changes: 2 additions & 2 deletions ark-client/src/boltz.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use crate::batch::BatchOutputType;
use crate::error::ErrorContext as _;
use crate::swap_storage::SwapStorage;
use crate::timeout_op;
use crate::wallet::BoardingWallet;
use crate::wallet::OnchainWallet;
use crate::wallet::Persistence;
use crate::Blockchain;
use crate::Client;
use crate::Error;
Expand Down Expand Up @@ -73,7 +73,7 @@ pub struct ClaimVhtlcResult {
impl<B, W, S, K> Client<B, W, S, K>
where
B: Blockchain,
W: BoardingWallet + OnchainWallet,
W: OnchainWallet + Persistence,
S: SwapStorage + 'static,
K: crate::KeyProvider,
{
Expand Down
6 changes: 3 additions & 3 deletions ark-client/src/coin_select.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::swap_storage::SwapStorage;
use crate::wallet::BoardingWallet;
use crate::wallet::OnchainWallet;
use crate::wallet::Persistence;
use crate::Blockchain;
use crate::Client;
use crate::Error;
Expand Down Expand Up @@ -34,11 +34,11 @@ pub async fn coin_select_for_onchain<B, W, S, K>(
>
where
B: Blockchain,
W: BoardingWallet + OnchainWallet,
W: OnchainWallet + Persistence,
S: SwapStorage + 'static,
K: crate::KeyProvider,
{
let boarding_outputs = client.inner.wallet.get_boarding_outputs()?;
let boarding_outputs = client.inner.wallet.load_boarding_outputs()?;

let now = Timestamp::now();

Expand Down
4 changes: 2 additions & 2 deletions ark-client/src/fee_estimation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::batch;
use crate::batch::BatchOutputType;
use crate::error::ErrorContext;
use crate::wallet::BoardingWallet;
use crate::wallet::OnchainWallet;
use crate::wallet::Persistence;
use crate::Client;
use crate::Error;
use crate::KeyProvider;
Expand All @@ -20,7 +20,7 @@ use rand::Rng;
impl<B, W, S, K> Client<B, W, S, K>
where
B: crate::Blockchain,
W: BoardingWallet + OnchainWallet,
W: OnchainWallet + Persistence,
S: SwapStorage + 'static,
K: KeyProvider,
{
Expand Down
57 changes: 22 additions & 35 deletions ark-client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use crate::error::ErrorContext;
use crate::key_provider::KeypairIndex;
use crate::utils::sleep;
use crate::utils::timeout_op;
use crate::wallet::BoardingWallet;
use crate::wallet::OnchainWallet;
use crate::wallet::Persistence;
use ark_core::build_anchor_tx;
use ark_core::history;
use ark_core::history::generate_incoming_vtxo_transaction_history;
Expand Down Expand Up @@ -85,11 +85,10 @@ pub const DEFAULT_GAP_LIMIT: u32 = 20;
/// # use ark_client::{Blockchain, Client, Error, SpendStatus, TxStatus};
/// # use ark_client::OfflineClient;
/// # use bitcoin::key::Keypair;
/// # use bitcoin::secp256k1::{Message, SecretKey};
/// # use bitcoin::secp256k1::SecretKey;
/// # use std::sync::Arc;
/// # use bitcoin::{Address, Amount, FeeRate, Network, Psbt, Transaction, Txid, XOnlyPublicKey};
/// # use bitcoin::secp256k1::schnorr::Signature;
/// # use ark_client::wallet::{Balance, BoardingWallet, OnchainWallet, Persistence};
/// # use bitcoin::{Address, Amount, FeeRate, Psbt, Transaction, Txid, XOnlyPublicKey};
/// # use ark_client::wallet::{Balance, OnchainWallet, Persistence};
/// # use ark_client::InMemorySwapStorage;
/// # use ark_core::{BoardingOutput, UtxoCoinSelection, ExplorerUtxo};
/// # use ark_client::StaticKeyProvider;
Expand Down Expand Up @@ -135,7 +134,7 @@ pub const DEFAULT_GAP_LIMIT: u32 = 20;
/// # }
///
/// struct MyWallet {}
/// # impl OnchainWallet for MyWallet where {
/// # impl OnchainWallet for MyWallet {
/// #
/// # fn get_onchain_address(&self) -> Result<Address, Error> {
/// # unimplemented!("You can implement this function using your preferred client library such as bdk")
Expand All @@ -162,9 +161,7 @@ pub const DEFAULT_GAP_LIMIT: u32 = 20;
/// # }
/// # }
/// #
///
/// struct InMemoryDb {}
/// # impl Persistence for InMemoryDb {
/// # impl Persistence for MyWallet {
/// #
/// # fn save_boarding_output(
/// # &self,
Expand All @@ -183,28 +180,6 @@ pub const DEFAULT_GAP_LIMIT: u32 = 20;
/// # }
/// # }
/// #
/// #
/// # impl BoardingWallet for MyWallet
/// # where
/// # {
/// # fn new_boarding_output(
/// # &self,
/// # server_pk: XOnlyPublicKey,
/// # exit_delay: bitcoin::Sequence,
/// # network: Network,
/// # ) -> Result<BoardingOutput, Error> {
/// # unimplemented!()
/// # }
/// #
/// # fn get_boarding_outputs(&self) -> Result<Vec<BoardingOutput>, Error> {
/// # unimplemented!()
/// # }
/// #
/// # fn sign_for_pk(&self, pk: &XOnlyPublicKey, msg: &Message) -> Result<Signature, Error> {
/// # unimplemented!()
/// # }
/// # }
/// #
/// // Initialize the client with a static keypair
/// async fn init_client_with_keypair() -> Result<Client<MyBlockchain, MyWallet, InMemorySwapStorage, ark_client::StaticKeyProvider>, ark_client::Error> {
/// // Create a keypair for signing transactions
Expand Down Expand Up @@ -371,7 +346,7 @@ pub trait Blockchain {
impl<B, W, S, K> OfflineClient<B, W, S, K>
where
B: Blockchain,
W: BoardingWallet + OnchainWallet,
W: OnchainWallet + Persistence,
S: SwapStorage + 'static,
K: KeyProvider,
{
Expand Down Expand Up @@ -578,7 +553,7 @@ where
impl<B, W, S, K> Client<B, W, S, K>
where
B: Blockchain,
W: BoardingWallet + OnchainWallet,
W: OnchainWallet + Persistence,
S: SwapStorage + 'static,
K: KeyProvider,
{
Expand Down Expand Up @@ -721,16 +696,28 @@ where
Ok(discovered_count)
}

// At the moment we are always generating the same address.
/// Get a boarding address for receiving funds on-chain that can later be settled into VTXOs.
///
/// Creates a new boarding output using the current keypair and saves it to persistence.
pub fn get_boarding_address(&self) -> Result<Address, Error> {
let server_info = &self.server_info;

let boarding_output = self.inner.wallet.new_boarding_output(
let keypair = self.next_keypair(KeypairIndex::LastUnused)?;
let owner_pk = keypair.x_only_public_key().0;

let boarding_output = ark_core::BoardingOutput::new(
self.secp(),
server_info.signer_pk.into(),
owner_pk,
server_info.boarding_exit_delay,
server_info.network,
)?;

// Save the boarding output and its secret key for later signing
self.inner
.wallet
.save_boarding_output(keypair.secret_key(), boarding_output.clone())?;

Ok(boarding_output.address().clone())
}

Expand Down
4 changes: 2 additions & 2 deletions ark-client/src/send_vtxo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::error::ErrorContext;
use crate::swap_storage::SwapStorage;
use crate::utils::timeout_op;
use crate::wallet::BoardingWallet;
use crate::wallet::OnchainWallet;
use crate::wallet::Persistence;
use crate::Blockchain;
use crate::Client;
use crate::Error;
Expand All @@ -27,7 +27,7 @@ use bitcoin::XOnlyPublicKey;
impl<B, W, S, K> Client<B, W, S, K>
where
B: Blockchain,
W: BoardingWallet + OnchainWallet,
W: OnchainWallet + Persistence,
S: SwapStorage + 'static,
K: crate::KeyProvider,
{
Expand Down
10 changes: 6 additions & 4 deletions ark-client/src/unilateral_exit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use crate::error::ErrorContext;
use crate::swap_storage::SwapStorage;
use crate::utils::sleep;
use crate::utils::timeout_op;
use crate::wallet::BoardingWallet;
use crate::wallet::OnchainWallet;
use crate::wallet::Persistence;
use crate::Blockchain;
use crate::Client;
use ark_core::build_unilateral_exit_tree_txids;
Expand All @@ -30,7 +30,7 @@ use std::collections::HashSet;
impl<B, W, S, K> Client<B, W, S, K>
where
B: Blockchain,
W: BoardingWallet + OnchainWallet,
W: OnchainWallet + Persistence,
S: SwapStorage + 'static,
K: crate::KeyProvider,
{
Expand Down Expand Up @@ -261,15 +261,17 @@ where
Some(script) => {
let mut res = vec![];
let pks = extract_checksig_pubkeys(script);
let secp = self.secp();

for pk in pks {
if let Ok(keypair) = self.keypair_by_pk(&pk) {
let sig = Secp256k1::new().sign_schnorr_no_aux_rand(&msg, &keypair);
let sig = secp.sign_schnorr_no_aux_rand(&msg, &keypair);
let pk = keypair.x_only_public_key().0;
res.push((sig, pk))
}

if let Ok(sig) = self.inner.wallet.sign_for_pk(&pk, &msg) {
if let Ok(sk) = self.inner.wallet.sk_for_pk(&pk) {
let sig = secp.sign_schnorr_no_aux_rand(&msg, &sk.keypair(&secp));
res.push((sig, pk))
}
Comment on lines 266 to 276
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid double-signing the same pubkey.

If both keypair_by_pk and sk_for_pk succeed, you’ll push two signatures for the same pubkey. Downstream signing code may expect a single signature per key, and duplicates can bloat or invalidate witnesses. Prefer else if/continue or deduplicate by pubkey.

Suggested fix 
                 for pk in pks {
                     if let Ok(keypair) = self.keypair_by_pk(&pk) {
                         let sig = secp.sign_schnorr_no_aux_rand(&msg, &keypair);
                         let pk = keypair.x_only_public_key().0;
                         res.push((sig, pk))
+                        continue;
                     }
 
                     if let Ok(sk) = self.inner.wallet.sk_for_pk(&pk) {
                         let sig = secp.sign_schnorr_no_aux_rand(&msg, &sk.keypair(&secp));
                         res.push((sig, pk))
                     }
                 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for pk in pks {
if let Ok(keypair) = self.keypair_by_pk(&pk) {
let sig = Secp256k1::new().sign_schnorr_no_aux_rand(&msg, &keypair);
let sig = secp.sign_schnorr_no_aux_rand(&msg, &keypair);
let pk = keypair.x_only_public_key().0;
res.push((sig, pk))
}
if let Ok(sig) = self.inner.wallet.sign_for_pk(&pk, &msg) {
if let Ok(sk) = self.inner.wallet.sk_for_pk(&pk) {
let sig = secp.sign_schnorr_no_aux_rand(&msg, &sk.keypair(&secp));
res.push((sig, pk))
}
for pk in pks {
if let Ok(keypair) = self.keypair_by_pk(&pk) {
let sig = secp.sign_schnorr_no_aux_rand(&msg, &keypair);
let pk = keypair.x_only_public_key().0;
res.push((sig, pk))
continue;
}
if let Ok(sk) = self.inner.wallet.sk_for_pk(&pk) {
let sig = secp.sign_schnorr_no_aux_rand(&msg, &sk.keypair(&secp));
res.push((sig, pk))
}
}
🤖 Prompt for AI Agents 
In `@ark-client/src/unilateral_exit.rs` around lines 266 - 276, The loop that
gathers signatures can double-sign the same pubkey because both
keypair_by_pk(&pk) and inner.wallet.sk_for_pk(&pk) may succeed; update the logic
in that loop to ensure only one signature per pubkey (e.g., use if ... else if
... or continue after a successful keypair_by_pk match, or track seen pubkeys in
a HashSet to deduplicate before pushing). Ensure you consistently derive the
pushed public key from the same source (keypair.x_only_public_key().0 or
sk.keypair(&secp).x_only_public_key().0) so the deduplication works correctly
when using secp.sign_schnorr_no_aux_rand.

}
Expand Down
Loading