f: review comments

Author: joostjager

bindings/ldk_node.udl

@@ -158,9 +158,9 @@ interface Node {
 	[Throws=NodeError]
 	bytes export_pathfinding_scores();
 	[Throws=NodeError]
-	void set_paths_to_static_invoice_server(sequence<u8> paths);
+	void set_paths_to_static_invoice_server(bytes paths);
 	[Throws=NodeError]
-	sequence<u8> blinded_paths_for_async_recipient(sequence<u8> recipient_id);
+	bytes blinded_paths_for_async_recipient(bytes recipient_id);
 };
 
 [Enum]
@@ -214,7 +214,7 @@ interface Bolt12Payment {
 	[Throws=NodeError]
 	Refund initiate_refund(u64 amount_msat, u32 expiry_secs, u64? quantity, string? payer_note);
 	[Throws=NodeError]
-	Offer get_async_receive_offer();
+	Offer receive_async();
 };
 
 interface SpontaneousPayment {
@@ -318,7 +318,6 @@ enum NodeError {
 	"LiquiditySourceUnavailable",
 	"LiquidityFeeTooHigh",
 	"InvalidBlindedPaths",
-	"OperationFailed",
 };
 
 dictionary NodeStatus {

src/error.rs

@@ -122,8 +122,6 @@ pub enum Error {
 	LiquidityFeeTooHigh,
 	/// The given blinded paths are invalid.
 	InvalidBlindedPaths,
-	/// The requested operation failed.
-	OperationFailed,
 }
 
 impl fmt::Display for Error {
@@ -198,7 +196,6 @@ impl fmt::Display for Error {
 				write!(f, "The given operation failed due to the LSP's required opening fee being too high.")
 			},
 			Self::InvalidBlindedPaths => write!(f, "The given blinded paths are invalid."),
-			Self::OperationFailed => write!(f, "The requested operation failed."),
 		}
 	}
 }

src/event.rs

@@ -5,9 +5,8 @@
 // http://opensource.org/licenses/MIT>, at your option. You may not use this file except in
 // accordance with one or both of these licenses.
 
-use crate::static_invoice_store::StaticInvoiceStore;
+use crate::payment::static_invoice_store::StaticInvoiceStore;
 use crate::types::{CustomTlvRecord, DynStore, PaymentStore, Sweeper, Wallet};
