From 1eb7be2fb2f7f05977147865d5684ea1a9151caa Mon Sep 17 00:00:00 2001 From: DanGould Date: Thu, 13 Feb 2025 16:40:59 -0500 Subject: [PATCH 1/7] Separate v1, v2 App via Boxed dyn trait --- payjoin-cli/src/app/mod.rs | 4 +- payjoin-cli/src/app/v1.rs | 9 ++++- payjoin-cli/src/app/v2.rs | 82 +++++++++++++++++++------------------- payjoin-cli/src/main.rs | 20 ++++++---- 4 files changed, 64 insertions(+), 51 deletions(-) diff --git a/payjoin-cli/src/app/mod.rs b/payjoin-cli/src/app/mod.rs index 4795999e3..53d8fda55 100644 --- a/payjoin-cli/src/app/mod.rs +++ b/payjoin-cli/src/app/mod.rs @@ -31,7 +31,9 @@ pub trait App { Self: Sized; fn bitcoind(&self) -> Result; async fn send_payjoin(&self, bip21: &str, fee_rate: FeeRate) -> Result<()>; - async fn receive_payjoin(self, amount: Amount) -> Result<()>; + async fn receive_payjoin(&self, amount: Amount) -> Result<()>; + #[cfg(feature = "v2")] + async fn resume_payjoins(&self) -> Result<()>; fn create_original_psbt(&self, uri: &PjUri, fee_rate: FeeRate) -> Result { let amount = uri.amount.ok_or_else(|| anyhow!("please specify the amount in the Uri"))?; diff --git a/payjoin-cli/src/app/v1.rs b/payjoin-cli/src/app/v1.rs index 537319f16..5b6037132 100644 --- a/payjoin-cli/src/app/v1.rs +++ b/payjoin-cli/src/app/v1.rs @@ -113,7 +113,7 @@ impl AppTrait for App { Ok(()) } - async fn receive_payjoin(self, amount: Amount) -> Result<()> { + async fn receive_payjoin(&self, amount: Amount) -> Result<()> { let pj_uri_string = self.construct_payjoin_uri(amount, None)?; println!( "Listening at {}. Configured to accept payjoin at BIP 21 Payjoin Uri:", @@ -130,6 +130,11 @@ impl AppTrait for App { } Ok(()) } + + #[cfg(feature = "v2")] + async fn resume_payjoins(&self) -> Result<()> { + unimplemented!("resume_payjoins not implemented for v1"); + } } impl App { @@ -153,7 +158,7 @@ impl App { Ok(pj_uri.to_string()) } - async fn start_http_server(self) -> Result<()> { + async fn start_http_server(&self) -> Result<()> { let addr = SocketAddr::from(([0, 0, 0, 0], self.config.port)); let listener = TcpListener::bind(addr).await?; let app = self.clone(); diff --git a/payjoin-cli/src/app/v2.rs b/payjoin-cli/src/app/v2.rs index 801539fc8..b43fa9ebb 100644 --- a/payjoin-cli/src/app/v2.rs +++ b/payjoin-cli/src/app/v2.rs @@ -76,7 +76,7 @@ impl AppTrait for App { self.spawn_payjoin_sender(req_ctx).await } - async fn receive_payjoin(self, amount: Amount) -> Result<()> { + async fn receive_payjoin(&self, amount: Amount) -> Result<()> { let address = self.bitcoind()?.get_new_address(None, None)?.assume_checked(); let ohttp_keys = unwrap_ohttp_keys_or_else_fetch(&self.config).await?; let session = @@ -84,6 +84,46 @@ impl AppTrait for App { self.db.insert_recv_session(session.clone())?; self.spawn_payjoin_receiver(session, Some(amount)).await } + + #[allow(clippy::incompatible_msrv)] + async fn resume_payjoins(&self) -> Result<()> { + let recv_sessions = self.db.get_recv_sessions()?; + let send_sessions = self.db.get_send_sessions()?; + + if recv_sessions.is_empty() && send_sessions.is_empty() { + println!("No sessions to resume."); + return Ok(()); + } + + let mut tasks = Vec::new(); + + for session in recv_sessions { + let self_clone = self.clone(); + tasks.push(tokio::spawn(async move { + self_clone.spawn_payjoin_receiver(session, None).await + })); + } + + for session in send_sessions { + let self_clone = self.clone(); + tasks.push(tokio::spawn(async move { self_clone.spawn_payjoin_sender(session).await })); + } + + let mut interrupt = self.interrupt.clone(); + tokio::select! { + _ = async { + for task in tasks { + let _ = task.await; + } + } => { + println!("All resumed sessions completed."); + } + _ = interrupt.changed() => { + println!("Resumed sessions were interrupted."); + } + } + Ok(()) + } } impl App { @@ -150,46 +190,6 @@ impl App { Ok(()) } - #[allow(clippy::incompatible_msrv)] - pub async fn resume_payjoins(&self) -> Result<()> { - let recv_sessions = self.db.get_recv_sessions()?; - let send_sessions = self.db.get_send_sessions()?; - - if recv_sessions.is_empty() && send_sessions.is_empty() { - println!("No sessions to resume."); - return Ok(()); - } - - let mut tasks = Vec::new(); - - for session in recv_sessions { - let self_clone = self.clone(); - tasks.push(tokio::spawn(async move { - self_clone.spawn_payjoin_receiver(session, None).await - })); - } - - for session in send_sessions { - let self_clone = self.clone(); - tasks.push(tokio::spawn(async move { self_clone.spawn_payjoin_sender(session).await })); - } - - let mut interrupt = self.interrupt.clone(); - tokio::select! { - _ = async { - for task in tasks { - let _ = task.await; - } - } => { - println!("All resumed sessions completed."); - } - _ = interrupt.changed() => { - println!("Resumed sessions were interrupted."); - } - } - Ok(()) - } - async fn long_poll_post(&self, req_ctx: &mut Sender) -> Result { match req_ctx.extract_v2(self.config.ohttp_relay.clone()) { Ok((req, ctx)) => { diff --git a/payjoin-cli/src/main.rs b/payjoin-cli/src/main.rs index c22489ebf..335dc82d9 100644 --- a/payjoin-cli/src/main.rs +++ b/payjoin-cli/src/main.rs @@ -9,18 +9,24 @@ use url::Url; mod app; mod db; -#[cfg(all(not(feature = "v2"), feature = "v1"))] -use app::v1::App; -#[cfg(feature = "v2")] -use app::v2::App; - #[tokio::main] async fn main() -> Result<()> { env_logger::init(); let matches = cli(); - let config = AppConfig::new(&matches).with_context(|| "Failed to parse config")?; - let app = App::new(config)?; + let config = AppConfig::new(&matches)?; + let app: Box = { + #[cfg(feature = "v2")] + { + Box::new(crate::app::v2::App::new(config)?) + } + #[cfg(all(feature = "v1", not(feature = "v2")))] + { + Box::new(crate::app::v1::App::new(config)?) + } + #[cfg(not(any(feature = "v1", feature = "v2")))] + compile_error!("Either feature \"v1\" or \"v2\" must be enabled"); + }; match matches.subcommand() { Some(("send", sub_matches)) => { From a37688020df8ca5234de6a271d9e94cec31286c2 Mon Sep 17 00:00:00 2001 From: DanGould Date: Thu, 13 Feb 2025 12:55:56 -0500 Subject: [PATCH 2/7] Make AppConfig hierarchical Use version-specific substructures. This organization gets closer to additive payjoin-cli features. --- payjoin-cli/example.config.toml | 45 +++++++++-------- payjoin-cli/src/app/config.rs | 85 +++++++++++++++++++-------------- payjoin-cli/src/app/mod.rs | 2 +- payjoin-cli/src/app/v1.rs | 20 ++++---- payjoin-cli/src/app/v2.rs | 32 +++++++------ 5 files changed, 102 insertions(+), 82 deletions(-) diff --git a/payjoin-cli/example.config.toml b/payjoin-cli/example.config.toml index 74041ab86..a5264507f 100644 --- a/payjoin-cli/example.config.toml +++ b/payjoin-cli/example.config.toml @@ -2,18 +2,25 @@ ## Payjoin config.toml configuration file. Lines beginning with # are comments. ## -# Bitcoin RPC Connection Settings -# ------------------------------ +# Common Settings +# -------------- +# The path to the database file +db_path = "payjoin.db" +# The maximum fee rate that the receiver is willing to pay (in sat/vB) +max_fee_rate = 2.0 + +# Bitcoin RPC Connection Settings +# ------------------------------ +[bitcoind] # The RPC host of the wallet to connect to. # For example, if the wallet is "sender", then default values are: # - mainnet: http://localhost:8332/wallet/sender # - testnet: http://localhost:18332/wallet/sender # - regtest: http://localhost:18443/wallet/sender # - signet: http://localhost:38332/wallet/sender -bitcoind_rpchost="http://localhost:18443/wallet/sender" - +rpchost = "http://localhost:18443/wallet/sender" # The RPC .cookie file used only for local authentication to bitcoind. # If rpcuser and rpcpassword are being used, this is not necessary. @@ -22,36 +29,32 @@ bitcoind_rpchost="http://localhost:18443/wallet/sender" # MacOS: ~/Library/Application Support/Bitcoin//.cookie # Windows Vista and later: C:\Users\YourUserName\AppData\Roaming\Bitcoin\\.cookie # Windows XP: C:\Documents and Settings\YourUserName\Application Data\Bitcoin\\.cookie -# bitcoind_cookie= - +cookie = "" # The rpcuser to connect to (specified in bitcoin.conf). -bitcoind_rpcuser="user" - +rpcuser = "user" # The rpcpassword of the user to connect to (specified in bitcoin.conf). -bitcoind_rpcpassword="password" - -## Payjoin Settings -## ---------------- - +rpcpassword = "password" +# Version 1 Settings +# ----------------- +[v1] # (v1, receiver only) The port for v1 receiving servers to bind to. -port="3000" - +port = 3000 # (v1, receiver only) The payjoin endpoint which coordinates the transaction. -pj_endpoint="https://localhost:3000" - +pj_endpoint = "https://localhost:3000" +# Version 2 Settings +# ----------------- +[v2] # (v2 only) The payjoin directory to rendezvous at. pj_directory = "https://payjo.in" - # (v2 only) The OHTTP relay that will forward requests to the OHTTP Gateway, which will forward to the pj_endpoint directory. -ohttp_relay="https://pj.bobspacebkk.com" - +ohttp_relay = "https://pj.bobspacebkk.com" # (v2 only, optional) The HPKE keys which need to be fetched ahead of time from the pj_endpoint for the payjoin packets to be encrypted. # These can now be fetched and no longer need to be configured. -ohttp_keys="./path/to/ohttp_keys" +ohttp_keys = "./path/to/ohttp_keys" diff --git a/payjoin-cli/src/app/config.rs b/payjoin-cli/src/app/config.rs index d32627a23..540d42970 100644 --- a/payjoin-cli/src/app/config.rs +++ b/payjoin-cli/src/app/config.rs @@ -10,72 +10,87 @@ use url::Url; use crate::db; #[derive(Debug, Clone, Deserialize)] -pub struct AppConfig { - pub bitcoind_rpchost: Url, - pub bitcoind_cookie: Option, - pub bitcoind_rpcuser: String, - pub bitcoind_rpcpassword: String, - pub db_path: PathBuf, - // receive-only - pub max_fee_rate: Option, +pub struct BitcoindConfig { + pub rpchost: Url, + pub cookie: Option, + pub rpcuser: String, + pub rpcpassword: String, +} - // v2 only - #[cfg(feature = "v2")] +#[cfg(feature = "v1")] +#[derive(Debug, Clone, Deserialize)] +pub struct V1Config { + pub port: u16, + pub pj_endpoint: Url, +} + +#[cfg(feature = "v2")] +#[derive(Debug, Clone, Deserialize)] +pub struct V2Config { #[serde(deserialize_with = "deserialize_ohttp_keys_from_path")] pub ohttp_keys: Option, - #[cfg(feature = "v2")] pub ohttp_relay: Url, - #[cfg(feature = "v2")] pub pj_directory: Url, +} - // v1 receive-only - #[cfg(not(feature = "v2"))] - pub port: u16, - #[cfg(not(feature = "v2"))] - pub pj_endpoint: Url, +#[derive(Debug, Clone, Deserialize)] +pub struct AppConfig { + pub db_path: PathBuf, + pub max_fee_rate: Option, + pub bitcoind: BitcoindConfig, + #[cfg(feature = "v1")] + pub v1: V1Config, + #[cfg(feature = "v2")] + pub v2: V2Config, } impl AppConfig { pub(crate) fn new(matches: &ArgMatches) -> Result { let builder = Config::builder() - .set_default("bitcoind_rpchost", "http://localhost:18443")? + // BitcoindConfig defaults + .set_default("bitcoind.rpchost", "http://localhost:18443")? .set_override_option( - "bitcoind_rpchost", + "bitcoind.rpchost", matches.get_one::("rpchost").map(|s| s.as_str()), )? - .set_default("bitcoind_cookie", None::)? + .set_default("bitcoind.cookie", None::)? .set_override_option( - "bitcoind_cookie", + "bitcoind.cookie", matches.get_one::("cookie_file").map(|s| s.as_str()), )? - .set_default("bitcoind_rpcuser", "bitcoin")? + .set_default("bitcoind.rpcuser", "bitcoin")? .set_override_option( - "bitcoind_rpcuser", + "bitcoind.rpcuser", matches.get_one::("rpcuser").map(|s| s.as_str()), )? - .set_default("bitcoind_rpcpassword", "")? + .set_default("bitcoind.rpcpassword", "")? .set_override_option( - "bitcoind_rpcpassword", + "bitcoind.rpcpassword", matches.get_one::("rpcpassword").map(|s| s.as_str()), )? + // Common defaults .set_default("db_path", db::DB_PATH)? .set_override_option( "db_path", matches.get_one::("db_path").map(|s| s.as_str()), )? - // Subcommand defaults without which file serialization fails. - .set_default("port", "3000")? - .set_default("pj_endpoint", "https://localhost:3000")? .add_source(File::new("config.toml", FileFormat::Toml).required(false)); + // Add V1Config defaults when not using V2 + #[cfg(feature = "v1")] + let builder = builder + .set_default("v1.port", 3000_u16)? + .set_default("v1.pj_endpoint", "https://localhost:3000")?; + + // Add V2Config defaults when using V2 #[cfg(feature = "v2")] let builder = builder .set_override_option( - "ohttp_relay", + "v2.ohttp_relay", matches.get_one::("ohttp_relay").map(|s| s.as_str()), )? - .set_default("pj_directory", "https://payjo.in")? - .set_default("ohttp_keys", None::)?; + .set_default("v2.pj_directory", "https://payjo.in")? + .set_default("v2.ohttp_keys", None::)?; let builder = match matches.subcommand() { Some(("send", _)) => builder, @@ -89,8 +104,8 @@ impl AppConfig { .map_err(|_| { ConfigError::Message("\"port\" must be a valid number".to_string()) })?; - builder.set_override_option("port", port)?.set_override_option( - "pj_endpoint", + builder.set_override_option("v1.port", port)?.set_override_option( + "v1.pj_endpoint", matches.get_one::("pj_endpoint").map(|s| s.as_str()), )? }; @@ -99,11 +114,11 @@ impl AppConfig { let builder = { builder .set_override_option( - "pj_directory", + "v2.pj_directory", matches.get_one::("pj_directory").map(|s| s.as_str()), )? .set_override_option( - "ohttp_keys", + "v2.ohttp_keys", matches.get_one::("ohttp_keys").map(|s| s.as_str()), )? }; diff --git a/payjoin-cli/src/app/mod.rs b/payjoin-cli/src/app/mod.rs index 53d8fda55..bf0814bf4 100644 --- a/payjoin-cli/src/app/mod.rs +++ b/payjoin-cli/src/app/mod.rs @@ -16,7 +16,7 @@ use tokio::sync::watch; pub mod config; use crate::app::config::AppConfig; -#[cfg(all(not(feature = "v2"), feature = "v1"))] +#[cfg(feature = "v1")] pub(crate) mod v1; #[cfg(feature = "v2")] pub(crate) mod v2; diff --git a/payjoin-cli/src/app/v1.rs b/payjoin-cli/src/app/v1.rs index 5b6037132..04e2eba7a 100644 --- a/payjoin-cli/src/app/v1.rs +++ b/payjoin-cli/src/app/v1.rs @@ -58,16 +58,16 @@ impl AppTrait for App { } fn bitcoind(&self) -> Result { - match &self.config.bitcoind_cookie { + match &self.config.bitcoind.cookie { Some(cookie) => bitcoincore_rpc::Client::new( - self.config.bitcoind_rpchost.as_str(), + self.config.bitcoind.rpchost.as_str(), bitcoincore_rpc::Auth::CookieFile(cookie.into()), ), None => bitcoincore_rpc::Client::new( - self.config.bitcoind_rpchost.as_str(), + self.config.bitcoind.rpchost.as_str(), bitcoincore_rpc::Auth::UserPass( - self.config.bitcoind_rpcuser.clone(), - self.config.bitcoind_rpcpassword.clone(), + self.config.bitcoind.rpcuser.clone(), + self.config.bitcoind.rpcpassword.clone(), ), ), } @@ -117,7 +117,7 @@ impl AppTrait for App { let pj_uri_string = self.construct_payjoin_uri(amount, None)?; println!( "Listening at {}. Configured to accept payjoin at BIP 21 Payjoin Uri:", - self.config.port + self.config.v1.port ); println!("{}", pj_uri_string); @@ -146,7 +146,7 @@ impl App { let pj_receiver_address = self.bitcoind()?.get_new_address(None, None)?.assume_checked(); let pj_part = match fallback_target { Some(target) => target, - None => self.config.pj_endpoint.as_str(), + None => self.config.v1.pj_endpoint.as_str(), }; let pj_part = payjoin::Url::parse(pj_part) .map_err(|e| anyhow!("Failed to parse pj_endpoint: {}", e))?; @@ -159,7 +159,7 @@ impl App { } async fn start_http_server(&self) -> Result<()> { - let addr = SocketAddr::from(([0, 0, 0, 0], self.config.port)); + let addr = SocketAddr::from(([0, 0, 0, 0], self.config.v1.port)); let listener = TcpListener::bind(addr).await?; let app = self.clone(); @@ -274,10 +274,10 @@ impl App { "{}?amount={}&pj={}", address.to_qr_uri(), amount.to_btc(), - self.config.pj_endpoint + self.config.v1.pj_endpoint ) } else { - format!("{}?pj={}", address.to_qr_uri(), self.config.pj_endpoint) + format!("{}?pj={}", address.to_qr_uri(), self.config.v1.pj_endpoint) }; let uri = Uri::try_from(uri_string.clone()) .map_err(|_| Implementation(anyhow!("Could not parse payjoin URI string.").into()))?; diff --git a/payjoin-cli/src/app/v2.rs b/payjoin-cli/src/app/v2.rs index b43fa9ebb..1f4c49fd5 100644 --- a/payjoin-cli/src/app/v2.rs +++ b/payjoin-cli/src/app/v2.rs @@ -38,16 +38,16 @@ impl AppTrait for App { } fn bitcoind(&self) -> Result { - match &self.config.bitcoind_cookie { + match &self.config.bitcoind.cookie { Some(cookie) => bitcoincore_rpc::Client::new( - self.config.bitcoind_rpchost.as_str(), + self.config.bitcoind.rpchost.as_str(), bitcoincore_rpc::Auth::CookieFile(cookie.into()), ), None => bitcoincore_rpc::Client::new( - self.config.bitcoind_rpchost.as_str(), + self.config.bitcoind.rpchost.as_str(), bitcoincore_rpc::Auth::UserPass( - self.config.bitcoind_rpcuser.clone(), - self.config.bitcoind_rpcpassword.clone(), + self.config.bitcoind.rpcuser.clone(), + self.config.bitcoind.rpcpassword.clone(), ), ), } @@ -80,7 +80,7 @@ impl AppTrait for App { let address = self.bitcoind()?.get_new_address(None, None)?.assume_checked(); let ohttp_keys = unwrap_ohttp_keys_or_else_fetch(&self.config).await?; let session = - Receiver::new(address, self.config.pj_directory.clone(), ohttp_keys.clone(), None)?; + Receiver::new(address, self.config.v2.pj_directory.clone(), ohttp_keys.clone(), None)?; self.db.insert_recv_session(session.clone())?; self.spawn_payjoin_receiver(session, Some(amount)).await } @@ -151,7 +151,6 @@ impl App { println!("Receive session established"); let mut pj_uri = session.pj_uri(); pj_uri.amount = amount; - println!("Request Payjoin by sharing this Payjoin Uri:"); println!("{}", pj_uri); @@ -169,12 +168,14 @@ impl App { let mut payjoin_proposal = match self.process_v2_proposal(receiver.clone()) { Ok(proposal) => proposal, Err(Error::ReplyToSender(e)) => { - return Err(handle_recoverable_error(e, receiver, &self.config.ohttp_relay).await); + return Err( + handle_recoverable_error(e, receiver, &self.config.v2.ohttp_relay).await + ); } Err(e) => return Err(e.into()), }; let (req, ohttp_ctx) = payjoin_proposal - .extract_v2_req(&self.config.ohttp_relay) + .extract_v2_req(&self.config.v2.ohttp_relay) .map_err(|e| anyhow!("v2 req extraction failed {}", e))?; println!("Got a request from the sender. Responding with a Payjoin proposal."); let res = post_request(req).await?; @@ -191,14 +192,15 @@ impl App { } async fn long_poll_post(&self, req_ctx: &mut Sender) -> Result { - match req_ctx.extract_v2(self.config.ohttp_relay.clone()) { + match req_ctx.extract_v2(self.config.v2.ohttp_relay.clone()) { Ok((req, ctx)) => { println!("Posting Original PSBT Payload request..."); let response = post_request(req).await?; println!("Sent fallback transaction"); let v2_ctx = Arc::new(ctx.process_response(&response.bytes().await?)?); loop { - let (req, ohttp_ctx) = v2_ctx.extract_req(self.config.ohttp_relay.clone())?; + let (req, ohttp_ctx) = + v2_ctx.extract_req(self.config.v2.ohttp_relay.clone())?; let response = post_request(req).await?; match v2_ctx.process_response(&response.bytes().await?, ohttp_ctx) { Ok(Some(psbt)) => return Ok(psbt), @@ -235,7 +237,7 @@ impl App { session: &mut payjoin::receive::v2::Receiver, ) -> Result { loop { - let (req, context) = session.extract_req(&self.config.ohttp_relay)?; + let (req, context) = session.extract_req(&self.config.v2.ohttp_relay)?; println!("Polling receive request..."); let ohttp_response = post_request(req).await?; let proposal = session @@ -369,13 +371,13 @@ fn try_contributing_inputs( } async fn unwrap_ohttp_keys_or_else_fetch(config: &AppConfig) -> Result { - if let Some(keys) = config.ohttp_keys.clone() { + if let Some(keys) = config.v2.ohttp_keys.clone() { println!("Using OHTTP Keys from config"); Ok(keys) } else { println!("Bootstrapping private network transport over Oblivious HTTP"); - let ohttp_relay = config.ohttp_relay.clone(); - let payjoin_directory = config.pj_directory.clone(); + let ohttp_relay = config.v2.ohttp_relay.clone(); + let payjoin_directory = config.v2.pj_directory.clone(); #[cfg(feature = "_danger-local-https")] let ohttp_keys = { let cert_der = crate::app::read_local_cert()?; From e18962e4707018351d1a7b001af214bd79854734 Mon Sep 17 00:00:00 2001 From: DanGould Date: Thu, 13 Feb 2025 12:57:48 -0500 Subject: [PATCH 3/7] Rename AppConfig app::Config --- payjoin-cli/src/app/config.rs | 10 +++++----- payjoin-cli/src/app/mod.rs | 4 ++-- payjoin-cli/src/app/v1.rs | 6 +++--- payjoin-cli/src/app/v2.rs | 8 ++++---- payjoin-cli/src/main.rs | 4 ++-- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/payjoin-cli/src/app/config.rs b/payjoin-cli/src/app/config.rs index 540d42970..00b540e81 100644 --- a/payjoin-cli/src/app/config.rs +++ b/payjoin-cli/src/app/config.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; use anyhow::Result; use clap::ArgMatches; -use config::{Config, ConfigError, File, FileFormat}; +use config::{ConfigError, File, FileFormat}; use payjoin::bitcoin::FeeRate; use serde::Deserialize; use url::Url; @@ -34,7 +34,7 @@ pub struct V2Config { } #[derive(Debug, Clone, Deserialize)] -pub struct AppConfig { +pub struct Config { pub db_path: PathBuf, pub max_fee_rate: Option, pub bitcoind: BitcoindConfig, @@ -44,9 +44,9 @@ pub struct AppConfig { pub v2: V2Config, } -impl AppConfig { +impl Config { pub(crate) fn new(matches: &ArgMatches) -> Result { - let builder = Config::builder() + let builder = config::Config::builder() // BitcoindConfig defaults .set_default("bitcoind.rpchost", "http://localhost:18443")? .set_override_option( @@ -132,7 +132,7 @@ impl AppConfig { }; let config = builder.build()?; - let app_config: AppConfig = config.try_deserialize()?; + let app_config: Config = config.try_deserialize()?; log::debug!("App config: {:?}", app_config); Ok(app_config) } diff --git a/payjoin-cli/src/app/mod.rs b/payjoin-cli/src/app/mod.rs index bf0814bf4..2b7696152 100644 --- a/payjoin-cli/src/app/mod.rs +++ b/payjoin-cli/src/app/mod.rs @@ -14,7 +14,7 @@ use tokio::signal; use tokio::sync::watch; pub mod config; -use crate::app::config::AppConfig; +use crate::app::config::Config; #[cfg(feature = "v1")] pub(crate) mod v1; @@ -26,7 +26,7 @@ pub const LOCAL_CERT_FILE: &str = "localhost.der"; #[async_trait::async_trait] pub trait App { - fn new(config: AppConfig) -> Result + fn new(config: Config) -> Result where Self: Sized; fn bitcoind(&self) -> Result; diff --git a/payjoin-cli/src/app/v1.rs b/payjoin-cli/src/app/v1.rs index 04e2eba7a..652f04c17 100644 --- a/payjoin-cli/src/app/v1.rs +++ b/payjoin-cli/src/app/v1.rs @@ -23,7 +23,7 @@ use payjoin::{Uri, UriExt}; use tokio::net::TcpListener; use tokio::sync::watch; -use super::config::AppConfig; +use super::config::Config; use super::App as AppTrait; use crate::app::{handle_interrupt, http_agent, input_pair_from_list_unspent}; use crate::db::Database; @@ -39,14 +39,14 @@ impl payjoin::receive::v1::Headers for Headers<'_> { #[derive(Clone)] pub(crate) struct App { - config: AppConfig, + config: Config, db: Arc, interrupt: watch::Receiver<()>, } #[async_trait::async_trait] impl AppTrait for App { - fn new(config: AppConfig) -> Result { + fn new(config: Config) -> Result { let db = Arc::new(Database::create(&config.db_path)?); let (interrupt_tx, interrupt_rx) = watch::channel(()); tokio::spawn(handle_interrupt(interrupt_tx)); diff --git a/payjoin-cli/src/app/v2.rs b/payjoin-cli/src/app/v2.rs index 1f4c49fd5..b2d0bfef2 100644 --- a/payjoin-cli/src/app/v2.rs +++ b/payjoin-cli/src/app/v2.rs @@ -12,21 +12,21 @@ use payjoin::send::v2::{Sender, SenderBuilder}; use payjoin::{bitcoin, Uri}; use tokio::sync::watch; -use super::config::AppConfig; +use super::config::Config; use super::App as AppTrait; use crate::app::{handle_interrupt, http_agent, input_pair_from_list_unspent}; use crate::db::Database; #[derive(Clone)] pub(crate) struct App { - config: AppConfig, + config: Config, db: Arc, interrupt: watch::Receiver<()>, } #[async_trait::async_trait] impl AppTrait for App { - fn new(config: AppConfig) -> Result { + fn new(config: Config) -> Result { let db = Arc::new(Database::create(&config.db_path)?); let (interrupt_tx, interrupt_rx) = watch::channel(()); tokio::spawn(handle_interrupt(interrupt_tx)); @@ -370,7 +370,7 @@ fn try_contributing_inputs( .commit_inputs()) } -async fn unwrap_ohttp_keys_or_else_fetch(config: &AppConfig) -> Result { +async fn unwrap_ohttp_keys_or_else_fetch(config: &Config) -> Result { if let Some(keys) = config.v2.ohttp_keys.clone() { println!("Using OHTTP Keys from config"); Ok(keys) diff --git a/payjoin-cli/src/main.rs b/payjoin-cli/src/main.rs index 335dc82d9..89f6c58a1 100644 --- a/payjoin-cli/src/main.rs +++ b/payjoin-cli/src/main.rs @@ -1,5 +1,5 @@ use anyhow::{Context, Result}; -use app::config::AppConfig; +use app::config::Config; use app::App as AppTrait; use clap::{arg, value_parser, Arg, ArgMatches, Command}; use payjoin::bitcoin::amount::ParseAmountError; @@ -14,7 +14,7 @@ async fn main() -> Result<()> { env_logger::init(); let matches = cli(); - let config = AppConfig::new(&matches)?; + let config = Config::new(&matches)?; let app: Box = { #[cfg(feature = "v2")] { From 9e1ad8148eceb70a61385c7d58c855e6b776d9b7 Mon Sep 17 00:00:00 2001 From: DanGould Date: Thu, 13 Feb 2025 13:03:56 -0500 Subject: [PATCH 4/7] Break up config into helper functions Avoid inline comments by using helpers with docstrings instead. --- payjoin-cli/src/app/config.rs | 193 ++++++++++++++++++++-------------- 1 file changed, 113 insertions(+), 80 deletions(-) diff --git a/payjoin-cli/src/app/config.rs b/payjoin-cli/src/app/config.rs index 00b540e81..9d67913d1 100644 --- a/payjoin-cli/src/app/config.rs +++ b/payjoin-cli/src/app/config.rs @@ -2,6 +2,7 @@ use std::path::PathBuf; use anyhow::Result; use clap::ArgMatches; +use config::builder::DefaultState; use config::{ConfigError, File, FileFormat}; use payjoin::bitcoin::FeeRate; use serde::Deserialize; @@ -9,6 +10,8 @@ use url::Url; use crate::db; +type Builder = config::builder::ConfigBuilder; + #[derive(Debug, Clone, Deserialize)] pub struct BitcoindConfig { pub rpchost: Url, @@ -46,90 +49,22 @@ pub struct Config { impl Config { pub(crate) fn new(matches: &ArgMatches) -> Result { - let builder = config::Config::builder() - // BitcoindConfig defaults - .set_default("bitcoind.rpchost", "http://localhost:18443")? - .set_override_option( - "bitcoind.rpchost", - matches.get_one::("rpchost").map(|s| s.as_str()), - )? - .set_default("bitcoind.cookie", None::)? - .set_override_option( - "bitcoind.cookie", - matches.get_one::("cookie_file").map(|s| s.as_str()), - )? - .set_default("bitcoind.rpcuser", "bitcoin")? - .set_override_option( - "bitcoind.rpcuser", - matches.get_one::("rpcuser").map(|s| s.as_str()), - )? - .set_default("bitcoind.rpcpassword", "")? - .set_override_option( - "bitcoind.rpcpassword", - matches.get_one::("rpcpassword").map(|s| s.as_str()), - )? - // Common defaults - .set_default("db_path", db::DB_PATH)? - .set_override_option( - "db_path", - matches.get_one::("db_path").map(|s| s.as_str()), - )? - .add_source(File::new("config.toml", FileFormat::Toml).required(false)); + let mut builder = config::Config::builder(); + builder = add_bitcoind_defaults(builder, matches)?; + builder = add_common_defaults(builder, matches)?; - // Add V1Config defaults when not using V2 #[cfg(feature = "v1")] - let builder = builder - .set_default("v1.port", 3000_u16)? - .set_default("v1.pj_endpoint", "https://localhost:3000")?; + { + builder = add_v1_defaults(builder)?; + } - // Add V2Config defaults when using V2 #[cfg(feature = "v2")] - let builder = builder - .set_override_option( - "v2.ohttp_relay", - matches.get_one::("ohttp_relay").map(|s| s.as_str()), - )? - .set_default("v2.pj_directory", "https://payjo.in")? - .set_default("v2.ohttp_keys", None::)?; - - let builder = match matches.subcommand() { - Some(("send", _)) => builder, - Some(("receive", matches)) => { - #[cfg(not(feature = "v2"))] - let builder = { - let port = matches - .get_one::("port") - .map(|port| port.parse::()) - .transpose() - .map_err(|_| { - ConfigError::Message("\"port\" must be a valid number".to_string()) - })?; - builder.set_override_option("v1.port", port)?.set_override_option( - "v1.pj_endpoint", - matches.get_one::("pj_endpoint").map(|s| s.as_str()), - )? - }; - - #[cfg(feature = "v2")] - let builder = { - builder - .set_override_option( - "v2.pj_directory", - matches.get_one::("pj_directory").map(|s| s.as_str()), - )? - .set_override_option( - "v2.ohttp_keys", - matches.get_one::("ohttp_keys").map(|s| s.as_str()), - )? - }; - - let max_fee_rate = matches.get_one::("max_fee_rate"); - builder.set_override_option("max_fee_rate", max_fee_rate.map(|f| f.to_string()))? - } - #[cfg(feature = "v2")] - Some(("resume", _)) => builder, - _ => unreachable!(), // If all subcommands are defined above, anything else is unreachabe!() - }; + { + builder = add_v2_defaults(builder, matches)?; + } + + builder = handle_subcommands(builder, matches)?; + builder = builder.add_source(File::new("config.toml", FileFormat::Toml).required(false)); let config = builder.build()?; let app_config: Config = config.try_deserialize()?; @@ -138,6 +73,104 @@ impl Config { } } +/// Set up default values and CLI overrides for Bitcoin RPC connection settings +fn add_bitcoind_defaults(builder: Builder, matches: &ArgMatches) -> Result { + builder + .set_default("bitcoind.rpchost", "http://localhost:18443")? + .set_override_option( + "bitcoind.rpchost", + matches.get_one::("rpchost").map(|s| s.as_str()), + )? + .set_default("bitcoind.cookie", None::)? + .set_override_option( + "bitcoind.cookie", + matches.get_one::("cookie_file").map(|s| s.as_str()), + )? + .set_default("bitcoind.rpcuser", "bitcoin")? + .set_override_option( + "bitcoind.rpcuser", + matches.get_one::("rpcuser").map(|s| s.as_str()), + )? + .set_default("bitcoind.rpcpassword", "")? + .set_override_option( + "bitcoind.rpcpassword", + matches.get_one::("rpcpassword").map(|s| s.as_str()), + ) +} + +/// Set up default values and CLI overrides for common settings shared between v1 and v2 +fn add_common_defaults(builder: Builder, matches: &ArgMatches) -> Result { + builder + .set_default("db_path", db::DB_PATH)? + .set_override_option("db_path", matches.get_one::("db_path").map(|s| s.as_str())) +} + +/// Set up default values for v1-specific settings when v2 is not enabled +#[cfg(feature = "v1")] +fn add_v1_defaults(builder: Builder) -> Result { + builder + .set_default("v1.port", 3000_u16)? + .set_default("v1.pj_endpoint", "https://localhost:3000") +} + +/// Set up default values and CLI overrides for v2-specific settings +#[cfg(feature = "v2")] +fn add_v2_defaults(builder: Builder, matches: &ArgMatches) -> Result { + builder + .set_override_option( + "v2.ohttp_relay", + matches.get_one::("ohttp_relay").map(|s| s.as_str()), + )? + .set_default("v2.pj_directory", "https://payjo.in")? + .set_default("v2.ohttp_keys", None::) +} + +/// Handles configuration overrides based on CLI subcommands +fn handle_subcommands(builder: Builder, matches: &ArgMatches) -> Result { + match matches.subcommand() { + Some(("send", _)) => Ok(builder), + Some(("receive", matches)) => { + let builder = handle_receive_command(builder, matches)?; + let max_fee_rate = matches.get_one::("max_fee_rate"); + builder.set_override_option("max_fee_rate", max_fee_rate.map(|f| f.to_string())) + } + #[cfg(feature = "v2")] + Some(("resume", _)) => Ok(builder), + _ => unreachable!(), // If all subcommands are defined above, anything else is unreachabe!() + } +} + +/// Handle configuration overrides specific to the receive command +fn handle_receive_command(builder: Builder, matches: &ArgMatches) -> Result { + #[cfg(not(feature = "v2"))] + let builder = { + let port = matches + .get_one::("port") + .map(|port| port.parse::()) + .transpose() + .map_err(|_| ConfigError::Message("\"port\" must be a valid number".to_string()))?; + builder.set_override_option("v1.port", port)?.set_override_option( + "v1.pj_endpoint", + matches.get_one::("pj_endpoint").map(|s| s.as_str()), + )? + }; + + #[cfg(feature = "v2")] + let builder = { + builder + .set_override_option( + "v2.pj_directory", + matches.get_one::("pj_directory").map(|s| s.as_str()), + )? + .set_override_option( + "v2.ohttp_keys", + matches.get_one::("ohttp_keys").map(|s| s.as_str()), + )? + }; + + Ok(builder) +} + #[cfg(feature = "v2")] fn deserialize_ohttp_keys_from_path<'de, D>( deserializer: D, From 7ddcc589f93cccc24dd1ee6e5b605a7547a2783c Mon Sep 17 00:00:00 2001 From: DanGould Date: Thu, 13 Feb 2025 16:02:14 -0500 Subject: [PATCH 5/7] Abstract bitcoind functions into wallet mod Give an implementer an idea of the abstract functions the wallet needs to implement. This can also become an abstract class in bindings libraries to make it even easier to implement. Hide PSBT parsing, error handling, etc. --- payjoin-cli/src/app/mod.rs | 83 +++------------- payjoin-cli/src/app/v1.rs | 98 +++++-------------- payjoin-cli/src/app/v2.rs | 86 ++++------------- payjoin-cli/src/app/wallet.rs | 175 ++++++++++++++++++++++++++++++++++ 4 files changed, 228 insertions(+), 214 deletions(-) create mode 100644 payjoin-cli/src/app/wallet.rs diff --git a/payjoin-cli/src/app/mod.rs b/payjoin-cli/src/app/mod.rs index 2b7696152..768c4bc35 100644 --- a/payjoin-cli/src/app/mod.rs +++ b/payjoin-cli/src/app/mod.rs @@ -1,20 +1,17 @@ use std::collections::HashMap; -use std::str::FromStr; -use anyhow::{anyhow, Context, Result}; -use bitcoin::psbt::Input as PsbtInput; -use bitcoin::TxIn; +use anyhow::{anyhow, Result}; use bitcoincore_rpc::bitcoin::Amount; -use bitcoincore_rpc::RpcApi; use payjoin::bitcoin::psbt::Psbt; use payjoin::bitcoin::FeeRate; -use payjoin::receive::InputPair; use payjoin::{bitcoin, PjUri}; use tokio::signal; use tokio::sync::watch; pub mod config; +pub mod wallet; use crate::app::config::Config; +use crate::app::wallet::BitcoindWallet; #[cfg(feature = "v1")] pub(crate) mod v1; @@ -29,7 +26,7 @@ pub trait App { fn new(config: Config) -> Result where Self: Sized; - fn bitcoind(&self) -> Result; + fn wallet(&self) -> BitcoindWallet; async fn send_payjoin(&self, bip21: &str, fee_rate: FeeRate) -> Result<()>; async fn receive_payjoin(&self, amount: Amount) -> Result<()>; #[cfg(feature = "v2")] @@ -41,53 +38,18 @@ pub trait App { // wallet_create_funded_psbt requires a HashMap let mut outputs = HashMap::with_capacity(1); outputs.insert(uri.address.to_string(), amount); - let fee_sat_per_kvb = - fee_rate.to_sat_per_kwu().checked_mul(4).ok_or(anyhow!("Invalid fee rate"))?; - let fee_per_kvb = Amount::from_sat(fee_sat_per_kvb); - log::debug!("Fee rate sat/kvb: {}", fee_per_kvb.display_in(bitcoin::Denomination::Satoshi)); - let options = bitcoincore_rpc::json::WalletCreateFundedPsbtOptions { - lock_unspent: Some(true), - fee_rate: Some(fee_per_kvb), - ..Default::default() - }; - let psbt = self - .bitcoind()? - .wallet_create_funded_psbt( - &[], // inputs - &outputs, - None, // locktime - Some(options), - None, - ) - .context("Failed to create PSBT")? - .psbt; - let psbt = self - .bitcoind()? - .wallet_process_psbt(&psbt, None, None, None) - .with_context(|| "Failed to process PSBT")? - .psbt; - let psbt = Psbt::from_str(&psbt).with_context(|| "Failed to load PSBT from base64")?; - log::debug!("Original psbt: {:#?}", psbt); - Ok(psbt) + + self.wallet().create_psbt(outputs, fee_rate, true) } fn process_pj_response(&self, psbt: Psbt) -> Result { log::debug!("Proposed psbt: {:#?}", psbt); - let psbt = self - .bitcoind()? - .wallet_process_psbt(&psbt.to_string(), None, None, None) - .with_context(|| "Failed to process PSBT")? - .psbt; - let tx = self - .bitcoind()? - .finalize_psbt(&psbt, Some(true)) - .with_context(|| "Failed to finalize PSBT")? - .hex - .ok_or_else(|| anyhow!("Incomplete PSBT"))?; - let txid = self - .bitcoind()? - .send_raw_transaction(&tx) - .with_context(|| "Failed to send raw transaction")?; + + let signed = self.wallet().process_psbt(&psbt)?; + let tx = self.wallet().finalize_psbt(&signed)?; + + let txid = self.wallet().broadcast_tx(&tx)?; + println!("Payjoin sent. TXID: {}", txid); Ok(txid) } @@ -120,27 +82,6 @@ fn read_local_cert() -> Result> { Ok(std::fs::read(local_cert_path)?) } -pub fn input_pair_from_list_unspent( - utxo: bitcoincore_rpc::bitcoincore_rpc_json::ListUnspentResultEntry, -) -> InputPair { - let psbtin = PsbtInput { - // NOTE: non_witness_utxo is not necessary because bitcoin-cli always supplies - // witness_utxo, even for non-witness inputs - witness_utxo: Some(bitcoin::TxOut { - value: utxo.amount, - script_pubkey: utxo.script_pub_key.clone(), - }), - redeem_script: utxo.redeem_script.clone(), - witness_script: utxo.witness_script.clone(), - ..Default::default() - }; - let txin = TxIn { - previous_output: bitcoin::OutPoint { txid: utxo.txid, vout: utxo.vout }, - ..Default::default() - }; - InputPair::new(txin, psbtin).expect("Input pair should be valid") -} - async fn handle_interrupt(tx: watch::Sender<()>) { if let Err(e) = signal::ctrl_c().await { eprintln!("Error setting up Ctrl-C handler: {}", e); diff --git a/payjoin-cli/src/app/v1.rs b/payjoin-cli/src/app/v1.rs index 652f04c17..1baaa3966 100644 --- a/payjoin-cli/src/app/v1.rs +++ b/payjoin-cli/src/app/v1.rs @@ -5,7 +5,6 @@ use std::sync::Arc; use anyhow::{anyhow, Context, Result}; use bitcoincore_rpc::bitcoin::Amount; -use bitcoincore_rpc::RpcApi; use http_body_util::combinators::BoxBody; use http_body_util::{BodyExt, Full}; use hyper::body::{Buf, Bytes, Incoming}; @@ -14,7 +13,7 @@ use hyper::service::service_fn; use hyper::{Method, Request, Response, StatusCode}; use hyper_util::rt::TokioIo; use payjoin::bitcoin::psbt::Psbt; -use payjoin::bitcoin::{self, FeeRate}; +use payjoin::bitcoin::FeeRate; use payjoin::receive::v1::{PayjoinProposal, UncheckedProposal}; use payjoin::receive::ImplementationError; use payjoin::receive::ReplyableError::{self, Implementation, V1}; @@ -24,8 +23,9 @@ use tokio::net::TcpListener; use tokio::sync::watch; use super::config::Config; +use super::wallet::BitcoindWallet; use super::App as AppTrait; -use crate::app::{handle_interrupt, http_agent, input_pair_from_list_unspent}; +use crate::app::{handle_interrupt, http_agent}; use crate::db::Database; #[cfg(feature = "_danger-local-https")] pub const LOCAL_CERT_FILE: &str = "localhost.der"; @@ -41,6 +41,7 @@ impl payjoin::receive::v1::Headers for Headers<'_> { pub(crate) struct App { config: Config, db: Arc, + wallet: BitcoindWallet, interrupt: watch::Receiver<()>, } @@ -50,29 +51,15 @@ impl AppTrait for App { let db = Arc::new(Database::create(&config.db_path)?); let (interrupt_tx, interrupt_rx) = watch::channel(()); tokio::spawn(handle_interrupt(interrupt_tx)); - let app = Self { config, db, interrupt: interrupt_rx }; - app.bitcoind()? - .get_blockchain_info() + let wallet = BitcoindWallet::new(&config.bitcoind)?; + let app = Self { config, db, wallet, interrupt: interrupt_rx }; + app.wallet() + .network() .context("Failed to connect to bitcoind. Check config RPC connection.")?; Ok(app) } - fn bitcoind(&self) -> Result { - match &self.config.bitcoind.cookie { - Some(cookie) => bitcoincore_rpc::Client::new( - self.config.bitcoind.rpchost.as_str(), - bitcoincore_rpc::Auth::CookieFile(cookie.into()), - ), - None => bitcoincore_rpc::Client::new( - self.config.bitcoind.rpchost.as_str(), - bitcoincore_rpc::Auth::UserPass( - self.config.bitcoind.rpcuser.clone(), - self.config.bitcoind.rpcpassword.clone(), - ), - ), - } - .with_context(|| "Failed to connect to bitcoind") - } + fn wallet(&self) -> BitcoindWallet { self.wallet.clone() } async fn send_payjoin(&self, bip21: &str, fee_rate: FeeRate) -> Result<()> { let uri = @@ -143,7 +130,7 @@ impl App { amount: Amount, fallback_target: Option<&str>, ) -> Result { - let pj_receiver_address = self.bitcoind()?.get_new_address(None, None)?.assume_checked(); + let pj_receiver_address = self.wallet.get_new_address()?; let pj_part = match fallback_target { Some(target) => target, None => self.config.v1.pj_endpoint.as_str(), @@ -263,12 +250,7 @@ impl App { &self, amount: Option, ) -> Result>, ReplyableError> { - let address = self - .bitcoind() - .map_err(|e| Implementation(e.into()))? - .get_new_address(None, None) - .map_err(|e| Implementation(e.into()))? - .assume_checked(); + let address = self.wallet.get_new_address().map_err(|e| Implementation(e.into()))?; let uri_string = if let Some(amount) = amount { format!( "{}?amount={}&pj={}", @@ -310,38 +292,18 @@ impl App { &self, proposal: UncheckedProposal, ) -> Result { - let bitcoind = self.bitcoind().map_err(|e| Implementation(e.into()))?; + let wallet = self.wallet(); // in a payment processor where the sender could go offline, this is where you schedule to broadcast the original_tx let _to_broadcast_in_failure_case = proposal.extract_tx_to_schedule_broadcast(); - // The network is used for checks later - let network = bitcoind.get_blockchain_info().map_err(|e| Implementation(e.into()))?.chain; - // Receive Check 1: Can Broadcast let proposal = - proposal.check_broadcast_suitability(None, |tx| { - let raw_tx = bitcoin::consensus::encode::serialize_hex(&tx); - let mempool_results = bitcoind - .test_mempool_accept(&[raw_tx]) - .map_err(|e| Implementation(e.into()))?; - match mempool_results.first() { - Some(result) => Ok(result.allowed), - None => Err(ImplementationError::from( - "No mempool results returned on broadcast check", - )), - } - })?; + proposal.check_broadcast_suitability(None, |tx| Ok(wallet.can_broadcast(tx)?))?; log::trace!("check1"); // Receive Check 2: receiver can't sign for proposal inputs - let proposal = proposal.check_inputs_not_owned(|input| { - if let Ok(address) = bitcoin::Address::from_script(input, network) { - Ok(bitcoind.get_address_info(&address).map(|info| info.is_mine.unwrap_or(false))?) - } else { - Ok(false) - } - })?; + let proposal = proposal.check_inputs_not_owned(|input| Ok(wallet.is_mine(input)?))?; log::trace!("check2"); // Receive Check 3: have we seen this input before? More of a check for non-interactive i.e. payment processor receivers. @@ -349,35 +311,25 @@ impl App { .check_no_inputs_seen_before(|input| Ok(self.db.insert_input_seen_before(*input)?))?; log::trace!("check3"); - let payjoin = payjoin.identify_receiver_outputs(|output_script| { - if let Ok(address) = bitcoin::Address::from_script(output_script, network) { - Ok(bitcoind.get_address_info(&address).map(|info| info.is_mine.unwrap_or(false))?) - } else { - Ok(false) - } - })?; + let payjoin = payjoin + .identify_receiver_outputs(|output_script| Ok(wallet.is_mine(output_script)?))?; let payjoin = payjoin .substitute_receiver_script( - &bitcoind - .get_new_address(None, None) - .map_err(|e| Implementation(e.into()))? - .require_network(network) + &self + .wallet + .get_new_address() .map_err(|e| Implementation(e.into()))? .script_pubkey(), ) .map_err(|e| Implementation(e.into()))? .commit_outputs(); - let provisional_payjoin = try_contributing_inputs(payjoin.clone(), &bitcoind) + let provisional_payjoin = try_contributing_inputs(payjoin.clone(), &self.wallet) .map_err(ReplyableError::Implementation)?; let payjoin_proposal = provisional_payjoin.finalize_proposal( - |psbt: &Psbt| { - let res = - bitcoind.wallet_process_psbt(&psbt.to_string(), None, None, Some(false))?; - Ok(Psbt::from_str(&res.psbt)?) - }, + |psbt| Ok(self.wallet.process_psbt(psbt)?), None, self.config.max_fee_rate, )?; @@ -387,13 +339,9 @@ impl App { fn try_contributing_inputs( payjoin: payjoin::receive::v1::WantsInputs, - bitcoind: &bitcoincore_rpc::Client, + wallet: &BitcoindWallet, ) -> Result { - let candidate_inputs = bitcoind - .list_unspent(None, None, None, None, None) - .map_err(ImplementationError::from)? - .into_iter() - .map(input_pair_from_list_unspent); + let candidate_inputs = wallet.list_unspent()?; let selected_input = payjoin.try_preserving_privacy(candidate_inputs).map_err(ImplementationError::from)?; diff --git a/payjoin-cli/src/app/v2.rs b/payjoin-cli/src/app/v2.rs index b2d0bfef2..5c8c2abb8 100644 --- a/payjoin-cli/src/app/v2.rs +++ b/payjoin-cli/src/app/v2.rs @@ -1,26 +1,26 @@ -use std::str::FromStr; use std::sync::Arc; use anyhow::{anyhow, Context, Result}; -use bitcoincore_rpc::RpcApi; use payjoin::bitcoin::consensus::encode::serialize_hex; use payjoin::bitcoin::psbt::Psbt; use payjoin::bitcoin::{Amount, FeeRate}; use payjoin::receive::v2::{Receiver, UncheckedProposal}; use payjoin::receive::{Error, ImplementationError, ReplyableError}; use payjoin::send::v2::{Sender, SenderBuilder}; -use payjoin::{bitcoin, Uri}; +use payjoin::Uri; use tokio::sync::watch; use super::config::Config; +use super::wallet::BitcoindWallet; use super::App as AppTrait; -use crate::app::{handle_interrupt, http_agent, input_pair_from_list_unspent}; +use crate::app::{handle_interrupt, http_agent}; use crate::db::Database; #[derive(Clone)] pub(crate) struct App { config: Config, db: Arc, + wallet: BitcoindWallet, interrupt: watch::Receiver<()>, } @@ -30,29 +30,15 @@ impl AppTrait for App { let db = Arc::new(Database::create(&config.db_path)?); let (interrupt_tx, interrupt_rx) = watch::channel(()); tokio::spawn(handle_interrupt(interrupt_tx)); - let app = Self { config, db, interrupt: interrupt_rx }; - app.bitcoind()? - .get_blockchain_info() + let wallet = BitcoindWallet::new(&config.bitcoind)?; + let app = Self { config, db, wallet, interrupt: interrupt_rx }; + app.wallet() + .network() .context("Failed to connect to bitcoind. Check config RPC connection.")?; Ok(app) } - fn bitcoind(&self) -> Result { - match &self.config.bitcoind.cookie { - Some(cookie) => bitcoincore_rpc::Client::new( - self.config.bitcoind.rpchost.as_str(), - bitcoincore_rpc::Auth::CookieFile(cookie.into()), - ), - None => bitcoincore_rpc::Client::new( - self.config.bitcoind.rpchost.as_str(), - bitcoincore_rpc::Auth::UserPass( - self.config.bitcoind.rpcuser.clone(), - self.config.bitcoind.rpcpassword.clone(), - ), - ), - } - .with_context(|| "Failed to connect to bitcoind") - } + fn wallet(&self) -> BitcoindWallet { self.wallet.clone() } async fn send_payjoin(&self, bip21: &str, fee_rate: FeeRate) -> Result<()> { use payjoin::UriExt; @@ -77,7 +63,7 @@ impl AppTrait for App { } async fn receive_payjoin(&self, amount: Amount) -> Result<()> { - let address = self.bitcoind()?.get_new_address(None, None)?.assume_checked(); + let address = self.wallet().get_new_address()?; let ohttp_keys = unwrap_ohttp_keys_or_else_fetch(&self.config).await?; let session = Receiver::new(address, self.config.v2.pj_directory.clone(), ohttp_keys.clone(), None)?; @@ -254,38 +240,18 @@ impl App { &self, proposal: payjoin::receive::v2::UncheckedProposal, ) -> Result { - let bitcoind = self.bitcoind().map_err(|e| ReplyableError::Implementation(e.into()))?; + let wallet = self.wallet(); // in a payment processor where the sender could go offline, this is where you schedule to broadcast the original_tx let _to_broadcast_in_failure_case = proposal.extract_tx_to_schedule_broadcast(); - // The network is used for checks later - let network = bitcoind - .get_blockchain_info() - .map_err(|e| ReplyableError::Implementation(e.into()))? - .chain; // Receive Check 1: Can Broadcast let proposal = - proposal.check_broadcast_suitability(None, |tx| { - let raw_tx = bitcoin::consensus::encode::serialize_hex(&tx); - let mempool_results = bitcoind.test_mempool_accept(&[raw_tx])?; - match mempool_results.first() { - Some(result) => Ok(result.allowed), - None => Err(ImplementationError::from( - "No mempool results returned on broadcast check", - )), - } - })?; + proposal.check_broadcast_suitability(None, |tx| Ok(wallet.can_broadcast(tx)?))?; log::trace!("check1"); // Receive Check 2: receiver can't sign for proposal inputs - let proposal = proposal.check_inputs_not_owned(|input| { - if let Ok(address) = bitcoin::Address::from_script(input, network) { - Ok(bitcoind.get_address_info(&address).map(|info| info.is_mine.unwrap_or(false))?) - } else { - Ok(false) - } - })?; + let proposal = proposal.check_inputs_not_owned(|input| Ok(wallet.is_mine(input)?))?; log::trace!("check2"); // Receive Check 3: have we seen this input before? More of a check for non-interactive i.e. payment processor receivers. @@ -294,26 +260,14 @@ impl App { log::trace!("check3"); let payjoin = payjoin - .identify_receiver_outputs(|output_script| { - if let Ok(address) = bitcoin::Address::from_script(output_script, network) { - Ok(bitcoind - .get_address_info(&address) - .map(|info| info.is_mine.unwrap_or(false))?) - } else { - Ok(false) - } - })? + .identify_receiver_outputs(|output_script| Ok(wallet.is_mine(output_script)?))? .commit_outputs(); - let provisional_payjoin = try_contributing_inputs(payjoin.clone(), &bitcoind) + let provisional_payjoin = try_contributing_inputs(payjoin.clone(), &wallet) .map_err(ReplyableError::Implementation)?; let payjoin_proposal = provisional_payjoin.finalize_proposal( - |psbt: &Psbt| { - let res = - bitcoind.wallet_process_psbt(&psbt.to_string(), None, None, Some(false))?; - Ok(Psbt::from_str(&res.psbt)?) - }, + |psbt| Ok(wallet.process_psbt(psbt)?), None, self.config.max_fee_rate, )?; @@ -353,13 +307,9 @@ async fn handle_recoverable_error( fn try_contributing_inputs( payjoin: payjoin::receive::v2::WantsInputs, - bitcoind: &bitcoincore_rpc::Client, + wallet: &BitcoindWallet, ) -> Result { - let candidate_inputs = bitcoind - .list_unspent(None, None, None, None, None) - .map_err(ImplementationError::from)? - .into_iter() - .map(input_pair_from_list_unspent); + let candidate_inputs = wallet.list_unspent()?; let selected_input = payjoin.try_preserving_privacy(candidate_inputs).map_err(ImplementationError::from)?; diff --git a/payjoin-cli/src/app/wallet.rs b/payjoin-cli/src/app/wallet.rs new file mode 100644 index 000000000..d496bbe41 --- /dev/null +++ b/payjoin-cli/src/app/wallet.rs @@ -0,0 +1,175 @@ +use std::collections::HashMap; +use std::str::FromStr; +use std::sync::Arc; + +use anyhow::{anyhow, Context, Result}; +use bitcoincore_rpc::json::WalletCreateFundedPsbtOptions; +use bitcoincore_rpc::{Auth, Client, RpcApi}; +use payjoin::bitcoin::consensus::encode::{deserialize, serialize_hex}; +use payjoin::bitcoin::consensus::Encodable; +use payjoin::bitcoin::psbt::{Input, Psbt}; +use payjoin::bitcoin::{ + Address, Amount, Denomination, FeeRate, Network, OutPoint, Script, Transaction, TxIn, TxOut, + Txid, +}; +use payjoin::receive::InputPair; + +/// Implementation of PayjoinWallet for bitcoind +#[derive(Clone, Debug)] +pub struct BitcoindWallet { + pub bitcoind: std::sync::Arc, +} + +impl BitcoindWallet { + pub fn new(config: &crate::app::config::BitcoindConfig) -> Result { + let client = match &config.cookie { + Some(cookie) => Client::new(config.rpchost.as_str(), Auth::CookieFile(cookie.into())), + None => Client::new( + config.rpchost.as_str(), + Auth::UserPass(config.rpcuser.clone(), config.rpcpassword.clone()), + ), + }?; + Ok(Self { bitcoind: Arc::new(client) }) + } +} + +impl BitcoindWallet { + /// Create a PSBT with the given outputs and fee rate + pub fn create_psbt( + &self, + outputs: HashMap, + fee_rate: FeeRate, + lock_unspent: bool, + ) -> Result { + let fee_sat_per_kvb = + fee_rate.to_sat_per_kwu().checked_mul(4).ok_or_else(|| anyhow!("Invalid fee rate"))?; + let fee_per_kvb = Amount::from_sat(fee_sat_per_kvb); + log::debug!("Fee rate sat/kvb: {}", fee_per_kvb.display_in(Denomination::Satoshi)); + + let options = WalletCreateFundedPsbtOptions { + lock_unspent: Some(lock_unspent), + fee_rate: Some(fee_per_kvb), + ..Default::default() + }; + + let psbt = self + .bitcoind + .wallet_create_funded_psbt( + &[], // inputs + &outputs, + None, // locktime + Some(options), + None, + ) + .context("Failed to create PSBT")? + .psbt; + + let psbt = self + .bitcoind + .wallet_process_psbt(&psbt, None, None, None) + .context("Failed to process PSBT")? + .psbt; + + Psbt::from_str(&psbt).context("Failed to load PSBT from base64") + } + + /// Process a PSBT, validating and signing inputs owned by this wallet + /// + /// Does not include bip32 derivations in the PSBT + pub fn process_psbt(&self, psbt: &Psbt) -> Result { + let psbt_str = psbt.to_string(); + let processed = self + .bitcoind + .wallet_process_psbt(&psbt_str, None, None, Some(false)) + .context("Failed to process PSBT")? + .psbt; + Psbt::from_str(&processed).context("Failed to parse processed PSBT") + } + + /// Finalize a PSBT and extract the transaction + pub fn finalize_psbt(&self, psbt: &Psbt) -> Result { + let result = self + .bitcoind + .finalize_psbt(&psbt.to_string(), Some(true)) + .context("Failed to finalize PSBT")?; + let tx = deserialize(&result.hex.ok_or_else(|| anyhow!("Incomplete PSBT"))?)?; + Ok(tx) + } + + pub fn can_broadcast(&self, tx: &Transaction) -> Result { + let raw_tx = serialize_hex(&tx); + let mempool_results = self.bitcoind.test_mempool_accept(&[raw_tx])?; + match mempool_results.first() { + Some(result) => Ok(result.allowed), + None => Err(anyhow!("No mempool results returned on broadcast check",)), + } + } + + /// Broadcast a raw transaction + pub fn broadcast_tx(&self, tx: &Transaction) -> Result { + let mut serialized_tx = Vec::new(); + tx.consensus_encode(&mut serialized_tx)?; + self.bitcoind + .send_raw_transaction(&serialized_tx) + .context("Failed to broadcast transaction") + } + + /// Check if a script belongs to this wallet + pub fn is_mine(&self, script: &Script) -> Result { + if let Ok(address) = Address::from_script(script, self.network()?) { + self.bitcoind + .get_address_info(&address) + .map(|info| info.is_mine.unwrap_or(false)) + .context("Failed to get address info") + } else { + Ok(false) + } + } + + /// Get a new address from the wallet + pub fn get_new_address(&self) -> Result
{ + self.bitcoind + .get_new_address(None, None) + .context("Failed to get new address")? + .require_network(self.network()?) + .context("Invalid network for address") + } + + /// List unspent UTXOs + pub fn list_unspent(&self) -> Result> { + let unspent = self + .bitcoind + .list_unspent(None, None, None, None, None) + .context("Failed to list unspent")?; + Ok(unspent.into_iter().map(input_pair_from_list_unspent).collect()) + } + + /// Get the network this wallet is operating on + pub fn network(&self) -> Result { + self.bitcoind + .get_blockchain_info() + .map_err(|_| anyhow!("Failed to get blockchain info")) + .map(|info| info.chain) + } +} + +pub fn input_pair_from_list_unspent( + utxo: bitcoincore_rpc::bitcoincore_rpc_json::ListUnspentResultEntry, +) -> InputPair { + let psbtin = Input { + // NOTE: non_witness_utxo is not necessary because bitcoin-cli always supplies + // witness_utxo, even for non-witness inputs + witness_utxo: Some(TxOut { + value: utxo.amount, + script_pubkey: utxo.script_pub_key.clone(), + }), + redeem_script: utxo.redeem_script.clone(), + witness_script: utxo.witness_script.clone(), + ..Default::default() + }; + let txin = TxIn { + previous_output: OutPoint { txid: utxo.txid, vout: utxo.vout }, + ..Default::default() + }; + InputPair::new(txin, psbtin).expect("Input pair should be valid") +} From c4266ea46859b56ae7145a5d2785f17702e37332 Mon Sep 17 00:00:00 2001 From: DanGould Date: Thu, 13 Feb 2025 16:30:04 -0500 Subject: [PATCH 6/7] Make payjoin-cli features additive Make use of the AppTrait and a `--bip78` cli argument to allow both to be compiled into the same binary and a user to choose the protocol at runtime. Parse only one config version "mode" at runtime so that errors in a version unused at runtime will not cause errors. Use a determine_version fn in config so that the default behavior without an explicit version flag runs the highest version available. Use argument matches in main to reduce presedence if multiple versions are available. --- payjoin-cli/contrib/test.sh | 3 +- payjoin-cli/example.config.toml | 38 ++++---- payjoin-cli/src/app/config.rs | 162 ++++++++++++++++++++++++++++---- payjoin-cli/src/app/mod.rs | 2 +- payjoin-cli/src/app/v1.rs | 12 ++- payjoin-cli/src/app/v2.rs | 24 +++-- payjoin-cli/src/main.rs | 49 +++++++++- payjoin-cli/tests/e2e.rs | 8 +- 8 files changed, 237 insertions(+), 61 deletions(-) diff --git a/payjoin-cli/contrib/test.sh b/payjoin-cli/contrib/test.sh index 71ffc5952..d59518991 100755 --- a/payjoin-cli/contrib/test.sh +++ b/payjoin-cli/contrib/test.sh @@ -1,5 +1,4 @@ #!/usr/bin/env bash set -e -cargo test --locked --package payjoin-cli --verbose --no-default-features --features=_danger-local-https,v2 --test e2e -cargo test --locked --package payjoin-cli --verbose --no-default-features --features=v1,_danger-local-https +cargo test --locked --package payjoin-cli --verbose --all-features diff --git a/payjoin-cli/example.config.toml b/payjoin-cli/example.config.toml index a5264507f..2b3ba5b50 100644 --- a/payjoin-cli/example.config.toml +++ b/payjoin-cli/example.config.toml @@ -37,24 +37,20 @@ rpcuser = "user" # The rpcpassword of the user to connect to (specified in bitcoin.conf). rpcpassword = "password" -# Version 1 Settings -# ----------------- -[v1] -# (v1, receiver only) The port for v1 receiving servers to bind to. -port = 3000 - -# (v1, receiver only) The payjoin endpoint which coordinates the transaction. -pj_endpoint = "https://localhost:3000" - -# Version 2 Settings -# ----------------- -[v2] -# (v2 only) The payjoin directory to rendezvous at. -pj_directory = "https://payjo.in" - -# (v2 only) The OHTTP relay that will forward requests to the OHTTP Gateway, which will forward to the pj_endpoint directory. -ohttp_relay = "https://pj.bobspacebkk.com" - -# (v2 only, optional) The HPKE keys which need to be fetched ahead of time from the pj_endpoint for the payjoin packets to be encrypted. -# These can now be fetched and no longer need to be configured. -ohttp_keys = "./path/to/ohttp_keys" +# Version Configuration +# ------------------- +# Uncomment ONE of the following version configurations depending on which version you want to use + +# Version 1 Configuration +# [v1] +# port = 3000 +# pj_endpoint = "https://localhost:3000" + +# Version 2 Configuration +# [v2] +# pj_directory = "https://payjo.in" +# ohttp_relay = "https://pj.bobspacebkk.com" +# # Optional: The HPKE keys which need to be fetched ahead of time from the pj_endpoint +# # for the payjoin packets to be encrypted. +# # These can now be fetched and no longer need to be configured. +# ohttp_keys = "./path/to/ohttp_keys" diff --git a/payjoin-cli/src/app/config.rs b/payjoin-cli/src/app/config.rs index 9d67913d1..b6cbc52c3 100644 --- a/payjoin-cli/src/app/config.rs +++ b/payjoin-cli/src/app/config.rs @@ -36,40 +36,170 @@ pub struct V2Config { pub pj_directory: Url, } +#[allow(clippy::large_enum_variant)] +#[derive(Debug, Clone, Deserialize)] +#[serde(tag = "version")] +pub enum VersionConfig { + #[cfg(feature = "v1")] + #[serde(rename = "v1")] + V1(V1Config), + #[cfg(feature = "v2")] + #[serde(rename = "v2")] + V2(V2Config), +} + #[derive(Debug, Clone, Deserialize)] pub struct Config { pub db_path: PathBuf, pub max_fee_rate: Option, pub bitcoind: BitcoindConfig, - #[cfg(feature = "v1")] - pub v1: V1Config, - #[cfg(feature = "v2")] - pub v2: V2Config, + #[serde(skip)] + pub version: Option, } impl Config { + /// Version flags in order of precedence (newest to oldest) + const VERSION_FLAGS: &'static [(&'static str, u8)] = &[("bip77", 2), ("bip78", 1)]; + + /// Check for multiple version flags and return the highest precedence version + fn determine_version(matches: &ArgMatches) -> Result { + let mut selected_version = None; + for &(flag, version) in Self::VERSION_FLAGS.iter() { + if matches.get_flag(flag) { + if selected_version.is_some() { + return Err(ConfigError::Message(format!( + "Multiple version flags specified. Please use only one of: {}", + Self::VERSION_FLAGS + .iter() + .map(|(flag, _)| format!("--{}", flag)) + .collect::>() + .join(", ") + ))); + } + selected_version = Some(version); + } + } + + if let Some(version) = selected_version { + return Ok(version); + } + + #[cfg(feature = "v2")] + return Ok(2); + #[cfg(all(feature = "v1", not(feature = "v2")))] + return Ok(1); + + #[cfg(not(any(feature = "v1", feature = "v2")))] + return Err(ConfigError::Message( + "No valid version available - must compile with v1 or v2 feature".to_string(), + )); + } + pub(crate) fn new(matches: &ArgMatches) -> Result { let mut builder = config::Config::builder(); builder = add_bitcoind_defaults(builder, matches)?; builder = add_common_defaults(builder, matches)?; - #[cfg(feature = "v1")] - { - builder = add_v1_defaults(builder)?; - } + let version = Self::determine_version(matches)?; - #[cfg(feature = "v2")] - { - builder = add_v2_defaults(builder, matches)?; + match version { + 1 => { + #[cfg(feature = "v1")] + { + builder = add_v1_defaults(builder)?; + } + #[cfg(not(feature = "v1"))] + return Err(ConfigError::Message( + "BIP78 (v1) selected but v1 feature not enabled".to_string(), + )); + } + 2 => { + #[cfg(feature = "v2")] + { + builder = add_v2_defaults(builder, matches)?; + } + #[cfg(not(feature = "v2"))] + return Err(ConfigError::Message( + "BIP77 (v2) selected but v2 feature not enabled".to_string(), + )); + } + _ => unreachable!("determine_version() should only return 1 or 2"), } builder = handle_subcommands(builder, matches)?; builder = builder.add_source(File::new("config.toml", FileFormat::Toml).required(false)); - let config = builder.build()?; - let app_config: Config = config.try_deserialize()?; - log::debug!("App config: {:?}", app_config); - Ok(app_config) + let built_config = builder.build()?; + + let mut config = Config { + db_path: built_config.get("db_path")?, + max_fee_rate: built_config.get("max_fee_rate").ok(), + bitcoind: built_config.get("bitcoind")?, + version: None, + }; + + match version { + 1 => { + #[cfg(feature = "v1")] + { + if let Ok(v1) = built_config.get::("v1") { + config.version = Some(VersionConfig::V1(v1)); + } else { + return Err(ConfigError::Message( + "V1 configuration is required for BIP78 mode".to_string(), + )); + } + } + #[cfg(not(feature = "v1"))] + return Err(ConfigError::Message( + "BIP78 (v1) selected but v1 feature not enabled".to_string(), + )); + } + 2 => { + #[cfg(feature = "v2")] + { + if let Ok(v2) = built_config.get::("v2") { + config.version = Some(VersionConfig::V2(v2)); + } else { + return Err(ConfigError::Message( + "V2 configuration is required for BIP77 mode".to_string(), + )); + } + } + #[cfg(not(feature = "v2"))] + return Err(ConfigError::Message( + "BIP77 (v2) selected but v2 feature not enabled".to_string(), + )); + } + _ => unreachable!("determine_version() should only return 1 or 2"), + } + + if config.version.is_none() { + return Err(ConfigError::Message( + "No valid version configuration found for the specified mode".to_string(), + )); + } + + log::debug!("App config: {:?}", config); + Ok(config) + } + + #[cfg(feature = "v1")] + pub fn v1(&self) -> Result<&V1Config, anyhow::Error> { + match &self.version { + Some(VersionConfig::V1(v1_config)) => Ok(v1_config), + #[allow(unreachable_patterns)] + _ => Err(anyhow::anyhow!("V1 configuration is required for BIP78 mode")), + } + } + + #[cfg(feature = "v2")] + pub fn v2(&self) -> Result<&V2Config, anyhow::Error> { + match &self.version { + Some(VersionConfig::V2(v2_config)) => Ok(v2_config), + #[allow(unreachable_patterns)] + _ => Err(anyhow::anyhow!("V2 configuration is required for v2 mode")), + } } } @@ -142,7 +272,7 @@ fn handle_subcommands(builder: Builder, matches: &ArgMatches) -> Result Result { - #[cfg(not(feature = "v2"))] + #[cfg(feature = "v1")] let builder = { let port = matches .get_one::("port") diff --git a/payjoin-cli/src/app/mod.rs b/payjoin-cli/src/app/mod.rs index 768c4bc35..050b13b31 100644 --- a/payjoin-cli/src/app/mod.rs +++ b/payjoin-cli/src/app/mod.rs @@ -22,7 +22,7 @@ pub(crate) mod v2; pub const LOCAL_CERT_FILE: &str = "localhost.der"; #[async_trait::async_trait] -pub trait App { +pub trait App: Send + Sync { fn new(config: Config) -> Result where Self: Sized; diff --git a/payjoin-cli/src/app/v1.rs b/payjoin-cli/src/app/v1.rs index 1baaa3966..b2b563814 100644 --- a/payjoin-cli/src/app/v1.rs +++ b/payjoin-cli/src/app/v1.rs @@ -100,11 +100,12 @@ impl AppTrait for App { Ok(()) } + #[allow(clippy::incompatible_msrv)] async fn receive_payjoin(&self, amount: Amount) -> Result<()> { let pj_uri_string = self.construct_payjoin_uri(amount, None)?; println!( "Listening at {}. Configured to accept payjoin at BIP 21 Payjoin Uri:", - self.config.v1.port + self.config.v1()?.port ); println!("{}", pj_uri_string); @@ -133,7 +134,7 @@ impl App { let pj_receiver_address = self.wallet.get_new_address()?; let pj_part = match fallback_target { Some(target) => target, - None => self.config.v1.pj_endpoint.as_str(), + None => self.config.v1()?.pj_endpoint.as_str(), }; let pj_part = payjoin::Url::parse(pj_part) .map_err(|e| anyhow!("Failed to parse pj_endpoint: {}", e))?; @@ -146,7 +147,7 @@ impl App { } async fn start_http_server(&self) -> Result<()> { - let addr = SocketAddr::from(([0, 0, 0, 0], self.config.v1.port)); + let addr = SocketAddr::from(([0, 0, 0, 0], self.config.v1()?.port)); let listener = TcpListener::bind(addr).await?; let app = self.clone(); @@ -250,16 +251,17 @@ impl App { &self, amount: Option, ) -> Result>, ReplyableError> { + let v1_config = self.config.v1().map_err(|e| Implementation(e.into()))?; let address = self.wallet.get_new_address().map_err(|e| Implementation(e.into()))?; let uri_string = if let Some(amount) = amount { format!( "{}?amount={}&pj={}", address.to_qr_uri(), amount.to_btc(), - self.config.v1.pj_endpoint + v1_config.pj_endpoint ) } else { - format!("{}?pj={}", address.to_qr_uri(), self.config.v1.pj_endpoint) + format!("{}?pj={}", address.to_qr_uri(), v1_config.pj_endpoint) }; let uri = Uri::try_from(uri_string.clone()) .map_err(|_| Implementation(anyhow!("Could not parse payjoin URI string.").into()))?; diff --git a/payjoin-cli/src/app/v2.rs b/payjoin-cli/src/app/v2.rs index 5c8c2abb8..dad11fb80 100644 --- a/payjoin-cli/src/app/v2.rs +++ b/payjoin-cli/src/app/v2.rs @@ -65,8 +65,12 @@ impl AppTrait for App { async fn receive_payjoin(&self, amount: Amount) -> Result<()> { let address = self.wallet().get_new_address()?; let ohttp_keys = unwrap_ohttp_keys_or_else_fetch(&self.config).await?; - let session = - Receiver::new(address, self.config.v2.pj_directory.clone(), ohttp_keys.clone(), None)?; + let session = Receiver::new( + address, + self.config.v2()?.pj_directory.clone(), + ohttp_keys.clone(), + None, + )?; self.db.insert_recv_session(session.clone())?; self.spawn_payjoin_receiver(session, Some(amount)).await } @@ -155,13 +159,13 @@ impl App { Ok(proposal) => proposal, Err(Error::ReplyToSender(e)) => { return Err( - handle_recoverable_error(e, receiver, &self.config.v2.ohttp_relay).await + handle_recoverable_error(e, receiver, &self.config.v2()?.ohttp_relay).await ); } Err(e) => return Err(e.into()), }; let (req, ohttp_ctx) = payjoin_proposal - .extract_v2_req(&self.config.v2.ohttp_relay) + .extract_v2_req(&self.config.v2()?.ohttp_relay) .map_err(|e| anyhow!("v2 req extraction failed {}", e))?; println!("Got a request from the sender. Responding with a Payjoin proposal."); let res = post_request(req).await?; @@ -178,7 +182,7 @@ impl App { } async fn long_poll_post(&self, req_ctx: &mut Sender) -> Result { - match req_ctx.extract_v2(self.config.v2.ohttp_relay.clone()) { + match req_ctx.extract_v2(self.config.v2()?.ohttp_relay.clone()) { Ok((req, ctx)) => { println!("Posting Original PSBT Payload request..."); let response = post_request(req).await?; @@ -186,7 +190,7 @@ impl App { let v2_ctx = Arc::new(ctx.process_response(&response.bytes().await?)?); loop { let (req, ohttp_ctx) = - v2_ctx.extract_req(self.config.v2.ohttp_relay.clone())?; + v2_ctx.extract_req(self.config.v2()?.ohttp_relay.clone())?; let response = post_request(req).await?; match v2_ctx.process_response(&response.bytes().await?, ohttp_ctx) { Ok(Some(psbt)) => return Ok(psbt), @@ -223,7 +227,7 @@ impl App { session: &mut payjoin::receive::v2::Receiver, ) -> Result { loop { - let (req, context) = session.extract_req(&self.config.v2.ohttp_relay)?; + let (req, context) = session.extract_req(&self.config.v2()?.ohttp_relay)?; println!("Polling receive request..."); let ohttp_response = post_request(req).await?; let proposal = session @@ -321,13 +325,13 @@ fn try_contributing_inputs( } async fn unwrap_ohttp_keys_or_else_fetch(config: &Config) -> Result { - if let Some(keys) = config.v2.ohttp_keys.clone() { + if let Some(keys) = config.v2()?.ohttp_keys.clone() { println!("Using OHTTP Keys from config"); Ok(keys) } else { println!("Bootstrapping private network transport over Oblivious HTTP"); - let ohttp_relay = config.v2.ohttp_relay.clone(); - let payjoin_directory = config.v2.pj_directory.clone(); + let ohttp_relay = config.v2()?.ohttp_relay.clone(); + let payjoin_directory = config.v2()?.pj_directory.clone(); #[cfg(feature = "_danger-local-https")] let ohttp_keys = { let cert_der = crate::app::read_local_cert()?; diff --git a/payjoin-cli/src/main.rs b/payjoin-cli/src/main.rs index 89f6c58a1..2851d123b 100644 --- a/payjoin-cli/src/main.rs +++ b/payjoin-cli/src/main.rs @@ -15,7 +15,31 @@ async fn main() -> Result<()> { let matches = cli(); let config = Config::new(&matches)?; - let app: Box = { + + #[allow(clippy::if_same_then_else)] + let app: Box = if matches.get_flag("bip78") { + #[cfg(feature = "v1")] + { + Box::new(crate::app::v1::App::new(config)?) + } + #[cfg(not(feature = "v1"))] + { + anyhow::bail!( + "BIP78 (v1) support is not enabled in this build. Recompile with --features v1" + ) + } + } else if matches.get_flag("bip77") { + #[cfg(feature = "v2")] + { + Box::new(crate::app::v2::App::new(config)?) + } + #[cfg(not(feature = "v2"))] + { + anyhow::bail!( + "BIP77 (v2) support is not enabled in this build. Recompile with --features v2" + ) + } + } else { #[cfg(feature = "v2")] { Box::new(crate::app::v2::App::new(config)?) @@ -25,7 +49,9 @@ async fn main() -> Result<()> { Box::new(crate::app::v1::App::new(config)?) } #[cfg(not(any(feature = "v1", feature = "v2")))] - compile_error!("Either feature \"v1\" or \"v2\" must be enabled"); + { + anyhow::bail!("No valid version available - must compile with v1 or v2 feature") + } }; match matches.subcommand() { @@ -43,6 +69,9 @@ async fn main() -> Result<()> { } #[cfg(feature = "v2")] Some(("resume", _)) => { + if matches.get_flag("bip78") { + anyhow::bail!("Resume command is only available with BIP77 (v2)"); + } println!("resume"); app.resume_payjoins().await?; } @@ -56,6 +85,20 @@ fn cli() -> ArgMatches { let mut cmd = Command::new("payjoin") .version(env!("CARGO_PKG_VERSION")) .about("Payjoin - bitcoin scaling, savings, and privacy by default") + .arg( + Arg::new("bip77") + .long("bip77") + .help("Use BIP77 (v2) protocol (default)") + .conflicts_with("bip78") + .action(clap::ArgAction::SetTrue), + ) + .arg( + Arg::new("bip78") + .long("bip78") + .help("Use BIP78 (v1) protocol") + .conflicts_with("bip77") + .action(clap::ArgAction::SetTrue), + ) .arg( Arg::new("rpchost") .long("rpchost") @@ -127,7 +170,7 @@ fn cli() -> ArgMatches { .help("The maximum effective fee rate the receiver is willing to pay (in sat/vB)") .value_parser(parse_fee_rate_in_sat_per_vb), ); - #[cfg(not(feature = "v2"))] + #[cfg(feature = "v1")] { receive_cmd = receive_cmd.arg( Arg::new("port") diff --git a/payjoin-cli/tests/e2e.rs b/payjoin-cli/tests/e2e.rs index 095d4982f..35f8111f2 100644 --- a/payjoin-cli/tests/e2e.rs +++ b/payjoin-cli/tests/e2e.rs @@ -19,9 +19,9 @@ mod e2e { const RECEIVE_SATS: &str = "54321"; - #[cfg(not(feature = "v2"))] + #[cfg(feature = "v1")] #[tokio::test(flavor = "multi_thread", worker_threads = 4)] - async fn send_receive_payjoin() -> Result<(), BoxError> { + async fn send_receive_payjoin_v1() -> Result<(), BoxError> { let (bitcoind, _sender, _receiver) = init_bitcoind_sender_receiver(None, None)?; let temp_dir = env::temp_dir(); let receiver_db_path = temp_dir.join("receiver_db"); @@ -38,6 +38,7 @@ mod e2e { let payjoin_cli = env!("CARGO_BIN_EXE_payjoin-cli"); let mut cli_receiver = Command::new(payjoin_cli) + .arg("--bip78") .arg("--rpchost") .arg(&receiver_rpchost) .arg("--cookie-file") @@ -79,6 +80,7 @@ mod e2e { log::debug!("Got bip21 {}", &bip21); let mut cli_sender = Command::new(payjoin_cli) + .arg("--bip78") .arg("--rpchost") .arg(&sender_rpchost) .arg("--cookie-file") @@ -142,7 +144,7 @@ mod e2e { #[cfg(feature = "v2")] #[tokio::test(flavor = "multi_thread", worker_threads = 4)] - async fn send_receive_payjoin() -> Result<(), Box> { + async fn send_receive_payjoin_v2() -> Result<(), Box> { use std::path::PathBuf; use payjoin_test_utils::{init_tracing, TestServices}; From bfa282f2dab27556e6102cc742aff3b092f5356f Mon Sep 17 00:00:00 2001 From: DanGould Date: Tue, 18 Feb 2025 16:24:52 -0500 Subject: [PATCH 7/7] Show compile_error if unusable feature combination is used Certain features must be present for the compilation to have any use at all. --- payjoin-cli/src/main.rs | 3 +++ payjoin/src/lib.rs | 3 +++ 2 files changed, 6 insertions(+) diff --git a/payjoin-cli/src/main.rs b/payjoin-cli/src/main.rs index 2851d123b..e95855bb6 100644 --- a/payjoin-cli/src/main.rs +++ b/payjoin-cli/src/main.rs @@ -9,6 +9,9 @@ use url::Url; mod app; mod db; +#[cfg(not(any(feature = "v1", feature = "v2")))] +compile_error!("At least one of the features ['v1', 'v2'] must be enabled"); + #[tokio::main] async fn main() -> Result<()> { env_logger::init(); diff --git a/payjoin/src/lib.rs b/payjoin/src/lib.rs index 2e88094e6..db78e0dbe 100644 --- a/payjoin/src/lib.rs +++ b/payjoin/src/lib.rs @@ -17,6 +17,9 @@ //! //! **Use at your own risk. This crate has not yet been reviewed by independent Rust and Bitcoin security professionals.** +#[cfg(not(any(feature = "directory", feature = "v1", feature = "v2")))] +compile_error!("At least one of the features ['directory', 'v1', 'v2'] must be enabled"); + #[cfg(feature = "_core")] pub extern crate bitcoin;