-
Notifications
You must be signed in to change notification settings - Fork 411
BOLT 12 Offers utilities #2578
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
BOLT 12 Offers utilities #2578
Changes from all commits
a6cd661
86e2b00
b78cb69
a4894bd
cfe6b95
81c6147
94573dd
840efd5
8b442fe
6dc4223
622f7f2
11fb9c4
7c6e62f
80ae66a
905028b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ use bitcoin::secp256k1::{SecretKey,PublicKey}; | |
use bitcoin::secp256k1::Secp256k1; | ||
use bitcoin::{LockTime, secp256k1, Sequence}; | ||
|
||
use crate::blinded_path::BlindedPath; | ||
use crate::chain; | ||
use crate::chain::{Confirm, ChannelMonitorUpdateStatus, Watch, BestBlock}; | ||
use crate::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator}; | ||
|
@@ -55,6 +56,9 @@ use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError}; | |
use crate::ln::outbound_payment; | ||
use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment, SendAlongPathArgs}; | ||
use crate::ln::wire::Encode; | ||
use crate::offers::offer::{DerivedMetadata, OfferBuilder}; | ||
use crate::offers::parse::Bolt12SemanticError; | ||
use crate::offers::refund::RefundBuilder; | ||
use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider, WriteableEcdsaChannelSigner}; | ||
use crate::util::config::{UserConfig, ChannelConfig, ChannelConfigUpdate}; | ||
use crate::util::wakers::{Future, Notifier}; | ||
|
@@ -4791,6 +4795,10 @@ where | |
/// with the current [`ChannelConfig`]. | ||
/// * Removing peers which have disconnected but and no longer have any channels. | ||
/// * Force-closing and removing channels which have not completed establishment in a timely manner. | ||
/// * Forgetting about stale outbound payments, either those that have already been fulfilled | ||
/// or those awaiting an invoice that hasn't been delivered in the necessary amount of time. | ||
/// The latter is determined using the system clock in `std` and the block time minus two | ||
/// hours in `no-std`. | ||
/// | ||
/// Note that this may cause reentrancy through [`chain::Watch::update_channel`] calls or feerate | ||
/// estimate fetches. | ||
|
@@ -5019,7 +5027,18 @@ where | |
self.finish_close_channel(shutdown_res); | ||
} | ||
|
||
self.pending_outbound_payments.remove_stale_payments(&self.pending_events); | ||
#[cfg(feature = "std")] | ||
let duration_since_epoch = std::time::SystemTime::now() | ||
.duration_since(std::time::SystemTime::UNIX_EPOCH) | ||
.expect("SystemTime::now() should come after SystemTime::UNIX_EPOCH"); | ||
#[cfg(not(feature = "std"))] | ||
let duration_since_epoch = Duration::from_secs( | ||
self.highest_seen_timestamp.load(Ordering::Acquire).saturating_sub(7200) as u64 | ||
jkczyz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
); | ||
|
||
self.pending_outbound_payments.remove_stale_payments( | ||
duration_since_epoch, &self.pending_events | ||
); | ||
|
||
// Technically we don't need to do this here, but if we have holding cell entries in a | ||
// channel that need freeing, it's better to do that here and block a background task | ||
|
@@ -7108,6 +7127,71 @@ where | |
} | ||
} | ||
|
||
/// Creates an [`OfferBuilder`] such that the [`Offer`] it builds is recognized by the | ||
/// [`ChannelManager`] when handling [`InvoiceRequest`] messages for the offer. The offer will | ||
/// not have an expiration unless otherwise set on the builder. | ||
/// | ||
/// Uses a one-hop [`BlindedPath`] for the offer with [`ChannelManager::get_our_node_id`] as the | ||
/// introduction node and a derived signing pubkey for recipient privacy. As such, currently, | ||
/// the node must be announced. Otherwise, there is no way to find a path to the introduction | ||
/// node in order to send the [`InvoiceRequest`]. | ||
/// | ||
/// [`Offer`]: crate::offers::offer::Offer | ||
/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest | ||
pub fn create_offer_builder( | ||
jkczyz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
&self, description: String | ||
) -> OfferBuilder<DerivedMetadata, secp256k1::All> { | ||
valentinewallace marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let node_id = self.get_our_node_id(); | ||
let expanded_key = &self.inbound_payment_key; | ||
let entropy = &*self.entropy_source; | ||
let secp_ctx = &self.secp_ctx; | ||
let path = self.create_one_hop_blinded_path(); | ||
|
||
OfferBuilder::deriving_signing_pubkey(description, node_id, expanded_key, entropy, secp_ctx) | ||
valentinewallace marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.chain_hash(self.chain_hash) | ||
.path(path) | ||
} | ||
|
||
/// Creates a [`RefundBuilder`] such that the [`Refund`] it builds is recognized by the | ||
/// [`ChannelManager`] when handling [`Bolt12Invoice`] messages for the refund. The builder will | ||
/// have the provided expiration set. Any changes to the expiration on the returned builder will | ||
/// not be honored by [`ChannelManager`]. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to mention the two-hour thing here, its a bit awkward its buried in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated in #2039. |
||
/// | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh also we should probably say something about how you can use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated in #2039. |
||
/// The provided `payment_id` is used to ensure that only one invoice is paid for the refund. | ||
/// | ||
/// Uses a one-hop [`BlindedPath`] for the refund with [`ChannelManager::get_our_node_id`] as | ||
/// the introduction node and a derived payer id for sender privacy. As such, currently, the | ||
/// node must be announced. Otherwise, there is no way to find a path to the introduction node | ||
/// in order to send the [`Bolt12Invoice`]. | ||
/// | ||
/// [`Refund`]: crate::offers::refund::Refund | ||
/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice | ||
pub fn create_refund_builder( | ||
&self, description: String, amount_msats: u64, absolute_expiry: Duration, | ||
payment_id: PaymentId, retry_strategy: Retry, max_total_routing_fee_msat: Option<u64> | ||
) -> Result<RefundBuilder<secp256k1::All>, Bolt12SemanticError> { | ||
let node_id = self.get_our_node_id(); | ||
let expanded_key = &self.inbound_payment_key; | ||
let entropy = &*self.entropy_source; | ||
let secp_ctx = &self.secp_ctx; | ||
let path = self.create_one_hop_blinded_path(); | ||
|
||
let builder = RefundBuilder::deriving_payer_id( | ||
description, node_id, expanded_key, entropy, secp_ctx, amount_msats, payment_id | ||
)? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pretty awkward to return a full huge enum just if the user provided an amount > MAX_VALUE_MSAT. Can we check it first and unwrap the builder? Then we don't have to add a dup payment id to Bolt12 errors, which feels wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't we still need two different errors? One for the amount and one for duplicate payment id? If we are fine just using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean we can almost just panic if the amount is too high, but I think its fine to tell users "we return an error if you duplicate the payment id or the amount is greater than max-amount". Users can figure out if the amount isnt invalid and just assume the payment id is duplicative, basically. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm going to leave this as-is. We have a similar problem in |
||
.chain_hash(self.chain_hash) | ||
.absolute_expiry(absolute_expiry) | ||
.path(path); | ||
|
||
self.pending_outbound_payments | ||
.add_new_awaiting_invoice( | ||
payment_id, absolute_expiry, retry_strategy, max_total_routing_fee_msat, | ||
valentinewallace marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
.map_err(|_| Bolt12SemanticError::DuplicatePaymentId)?; | ||
|
||
Ok(builder) | ||
} | ||
|
||
/// Gets a payment secret and payment hash for use in an invoice given to a third party wishing | ||
/// to pay us. | ||
/// | ||
|
@@ -7208,6 +7292,14 @@ where | |
inbound_payment::get_payment_preimage(payment_hash, payment_secret, &self.inbound_payment_key) | ||
} | ||
|
||
/// Creates a one-hop blinded path with [`ChannelManager::get_our_node_id`] as the introduction | ||
/// node. | ||
fn create_one_hop_blinded_path(&self) -> BlindedPath { | ||
let entropy_source = self.entropy_source.deref(); | ||
let secp_ctx = &self.secp_ctx; | ||
BlindedPath::one_hop_for_message(self.get_our_node_id(), entropy_source, secp_ctx).unwrap() | ||
} | ||
|
||
/// Gets a fake short channel id for use in receiving [phantom node payments]. These fake scids | ||
/// are used when constructing the phantom invoice's route hints. | ||
/// | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind too much, but why remove this?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restored it but now the test needs to check against
InvalidFirstHop
, which I'm not sure if that is intended?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mm that shouldn't be intended. It looks like that's because the first-hop peer in the reply path isn't connected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will reexamine this but want to get the other changes pushed for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'd also be fine with saving this for follow-up if you want to note it in #1970.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, other than the introduction node being wrong, when hardcoding a fix I'm also seeing this logging:
Instead of the expected:
But I don't understand how my change would affect the
path_id
. Any ideas?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, nevermind on this. That was just because I had been building using some unpublished commits instead of during the rebase. 🤦♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #2671