-
 use crate::{
 	hex_utils, BumpTransactionEventHandler, ChannelManager, Error, Graph, PeerInfo, PeerStore,
 	UserChannelId,
@@ -1526,10 +1525,10 @@ where
 
 				match invoice {
 					Ok(Some(invoice)) => {
-						if let Err(_) =
+						if let Err(e) =
 							self.channel_manager.send_static_invoice(invoice, reply_path)
 						{
-							log_error!(self.logger, "Failed to send static invoice");
+							log_error!(self.logger, "Failed to send static invoice: {:?}", e);
 						}
 					},
 					Ok(None) => {},

src/io/mod.rs

@@ -73,3 +73,8 @@ pub(crate) const BDK_WALLET_TX_GRAPH_KEY: &str = "tx_graph";
 pub(crate) const BDK_WALLET_INDEXER_PRIMARY_NAMESPACE: &str = "bdk_wallet";
 pub(crate) const BDK_WALLET_INDEXER_SECONDARY_NAMESPACE: &str = "";
 pub(crate) const BDK_WALLET_INDEXER_KEY: &str = "indexer";
+
+// Static invoices will be persisted at "static_invoices/<sha256(recipient_id)>/<invoice_slot>".
+//
+// Example: static_invoices/039058c6f2c0cb492c533b0a4d14ef77cc0f78abccced5287d84a1a2011cfb81/001f
+pub(crate) const STATIC_INVOICES_PRIMARY_NAMESPACE: &str = "static_invoices";

src/lib.rs

@@ -94,9 +94,7 @@ pub mod logger;
 mod message_handler;
 pub mod payment;
 mod peer_store;
-mod rate_limiter;
 mod runtime;
-mod static_invoice_store;
 mod tx_broadcaster;
 mod types;
 mod wallet;
@@ -138,6 +136,7 @@ use gossip::GossipSource;
 use graph::NetworkGraph;
 use io::utils::write_node_metrics;
 use liquidity::{LSPS1Liquidity, LiquiditySource};
+use payment::static_invoice_store::StaticInvoiceStore;
 use payment::{
 	Bolt11Payment, Bolt12Payment, OnchainPayment, PaymentDetails, SpontaneousPayment,
 	UnifiedQrPayment,
@@ -175,8 +174,6 @@ use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Mutex, RwLock};
 use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
 
-use crate::static_invoice_store::StaticInvoiceStore;
-
 #[cfg(feature = "uniffi")]
 uniffi::include_scaffolding!("ldk_node");
 
@@ -1512,10 +1509,10 @@ impl Node {
 		let paths = self
 			.channel_manager
 			.blinded_paths_for_async_recipient(recipient_id, None)
-			.or(Err(Error::OperationFailed))?;
+			.or(Err(Error::InvalidBlindedPaths))?;
 
 		let mut bytes = Vec::new();
-		paths.write(&mut bytes).or(Err(Error::OperationFailed))?;
+		paths.write(&mut bytes).or(Err(Error::InvalidBlindedPaths))?;
 
 		Ok(bytes)
 	}

src/payment/bolt12.rs

@@ -451,15 +451,16 @@ impl Bolt12Payment {
 		Ok(maybe_wrap(refund))
 	}
 
-	/// Retrieve an [`Offer`] for receiving async payments as an often-offline recipient. Will only return an offer if
-	/// [`Node::set_paths_to_static_invoice_server`] was called and we succeeded in interactively building a
-	/// [`StaticInvoice`] with the static invoice server.
+	/// Retrieve an [`Offer`] for receiving async payments as an often-offline recipient.
+	///
+	/// Will only return an offer if [`Node::set_paths_to_static_invoice_server`] was called and we succeeded in
+	/// interactively building a [`StaticInvoice`] with the static invoice server.
 	///
 	/// Useful for posting offers to receive payments later, such as posting an offer on a website.
 	///
 	/// [`Node::set_paths_to_static_invoice_server`]: crate::Node::set_paths_to_static_invoice_server
 	/// [`StaticInvoice`]: lightning::offers::static_invoice::StaticInvoice
-	pub fn get_async_receive_offer(&self) -> Result<Offer, Error> {
+	pub fn receive_async(&self) -> Result<Offer, Error> {
 		self.channel_manager
 			.get_async_receive_offer()
 			.map(maybe_wrap)

src/payment/mod.rs

@@ -10,7 +10,9 @@
 mod bolt11;
 mod bolt12;
 mod onchain;
+mod rate_limiter;
 mod spontaneous;
+pub(crate) mod static_invoice_store;
 pub(crate) mod store;
 mod unified_qr;
 

src/rate_limiter.rs src/payment/rate_limiter.rssrc/rate_limiter.rs renamed to src/payment/rate_limiter.rs

@@ -2,9 +2,11 @@ use std::collections::HashMap;
 use std::time::{Duration, Instant};
 
 /// Implements a leaky-bucket style rate limiter parameterized by the max capacity of the bucket, the refill interval,
-/// and the max idle duration. For every passing of the refill interval, one token is added to the bucket, up to the
-/// maximum capacity. When the bucket has remained at the maximum capacity for longer than the max idle duration, it is
-/// removed to prevent memory leakage.
+/// and the max idle duration.
+///
+/// For every passing of the refill interval, one token is added to the bucket, up to the maximum capacity. When the
+/// bucket has remained at the maximum capacity for longer than the max idle duration, it is removed to prevent memory
+/// leakage.
 pub(crate) struct RateLimiter {
 	users: HashMap<Vec<u8>, Bucket>,
 	capacity: u32,

src/static_invoice_store.rs src/payment/static_invoice_store.rssrc/static_invoice_store.rs renamed to src/payment/static_invoice_store.rs

@@ -1,10 +1,15 @@
-use crate::{hex_utils, rate_limiter::RateLimiter, types::DynStore};
-use bitcoin::hashes::{sha256, Hash};
+use crate::hex_utils;
+use crate::io::STATIC_INVOICES_PRIMARY_NAMESPACE;
+use crate::payment::rate_limiter::RateLimiter;
+use crate::types::DynStore;
+
+use bitcoin::hashes::sha256::Hash as Sha256;
+use bitcoin::hashes::Hash;
+
 use lightning::{offers::static_invoice::StaticInvoice, util::ser::Writeable};
-use std::{
-	sync::{Arc, Mutex},
-	time::Duration,
-};
+
+use std::sync::{Arc, Mutex};
+use std::time::Duration;
 
 pub(crate) struct StaticInvoiceStore {
 	kv_store: Arc<DynStore>,
@@ -13,12 +18,6 @@ pub(crate) struct StaticInvoiceStore {
 }
 
 impl StaticInvoiceStore {
-	// Static invoices are stored at "static_invoices/<sha256(recipient_id)>/<invoice_slot>".
-	//
-	// Example:
-	// static_invoices/039058c6f2c0cb492c533b0a4d14ef77cc0f78abccced5287d84a1a2011cfb81/001f
-	const PRIMARY_NAMESPACE: &str = "static_invoices";
-
 	const RATE_LIMITER_BUCKET_CAPACITY: u32 = 5;
 	const RATE_LIMITER_REFILL_INTERVAL: Duration = Duration::from_millis(100);
 	const RATE_LIMITER_MAX_IDLE: Duration = Duration::from_secs(600);
@@ -57,14 +56,16 @@ impl StaticInvoiceStore {
 
 		let (secondary_namespace, key) = Self::get_storage_location(invoice_slot, recipient_id);
 
-		self.kv_store.read(Self::PRIMARY_NAMESPACE, &secondary_namespace, &key).and_then(|data| {
-			data.try_into().map(Some).map_err(|e| {
-				lightning::io::Error::new(
-					lightning::io::ErrorKind::InvalidData,
-					format!("Failed to parse static invoice: {:?}", e),
-				)
-			})
-		})
+		self.kv_store.read(STATIC_INVOICES_PRIMARY_NAMESPACE, &secondary_namespace, &key).and_then(
+			|data| {
+				data.try_into().map(Some).map_err(|e| {
+					lightning::io::Error::new(
+						lightning::io::ErrorKind::InvalidData,
+						format!("Failed to parse static invoice: {:?}", e),
+					)
+				})
+			},
+		)
 	}
 
 	pub(crate) async fn handle_persist_static_invoice(
@@ -77,14 +78,14 @@ impl StaticInvoiceStore {
 		let mut buf = Vec::new();
 		invoice.write(&mut buf)?;
 
-		self.kv_store.write(Self::PRIMARY_NAMESPACE, &secondary_namespace, &key, buf)
+		self.kv_store.write(STATIC_INVOICES_PRIMARY_NAMESPACE, &secondary_namespace, &key, buf)
 	}
 
 	fn get_storage_location(invoice_slot: u16, recipient_id: Vec<u8>) -> (String, String) {
-		let hash = sha256::Hash::hash(&recipient_id).to_byte_array();
+		let hash = Sha256::hash(&recipient_id).to_byte_array();
 		let secondary_namespace = hex_utils::to_string(&hash);
 
-		let key = hex_utils::to_string(&invoice_slot.to_be_bytes());
+		let key = format!("{:05}", invoice_slot);
 		(secondary_namespace, key)
 	}
 }

tests/integration_tests_rust.rs

@@ -1214,7 +1214,7 @@ fn static_invoice_server() {
 	node_receiver.set_paths_to_static_invoice_server(blinded_paths).unwrap();
 
 	let offer = loop {
-		if let Ok(offer) = node_receiver.bolt12_payment().get_async_receive_offer() {
+		if let Ok(offer) = node_receiver.bolt12_payment().receive_async() {
 			break offer;
 		}