Audit the reference implementation

MENTAL-MODEL.md

@@ -0,0 +1,324 @@
+# What is the Payjoin SDK and How does it work?
+
+The Payjoin SDK/`rust-payjoin` is the most well-tested and flexible library for BIP 78 Payjoin and related privacy practices.
+
+The primary crate, `payjoin`, is runtime-agnostic. Data persistence, chain interactions, and networking may be provided by custom implementations or copy the reference `payjoin-client` + bitcoind, `nolooking` + LND integration, or `bitmask-core` + BDK integrations.
+
+The following is a breakdown of the existing documentation and its application to the `payjoin-client` reference implementation. Errors are mostly handled with `anyhow`.
+
+## Send a PayJoin
+
+The `sender` feature provides all of the checks and most every PSBT formatting utility necessary to send payjoin out of the box. Sending requires an HTTP client and a bitcoin wallet. The reference implementation uses `reqwest` and Bitcoin Core RPC. Only a few non-default parameters are required:
+
+```rs
+fn send_payjoin(
+    bitcoind: bitcoincore_rpc::Client,
+    bip21: &str,
+    danger_accept_invalid_certs: bool,
+) -> Result<()>
+```
+
+### 1. Parse BIP21 as [`payjoin::Uri`](crate::Uri)
+
+Start by parseing and checking that there is a valid BIP 21 uri with PJ parameter. Uses the bip21 crate
+
+```rs
+let link = payjoin::Uri::try_from(bip21)
+    .map_err(|e| anyhow!("Failed to create URI from BIP21: {}", e))?;
+
+let link = link
+    .check_pj_supported()
+    .map_err(|e| anyhow!("The provided URI doesn't support payjoin (BIP78): {}", e))?;
+```
+
+Using a builder pattern may eliminate check_pj_supported step #19  [#19](https://github.com/payjoin/rust-payjoin/issues/19)
+
+### 2. Construct URI request parameters, a finalized "Original PSBT" paying `.amount` to `.address`
+
+```rs
+let mut outputs = HashMap::with_capacity(1);
+outputs.insert(link.address.to_string(), amount);
+
+let options = bitcoincore_rpc::json::WalletCreateFundedPsbtOptions {
+    lock_unspent: Some(true),
+    fee_rate: Some(Amount::from_sat(2000)), // SHOULD CHANGE TO AN OPTIONAL FEE RATE
+    ..Default::default()
+};
+let psbt = bitcoind
+    .wallet_create_funded_psbt(
+        &[], // inputs
+        &outputs,
+        None, // locktime
+        Some(options),
+        None,
+    )
+    .context("Failed to create PSBT")?
+    .psbt;
+let psbt = bitcoind
+    .wallet_process_psbt(&psbt, None, None, None)
+    .with_context(|| "Failed to process PSBT")?
+    .psbt;
+let psbt = load_psbt_from_base64(psbt.as_bytes()) // SHOULD BE PROVIDED BY CRATE AS HELPER USING rust-bitcoin base64 feature
+    .with_context(|| "Failed to load PSBT from base64")?;
+log::debug!("Original psbt: {:#?}", psbt);
+let pj_params = payjoin::sender::Configuration::with_fee_contribution(
+    payjoin::bitcoin::Amount::from_sat(10000),
+    None,
+);
+let (req, ctx) = link
+    .create_pj_request(psbt, pj_params)
+    .with_context(|| "Failed to create payjoin request")?;
+```
+
+### 3. (optional) Spawn a thread or async task that will broadcast the transaction after one minute unless canceled
+
+In case the payjoin goes through but you still want to pay by default. This missing `payjoin-client`
+
+Writing this, I think of [Signal's contributing guidelines](The answer is not more options. If you feel compelled to add a preference that's exposed to the user, it's very possible you've made a wrong turn somewhere.):
+> The answer is not more options. If you feel compelled to add a preference that's exposed to the user, it's very possible you've made a wrong turn somewhere.
+
+### 4. Construct the request with the PSBT and parameters
+
+```rs
+let (req, ctx) = link
+    .create_pj_request(psbt, pj_params)
+    .with_context(|| "Failed to create payjoin request")?;
+```
+
+### 5. Send the request and receive response
+
+Payjoin participants construct a transaction and require authentication and should have e2ee to prevent that transaction from being modified by a malicious third party during transit and being snooped on. Only https and .onion endpoints are spec-compatible payjoin endpoints.
+
+For testing, it's nice to be able to avoid this trust requirement (but again, Signal guidelines remind me of the folly of options).
+
+```rs
+let client = reqwest::blocking::Client::builder()
+    .danger_accept_invalid_certs(danger_accept_invalid_certs)
+    .build()
+    .with_context(|| "Failed to build reqwest http client")?;
+let response = client
+    .post(req.url)
+    .body(req.body)
+    .header("Content-Type", "text/plain")
+    .send()
+    .with_context(|| "HTTP request failed")?;
+```
+
+### 6. Process the response
+
+An Ok response should include a Payjoin PSBT signed by the receiver. Check that it's not trying to steal from us or otherwise do something wrong.
+
+```rs
+// TODO display well-known errors and log::debug the rest
+let psbt = ctx.process_response(response).with_context(|| "Failed to process response")?;
+log::debug!("Proposed psbt: {:#?}", psbt);
+```
+
+Payjoin response errors (called [receiver's errors in spec](https://github.com/bitcoin/bips/blob/master/bip-0078.mediawiki#receivers-well-known-errors)) come from a remote server and can be used to "maliciously to phish a non technical user." Errors from `process_response` may be improved by being separated into safe `ReceiverError::WellKnown` standard error types that can be displayed in the UI and `ReceiverError::DebugOnly` which "can only appear in debug logs." Separation would simplify an integration's error handling.
+
+### 7. Sign and finalize the Payjoin PSBT
+
+```rs
+let psbt = bitcoind
+    .wallet_process_psbt(&serialize_psbt(&psbt), None, None, None)
+    .with_context(|| "Failed to process PSBT")?
+    .psbt;
+let tx = bitcoind
+    .finalize_psbt(&psbt, Some(true))
+    .with_context(|| "Failed to finalize PSBT")?
+    .hex
+    .ok_or_else(|| anyhow!("Incomplete PSBT"))?;
+```
+
+The signature step could be combined with the Process step if a signing closure were passed to process_response. That may error if the closure is an async runtime, however.
+
+### 8. Broadcast the resulting PSBT
+
+```rs
+let txid =
+    bitcoind.send_raw_transaction(&tx).with_context(|| "Failed to send raw transaction")?;
+log::info!("Transaction sent: {}", txid);
+```
+
+
+## Receive a Payjoin
+
+The `receiver` feature provides all of the checks and most every PSBT formatting utility necessary to send payjoin out of the box. Receiving payjoin requires a live http endpoint listening for inbound requests. The `endpoint` (displayed in the BIP 21 URI pj parameter) could be configured rather than passed as an argument, and the amount is optional. The reference implementation uses `rouille` sync http server and Bitcoin Core RPC.
+
+```rs
+fn receive_payjoin(
+    bitcoind: bitcoincore_rpc::Client,
+    amount_arg: &str,
+    endpoint_arg: &str,
+) -> Result<()>
+```
+
+### 1. Generate a pj_uri BIP21 using `payjoin::Uri::from_str`
+
+A BIP 21 URI supporting payjoin requires an address and a secure `pj` endpoint.
+
+```rs
+let pj_receiver_address = bitcoind.get_new_address(None, None)?;
+let amount = Amount::from_sat(amount_arg.parse()?);
+let pj_uri_string = format!(
+    "{}?amount={}&pj={}",
+    pj_receiver_address.to_qr_uri(),
+    amount.to_btc(),
+    endpoint_arg
+);
+let pj_uri = Uri::from_str(&pj_uri_string)
+    .map_err(|e| anyhow!("Constructed a bad URI string from args: {}", e))?;
+let _pj_uri = pj_uri
+    .check_pj_supported()
+    .map_err(|e| anyhow!("Constructed URI does not support payjoin: {}", e))?;
+```
+
+A URI builder may be a more ergonomic interface. The `bip21` crate might be a good candidate to start to explore bindings with.
+
+### 2. Listen for an original PSBT on the endpoint specified in the URI
+
+```rs
+rouille::start_server("0.0.0.0:3000", move |req| handle_web_request(&req, &bitcoind));
+// ...
+fn handle_web_request(req: &Request, bitcoind: &bitcoincore_rpc::Client) -> Response {
+    handle_payjoin_request(req, bitcoind)
+        .map_err(|e| match e {
+            ReceiveError::RequestError(e) => {
+                log::error!("Error handling request: {}", e);
+                Response::text(e.to_string()).with_status_code(400)
+            }
+            e => {
+                log::error!("Error handling request: {}", e);
+                Response::text(e.to_string()).with_status_code(500)
+            }
+        })
+        .unwrap_or_else(|err_resp| err_resp)
+}
+```
+
+This server only responds to payjoin protocol POST messages. It would be a better experience to respond with a payjoin QuickStart page for those sending GET requests to the payjoin endpoint.
+
+### 3. Parse an incoming request using `UncheckedProposal::from_request()`
+
+Parse incoming HTTP request and check that it follows protocol.
+
+```rs
+let headers = Headers(req.headers());
+let proposal = payjoin::receiver::UncheckedProposal::from_request(
+    req.data().context("Failed to read request body")?,
+    req.raw_query_string(),
+    headers,
+)?;
+```
+
+Headers are parsed using the `payjoin::receiver::Headers` Trait so that the library can iterate through them, ideally without cloning.
+I reckon bindings will run into a problem with this design.
+
+```rs
+struct Headers<'a>(rouille::HeadersIter<'a>);
+impl payjoin::receiver::Headers for Headers<'_> {
+    fn get_header(&self, key: &str) -> Option<&str> {
+        let mut copy = self.0.clone(); // lol
+        copy.find(|(k, _)| k.eq_ignore_ascii_case(key)).map(|(_, v)| v)
+    }
+}
+```
+
+### 4. Validate the proposal using the `check` methods
+
+The receiver checks the sender's Original PSBT to prevent it from stealing, probing to fingerprint its wallet, and so that it doesn't trip over any privacy gotchas.
+
+```rs
+// 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.get_transaction_to_schedule_broadcast();
+
+// The network is used for checks later
+let network = match bitcoind.get_blockchain_info()?.chain.as_str() {
+    "main" => bitcoin::Network::Bitcoin,
+    "test" => bitcoin::Network::Testnet,
+    "regtest" => bitcoin::Network::Regtest,
+    _ => return Err(ReceiveError::Other(anyhow!("Unknown network"))),
+};
+
+// Receive Check 1: Can Broadcast
+let proposal = proposal.check_can_broadcast(|tx| {
+    bitcoind
+        .test_mempool_accept(&[bitcoin::consensus::encode::serialize(&tx).to_hex()])
+        .unwrap()
+        .first()
+        .unwrap()
+        .allowed
+})?;
+log::trace!("check1");
+
+// Receive Check 2: receiver can't sign for proposal inputs
+let proposal = proposal.check_inputs_not_owned(|input| {
+    let address = bitcoin::Address::from_script(&input, network).unwrap();
+    bitcoind.get_address_info(&address).unwrap().is_mine.unwrap()
+})?;
+log::trace!("check2");
+// Receive Check 3: receiver can't sign for proposal inputs
+let proposal = proposal.check_no_mixed_input_scripts()?;
+log::trace!("check3");
+
+// Receive Check 4: have we seen this input before? More of a check for non-interactive i.e. payment processor receivers.
+let mut payjoin = proposal
+    .check_no_inputs_seen_before(|_| false)
+    .unwrap()
+    .identify_receiver_outputs(|output_script| {
+        let address = bitcoin::Address::from_script(&output_script, network).unwrap();
+        bitcoind.get_address_info(&address).unwrap().is_mine.unwrap()
+    })?;
+log::trace!("check4");
+```
+
+### 5. Assuming the request is valid, augment with the available `try_preserving_privacy` and `substitute_output_address` methods
+
+Here's where the PSBT is modified. Inputs may be added to break common input ownership heurstic. There are a number of ways to select coins that break common input heuristic but violate trecherous Unnecessary Input Heuristic (UIH) so that privacy preservation is destroyed are moot. Until February 2023, even BTCPay occasionally made these errors. Privacy preserving coin selection done here is precarious and may be the most sensitive and valuable part of the kit. The Original PSBT output address that pays the receiver may be substituted with a unique one even from a watch-only wallet. The substitution may also be used to direct incoming funds and consolidate funds from a hot payjoin wallet to a cold wallet.
+
+PSBTv0 was not designed for input/output modification so these functions are complicated. PSBTv2 would simplify this part of the code under the hood.
+
+```rs
+// Select receiver payjoin inputs.
+_ = try_contributing_inputs(&mut payjoin, bitcoind)
+    .map_err(|e| log::warn!("Failed to contribute inputs: {}", e));
+
+let receiver_substitute_address = bitcoind.get_new_address(None, None)?;
+payjoin.substitute_output_address(receiver_substitute_address);
+```
+
+The industry may benefit if we expose the selection algorithm inside `try_preserving_privacy` as a static function to make it easier and more lightweight to bind to and avoid dangerous instances of UIH.
+
+Serious, in-depth research has gone into proper transaction construction. [Here's a good starting point from the JoinMarket repo](https://gist.github.com/AdamISZ/4551b947789d3216bacfcb7af25e029e#gistcomment-2796539). This part is critical to do right.
+
+### 6. Extract the payjoin PSBT and sign it
+
+Fees are applied to the proposal PSBT, having been modified by the receiver, using calculation factoring the receiver's minnimum feerate and the sender's fee-related [optional parameters](https://github.com/bitcoin/bips/blob/master/bip-0078.mediawiki#optional-parameters) into account. The `payjoin` implementation is very simple, disregarding PSBT fee estimation and only adding fees according to the sender's budget. More accurate fee calculation could be done with an algorithm to predict a PSBT's final weight (slightly more complicated than it sounds, but solved, just unimplemented in rust-bitcoin).
+
+```rs
+let payjoin_proposal_psbt = payjoin.apply_fee(min_feerate_sat_per_vb: Some(1))?;
+
+log::debug!("Extracted PSBT: {:#?}", payjoin_proposal_psbt);
+// Sign payjoin psbt
+let payjoin_base64_string =
+    base64::encode(bitcoin::consensus::serialize(&payjoin_proposal_psbt));
+// `wallet_process_psbt` adds available utxo data and finalizes
+let payjoin_proposal_psbt =
+    bitcoind.wallet_process_psbt(&payjoin_base64_string, sign: None, sighash_type: None, bip32derivs: Some(false))?.psbt;
+let payjoin_proposal_psbt =
+    load_psbt_from_base64(payjoin_proposal_psbt.as_bytes()).context("Failed to parse PSBT")?;
+```
+
+The signing algorithm could be passed as a closure, but runs into the same runtime issues as mentioned previously. I.e., rust does not support async closures.
+
+### 7. Respond to the sender's http request with the signed PSBT as payload
+
+BIP 78 defines a very specific PSBT construction that the sender will find acceptable, which prepare_psbt handles. PSBTv0 was not designed to support input/output modification, so the protocol requires this step to be carried out precisely. A PSBTv2 protocol may not.
+
+It is critical to pay special care in the error response messages. Without special care, a receiver could make itself vulnerable to probing attacks which cluster its UTXOs.
+
+```rs
+let payjoin_proposal_psbt = payjoin.prepare_psbt(payjoin_proposal_psbt)?;
+let payload = base64::encode(bitcoin::consensus::serialize(&payjoin_proposal_psbt));
+Ok(Response::text(payload))
+```