Skip to content

Commit 5eb8a0f

Browse files
De-dedup paths between flow and async offer cache
We were previously duplicating paths_to_static_invoice_server between the OffersMessageFlow and the AsyncReceiveOfferCache, even though the paths were only used in one place in the flow. De-duplicate that to remove the risk of the structs getting out-of-sync.
1 parent 7833e53 commit 5eb8a0f

File tree

2 files changed

+11
-24
lines changed

2 files changed

+11
-24
lines changed

lightning/src/offers/async_receive_offer_cache.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,8 @@ impl AsyncReceiveOfferCache {
143143
}
144144
}
145145

146-
pub(super) fn paths_to_static_invoice_server(&self) -> Vec<BlindedMessagePath> {
147-
self.paths_to_static_invoice_server.clone()
146+
pub(super) fn paths_to_static_invoice_server(&self) -> &[BlindedMessagePath] {
147+
&self.paths_to_static_invoice_server[..]
148148
}
149149

150150
/// Sets the [`BlindedMessagePath`]s that we will use as an async recipient to interactively build

lightning/src/offers/flow.rs

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,6 @@ where
104104

105105
pending_async_payments_messages: Mutex<Vec<(AsyncPaymentsMessage, MessageSendInstructions)>>,
106106
async_receive_offer_cache: Mutex<AsyncReceiveOfferCache>,
107-
/// Blinded paths used to request offer paths from the static invoice server, if we are an async
108-
/// recipient.
109-
paths_to_static_invoice_server: Mutex<Vec<BlindedMessagePath>>,
110107

111108
#[cfg(feature = "dnssec")]
112109
pub(crate) hrn_resolver: OMNameResolver,
@@ -146,7 +143,6 @@ where
146143
pending_dns_onion_messages: Mutex::new(Vec::new()),
147144

148145
async_receive_offer_cache: Mutex::new(AsyncReceiveOfferCache::new()),
149-
paths_to_static_invoice_server: Mutex::new(Vec::new()),
150146
}
151147
}
152148

@@ -158,8 +154,6 @@ where
158154
pub fn with_async_payments_offers_cache(
159155
mut self, async_receive_offer_cache: AsyncReceiveOfferCache,
160156
) -> Self {
161-
self.paths_to_static_invoice_server =
162-
Mutex::new(async_receive_offer_cache.paths_to_static_invoice_server());
163157
self.async_receive_offer_cache = Mutex::new(async_receive_offer_cache);
164158
self
165159
}
@@ -174,15 +168,9 @@ where
174168
pub fn set_paths_to_static_invoice_server(
175169
&self, paths_to_static_invoice_server: Vec<BlindedMessagePath>,
176170
) -> Result<(), ()> {
177-
// Store the paths in the async receive cache so they are persisted with the cache, but also
178-
// store them in-memory in the `OffersMessageFlow` so the flow has access to them when building
179-
// onion messages to send to the static invoice server, without introducing undesirable lock
180-
// dependencies with the cache.
181-
*self.paths_to_static_invoice_server.lock().unwrap() =
182-
paths_to_static_invoice_server.clone();
183-
184171
let mut cache = self.async_receive_offer_cache.lock().unwrap();
185-
cache.set_paths_to_static_invoice_server(paths_to_static_invoice_server)
172+
cache.set_paths_to_static_invoice_server(paths_to_static_invoice_server.clone())?;
173+
Ok(())
186174
}
187175

188176
/// Gets the node_id held by this [`OffersMessageFlow`]`
@@ -1264,19 +1252,17 @@ where
12641252
R::Target: Router,
12651253
{
12661254
// Terminate early if this node does not intend to receive async payments.
1267-
if self.paths_to_static_invoice_server.lock().unwrap().is_empty() {
1255+
let mut cache = self.async_receive_offer_cache.lock().unwrap();
1256+
if cache.paths_to_static_invoice_server().is_empty() {
12681257
return Ok(());
12691258
}
12701259

12711260
let duration_since_epoch = self.duration_since_epoch();
12721261

12731262
// Update the cache to remove expired offers, and check to see whether we need new offers to be
12741263
// interactively built with the static invoice server.
1275-
let needs_new_offers = self
1276-
.async_receive_offer_cache
1277-
.lock()
1278-
.unwrap()
1279-
.prune_expired_offers(duration_since_epoch, timer_tick_occurred);
1264+
let needs_new_offers =
1265+
cache.prune_expired_offers(duration_since_epoch, timer_tick_occurred);
12801266

12811267
// If we need new offers, send out offer paths request messages to the static invoice server.
12821268
if needs_new_offers {
@@ -1292,18 +1278,19 @@ where
12921278
};
12931279

12941280
// We can't fail past this point, so indicate to the cache that we've requested new offers.
1295-
self.async_receive_offer_cache.lock().unwrap().new_offers_requested();
1281+
cache.new_offers_requested();
12961282

12971283
let mut pending_async_payments_messages =
12981284
self.pending_async_payments_messages.lock().unwrap();
12991285
let message = AsyncPaymentsMessage::OfferPathsRequest(OfferPathsRequest {});
13001286
enqueue_onion_message_with_reply_paths(
13011287
message,
1302-
&self.paths_to_static_invoice_server.lock().unwrap()[..],
1288+
cache.paths_to_static_invoice_server(),
13031289
reply_paths,
13041290
&mut pending_async_payments_messages,
13051291
);
13061292
}
1293+
core::mem::drop(cache);
13071294

13081295
if timer_tick_occurred {
13091296
self.check_refresh_static_invoices(

0 commit comments

Comments
 (0)