Skip to content
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

Jump to bottom

Add amount checks to integration tests #351

Merged
merged 3 commits into from
Aug 22, 2024
Merged

Add amount checks to integration tests #351

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c7b5c17
Add amount checks to integration tests
spacebear21 Aug 13, 2024
7391636
Explain why MIN_RELAY_TX_FEE is used
DanGould Aug 21, 2024
f1401ce
Use the rust-bitcoin fee calculator instead of hardcoded estimates
spacebear21 Aug 22, 2024
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
94 changes: 77 additions & 17 deletions payjoin/tests/integration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ mod integration {
use std::env;
use std::str::FromStr;

use bitcoin::policy::DEFAULT_MIN_RELAY_TX_FEE;
use bitcoin::psbt::Psbt;
use bitcoin::{Amount, FeeRate, OutPoint};
use bitcoin::{Amount, FeeRate, OutPoint, Weight};
use bitcoind::bitcoincore_rpc::json::{AddressType, WalletProcessPsbtResult};
use bitcoind::bitcoincore_rpc::{self, RpcApi};
use log::{log_enabled, Level};
Expand Down Expand Up @@ -39,6 +40,8 @@ mod integration {
.amount(Amount::ONE_BTC)
.build();

// **********************
// Inside the Sender:
// Sender create a funded PSBT (not broadcasted) to address with amount given in the pj_uri
let uri = Uri::from_str(&pj_uri.to_string())
.unwrap()
Expand All @@ -55,16 +58,25 @@ mod integration {
// **********************
// Inside the Receiver:
// this data would transit from one party to another over the network in production
let response = handle_v1_pj_request(req, headers, receiver);
let response = handle_v1_pj_request(req, headers, &receiver);
// this response would be returned as http response to the sender

// **********************
// Inside the Sender:

// Sender checks, signs, finalizes, extracts, and broadcasts
let checked_payjoin_proposal_psbt = ctx.process_response(&mut response.as_bytes())?;
let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?;
sender.send_raw_transaction(&payjoin_tx)?;

// Check resulting transaction and balances
let network_fees = predicted_tx_weight(&payjoin_tx) * FeeRate::BROADCAST_MIN;
assert_eq!(payjoin_tx.input.len(), 2);
assert_eq!(payjoin_tx.output.len(), 2);
assert_eq!(receiver.get_balances()?.mine.untrusted_pending, Amount::from_btc(51.0)?);
assert_eq!(
sender.get_balances()?.mine.untrusted_pending,
Amount::from_btc(49.0)? - network_fees
);
Ok(())
}
}
Expand Down Expand Up @@ -264,7 +276,7 @@ mod integration {
.unwrap();
let psbt = build_sweep_psbt(&sender, &pj_uri)?;
let mut req_ctx = RequestBuilder::from_psbt_and_uri(psbt.clone(), pj_uri.clone())?
.build_recommended(payjoin::bitcoin::FeeRate::BROADCAST_MIN)?;
.build_recommended(FeeRate::BROADCAST_MIN)?;
let (Request { url, body, content_type, .. }, send_ctx) =
req_ctx.extract_v2(directory.to_owned())?;
let response = agent
Expand All @@ -290,7 +302,7 @@ mod integration {
// POST payjoin
let proposal =
session.process_res(response.bytes().await?.to_vec().as_slice(), ctx)?.unwrap();
let mut payjoin_proposal = handle_directory_proposal(receiver, proposal);
let mut payjoin_proposal = handle_directory_proposal(&receiver, proposal);
assert!(!payjoin_proposal.is_output_substitution_disabled());
let (req, ctx) = payjoin_proposal.extract_v2_req()?;
let response = agent.post(req.url).body(req.body).send().await?;
Expand All @@ -300,7 +312,6 @@ mod integration {
// **********************
// Inside the Sender:
// Sender checks, signs, finalizes, extracts, and broadcasts

// Replay post fallback to get the response
let (Request { url, body, .. }, send_ctx) =
req_ctx.extract_v2(directory.to_owned())?;
Expand All @@ -311,6 +322,20 @@ mod integration {
let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?;
sender.send_raw_transaction(&payjoin_tx)?;
log::info!("sent");

// Check resulting transaction and balances
// NOTE: No one is contributing fees for the receiver input because the sender has
// no change output and the receiver doesn't contribute fees. Temporary workaround
// is to ensure the sender overpays in the original psbt for the receiver's input.
let network_fees = psbt.fee()?;
// Sender sent the entire value of their utxo to receiver (minus fees)
assert_eq!(payjoin_tx.input.len(), 2);
assert_eq!(payjoin_tx.output.len(), 1);
assert_eq!(
receiver.get_balances()?.mine.untrusted_pending,
Amount::from_btc(100.0)? - network_fees
);
assert_eq!(sender.get_balances()?.mine.untrusted_pending, Amount::from_btc(0.0)?);
Ok(())
}
}
Expand All @@ -335,24 +360,33 @@ mod integration {
.unwrap();
let psbt = build_original_psbt(&sender, &pj_uri)?;
let mut req_ctx = RequestBuilder::from_psbt_and_uri(psbt.clone(), pj_uri.clone())?
.build_recommended(payjoin::bitcoin::FeeRate::BROADCAST_MIN)?;
.build_recommended(FeeRate::BROADCAST_MIN)?;
let (req, ctx) = req_ctx.extract_v2(EXAMPLE_URL.to_owned())?;
let headers = HeaderMock::new(&req.body, req.content_type);

// **********************
// Inside the Receiver:
// this data would transit from one party to another over the network in production
let response = handle_v1_pj_request(req, headers, receiver);
let response = handle_v1_pj_request(req, headers, &receiver);
// this response would be returned as http response to the sender

// **********************
// Inside the Sender:

// Sender checks, signs, finalizes, extracts, and broadcasts
let checked_payjoin_proposal_psbt =
ctx.process_response(&mut response.as_bytes())?.unwrap();
let payjoin_tx = extract_pj_tx(&sender, checked_payjoin_proposal_psbt)?;
sender.send_raw_transaction(&payjoin_tx)?;

// Check resulting transaction and balances
let network_fees = predicted_tx_weight(&payjoin_tx) * FeeRate::BROADCAST_MIN;
assert_eq!(payjoin_tx.input.len(), 2);
assert_eq!(payjoin_tx.output.len(), 2);
assert_eq!(receiver.get_balances()?.mine.untrusted_pending, Amount::from_btc(51.0)?);
assert_eq!(
sender.get_balances()?.mine.untrusted_pending,
Amount::from_btc(49.0)? - network_fees
);
Ok(())
}

Expand Down Expand Up @@ -427,6 +461,8 @@ mod integration {
// **********************
// Inside the Receiver:
let agent_clone: Arc<Client> = agent.clone();
let receiver: Arc<bitcoincore_rpc::Client> = Arc::new(receiver);
let receiver_clone = receiver.clone();
let receiver_loop = tokio::task::spawn(async move {
let agent_clone = agent_clone.clone();
let (response, ctx) = loop {
Expand All @@ -446,7 +482,7 @@ mod integration {
}
};
let proposal = session.process_res(response.as_slice(), ctx).unwrap().unwrap();
let mut payjoin_proposal = handle_directory_proposal(receiver, proposal);
let mut payjoin_proposal = handle_directory_proposal(&receiver_clone, proposal);
assert!(payjoin_proposal.is_output_substitution_disabled());
// Respond with payjoin psbt within the time window the sender is willing to wait
// this response would be returned as http response to the sender
Expand Down Expand Up @@ -476,6 +512,19 @@ mod integration {
receiver_loop.await.is_ok(),
"The spawned task panicked or returned an error"
);

// Check resulting transaction and balances
let network_fees = predicted_tx_weight(&payjoin_tx) * FeeRate::BROADCAST_MIN;
assert_eq!(payjoin_tx.input.len(), 2);
assert_eq!(payjoin_tx.output.len(), 2);
assert_eq!(
receiver.get_balances()?.mine.untrusted_pending,
Amount::from_btc(51.0)?
);
assert_eq!(
sender.get_balances()?.mine.untrusted_pending,
Amount::from_btc(49.0)? - network_fees
);
Ok(())
}
}
Expand Down Expand Up @@ -535,7 +584,7 @@ mod integration {
}

fn handle_directory_proposal(
receiver: bitcoincore_rpc::Client,
receiver: &bitcoincore_rpc::Client,
proposal: UncheckedProposal,
) -> PayjoinProposal {
// in a payment processor where the sender could go offline, this is where you schedule to broadcast the original_tx
Expand Down Expand Up @@ -618,7 +667,7 @@ mod integration {
})
.unwrap())
},
Some(bitcoin::FeeRate::MIN),
Some(FeeRate::BROADCAST_MIN),
)
.unwrap();
payjoin_proposal
Expand Down Expand Up @@ -671,7 +720,9 @@ mod integration {
outputs.insert(pj_uri.address.to_string(), Amount::from_btc(50.0)?);
let options = bitcoincore_rpc::json::WalletCreateFundedPsbtOptions {
lock_unspent: Some(true),
fee_rate: Some(Amount::from_sat(2000)),
// The current API doesn't let the receiver pay for additional fees,
// so we double the minimum relay fee to ensure that the sender pays for the receiver's inputs
fee_rate: Some(Amount::from_sat((DEFAULT_MIN_RELAY_TX_FEE * 2).into())),
subtract_fee_from_outputs: vec![0],
..Default::default()
};
Expand Down Expand Up @@ -740,7 +791,9 @@ mod integration {
outputs.insert(pj_uri.address.to_string(), pj_uri.amount.unwrap_or(Amount::ONE_BTC));
let options = bitcoincore_rpc::json::WalletCreateFundedPsbtOptions {
lock_unspent: Some(true),
fee_rate: Some(Amount::from_sat(2000)),
// The minimum relay feerate ensures that tests fail if the receiver would add inputs/outputs
// that cannot be covered by the sender's additional fee contributions.
fee_rate: Some(Amount::from_sat(DEFAULT_MIN_RELAY_TX_FEE.into())),
..Default::default()
};
let psbt = sender
Expand All @@ -761,7 +814,7 @@ mod integration {
fn handle_v1_pj_request(
req: Request,
headers: impl payjoin::receive::Headers,
receiver: bitcoincore_rpc::Client,
receiver: &bitcoincore_rpc::Client,
) -> String {
// Receiver receive payjoin proposal, IRL it will be an HTTP request (over ssl or onion)
let proposal = payjoin::receive::UncheckedProposal::from_request(
Expand All @@ -779,7 +832,7 @@ mod integration {

fn handle_proposal(
proposal: payjoin::receive::UncheckedProposal,
receiver: bitcoincore_rpc::Client,
receiver: &bitcoincore_rpc::Client,
) -> payjoin::receive::PayjoinProposal {
// 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();
Expand Down Expand Up @@ -861,7 +914,7 @@ mod integration {
})
.unwrap())
},
Some(bitcoin::FeeRate::MIN),
Some(FeeRate::BROADCAST_MIN),
)
.unwrap();
payjoin_proposal
Expand All @@ -879,6 +932,13 @@ mod integration {
Ok(payjoin_psbt.extract_tx()?)
}

fn predicted_tx_weight(tx: &bitcoin::Transaction) -> Weight {
bitcoin::transaction::predict_weight(
vec![bitcoin::transaction::InputWeightPrediction::P2WPKH_MAX; tx.input.len()],
Copy link
Contributor

Choose a reason for hiding this comment

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

I find it mildly irritating that we have to assume P2WPKH script since the tx primitive doesn't have prevout data, but I'm OK living with it. PSBTv2 fixes this.

tx.script_pubkey_lens(),
)
}

struct HeaderMock(HashMap<String, String>);

impl payjoin::receive::Headers for HeaderMock {
Expand Down
Loading