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.

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

multi: config timeout receiving invoice #122

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions config_spec.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,9 @@ name = "grpc_port"
type = "u16"
optional = true
doc = "The port the grpc server will run on. Defaults to 7000."

[[param]]
name = "response_invoice_timeout"
type = "u32"
optional = true
doc = "Amount of time in seconds that server waits for an offer creator to respond with an invoice. Defaults to 30s."
1 change: 1 addition & 0 deletions proto/lndkrpc.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ service Offers {
message PayOfferRequest {
string offer = 1;
optional uint64 amount = 2;
optional uint32 response_invoice_timeout = 3;
}

message PayOfferResponse {
Expand Down
1 change: 1 addition & 0 deletions sample-lndk.conf
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ macaroon_path="/home/<USERNAME>/.lnd/data/chain/bitcoin/regtest/admin.macaroon"
# -----END CERTIFICATE-----"
# # This should be a hex-encoded macaroon string.
# macaroon-hex="0201036C6E6402F801030A1034F41C28A3B5190702FEA607DD4045AE1201301A160A0761646472657373120472656164120577726974651A130A04696E666F120472656164120577726974651A170A08696E766F69636573120472656164120577726974651A210A086D616361726F6F6E120867656E6572617465120472656164120577726974651A160A076D657373616765120472656164120577726974651A170A086F6666636861696E120472656164120577726974651A160A076F6E636861696E120472656164120577726974651A140A057065657273120472656164120577726974651A180A067369676E6572120867656E6572617465120472656164000006205CF3E77A97764FB2A68967832608591BE375B36F51A6269AB425AA767BC22B55"
response_invoice_timeout=10
7 changes: 7 additions & 0 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ enum Commands {
/// whatever the offer amount is.
#[arg(required = false)]
amount: Option<u64>,

/// The amount of time in seconds that the user would like to wait for an invoice to
/// arrive. If this isn't set, we'll use the default value.
#[arg(long, global = false, required = false)]
response_invoice_timeout: Option<u32>,
},
}

Expand Down Expand Up @@ -98,6 +103,7 @@ async fn main() -> Result<(), ()> {
Commands::PayOffer {
ref offer_string,
amount,
response_invoice_timeout,
} => {
let data_dir = home::home_dir().unwrap().join(DEFAULT_DATA_DIR);
let pem = match args.cert_pem {
Expand Down Expand Up @@ -162,6 +168,7 @@ async fn main() -> Result<(), ()> {
let mut request = Request::new(PayOfferRequest {
offer: offer.to_string(),
amount,
response_invoice_timeout,
});
add_metadata(&mut request, macaroon).map_err(|_| ())?;

Expand Down
42 changes: 37 additions & 5 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ pub const DEFAULT_DATA_DIR: &str = ".lndk";

pub const TLS_CERT_FILENAME: &str = "tls-cert.pem";
pub const TLS_KEY_FILENAME: &str = "tls-key.pem";
pub const DEFAULT_RESPONSE_INVOICE_TIMEOUT: u32 = 30;

#[allow(clippy::result_unit_err)]
pub fn setup_logger(log_level: Option<String>, log_dir: Option<String>) -> Result<(), ()> {
Expand Down Expand Up @@ -226,6 +227,9 @@ pub struct OfferHandler {
pending_messages: Mutex<Vec<PendingOnionMessage<OffersMessage>>>,
pub messenger_utils: MessengerUtilities,
expanded_key: ExpandedKey,
/// Default timeout in seconds that we will wait for offer creator to respond with
/// an invoice.
pub response_invoice_timeout: u32,
}

#[derive(Clone)]
Expand All @@ -239,10 +243,28 @@ pub struct PayOfferParams {
/// The path we will send back to the offer creator, so it knows where to send back the
/// invoice.
pub reply_path: Option<BlindedPath>,
/// The amount of time in seconds that we will wait for offer creator to respond with
/// an invoice. If not provided, we will use the default value.
pub response_invoice_timeout: Option<u32>,
}

impl OfferHandler {
pub fn new() -> Self {
pub fn new_default() -> Self {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm I think I'd rather remove this new_default function and in impl Default (and wherever else new_default is called) we can just call new(None) instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

I addressed this in e989fbb

Self::new(None)
}

pub fn new(response_invoice_timeout: Option<u32>) -> Self {
let response_invoice_timeout = match response_invoice_timeout {
Some(timeout) => {
if timeout < 1 {
Copy link
Collaborator

@orbitalturtle orbitalturtle Jun 30, 2024

Choose a reason for hiding this comment

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

Ah is this to make sure a user doesn't set a timeout of zero? I think it'd be good to do validation of this first -- so before OfferHandler::new is called in main.rs, check that the timeout isn't less than 1. If it is less than one log an error! an exit so the user can then provide a valid timeout

Copy link
Contributor

Choose a reason for hiding this comment

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

I addressed this in 6f18e38

DEFAULT_RESPONSE_INVOICE_TIMEOUT
} else {
timeout
}
}
None => DEFAULT_RESPONSE_INVOICE_TIMEOUT,
};

let messenger_utils = MessengerUtilities::new();
let random_bytes = messenger_utils.get_secure_random_bytes();
let expanded_key = ExpandedKey::new(&KeyMaterial(random_bytes));
Expand All @@ -253,6 +275,7 @@ impl OfferHandler {
pending_messages: Mutex::new(Vec::new()),
messenger_utils,
expanded_key,
response_invoice_timeout,
}
}

Expand All @@ -264,19 +287,28 @@ impl OfferHandler {
) -> Result<Payment, OfferError<Secp256k1Error>> {
let client_clone = cfg.client.clone();
let offer_id = cfg.offer.clone().to_string();
let cfg_timeout = match cfg.response_invoice_timeout {
Some(timeout) => timeout,
None => self.response_invoice_timeout,
};
let validated_amount = self.send_invoice_request(cfg).await.map_err(|e| {
let mut active_offers = self.active_offers.lock().unwrap();
active_offers.remove(&offer_id.clone());
e
})?;

let invoice = match timeout(Duration::from_secs(100), self.wait_for_invoice()).await {
let invoice = match timeout(
Duration::from_secs(cfg_timeout as u64),
self.wait_for_invoice(),
)
.await
{
Ok(invoice) => invoice,
Err(_) => {
error!("Did not receive invoice in 100 seconds.");
error!("Did not receive invoice in {cfg_timeout} seconds.");
let mut active_offers = self.active_offers.lock().unwrap();
active_offers.remove(&offer_id.clone());
return Err(OfferError::InvoiceTimeout);
return Err(OfferError::InvoiceTimeout(cfg_timeout));
}
};
{
Expand Down Expand Up @@ -327,7 +359,7 @@ impl OfferHandler {

impl Default for OfferHandler {
fn default() -> Self {
Self::new()
Self::new_default()
}
}

Expand Down
22 changes: 11 additions & 11 deletions src/lndk_offers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pub enum OfferError<Secp256k1Error> {
/// Failed to send payment.
PaymentFailure,
/// Failed to receive an invoice back from offer creator before the timeout.
InvoiceTimeout,
InvoiceTimeout(u32),
}

impl Display for OfferError<Secp256k1Error> {
Expand All @@ -83,7 +83,7 @@ impl Display for OfferError<Secp256k1Error> {
OfferError::RouteFailure(e) => write!(f, "Error routing payment: {e:?}"),
OfferError::TrackFailure(e) => write!(f, "Error tracking payment: {e:?}"),
OfferError::PaymentFailure => write!(f, "Failed to send payment"),
OfferError::InvoiceTimeout => write!(f, "Did not receive invoice in 100 seconds."),
OfferError::InvoiceTimeout(e) => write!(f, "Did not receive invoice in {e:?} seconds."),
}
}
}
Expand Down Expand Up @@ -699,7 +699,7 @@ mod tests {
.returning(move |_, _| Ok(get_invoice_request(offer.clone(), amount)));

let offer = decode(get_offer()).unwrap();
let handler = OfferHandler::new();
let handler = OfferHandler::new_default();
let resp = handler
.create_invoice_request(signer_mock, offer, vec![], Network::Regtest, amount)
.await;
Expand All @@ -719,7 +719,7 @@ mod tests {
.returning(move |_, _| Ok(get_invoice_request(decode(get_offer()).unwrap(), 10000)));

let offer = decode(get_offer()).unwrap();
let handler = OfferHandler::new();
let handler = OfferHandler::new_default();
assert!(handler
.create_invoice_request(signer_mock, offer, vec![], Network::Regtest, 10000,)
.await
Expand All @@ -744,7 +744,7 @@ mod tests {
});

let offer = decode(get_offer()).unwrap();
let handler = OfferHandler::new();
let handler = OfferHandler::new_default();
assert!(handler
.create_invoice_request(signer_mock, offer, vec![], Network::Regtest, 10000,)
.await
Expand Down Expand Up @@ -885,7 +885,7 @@ mod tests {
});

let receiver_node_id = PublicKey::from_str(&get_pubkey()).unwrap();
let handler = OfferHandler::new();
let handler = OfferHandler::new_default();
assert!(handler
.create_reply_path(connector_mock, receiver_node_id)
.await
Expand All @@ -902,7 +902,7 @@ mod tests {
.returning(|| Ok(ListPeersResponse { peers: vec![] }));

let receiver_node_id = PublicKey::from_str(&get_pubkey()).unwrap();
let handler = OfferHandler::new();
let handler = OfferHandler::new_default();
assert!(handler
.create_reply_path(connector_mock, receiver_node_id)
.await
Expand All @@ -918,7 +918,7 @@ mod tests {
.returning(|| Err(Status::unknown("unknown error")));

let receiver_node_id = PublicKey::from_str(&get_pubkey()).unwrap();
let handler = OfferHandler::new();
let handler = OfferHandler::new_default();
assert!(handler
.create_reply_path(connector_mock, receiver_node_id)
.await
Expand Down Expand Up @@ -953,7 +953,7 @@ mod tests {

let blinded_path = get_blinded_path();
let payment_hash = MessengerUtilities::new().get_secure_random_bytes();
let handler = OfferHandler::new();
let handler = OfferHandler::new_default();
let params = PayInvoiceParams {
path: blinded_path,
cltv_expiry_delta: 200,
Expand All @@ -976,7 +976,7 @@ mod tests {

let blinded_path = get_blinded_path();
let payment_hash = MessengerUtilities::new().get_secure_random_bytes();
let handler = OfferHandler::new();
let handler = OfferHandler::new_default();
let params = PayInvoiceParams {
path: blinded_path,
cltv_expiry_delta: 200,
Expand Down Expand Up @@ -1009,7 +1009,7 @@ mod tests {

let blinded_path = get_blinded_path();
let payment_hash = MessengerUtilities::new().get_secure_random_bytes();
let handler = OfferHandler::new();
let handler = OfferHandler::new_default();
let params = PayInvoiceParams {
path: blinded_path,
cltv_expiry_delta: 200,
Expand Down
2 changes: 1 addition & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
signals,
};

let handler = Arc::new(OfferHandler::new());
let handler = Arc::new(OfferHandler::new(config.response_invoice_timeout));

Check warning on line 57 in src/main.rs

View check run for this annotation

Codecov / codecov/patch

src/main.rs#L57

Added line #L57 was not covered by tests
let messenger = LndkOnionMessenger::new();

let data_dir =
Expand Down
1 change: 1 addition & 0 deletions src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ impl Offers for LNDKServer {
client,
destination,
reply_path: Some(reply_path),
response_invoice_timeout: inner_request.response_invoice_timeout,
};

let payment = match self.offer_handler.pay_offer(cfg).await {
Expand Down
10 changes: 6 additions & 4 deletions tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ async fn test_lndk_forwards_onion_message() {
);
setup_logger(None, log_dir).unwrap();

let handler = Arc::new(lndk::OfferHandler::new());
let handler = Arc::new(lndk::OfferHandler::new_default());
let messenger = lndk::LndkOnionMessenger::new();
select! {
val = messenger.run(lndk_cfg, Arc::clone(&handler)) => {
Expand Down Expand Up @@ -245,7 +245,7 @@ async fn test_lndk_send_invoice_request() {
setup_logger(None, log_dir).unwrap();

// Make sure lndk successfully sends the invoice_request.
let handler = Arc::new(lndk::OfferHandler::new());
let handler = Arc::new(lndk::OfferHandler::new_default());
let messenger = lndk::LndkOnionMessenger::new();
let pay_cfg = PayOfferParams {
offer: offer.clone(),
Expand All @@ -254,6 +254,7 @@ async fn test_lndk_send_invoice_request() {
client: client.clone(),
destination: Destination::BlindedPath(blinded_path.clone()),
reply_path: Some(reply_path.clone()),
response_invoice_timeout: None,
};
select! {
val = messenger.run(lndk_cfg, Arc::clone(&handler)) => {
Expand Down Expand Up @@ -290,7 +291,7 @@ async fn test_lndk_send_invoice_request() {
);
setup_logger(None, log_dir).unwrap();

let handler = Arc::new(lndk::OfferHandler::new());
let handler = Arc::new(lndk::OfferHandler::new_default());
let messenger = lndk::LndkOnionMessenger::new();
select! {
val = messenger.run(lndk_cfg, Arc::clone(&handler)) => {
Expand Down Expand Up @@ -420,7 +421,7 @@ async fn test_lndk_pay_offer() {
BlindedPath::new_for_message(&[pubkey_2, lnd_pubkey], &messenger_utils, &secp_ctx).unwrap();

// Make sure lndk successfully sends the invoice_request.
let handler = Arc::new(lndk::OfferHandler::new());
let handler = Arc::new(lndk::OfferHandler::new_default());
let messenger = lndk::LndkOnionMessenger::new();
let pay_cfg = PayOfferParams {
offer,
Expand All @@ -429,6 +430,7 @@ async fn test_lndk_pay_offer() {
client: client.clone(),
destination: Destination::BlindedPath(blinded_path.clone()),
reply_path: Some(reply_path),
response_invoice_timeout: None,
};
let log_dir = Some(
lndk_dir
Expand Down
Loading