Stop storing pending inbound payment data; instead make it derivable on receive #1177

valentinewallace · 2021-11-18T17:10:22Z

Based on #1180

codecov · 2021-11-18T17:44:36Z

Codecov Report

Merging #1177 (a47eb4d) into main (ba50dd5) will increase coverage by 1.26%.
The diff coverage is 97.17%.

@@            Coverage Diff             @@
##             main    #1177      +/-   ##
==========================================
+ Coverage   90.46%   91.72%   +1.26%     
==========================================
  Files          69       69              
  Lines       37017    43475    +6458     
==========================================
+ Hits        33486    39879    +6393     
- Misses       3531     3596      +65

Impacted Files	Coverage Δ
lightning/src/ln/channel.rs	`92.93% <0.00%> (+4.10%)`	⬆️
lightning/src/ln/functional_test_utils.rs	`96.56% <ø> (+1.42%)`	⬆️
lightning/src/util/test_utils.rs	`81.74% <50.00%> (-0.17%)`	⬇️
lightning/src/ln/channelmanager.rs	`86.85% <98.01%> (+3.10%)`	⬆️
lightning/src/chain/keysinterface.rs	`98.32% <100.00%> (+3.19%)`	⬆️
lightning/src/ln/functional_tests.rs	`98.54% <100.00%> (+1.21%)`	⬆️
lightning/src/util/chacha20.rs	`96.11% <100.00%> (+0.73%)`	⬆️
lightning/src/ln/onion_route_tests.rs	`96.49% <0.00%> (-0.12%)`	⬇️
lightning/src/ln/monitor_tests.rs	`100.00% <0.00%> (ø)`
lightning/src/ln/payment_tests.rs	`99.10% <0.00%> (+<0.01%)`	⬆️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba50dd5...a47eb4d. Read the comment docs.

TheBlueMatt

Just looked at the creation side, I assume the receiving side matches. Looks good, but we're gonna need an AES implementation (or, I guess, any block cipher, but dunno why we'd do anything else).

The simplest approach may be to just transliterate https://github.com/bitcoin-core/ctaes/blob/master/ctaes.c into Rust and put it in util/.

lightning/src/ln/channelmanager.rs

lightning/src/chain/keysinterface.rs

TheBlueMatt · 2021-11-23T20:15:53Z

lightning/src/ln/channelmanager.rs

+	// `create_inbound_payment_for_hash` for details on our checks.
+	fn verify_inbound_payment_data(&self, payment_hash: PaymentHash, payment_data: msgs::FinalOnionHopData) -> Result<Option<PaymentPreimage>, ()> {
+		let (iv_bytes, encrypted_metadata_bytes) = payment_data.payment_secret.0.split_at(16);
+		let mut chacha = ChaCha20::new(&self.inbound_payment_key[..], &iv_bytes[..12]);


Right, we're gonna have to define our own custom variant of ChaCha here to use the full 16 byte seed. ChaCha20-RFC defaults to using 4 bytes for the counter and 12 bytes for the IV, but we actually only want one block and really want to use the full 16 bytes for the IV. If we drop the counter bytes we can replace them with the other 4 bytes of IV. This is algorithmically identical to using the 12 byte IV and then having 4 bytes of magic "seek to output index X", i.e. it can't be worse than simply using the 12-byte nonce, but it should actually provide 16 byte IV. API-wise, I think we should have a function in ChaCha20 that takes a 32-byte key and 16-byte nonce and then simply returns 32/64 bytes from the ChaCha stream, communicating via API that this variant only supports one block.

lightning/src/ln/channelmanager.rs

TheBlueMatt · 2021-11-23T20:37:54Z

lightning/src/ln/channelmanager.rs

 	/// [`PaymentReceived`]: events::Event::PaymentReceived
 	/// [`PaymentReceived::payment_preimage`]: events::Event::PaymentReceived::payment_preimage
 	/// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash
+	// LDK does not store any data for pending inbound payments. Instead, in the case of


Probably worth putting part of this, but probably not the actual algorithm, in the docs, no?

TheBlueMatt

Looking pretty good. One major question about how we do the upgrade process here.

TheBlueMatt · 2021-11-26T18:09:31Z

lightning/src/ln/channelmanager.rs

-													},
-													amt: total_value,
-												});
-												// Only ever generate at most one PaymentReceived


Presumably the removal of this should imply a docs change on Event::PaymentReceived?

I don't believe this was resolved? We need to state explicitly that a PaymentReceived event may be called multiple times for the same payment. Also, shouldn't we still remove the entry for users of the old API? Otherwise does the payment ever get removed?

Also, shouldn't we still remove the entry for users of the old API? Otherwise does the payment ever get removed?

Good catch. These payments would still have gotten removed on best_block_updated if they've expired, but that was a bug

Let me know if the docs updates are more what you're thinking.

lightning/src/ln/channelmanager.rs

lightning/src/util/chacha20.rs

valentinewallace · 2021-11-30T18:06:36Z

I don't think the failures on beta are related to my PR 🤔

TheBlueMatt · 2021-11-30T18:39:40Z

I don't think the failures on beta are related to my PR thinking

They're hitting on main too. Will report to rustc...Another day, another rustc bug.

TheBlueMatt · 2021-11-30T19:03:29Z

Yep, issue is rust-lang/rust#91398

TheBlueMatt

Rapidly getting there, pretty happy with this.

lightning/src/chain/keysinterface.rs

lightning/src/ln/channelmanager.rs

lightning/src/chain/keysinterface.rs

lightning/src/ln/channelmanager.rs

lightning-invoice/src/utils.rs

lightning/src/util/chacha20.rs

lightning/src/ln/channelmanager.rs

jkczyz

Just going through verify now but wanted to send out accumulating comments. Almost there!

lightning/src/util/chacha20.rs

lightning/src/ln/channelmanager.rs

jkczyz · 2021-12-15T18:40:38Z

lightning/src/ln/channelmanager.rs

+		match inbound_payment::verify(bad_payment_hash, payment_data.clone(), nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger) {
+			Ok(_) => panic!("Unexpected ok"),
+			Err(()) => {
+				nodes[0].logger.assert_log_contains("lightning::ln::channelmanager::inbound_payment".to_string(), "Failing HTLC with user-generated payment_hash".to_string(), 1);


Would be nice if the error was an enum (implementing Debug) to check against and logged at the call site rather than asserting for a particular log message. Happy if you want to leave this to a follow-up, though.

Cool, there should be a quick follow-up for #1213 so happy to put it in there

lightning/src/ln/channelmanager.rs

jkczyz

LGTM. inbound_payment is super easy to follow!

This will allow us to retrieve key material for encrypting/decrypting inbound payment info, in upcoming commits

This DRY-ed code will be used in upcoming commits when we stop storing inbound payment data

In the next commit, we'll want to get a single block from a chacha stream that takes a 16-byte nonce.

and replace payment_secret with encrypted metadata See docs on `inbound_payment::verify` for details Also add min_value checks to all create_inbound_payment* methods

…ate_inbound_payment` in an invoice creation utility. Note that if the error type of `create_inbound_payment` ever changed, we'd be forced to update the invoice utility's callsite to handle the new error

TheBlueMatt · 2021-12-16T23:53:04Z

lightning/src/ln/channelmanager.rs

+
+	impl ExpandedKey {
+		pub(super) fn new(key_material: &KeyMaterial) -> ExpandedKey {
+			hkdf_extract_expand(&vec![0], &key_material)


If you have a sec before Jeff reviews, could you change the salt here to some string? eg b"LDK Inbound Payment Key Expansion"?

TheBlueMatt · 2021-12-16T23:53:41Z

lightning/src/util/events.rs

 	/// [`ChannelManager::fail_htlc_backwards`]: crate::ln::channelmanager::ChannelManager::fail_htlc_backwards
 	PaymentReceived {
-		/// The hash for which the preimage should be handed to the ChannelManager.
+		/// The hash for which the preimage should be handed to the ChannelManager. Note that LDK will


This seems like something that should be documented on ChannelManager::create_inbound_payment_for_hash.

Leftover feedback from lightningdevkit#1177

Add methods to count total fees and total amount in a Route #999 * Added `get_total_fees` method to route, to calculate all the fees paid accross each path. * Added `get_total_amount` method to route, to calculate the total of actual amounts paid in each path. Update docs to specify where process events is called Add Event::ChannelClosed generation at channel shutdown Rename MonitorEvent::CommitmentTxBroadcasted to CommitmentTxConfirmed Extend MsgHandleErrInternal with a new chan_id field Option<[u8; 32]> This field is used in next commit to generate appropriate ChannelClosed event at `handle_error()` processing. Add ChannelClosed generation at cooperative/force-close/error processing When we detect a channel `is_shutdown()` or call on it `force_shutdown()`, we notify the user with a Event::ChannelClosed informing about the id and closure reason. Add `pending_events` deadlock detection in `handle_error` Bump HTTP read timeout to match reality of Bitcoin Core blocking Fix future unknown `Event` variant backwards compatibility In 8ffc2d1742ff1171a87b0410b21cbbd557ff8247, in 0.0.100, we added a backwards compatibility feature to the reading of `Event`s - if the type was unknown and odd, we'd simply ignore the event and treat it as no event. However, we failed to read the length-prefixed TLV stream when doing so, resulting in us reading some of the skipped-event data as the next event or other data in the ChannelManager. We fix this by reading the varint length prefix written, then skipping that many bytes when we come across an unknown odd event type. Fix a panic in Route's new fee-calculation methods and clean up This addresses Val's feedback on the new Route fee- and amount-calculation methods, including fixing the panic she identified and cleaning up various docs and comments. Adds Transaction to lighting-block-sync::convert Includes disclaimer in docs, see https://github.com/rust-bitcoin/rust-lightning/pull/1061#issuecomment-911960862 Rename PaymentFailed -> PaymentPathFailed Since we don't want to imply to users that a payment has completely failed when it really has just partially failed Add path field to PaymentPathFailed event Fix windows-only test failure added in #997 This is a trivial bugfix to add a missing test updated required in PR 997. Move trait bounds on `wire::Type` from use to the trait itself `wire::Type` is only (publicly) used as the `CustomMessage` associated type in `CustomMessageReader`, where it has additional trait bounds on `Debug` and `Writeable`. The documentation for `Type` even mentions that you need to implement `Writeable` because this is the one place it is used. To make this more clear, we move the type bounds onto the trait itself and not on the associated type. This is also the only practical way to build C bindings for `Type` as we cannot have a concrete, single, `Type` struct in C which only optionally implements various subtraits, at least not without runtime checking of the type bounds. Make `ChainMonitor::get_claimable_balances` take a slice of refs For the same reason as `get_route`, a slice of objects isn't practical to map to bindings - the objects in the bindings space are structs with a pointer and some additional metadata. Thus, to create a slice of them, we'd need to take ownership of the objects behind the pointer, place them into a slace, and then restore them to the pointer. This would be a lot of memory copying and marshalling, not to mention wouldn't be thread-safe, which the same function otherwise would be if we used a slice of references instead of a slice of objects. Use Infallible for the unconstructable default custom message type When we landed custom messages, we used the empty tuple for the custom message type for `IgnoringMessageHandler`. This was fine, except that we also implemented `Writeable` to panic when writing a `()`. Later, we added support for anchor output construction in CommitmentTransaction, signified by setting a field to `Some(())`, which is serialized as-is. This causes us to panic when writing a `CommitmentTransaction` with `opt_anchors` set. Note that we never set it inside of LDK, but downstream users may. Instead, we implement `Writeable` to write nothing for `()` and use `core::convert::Infallible` for the default custom message type as it is, appropriately, unconstructable. This also makes it easier to implement various things in bindings, as we can always assume `Infallible`-conversion logic is unreachable. Drop redundant generic bounds when the trait requires the bounds Make method time on trait impl explitit to help bindings generator Associated types in C bindings is somewhat of a misnomer - we concretize each trait to a single struct. Thus, different trait implementations must still have the same type, which defeats the point of associated types. In this particular case, however, we can reasonably special-case the `Infallible` type, as an instance of it existing implies something has gone horribly wrong. In order to help our bindings code figure out how to do so when referencing a parent trait's associated type, we specify the explicit type in the implementation method signature. Update CHANGELOG for 0.0.101 Bump Crate versions to 0.0.101 (and invoice to 0.9) Make `NetworkGraph` Clone-able again There isn't a lot of user-utility for cloning `NetworkGraph` directly (its a rather large struct, and there probably isn't a lot of reason to have *multiple* `NetworkGraph`s). Thus, when locks were pushed down into it, the `Clone`-ability of it was dropped as well. Sadly, mapping the Java memory model onto: * `Read`-ing a `NetworkGraph`, creating a Java-owned `NetworkGraph` object that the JVM will destruct for us, * Passing it to a `NetGraphMsgHandler`, which now expects to own the `NetworkGraph`, including destructing it, isn't really practical without adding a clone in between. Given this, and the fact that there's nothing inherently wrong with clone-ing a `NetworkGraph`, we simply re-add `Clone` here. Drop broken test that is unfixable due to being undocumented This should be reverted at some point, but the test is deficient and breaks on later changes that are important to land ASAP. Increase our default/minimum dust limit to 354 sat/vbytes 330 sat/vbyte, the current value, is not sufficient to ensure a future segwit script longer than 32 bytes meets the dust limit if used for a shutdown script. Thus, we can either check the value on shutdown or we can simply require segwit outputs and require a dust value of no less than 354 sat/vbyte. We swap the minimum dust value to 354 sat/vbyte here, requiring segwit scripts in a future commit. See https://github.com/lightningnetwork/lightning-rfc/issues/905 Reduce the maximum allowed counterparty dust limit to 546 sat/vbyte 546 sat/vbyte is the current default dust limit on most implementations, matching the network dust limit for P2SH outputs. Implementations don't currently appear to send any larger dust limits, and allowing a larger dust limit implies higher payment failure risk, so we'd like to be as tight as we can here. Require user cooperative close payout scripts to be Segwit There is little reason for users to be paying out to non-Segwit scripts when closing channels at this point. Given we will soon, in rare cases, force-close during shutdown when a counterparty closes to a non-Segwit script, we should also require it of our own users. Force-close channels if closing transactions may be non-standard If a counterparty (or an old channel of ours) uses a non-segwit script for their cooperative close payout, they may include an output which is unbroadcastable due to not meeting the network dust limit. Here we check for this condition, force-closing the channel instead if we find an output in the closing transaction which does not meet the limit. Rename MIN_DUST_LIMIT_SATOSHIS constant to disambiguate chan vs P2P While channel and P2P network dust limits are related, they're ultimately two different things, and thus their constant names should reference that. Regenerate PendingHTLCsForwardable on reload instead of serializing When we are prepared to forward HTLCs, we generate a PendingHTLCsForwardable event with a time in the future when the user should tell us to forward. This provides some basic batching of forward events, improving privacy slightly. After we generate the event, we expect users to spawn a timer in the background and let us know when it finishes. However, if the user shuts down before the timer fires, the user will restart and have no idea that HTLCs are waiting to be forwarded/received. To fix this, instead of serializing PendingHTLCsForwardable events to disk while they're pending (before the user starts the timer), we simply regenerate them when a ChannelManager is deserialized with HTLCs pending. Fixes #1042 Don't apply monitor updates after watch_channel PermFail The full stack fuzzer found an unreachable panic where we receive a FundingSigned with a duplicate channel outpoint. Update Watch docs to disallow dup channel outpoints on watch_channel Rename MppId to PaymentId Leftover from previous PR Jeff feedback. Useful in upcoming commits as we'll expose this to users for payment retries Return PaymentId from send_*payment functions Used in upcoming commits for retries Refactor send_payment internals for retries We want to reuse send_payment internal functions for retries, so some need to now be parameterized by PaymentId to avoid generating a new PaymentId on retry Refactor send_payment internals for retries 2 Retrying a partial payment means send_payment_internal needs to be parameterized by a total payment amount, else 'HTLC values do not match' errors channelmanager: Add retry data to pending_outbound_payments Add method to retry payments Don't remove failed payments when all paths fail This is because we want the ability to retry completely failed payments. Upcoming commits will remove these payments on timeout to prevent DoS issues Also test that this removal allows retrying single-path payments Expire outbound payments after 3 blocks if no parts are pending Correct step number in `get_route` Consider many first-hop paths to the same counterparty in routing Previously we'd simply overwritten "the" first hop path to each counterparty when routing, however this results in us ignoring all channels except the last one in the `ChannelDetails` list per counterparty. f readability improvements from val Update Event::PaymentReceived docs since we require payment secret Users no longer need to verify the amounts of received payments as the payment secret will protect us against the probing attacks such verification was intended to fix. Move tests of payment retries into a new module Move pending payment tracking to after the new HTLC flies If we attempt to send a payment, but the HTLC cannot be send due to local channel limits, we'll provide the user an error but end up with an entry in our pending payment map. This will result in a memory leak as we'll never reclaim the pending payment map entry. Correct error returned when `retry_payment` doesn't have a payment Add payment_hash to PaymentSent #999 Replace PublicKey with [u8; 33] in NetworkGraph Adds DiscardFunding event During the event of a channel close, if the funding transaction is yet to be broadcasted then a DiscardFunding event is issued along with the ChannelClose event. Use local channel state when constructing routes in test macro This is a bit more realistic and needed to route over non-public channels. Fix loop label shadowing warning Remove special case for onion error expiry_too_far With channel scoring and payment retries, it is no longer necessary to have expiry_too_far imply a payment failure. Pass hop index in construct_onion_keys_callback This simplifies failing route hop calculation, which will be useful for later identifying the failing hop for PaymentFailed events. Clean up fee_insufficient computation Add failing short channel id to PaymentPathFailed This will be useful for scoring channels when a payment fails. Expose ReadOnlyNetworkGraph::get_addresses to C by cloning result We cannot expose ReadOnlyNetworkGraph::get_addresses as is in C as it returns a list of references to an enum, which the bindings dont support. Instead, we simply clone the result so that it doesn't contain references. Speed up test_timer_tick_called Fix unused variable warnings in fuzzer Replace get_route with get_route_and_payment_hash The interface for get_route will change to take a scorer. Using get_route_and_payment_hash whenever possible allows for keeping the scorer inside get_route_and_payment_hash rather than at every call site. Replace get_route with get_route_and_payment_hash wherever possible. Additionally, update get_route_and_payment_hash to use the known invoice features and the sending node's logger. Move mpp_failure test to payment_tests.rs Move `Persist` trait to chainmonitor as that's the only reference Move ChannelMonitorUpdateErr to chain as it is a chain::Watch val Make `ChainMonitor::monitors` private and expose monitor via getter Exposing a `RwLock<HashMap<>>` directly was always a bit strange, and in upcoming changes we'd like to change the internal datastructure in `ChainMonitor`. Further, the use of `RwLock` and `HashMap` meant we weren't able to expose the ChannelMonitors themselves to users in bindings, leaving a bindings/rust API gap. Thus, we take this opportunity go expose ChannelMonitors directly via a wrapper, hiding the internals of `ChainMonitor` behind getters. We also update tests to use the new API. Simplify channelmonitor tests which use chain::Watch and Persister test_simple_monitor_permanent_update_fail and test_simple_monitor_temporary_update_fail both have a mode where they use either chain::Watch or persister to return errors. As we won't be doing any returns directly from the chain::Watch wrapper in a coming commit, the chain::Watch-return form of the test will no longer make sense. Handle Persister returning TemporaryFailure for new channels Previously, if a Persister returned a TemporaryFailure error when we tried to persist a new channel, the ChainMonitor wouldn't track the new ChannelMonitor at all, generating a PermanentFailure later when the updating is restored. This fixes that by correctly storing the ChannelMonitor on TemporaryFailures, allowing later update restoration to happen normally. This is (indirectly) tested in the next commit where we use Persister to return all monitor-update errors. Use Persister to return errors in tests not chain::Watch As ChainMonitor will need to see those errors in a coming PR, we need to return errors via Persister so that our ChainMonitor chain::Watch implementation sees them. Use Persister to return errors in fuzzers not chain::Watch Fixed 'Advancing Bitcoin' video URL. Add channel scoring to get_route Failed payments may be retried, but calling get_route may return a Route with the same failing path. Add a routing::Score trait used to parameterize get_route, which it calls to determine how much a channel should be penalized in terms of msats willing to pay to avoid the channel. Also, add a Scorer struct that implements routing::Score with a constant constant penalty. Subsequent changes will allow for more robust scoring by feeding back payment path success and failure to the scorer via event handling. Return the temporary channel id in success from `create_channel` This makes it more practical for users to track channels prior to funding, especially if the channel fails because the peer rejects it for a parameter mismatch. Include the user channel id counter in Event::ChannelClosed This makes it more practical for users to track channels using their own IDs, especially across funding. Rename create_channel param to user_channel_id to standardize it Add CHANGELOG entries for 0.0.102 Bump crate versions to 0.0.102 and lightning-invoice 0.10 Simplify prefers_shorter_route_with_higher_fees In order to make the scoring tests easier to read, only check the relevant RouteHop fields. The remaining fields are tested elsewhere. Expand the test to show the path used without scoring. Add source and target nodes to routing::Score Expand routing::Score::channel_penalty_msat to include the source and target node ids of the channel. This allows scorers to avoid certain nodes altogether if desired. Move MonitorEvent serialization to TLV-enum-upgradable from custom Move the two-AtomicUsize counter in peer_handler to a util struct We also take this opportunity to drop byte_utils::le64_to_array, as our MSRV now supports the native to_le_bytes() call. Move ChannelManager::monitor_updated to a MonitorEvent In the next commit we'll need ChainMonitor to "see" when a monitor persistence completes, which means `monitor_updated` needs to move to `ChainMonitor`. The simplest way to then communicate that information to `ChannelManager` is via `MonitorEvet`s, which seems to line up ok, even if they're now constructed by multiple different places. Use an opaque type to describe monitor updates in Persist In the next commit, we'll be originating monitor updates both from the ChainMonitor and from the ChannelManager, making simple sequential update IDs impossible. Further, the existing async monitor update API was somewhat hard to work with - instead of being able to generate monitor_updated callbacks whenever a persistence process finishes, you had to ensure you only did so at least once all previous updates had also been persisted. Here we eat the complexity for the user by moving to an opaque type for monitor updates, tracking which updates are in-flight for the user and only generating monitor-persisted events once all pending updates have been committed. Persist `ChannelMonitor`s after new blocks are connected This resolves several user complaints (and issues in the sample node) where startup is substantially delayed as we're always waiting for the chain data to sync. Further, in an upcoming PR, we'll be reloading pending payments from ChannelMonitors on restart, at which point we'll need the change here which avoids handling events until after the user has confirmed the `ChannelMonitor` has been persisted to disk. It will avoid a race where we * send a payment/HTLC (persisting the monitor to disk with the HTLC pending), * force-close the channel, removing the channel entry from the ChannelManager entirely, * persist the ChannelManager, * connect a block which contains a fulfill of the HTLC, generating a claim event, * handle the claim event while the `ChannelMonitor` is being persisted, * persist the ChannelManager (before the CHannelMonitor is persisted fully), * restart, reloading the HTLC as a pending payment in the ChannelManager, which now has no references to it except from the ChannelMonitor which still has the pending HTLC, * replay the block connection, generating a duplicate PaymentSent event. Update test_dup_htlc_onchain_fails_on_reload for new persist API ChannelMonitors now require that they be re-persisted before MonitorEvents be provided to the ChannelManager, the exact thing that test_dup_htlc_onchain_fails_on_reload was testing for when it *didn't* happen. As such, test_dup_htlc_onchain_fails_on_reload is now testing that we bahve correctly when the API guarantees are not met, something we don't need to do. Here, we adapt it to test the new API requirements through ChainMonitor's calls to the Persist trait instead. Always release `MonitorEvent`s to `ChannelManager` after 3 blocks If we have a `ChannelMonitor` update from an on-chain event which returns a `TemporaryFailure`, we block `MonitorEvent`s from that `ChannelMonitor` until the update is persisted. This prevents duplicate payment send events to the user after payments get reloaded from monitors on restart. However, if the event being avoided isn't going to generate a PaymentSent, but instead result in us claiming an HTLC from an upstream channel (ie the HTLC was forwarded), then the result of a user delaying the event is that we delay getting our money, not a duplicate event. Because user persistence may take an arbitrary amount of time, we need to bound the amount of time we can possibly wait to return events, which we do here by bounding it to 3 blocks. Thanks to Val for catching this in review. Clarify the contexts in which persist_new_channel may be called Its somewhat confusing that `persist_new_channel` is called on startup for an existing channel in common deployments, so we call it out explicitly. Make `Channel::revoke_and_ack`'s return tuple a struct This substantially improves readability at the callsite and in the function. Make `Channel::monitor_updating_restored`'s return tuple a struct This improves readability at the callsite and in the function. Implement `HashMap` read for `MaybeReadable` values This allows us to read a `HashMap` that has values which may be skipped if they are some backwards-compatibility type. We also take this opportunity to fail deserialization if keys are duplicated. Inform ChannelManager when fulfilled HTLCs are finalized When an HTLC has been failed, we track it up until the point there exists no broadcastable commitment transaction which has the HTLC present, at which point Channel returns the HTLCSource back to the ChannelManager, which fails the HTLC backwards appropriately. When an HTLC is fulfilled, however, we fulfill on the backwards path immediately. This is great for claiming upstream HTLCs, but when we want to track pending payments, we need to ensure we can check with ChannelMonitor data to rebuild pending payments. In order to do so, we need an event similar to the HTLC failure event, but for fulfills instead. Specifically, if we force-close a channel, we remove its off-chain `Channel` object entirely, at which point, on reload, we may notice HTLC(s) which are not present in our pending payments map (as they may have received a payment preimage, but not fully committed to it). Thus, we'd conclude we still have a retryable payment, which is untrue. This commit does so, informing the ChannelManager via a new return element where appropriate of the HTLCSource corresponding to the failed HTLC. Track payments after they resolve until all HTLCs are finalized In the next commit, we will reload lost pending payments from ChannelMonitors during restart. However, in order to avoid re-adding pending payments which have already been fulfilled, we must ensure that we do not fully remove pending payments until all HTLCs for the payment have been fully removed from their ChannelMonitors. We do so here, introducing a new PendingOutboundPayment variant called `Completed` which only tracks the set of pending HTLCs. Rename payment object vars to refer to payments and not session IDs Add PaymentSecrets to HTLCSource::OutboundRoute objects Reload pending payments from ChannelMonitor HTLC data on reload If we go to send a payment, add the HTLC(s) to the channel(s), commit the ChannelMonitor updates to disk, and then crash, we'll come back up with no pending payments but HTLC(s) ready to be claim/failed. This makes it rather impractical to write a payment sender/retryer, as you cannot guarantee atomicity - you cannot guarantee you'll have retry data persisted even if the HTLC(s) are actually pending. Because ChannelMonitors are *the* atomically-persisted data in LDK, we lean on their current HTLC data to figure out what HTLC(s) are a part of an outbound payment, rebuilding the pending payments list on reload. Add some basic test coverage of monitor payment data reloading Move test_dup_htlc_onchain_fails_on_reload to payment_tests test_dup_htlc_onchain_fails_on_reload is now more of a payment_test than a functional_test, testing for handling of pending payments. Add a test of an HTLC being fulfilled and then later failed Peers probably shouldn't do this, but if they want to give us free money, we should take it and not generate any spurious events. Define Payee abstraction for use in get_route A payee can be identified by a pubkey and optionally have an associated set of invoice features and route hints. Use this in get_route instead of three separate parameters. This may be included in PaymentPathFailed later to use when finding a new route. Remove outdated line from get_route docs Include PaymentPathRetry data in PaymentPathFailed When a payment path fails, it may be retried. Typically, this means re-computing the route after updating the NetworkGraph and channel scores in order to avoid the failing hop. The last hop in PaymentPathFailed's path field contains the pubkey, amount, and CLTV values needed to pass to get_route. However, it does not contain the payee's features and route hints from the invoice. Include the entire set of parameters in PaymentPathRetry and add it to the PaymentPathFailed event. Add a get_retry_route wrapper around get_route that takes PaymentPathRetry. This allows an EventHandler to retry failed payment paths using the payee's route hints and features. Use option TLV decoding for short_channel_id Using ignorable TLV decoding is only applicable for an Option containing an enum, but short_channel_id is an Option<u64>. Use option TLV encoding instead. Make `Payee::pubkey` pub. `Payee` is expected to be used by users to get routes for payment retries, potentially with their own router. Thus, its helpful if it is pub, even if it is redundant with the last hop in the `path` field in `Events::PaymentPathFailed`. Copy `Payee` into `Route`s to provide them to `ChannelManager` Store `Payee` information in `HTLCSource::OutboundRoute`. This stores and tracks HTLC payee information with HTLCSource info, allowing us to provide it back to the user if the HTLC fails and ensuring persistence by keeping it with the HTLC itself as it passes between Channel and ChannelMonitor. Expose log_bytes! macro for use in other crates Needed to log PaymentHash in the lightning-invoice crate when retrying payments. Add PaymentId to PaymentSent event The payment_hash may not uniquely identify the payment if it has been reused. Include the payment_id in PaymentSent events so it can correlated with the send_payment call. Add PaymentId to PaymentPathFailed event The PaymentId is needed when retrying payments. Include it in the PaymentPathFailed event so it can be used in that manner. Rewrite Invoice's interface in terms of msats InvoiceBuilder's interface was changed recently to work in terms of msats. Update Invoice's interface to return the amount in msats, too, and make amount_pico_btc private. Unify route finding methods An upcoming Router interface will be used for finding a Route both when initially sending a payment and also when retrying failed payment paths. Unify the three varieties of get_route so the interface can consist of a single method implemented by the new `find_route` method. Give get_route pub(crate) visibility so it can still be used in tests. Add InvoicePayer for retrying failed payments When a payment fails, it's useful to retry the payment once the network graph and channel scores are updated. InvoicePayer is a utility for making payments which will retry any failed payment paths for a payment up to a configured number of total attempts. It is parameterized by a Payer and Router for ease of customization and testing. Implement EventHandler for InvoicePayer as a decorator that intercepts PaymentPathFailed events and retries that payment using the parameters from the event. It delegates to the decorated EventHandler after retries have been exhausted and for other events. Support paying zero-value invoices Fail payment retry if Invoice is expired According to BOLT 11: - after the `timestamp` plus `expiry` has passed - SHOULD NOT attempt a payment Add a convenience method for checking if an Invoice has expired, and use it to short-circuit payment retries. Implement Payer and Router for lightning crate Implements Payer for ChannelManager and Rotuer for find_route, which can be used to parameterize InvoicePayer when needing payment retries. Add a utility trait in `router` to get the fees along a given path Pass the failing/succeeding `Path` to PendingOutboundPayment meths This will make the next commit much simpler Track the amount spent on fees as payments are retried Especially once we merge the `InvoicePayer` logic soon, we'll want to expose the total fee paid in the `PaymentSent` event. Correct send-bounding logic in `TestRoutingMessageHandler` The `cmp::min` appeared to confused `end` for a count. Add `PeerManager::disconnect_all_peers` to avoid complexity in BP In the coming commits simply calling `timer_tick_occurred` will no longer disconnect all peers, so its helpful to have a utility method. Constify the ratio in buf limits between forward and init sync msgs Util-ify enqueueing an encoded message in peer_handler This marginally simplifies coming commits. Give peers which are sending us messages longer to respond to ping See comment for rationale. Give peers one timer tick to finish handshake before disconnecting This ensures we don't let a hung connection stick around forever if the peer never completes the initial handshake. This also resolves a race where, on receiving a second connection from a peer, we may reset their_node_id to None to prevent sending messages even though the `channel_encryptor` `is_ready_for_encryption()`. Sending pings only checks the `channel_encryptor` status, not `their_node_id` resulting in an `unwrap` on `None` in `enqueue_message`. Log peer public key more thoroughly when logging in peer_handler Notify scorer of failing payment path and channel Upon receiving a PaymentPathFailed event, the failing payment may be retried on a different path. To avoid using the channel responsible for the failure, a scorer should be notified of the failure before being used to find a new route. Add a payment_path_failed method to routing::Score and call it in InvoicePayer's event handler. Introduce a LockableScore parameterization to InvoicePayer so the scorer is locked only once before calling find_route. Penalize failed channels in Scorer As payments fail, the channel responsible for the failure may be penalized. Implement Scorer::payment_path_failed to penalize the failed channel using a configured penalty. As time passes, the penalty is reduced using exponential decay, though penalties will accumulate if the channel continues to fail. The decay interval is also configurable. Test InvoicePayer in BackgroundProcessor Proof of concept showing InvoicePayer can be used with an Arc<ChannelManager> passed to BackgroundProcessor. Likely do not need to merge this commit. Move PaymentId to a [u8; 32] in bindings as for other hash objects This should allow us to fix https://github.com/lightningdevkit/ldk-garbagecollected/issues/52 Provide payment retry data when an MPP payment failed partially This will allow `InvoicePayer` to properly retry payments that only partially failed to send. Expand `InvoicePayer` documentation somewhat to clarify edge-cases Dont unwrap `RouteParameter::expiry_time` as users can set it Users can provide anything they want as `RouteParameters` so we shouldn't assume any fields are set any particular way, including `expiry_time` set at all. Rewrite InvoicePayer retry to correctly handle MPP partial failures This rewrites a good chunk of the retry logic in `InvoicePayer` to address two issues: * it was not considering the return value of `send_payment` (and `retry_payment`) may indicate a failure on some paths but not others, * it was not considering that more failures may still come later when removing elements from the retry count map. This could result in us seeing an MPP-partial-failure, failing to retry, removing the retries count entry, and then retrying other parts, potentially forever. Add an integration test for InvoicePayer paying when one part fails This tests the multi-part-single-failure-immediately fixes in the previous commit. Add integration test for InvoicePayerretry on an immediate failure Check for invoice expiry in InvoicePayer before we send any HTLCs Parameterize NetGraphMsgHandler with NetworkGraph NetworkGraph is owned by NetGraphMsgHandler, but DefaultRouter requires a reference to it. Introduce shared ownership to NetGraphMsgHandler so that both can use the same NetworkGraph. Make NetGraphMsgHandler::network_graph private Since NetworkGraph has shared ownership, NetGraphMsgHandler does not need to expose its field. Clarify Scorer docs around penalizing channels Refactor channel failure penalty logic Move channel failure penalty logic into a ChannelFailure abstraction. This encapsulates the logic for accumulating penalties and decaying them over time. It also is responsible for the no-std behavior. This cleans up Scorer and will make it easier to serialize it. Parameterize Scorer by a Time trait Scorer uses time to determine how much to penalize a channel after a failure occurs. Parameterizing it by time cleans up the code such that no-std support is in a single AlwaysPresent struct, which implements the Time trait. Time is implemented for std::time::Instant when std is available. This parameterization also allows for deterministic testing since a clock could be devised to advance forward as needed. Implement (de)serialization for Scorer Scorer should be serialized to retain penalty data between restarts. Implement (de)serialization for Scorer by serializing last failure times as duration since the UNIX epoch. For no-std, the zero-Duration is used. Remove trailing ;s from macro calls to silence new rustc warnings Rename Payee::new to Payee::from_node_id to clarify it somewhat This also differentiates it from the bindings default-constructed `new` method which is constructed when all fields are exposed and of mappable types. Make payment_path_failed path type bindings-mappable The bindings don't currently support passing `Vec`s of objects which it mappes as "opaque types". This is because it will require clones to convert its own list of references to Rust's list of objects. In the near future we should resolve this limitation, allowing us to revert this (and make `find_route`'s method signature similarly cleaner), but for now we must avoid `Vec<OpaqueType>`. Remove now-unused import in routing/mod.rs Add `(C-not exported)` tag to a `Payee` modifier with move semantics This matches the other `Payee` move-modifier functions. Add `(C-not exported)` tags as required in tuple types This prepares us for C bindings auto-exporting tuple type fields. Tweak serialization of ScorerUsingTime for better forward compat Update CHANGELOG for 0.0.103 Bump crate versions to 0.0.103/invoice 0.11 Fix `cargo doc` on older rustc Apparently at least rustc 1.48 doesn't support `Self` in doc links, so we make it explicit. Add a ChannelTypeFeatures features object for the new channel_type Its semantics are somewhat different from existing features, however not enough to merit a different struct entirely. Specifically, it only supports required features (if you send a channel_type, the counterparty has to accept it wholesale or try again, it cannot select only a subset of the flags) and it is serialized differently (only appearing in TLVs). Support de/ser of the new channel_type field in open_channel Support send/recv'ing the new channel_type field in open_channel This implements the channel type negotiation, though as we currently only support channels with only static_remotekey set, it doesn't implement the negotiation explicitly. 0.0.103 CHANGELOG tweaks from Jeff Add note about PaymentId fields to 0.0.103 changelog Add SinceEpoch time to test Scorer hermetically In order to test Scorer hermetically, sleeps must be avoided. Add a SinceEpoch abstraction for manually advancing time. Implement the Time trait for SinceEpoch so that it can be used with ScorerUsingTime in tests. Add unit tests for Scorer Test basic and channel failure penalties, including after a (de-)serialization round trip. Log before+after ChannelMonitor/Manager updates for visibility I realized on my own node that I don't have any visibility into how long a monitor or manager persistence call takes, potentially blocking other operations. This makes it much more clear by adding a relevant log_trace!() print immediately before and immediately after persistence. Fix to_remote output redeemscript when anchors enabled Renamed script_for_p2wpkh to get_p2wpkh_redeemscript to match convention Fix a minor memory leak on PermanentFailure mon errs when sending If we send a payment and fail to update the first-hop channel state with a `PermanentFailure` ChannelMonitorUpdateErr, we would have an entry in our pending payments map, but possibly not return the PaymentId back to the user to retry the payment, leading to a (rare and relatively minor) memory leak. Use upstream rust-bitcoin's dust calculation instead of our own Not only does this move to common code, but it fixes handling of all output types except for a few trivial cases. Be less aggressive in outbound HTLC CLTV timeout checks We currently assume our counterparty is naive and misconfigured and may force-close a channel to get an HTLC we just forwarded them. There shouldn't be any reason to do this - we don't have any such bug, and we shouldn't start by assuming our counterparties are buggy. Worse, this results in refusing to forward payments today, failing HTLCs for largely no reason. Instead, we keep a fairly conservative check, but not one which will fail HTLC forwarding spuriously - testing only that the HTLC doesn't expire for a few blocks from now. Fixes #1114. Correct Channel type serialization logic Currently, we write out the Channel's `ChannelTypeFeatures` as an odd type, implying clients which don't understand the `ChannelTypeFeatures` field can simply ignore it. This is obviously nonsense if the channel type is some future version - the client needs to fail to deserialize as it doesn't understand the channel's type. We adapt the serialization logic here to only write out the `ChannelTypeFeatures` field if it is something other than only-static-remote-key, and simply consider that "default" (as it is the only supported type today). Then, we write out the channel type as an even TLV, implying clients which do not understand it must fail to read the `Channel`. Note that we do not need to bother reserving the TLV type no longer written as it never appeared in a release (merged post-0.0.103). Refactor InvoicePayer for spontaneous payments To support spontaneous payments, InvoicePayer's sending logic must be invoice-agnostic. Refactor InvoicePayer::pay_invoice_internal such that invoice-specific code is in pay_invoice_using_amount and the remaining logic is in pay_internal. Further refactor the code's payment_cache locking such that it is accessed consistently when needed, and tidy up the code a bit. Support spontaneous payments in InvoicePayer InvoicePayer handles retries not only when handling PaymentPathFailed events but also for some types of PaymentSendFailure on the initial send. Expand InvoicePayer's interface with a pay_pubkey function for spontaneous (keysend) payments. Add a send_spontaneous_payment function to the Payer trait to support this and implement it for ChannelManager. Replace expect_value_msat with expect_send Modify all InvoicePayer unit tests to use expect_send instead of expect_value_msat, since the former can discern whether the send was for an invoice, spontaneous payment, or a retry. Updates tests to set payer expectations if they weren't already and assert these before returning a failure. Test retrying payment on partial send failure Add some test coverage for when InvoicePayer retries within pay_invoice because the Payer returned a partial failure. Add PaymentHash parameter to Router::find_route Implementations of Router may need the payment hash in order to look up pre-computed routes from a probe for a given payment. Add a PaymentHash parameter to Router::find_route to allow for this. Provide `Score` the HTLC amount and channel capacity This should allow `Score` implementations to make substantially better decisions, including of the form "willing to pay X to avoid routing over this channel which may have a high failure rate". Penalize large HTLCs relative to channels in default `Scorer` Sending HTLCs which are any greater than a very small fraction of the channel size tend to fail at a much higher rate. Thus, by default we start applying a penalty at only 1/8th the channel size and increase it linearly as the amount reaches the channel's capacity, 20 msat per 1024th of the channel capacity. Move `Score` into a `scoring` module instead of a top-level module Traits in top-level modules is somewhat confusing - generally top-level modules are just organizational modules and don't contain things themselves, instead placing traits and structs in sub-modules. Further, its incredibly awkward to have a `scorer` sub-module, but only have a single struct in it, with the relevant trait it is the only implementation of somewhere else. Not having `Score` in the `scorer` sub-module is further confusing because it's the only module anywhere that references scoring at all. Return `ClosureReason` from `Channel` chain update methods This fixes a few `ClosureReason`s and allows us to have finer-grained user-visible errors when a channel closes due to an on-chain event. Automatically close channels that go unfunded for 2016 blocks As recommended by BOLT 2 added in https://github.com/lightningnetwork/lightning-rfc/pull/839 Add 'accept_inbound_channels' config option. Limit minimum output size to the dust limit when RBF-bumping Check all outputs meet the dust threshold in check_spends!() Correct txid logging to reverse bytes. We also take this opportunity to log the channel being closed when one is closed by an on-chain spend of the funding output. Ensure current channel state is logged for all channels on startup Remove user_payment_id In upcoming commits, we'll be making the payment secret and payment hash/preimage derivable from info about the payment + a node secret. This means we don't need to store any info about incoming payments and can eventually get rid of the channelmanager::pending_inbound_payments map. Add a new log-level for gossip messages. Fix MPP routefinding when we first collect 95% of payment value See comment in new test for more details. Introduce new helper commit_tx_fee_sat Cancel the outbound feerate update if above what we can afford Check we won't overflow `max_dust_htlc_exposure_msat` at outbound feerate update Re-add `test_max_dust_htlc_exposure` Introduce CommitmentStats Check outbound update_fee affordance incremented with holding cell HTLCs Add anchor support to commitment HTLC outputs Increase visibility of anchor related methods Add anchor support to build_htlc_transaction Adjust HTLC_{SUCCESS,TIMEOUT}_TX_WEIGHT when anchors used Add test vectors for get_htlc_redeemscript wrt anchors Generate PaymentPathSuccessful event for each path A single PaymentSent event is generated when a payment is fulfilled. This is occurs when the preimage is revealed on the first claimed HTLC. For subsequent HTLCs, the event is not generated. In order to score channels involved with a successful payments, the scorer must be notified of each successful path involved in the payment. Add a PaymentPathSuccessful event for this purpose. Generate it whenever a part is removed from a pending outbound payment. This avoids duplicate events when reconnecting to a peer. Make Channel::commit_tx_fee_msat static and take fee explicitly This may avoid risk of bugs in the future as it requires the caller to think about the fee being used, not just blindly use the current (committed) channel feerate. Rewrite test_update_fee_that_funder_cannot_afford to avoid magic Instead of magic hard-coded constants, its better for tests to derive the values used so that they change if constants are changed and so that it is easier to re-derive constants in the future as needed. Correct initial commitment tx fee affordability checks on open Previously, we would reject inbound channels if the funder wasn't able to meet our channel reserve on their first commitment transaction only if they also failed to push enough to us for us to not meet their initial channel reserve as well. There's not a lot of reason to care about us meeting their reserve, however - its largely expected that they may not push enough to us in the initial open to meet it, and its not actually our problem if they don't. Further, we used our own fee, instead of the channel's actual fee, to calculate fee affordability of the initial commitment transaction. We resolve both issues here, rewriting the combined affordability check conditionals in inbound channel open handling and adding a fee affordability check for outbound channels as well. The prior code may have allowed a counterparty to start the channel with "no punishment" states - violating the reason for the reserve threshold. Test fixed channel reserve checks on channel open Fix compilation with the max_level_trace feature Test all log-limiting features in CI Support `logger::Record` in C by String-ing the fmt::Arguments This adds a new (non-feature) cfg argument `c_bindings` which will be set when building C bindings. With this, we can (slightly) tweak behavior and API based on whether we are being built for Rust or C users. Ideally we'd never need this, but as long as we can keep the API consistent-enough to avoid material code drift, this gives us a cheap way of doing the "right" thing for both C and Rust when the two are in tension. We also move lightning-background-processor to support the same MSRV as the main lightning crate, instead of only lightning-net-tokio's MSRV. Make `Score : Writeable` in c_bindings and impl on `LockedScore` Ultimately we likely need to wrap the locked `Score` in a struct that exposes writeable somehow, but because all traits have to be fully concretized for C bindings we'll still need `Writeable` on all `Score` in order to expose `Writeable` on the locked score. Otherwise, we'll only have a `LockedScore` with a `Score` visible that only has the `Score` methods, never the original type. Seal `scoring::Time` and only use `Instant` or `Eternity` publicly `scoring::Time` exists in part to make testing the passage of time in `Scorer` practical. To allow no-std users to provide a time source it was exposed as a trait as well. However, it seems somewhat unlikely that a no-std user is going to have a use for providing their own time source (otherwise they wouldn't be a no-std user), and likely they won't have a graph in memory either. `scoring::Time` as currently written is also exceptionally hard to write C bindings for - the C bindings trait mappings relies on the ability to construct trait implementations at runtime with function pointers (i.e. `dyn Trait`s). `scoring::Time`, on the other hand, is a supertrait of `core::ops::Sub` which requires a `sub` method which takes a type parameter and returns a type parameter. Both of which aren't practical in bindings, especially given the `Sub::Output` associated type is not bound by any trait bounds at all (implying we cannot simply map the `sub` function to return an opaque trait object). Thus, for simplicity, we here simply seal `scoring::Time` and make it effectively-private, ensuring the bindings don't need to bother with it. Implement Clone, Hash, PartialEq for ClosingTransaction This is a public struct intended to be used as an object by users, so it should likely have common implementations, given they're trivial. Implement Clone for InvalidShutdownScript Users hopefully shouldn't have much of a reason to use this, but the bindings may need it to ensure no leaking pointers over an ffi. Store holder channel reserve and max-htlc-in-flight explicitly Previously, `holder_selected_channel_reserve_satoshis` and `holder_max_htlc_value_in_flight_msat` were constant functions of the channel value satoshis. However, in the future we may allow allow users to specify it. In order to do so, we'll need to track them explicitly, including serializing them as appropriate. We go ahead and do so here, in part as it will make testing different counterparty-selected channel reserve values easier. Explicitly support counterparty setting 0 channel reserve A peer providing a channel_reserve_satoshis of 0 (or less than our dust limit) is insecure, but only for them. Because some LSPs do it with some level of trust of the clients (for a substantial UX improvement), we explicitly allow it. Because its unlikely to happen often in normal testing, we test it explicitly here. Fix regression when reading `Event::PaymentReceived` in some cases For some reason rustc was deciding on a type for the `Option` being deserialized for us as `_user_payment_id`. This really, really, absolutely should have been a compile failure - the type (with methods called on it!) was ambiguous! Instead, rustc seems to have been defaulting to `Option<()>`, causing us to read zero of the eight bytes in the `user_payment_id` field, which returns an `Err(InvalidValue)` error as TLVs must always be read fully. This should likely be reported to rustc as its definitely a bug, but I cannot seem to cause the same error on any kinda of vaguely-minimized version of the same code. Found by `chanmon_consistency` fuzz target. Fix compilation in `payment` rustdoc examples The samples were not valid rust, but previous versions of rustc had a bug where they were accepted anyway. Latest rustc beta no longer accepts these. Score successful payment paths Expand the Score trait with a payment_path_successful function for scoring successful payment paths. Called by InvoicePayer's EventHandler implementation when processing PaymentPathSuccessful events. May be used by Score implementations to revert any channel penalties that were applied by calls to payment_path_failed. Decay channel failure penalty upon success If a payment failed to route through a channel, a penalty is applied to the channel in the future when finding a route. This penalty decays over time. Immediately decay the penalty by one half life when a payment is successfully routed through the channel. Fix shift overflow in Scorer::channel_penalty_msat An unchecked shift of more than 64 bits on u64 values causes a shift overflow panic. This may happen if a channel is penalized only once and (1) is not successfully routed through and (2) after 64 or more half life decays. Use a checked shift to prevent this from happening. Allow missing-docs on test-only macros Prefer fully-specified paths in test macros This avoids macros being context-specific use-dependent. Continue after a single failure in `ChannelMonitor::update_monitor` `ChannelMonitorUpdate`s may contain multiple updates, including, eg a payment preimage after a commitment transaction update. While such updates are generally not generated today, we shouldn't return early out of the update loop, causing us to miss any updates after an earlier update fails. Always return failure in `update_monitor` after funding spend Previously, monitor updates were allowed freely even after a funding-spend transaction confirmed. This would allow a race condition where we could receive a payment (including the counterparty revoking their broadcasted state!) and accept it without recourse as long as the ChannelMonitor receives the block first, the full commitment update dance occurs after the block is connected, and before the ChannelManager receives the block. Obviously this is an incredibly contrived race given the counterparty would be risking their full channel balance for it, but its worth fixing nonetheless as it makes the potential ChannelMonitor states simpler to reason about. The test in this commit also tests the behavior changed in the previous commit. Drop `MonitorUpdateErr` in favor of opaque errors. We don't expect users to ever change behavior based on the string contained in a `MonitorUpdateErr`, except log it, so there's little reason to not just log it ourselves and return a `()` for errors. We do so here, simplifying the callsite in `ChainMonitor` as well. Make APIError debug output more clear by including the variant Add a simple test for ChainMonitor MonitorUpdate-holding behavior Add a test for MonitorEvent holding when they complete out-of-order Add trivial test of monitor update failure during block connection Remove OnionV2 parsing support OnionV2s don't (really) work on Tor anymore anyway, and the field is set for removal in the BOLTs [1]. Sadly because of the way addresses are parsed we have to continue to understand that type 3 addresses are 12 bytes long. Thus, for simplicity we keep the `OnionV2` enum variant around and just make it an opaque 12 bytes, with the documentation updated to note the deprecation. [1] https://github.com/lightning/bolts/pull/940 Ensure ChannelManager methods are idempotent During event handling, ChannelManager methods may need to be called as indicated in the Event documentation. Ensure that these calls are idempotent for the same event rather than panicking. This allows users to persist events for later handling without needing to worry about processing the same event twice (e.g., if ChannelManager is not persisted but the events were, the restarted ChannelManager would return some of the same events). Define public getters for all feature flags There's not a ton of reason not to do this, and moving it to the macro removes some code. Support the `channel_type` feature bit. Note that this feature bit does absolutely nothing. We signal it (as we already support channel type negotiation), but do not bother to look to see if peers support it, as we don't care - we simply look for the TLV entry and deduce if a peer supports channel type negotiation from that. The only behavioral change at all here is that we don't barf if a peer sets channel type negotiation to required via the feature bit (instead of failing the channel at open-time), but of course no implementations do this, and likely won't for some time (if ever - you can simply fail channels with unknown types later, and there's no reason to refuse connections, really). As defined in https://github.com/lightning/bolts/pull/906 Reduce force-closures with user fee estimators which round poorly See comment for more Upgrade to codecov uploader v2 Some time ago codecov stopped supporting their old v1 uploader, and it seems they've now finally turned it off, so we aren't getting any coverage reports anymore. Hopefully upgrading is pretty trivial. Getter for the total channel balance The existing balance getters subtract reserve, this one does not. Separate ChannelAnnouncement and ChannelUpdate broadcast conditions When a `ChannelUpdate` message is generated for broadcast as a part of a `BroadcastChannelAnnouncement` event, it may be newer than our previous `ChannelUpdate` and need to be broadcast. However, if the `ChannelAnnouncement` had already been seen we wouldn't re-broadcast either message as the `handle_channel_announcement` call would fail, short-circuiting the condition to broadcast both. Instead, we split the broadcast of each message as well as the conditional so that we always attempt to handle each message and update our local graph state, then broadcast the message if its update was processed successfully. Update `ChannelUpdate::timestamp` when channels are dis-/en-abled We update the `Channel::update_time_counter` field (which is copied into `ChannelUpdate::timestamp`) only when the channel is initialized or closes, and when a new block is connected. However, if a peer disconnects or reconnects, we may wish to generate `ChannelUpdate` updates in between new blocks. In such a case, we need to make sure the `timestamp` field is newer than any previous updates' `timestamp` fields, which we do here by simply incrementing it when the channel status is changed. As a side effect of this we have to update `test_background_processor` to ensure it eventually succeeds even if the serialization of the `ChannelManager` changes after the test begins. Re-broadcast our own gossip even if its same as the last broadcast Even if our gossip hasn't changed, we should be willing to re-broadcast it to our peers. All our peers may have been disconnected the last time we broadcasted it. Add a comment describing `update_time_counter` and when its updated Add a comment describing the timestamp field's use DRY up payment failure macros in functional_test_utils ... with a more extensible expectation-checking framework for them. Add a variant to `PendingOutboundPayment` for retries-exceeded When a payer gives up trying to retry a payment, they don't know for sure what the current state of the event queue is. Specifically, they cannot be sure that there are not multiple additional `PaymentPathFailed` or even `PaymentSuccess` events pending which they will see later. Thus, they have a very hard time identifying whether a payment has truly failed (and informing the UI of that fact) or if it is still pending. See [1] for more information. In order to avoid this mess, we will resolve it here by having the payer give `ChannelManager` a bit more information - when they have given up on a payment - and using that to generate a `PaymentFailed` event when all paths have failed. This commit adds the neccessary storage and changes for the new state inside `ChannelManager` and a public method to mark a payment as failed, the next few commits will add the new `Event` and use the new features in our `PaymentRetrier`. [1] https://github.com/lightningdevkit/rust-lightning/issues/1164 Expose an event when a payment has failed and retries complete When a payment fails, a payer needs to know when they can consider a payment as fully-failed, and when only some of the HTLCs in the payment have failed. This isn't possible with the current event scheme, as discovered recently and as described in the previous commit. This adds a new event which describes when a payment is fully and irrevocably failed, generating it only after the payment has expired or been marked as expired with `ChannelManager::mark_retries_exceeded` *and* all HTLCs for it have failed. With this, a payer can more simply deduce when a payment has failed and use that to remove payment state or finalize a payment failure. Use `Event::PaymentFailed` in `InvoicePayer` to remove retry count This finally fixes the bug described in the previous commits where we retry a payment after its retry count has expired due to early removal of the payment from the retry count tracking map. A test is also added which demonstrates the bug in previous versions and which passes now. Fixes #1164. Make attempting to retry a succeeded payment an APIError, not Route This is symmetric with the new failure once a payment is abandoned. Updates Fee estimator docs to include a max return value for get_est_sat_per_1000_weight Updates docs to include a max return value for get_est_sat_per_1000_weight Adds max to both conversions Additional detail Add mpp_timeout and invalid_onion_payload descriptions & handling Pin tokio to 1.14.0 in CI for older rustc's Build no-std on 1.47 now that core2 supports it DRY up network_graph tests substantially with message creation fns Drop `allow_wallclock_use` feature in favor of simply using `std` Fixes #1147. Add a method to prune stale channels from `NetworkGraph` We define "stale" as "haven't heard an updated channel_update in two weeks", as described in BOLT 7. We also filter out stale channels at write-time for `std` users, as we can look up the current time. Reject channel_update messages with timestamps too old or new Because we time out channel info that is older than two weeks now, we should also reject new channel info that is older than two weeks, in addition to rejecting future channel info. Automatically prune NetworkGraph of stale channels hourly in BP Add get_inbound_payment_key_material to KeysInterface This will allow us to retrieve key material for encrypting/decrypting inbound payment info, in upcoming commits Macro-ize checking that the total value of an MPP's parts is sane This DRY-ed code will be used in upcoming commits when we stop storing inbound payment data Add get_single_block to chacha20 module In the next commit, we'll want to get a single block from a chacha stream that takes a 16-byte nonce. Drop need to store pending inbound payments and replace payment_secret with encrypted metadata See docs on `inbound_payment::verify` for details Also add min_value checks to all create_inbound_payment* methods Add new invoice CreationError::InvalidAmount for use in checking `create_inbound_payment` in an invoice creation utility. Note that if the error type of `create_inbound_payment` ever changed, we'd be forced to update the invoice utility's callsite to handle the new error Salt inbound payment ExpandedKey Leftover feedback from #1177 create_inbound_payment: warn about dup hashes Leftover feedback from #1177 inbound_payment: DRY verify method for use in getting preimage in the next commit(s) inbound_payment: Add utility to get payment preimage given hash/secret User-requested feature Remove trailing whitespace in the FeeEstimator interface Update CHANGELOG for 0.0.104 Bump versions to 0.0.104/invoice 0.12 Swap around generic argument ordering in DefaultRouter for bindings The bindings generation really should support default generic types in where clauses, but currently does not. To avoid needing to add support during the current release process, we simply swap around the arguments to move them to the first <> instead of the where. Add a constructor to MultiThreadedLockableScore ...as otherwise the struct is rather useless. Add a C-not exported tag to `NetGraphMsgHandler.network_graph` Swap around generic argument ordering in InvoicePayer for bindings The bindings generation really should support generic bounds other than Deref::Target in where clauses, but currently does not. To avoid needing to add support during the current release process, we simply swap around the arguments to move them to the first <> instead of the where. update repo name to use lightningdevkit Log gossip rejections due to stale channel_updates at GOSSIP level This further reduces noise at the TRACE level during initial gossip sync. Support building `cfg=c_bindings` with `no-std` This will be needed for JavaScript bindings eventually. Adapt lightning-invoice to no_std Do not turn on bech32/std by default for lightning-invoice Add a new `WarningMessage` message to send and receive warnings Handle sending and receiving warning messages Send warning messages when appropriate in gossip handling pipeline Convert `shutdown` invalid script checks to warning messages As required by the warning messages PR, we should simply warn our counterparty in this case and let them try again, continuing to try to use the channel until they tell us otherwise. Send `warning` instead of `error` when we incounter bogus gossip Rely on Error/Warning message data lengths being correct In https://github.com/lightning/bolts/pull/950, the (somewhat strange) requirement that error messages be handled even if the length field is set larger than the size of the package was removed. Here we change the code to drop the special handling for this, opting to just fail to read the message if the length is incorrect. Set the SigHashType of remote htlc signatures w/ anchors to SinglePlusAnyoneCanPay Isolated channelmonitor weight unit tests and added anchor loops make WEIGHT{_REVOKED,}_{OFFERED,RECEIVED}_HTLC functions with opt_anchors parameter Add unit test coverage for package::weight_{offered,received}_htlc Convert COMMITMENT_TX_BASE_WEIGHT to anchor-aware function Convert HTLC_{SUCCESS,TIMEOUT}_TX_WEIGHT to anchor-aware functions Debit funder's output to cover anchors Add anchor tests to outbound_commitment_test Set opt_anchors for calls to CommitmentTransaction::new_with_auxiliary_htlc_data Fixed comment on weight_received_htlc Make lockorder consistent in channelmanager This resolves a lockorder inversion in `ChannelManager::finalize_claims` where `pending_outbound_payments` is locked after `pending_events`, opposite of, for example, the lockorder in `ChannelManager::fail_htlc_backwards_internal` where `pending_outbound_payments` is locked at the top of the `HTLCSource::OutboundRoute` handling and then `pending_events` is locked at the end. Fix some (non-bug) lock-order-inversions in tests Check lockorders in tests with a trivial lockorder tracker Fix build errors Create script using p2wsh for comparison Using p2wpkh for generating the payment script spendable_outputs sanity check

commit 1dff7564dd698c616e40c8ef208249202e3bef71 Author: vss96 <shettyvikas209@gmail.com> Date: Wed Jan 19 23:19:32 2022 +0530 spendable_outputs sanity check commit 7cb7b1558e7be1e9e70d1e7fd01aeca6a060384b Author: vss96 <shettyvikas209@gmail.com> Date: Wed Jan 19 12:08:28 2022 +0530 Using p2wpkh for generating the payment script commit be70ba1f9a383715aa88aab777f39e2d5f875852 Author: vss96 <shettyvikas209@gmail.com> Date: Wed Jan 19 00:58:41 2022 +0530 Create script using p2wsh for comparison commit 4c7d2c0e8cb6cd5d37d11b93b8deca99d339aa30 Author: vss96 <shettyvikas209@gmail.com> Date: Tue Jan 18 20:20:24 2022 +0530 Fix build errors commit 3c48ce338c3d7d243dd2c1d28d5efc9405fc0742 Merge: b647d10d 34cdca91 Author: Vikas <shettyvikas209@gmail.com> Date: Tue Jan 18 17:35:15 2022 +0530 Merge branch 'main' into sanity_check commit b647d10dcf9d1450656bcf4551b2ce3c25e0f3e3 Author: vss96 <shettyvikas209@gmail.com> Date: Tue Jan 18 17:30:19 2022 +0530 Sanity check for ChannelManager and KeysInterface commit 34cdca91baa0187d13969855129073c764f4c895 Merge: 89cbb6d7 8f007c7d Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Fri Jan 14 16:59:42 2022 +0000 Merge pull request #1238 from TheBlueMatt/2022-01-lockorder-checks Fix and test lockorder commit 8f007c7dbb3635cd92b9fa78ca19dc06a842f6ee Author: Matt Corallo <git@bluematt.me> Date: Thu Jan 13 01:00:43 2022 +0000 Check lockorders in tests with a trivial lockorder tracker commit 89cbb6d74b32b1d23090761dec216c942a846a04 Merge: a82067d3 9c2270c7 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Fri Jan 14 04:10:14 2022 +0000 Merge pull request #1229 from lightning-signer/2021-12-htlc-anchor-sighashtype anchors: Fix SigHashType and weight calculations for anchors commit 9c2270c72225afa1b8b1ad14c61799e04bc93c7a Author: Ken Sedgwick <ken@bonsai.com> Date: Wed Jan 12 18:29:05 2022 -0800 Fixed comment on weight_received_htlc commit bee00124d2b5f36b6b2d8979cfc1c22e984b762c Author: Ken Sedgwick <ken@bonsai.com> Date: Wed Jan 12 17:10:07 2022 -0800 Set opt_anchors for calls to CommitmentTransaction::new_with_auxiliary_htlc_data commit 299b6657d53dc77e55d5fb85774747b5337c4092 Author: Ken Sedgwick <ken@bonsai.com> Date: Tue Jan 4 14:53:44 2022 -0800 Add anchor tests to outbound_commitment_test commit 35a6e00b03835f87adbae0b3a4229f6a13b9ca8b Author: Ken Sedgwick <ken@bonsai.com> Date: Tue Jan 4 16:05:28 2022 -0800 Debit funder's output to cover anchors commit 557a83096f694a2e638361b65b828f52e979591a Author: Ken Sedgwick <ken@bonsai.com> Date: Wed Jan 5 13:40:08 2022 -0800 Convert HTLC_{SUCCESS,TIMEOUT}_TX_WEIGHT to anchor-aware functions commit 9566795c97ec241a7102b741475318747645fd91 Author: Ken Sedgwick <ken@bonsai.com> Date: Tue Jan 4 15:54:54 2022 -0800 Convert COMMITMENT_TX_BASE_WEIGHT to anchor-aware function commit c47d014a8ef76018edd1d82185d6f783f406eed9 Author: Ken Sedgwick <ken@bonsai.com> Date: Sun Jan 2 15:00:54 2022 -0800 Add unit test coverage for package::weight_{offered,received}_htlc commit 37001b8b0e730f37fbe66b5f29ec75b1b266155d Author: Ken Sedgwick <ken@bonsai.com> Date: Fri Dec 31 12:21:22 2021 -0800 make WEIGHT{_REVOKED,}_{OFFERED,RECEIVED}_HTLC functions with opt_anchors parameter commit 3a163d2c6152c83539469a8674d8a9f7bbabcbff Author: Ken Sedgwick <ken@bonsai.com> Date: Fri Dec 31 08:47:38 2021 -0800 Isolated channelmonitor weight unit tests and added anchor loops commit 8f09e5a7ff464109ca1d7de065f56635c17bd0ac Author: Ken Sedgwick <ken@bonsai.com> Date: Tue Dec 28 20:37:20 2021 -0800 Set the SigHashType of remote htlc signatures w/ anchors to SinglePlusAnyoneCanPay commit feb203d3b16538e279dba05fe3a8c6cbe1d9be97 Author: Matt Corallo <git@bluematt.me> Date: Thu Jan 13 01:51:29 2022 +0000 Fix some (non-bug) lock-order-inversions in tests commit 6ccd07bc2d6213061b2ae579ccad9cfef4bc02cd Author: Matt Corallo <git@bluematt.me> Date: Wed Jan 12 19:58:08 2022 +0000 Make lockorder consistent in channelmanager This resolves a lockorder inversion in `ChannelManager::finalize_claims` where `pending_outbound_payments` is locked after `pending_events`, opposite of, for example, the lockorder in `ChannelManager::fail_htlc_backwards_internal` where `pending_outbound_payments` is locked at the top of the `HTLCSource::OutboundRoute` handling and then `pending_events` is locked at the end. commit a82067d3595a782d656f18d544145b547e27abc4 Merge: bb248617 d786bfae Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue Jan 11 22:52:44 2022 +0000 Merge pull request #1013 from TheBlueMatt/2021-07-warning-msgs commit d786bfaef27b9e49f71ac5b8bf5a4e02cd484f79 Author: Matt Corallo <git@bluematt.me> Date: Fri Jan 7 20:11:31 2022 +0000 Rely on Error/Warning message data lengths being correct In https://github.com/lightning/bolts/pull/950, the (somewhat strange) requirement that error messages be handled even if the length field is set larger than the size of the package was removed. Here we change the code to drop the special handling for this, opting to just fail to read the message if the length is incorrect. commit 2d7b06e6194df42f171bde203500fe10e8b28370 Author: Matt Corallo <git@bluematt.me> Date: Fri Jan 7 20:30:50 2022 +0000 Send `warning` instead of `error` when we incounter bogus gossip commit 26fe0f753dc5116ab6fe5d078c9f30a318559502 Author: Matt Corallo <git@bluematt.me> Date: Thu Sep 30 22:45:07 2021 +0000 Convert `shutdown` invalid script checks to warning messages As required by the warning messages PR, we should simply warn our counterparty in this case and let them try again, continuing to try to use the channel until they tell us otherwise. commit e137cfb3c48e11ee1ad1e1febdb6511b4dbbbcb4 Author: Matt Corallo <git@bluematt.me> Date: Thu Jul 22 16:06:33 2021 +0000 Send warning messages when appropriate in gossip handling pipeline commit 1b3249a1929929170bba9d2553d7a9c8670193e5 Author: Matt Corallo <git@bluematt.me> Date: Thu Jul 22 16:05:48 2021 +0000 Handle sending and receiving warning messages commit f676f5585f46eefbababdc19a5a90d5e0c77f0ca Author: Matt Corallo <git@bluematt.me> Date: Thu Jul 22 15:25:13 2021 +0000 Add a new `WarningMessage` message to send and receive warnings commit bb248617a52c073d317538119a9dbe2703999993 Merge: b62b244c 1a779ce6 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon Jan 10 18:34:40 2022 +0000 Merge pull request #1230 from lightning-signer/2021-01-invoice-bech32-dep Do not turn on bech32/std by default for lightning-invoice commit 1a779ce6583e53ea4fd62e0bc8a9897b477c7e60 Author: Devrandom <c1.devrandom@niftybox.net> Date: Sun Jan 9 12:26:14 2022 +0100 Do not turn on bech32/std by default for lightning-invoice commit b62b244c3c826523a81631037cd060bafef8e402 Merge: 94c41d8c 01915810 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Thu Jan 6 19:25:36 2022 +0000 Merge pull request #1223 from lightning-signer/2021-12-invoice-nostd Adapt lightning-invoice to no_std commit 01915810d42506addbaef244b0cbb31e8b3856b5 Author: Devrandom <c1.devrandom@niftybox.net> Date: Wed Dec 22 17:43:25 2021 +0100 Adapt lightning-invoice to no_std commit 94c41d8c0ac6e0ae2409719e7ca67213cc14ecbf Merge: 3ca63426 ffbef9fe Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Wed Jan 5 21:01:06 2022 +0000 Merge pull request #1226 from TheBlueMatt/2022-01-bindings-no-std Support building `cfg=c_bindings` with `no-std` commit ffbef9fec72fa33ce86d3c68dbcebf45ebc34927 Author: Matt Corallo <git@bluematt.me> Date: Tue Jan 4 22:34:15 2022 +0000 Support building `cfg=c_bindings` with `no-std` This will be needed for JavaScript bindings eventually. commit 3ca63426f98af6817516597b6ad762324bfd2ac4 Merge: 036ea119 d5a14359 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon Jan 3 19:54:20 2022 +0000 Merge pull request #1220 from TheBlueMatt/2021-12-stale-update-gossip-log Log gossip rejections due to stale channel_updates at GOSSIP level commit 036ea119541553bdd17db84a48eb18e0232b2333 Merge: 56555900 d46c2a20 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Wed Dec 29 19:57:05 2021 +0000 Merge pull request #1225 from hackerrdave/update-repo-lightningdevkit update repo name to use lightningdevkit commit d46c2a20e125b3c5af6862ac556fc92a736c9864 Author: hackerrdave <dave@hackerrdave.com> Date: Sun Dec 26 22:53:16 2021 -0500 update repo name to use lightningdevkit commit 565559005ec241d8f4f45a373652eb4ee4e13b9c Merge: 2a8a396f 7d1e1873 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Sun Dec 19 23:55:09 2021 +0000 Merge pull request #1218 from TheBlueMatt/2021-12-minor-bindings-tweaks Minor Bindings Tweaks commit d5a1435905a82045bd9a984421d180d0f12187a4 Author: Matt Corallo <git@bluematt.me> Date: Sun Dec 19 20:13:38 2021 +0000 Log gossip rejections due to stale channel_updates at GOSSIP level This further reduces noise at the TRACE level during initial gossip sync. commit 7d1e1873f34b1de87ec9cfbe13c9111f892b5c8d Author: Matt Corallo <git@bluematt.me> Date: Sat Dec 18 19:52:11 2021 +0000 Swap around generic argument ordering in InvoicePayer for bindings The bindings generation really should support generic bounds other than Deref::Target in where clauses, but currently does not. To avoid needing to add support during the current release process, we simply swap around the arguments to move them to the first <> instead of the where. commit d26469a77a96bcf3717d321b6befce64661e8ed4 Author: Matt Corallo <git@bluematt.me> Date: Sat Dec 18 03:43:34 2021 +0000 Add a C-not exported tag to `NetGraphMsgHandler.network_graph` commit ae4f6198db4bf1dd5ab708c625276143d53239ba Author: Matt Corallo <git@bluematt.me> Date: Sat Dec 18 03:38:15 2021 +0000 Add a constructor to MultiThreadedLockableScore ...as otherwise the struct is rather useless. commit 2a8a396f1ed612ed0cbd2b6cef7d1816b128c5c9 Merge: 1c157a23 27d5c7a2 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Fri Dec 17 23:30:54 2021 +0000 Merge pull request #1217 from TheBlueMatt/2021-12-0.0.104 Cut 0.0.104 commit 27d5c7a2106f163f1c0df462db4ca6247f891af2 Author: Matt Corallo <git@bluematt.me> Date: Fri Dec 17 22:38:46 2021 +0000 Swap around generic argument ordering in DefaultRouter for bindings The bindings generation really should support default generic types in where clauses, but currently does not. To avoid needing to add support during the current release process, we simply swap around the arguments to move them to the first <> instead of the where. commit ec86e2a1a7a1b2cd680a46e2bdd4274dd39bb78c Author: Matt Corallo <git@bluematt.me> Date: Fri Dec 17 21:28:24 2021 +0000 Bump versions to 0.0.104/invoice 0.12 commit 8f9d1c9a8cd9af70b7d93cf905ce3ba1352787c5 Author: Matt Corallo <git@bluematt.me> Date: Fri Dec 17 01:49:39 2021 +0000 Update CHANGELOG for 0.0.104 commit 1c157a2328d23079230143314819d0128ec314fd Merge: 86d5944b 41cfd833 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Fri Dec 17 17:34:47 2021 +0000 Merge pull request #1216 from valentinewallace/2021-12-preimage-getter Add utility for retrieving an LDK payment preimage commit a3330acecfd50611a3b3ab09dc4dd564603bd4dc Author: Matt Corallo <git@bluematt.me> Date: Fri Dec 17 00:32:28 2021 +0000 Remove trailing whitespace in the FeeEstimator interface commit 41cfd833f1337c890d3fec0fe18bfa2c6bea75f2 Author: Valentine Wallace <vwallace@protonmail.com> Date: Mon Dec 13 18:40:16 2021 -0500 inbound_payment: Add utility to get payment preimage given hash/secret User-requested feature commit d734ad814e14097f1b1a053e8d63b3a84b1c9435 Author: Valentine Wallace <vwallace@protonmail.com> Date: Mon Dec 13 17:36:09 2021 -0500 inbound_payment: DRY verify method for use in getting preimage in the next commit(s) commit d20239bbeb28daf94b8ca34304a0e7c3a12a4808 Author: Valentine Wallace <vwallace@protonmail.com> Date: Thu Dec 16 16:27:13 2021 -0800 create_inbound_payment: warn about dup hashes Leftover feedback from #1177 commit 6e0820ca197d8e318ad74ad959b17f70889434be Author: Valentine Wallace <vwallace@protonmail.com> Date: Thu Dec 16 16:24:24 2021 -0800 Salt inbound payment ExpandedKey Leftover feedback from #1177 commit 86d5944b0d4b85577d9eede80843dda445ad28e9 Merge: cec8ce0f d41499a2 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Fri Dec 17 00:15:24 2021 +0000 Merge pull request #1177 from valentinewallace/2021-11-derivable-payment-secret Stop storing pending inbound payment data; instead make it derivable on receive commit d41499a26084c828de6542380b0b615093bda954 Author: Valentine Wallace <vwallace@protonmail.com> Date: Thu Dec 9 17:41:33 2021 -0500 Add new invoice CreationError::InvalidAmount for use in checking `create_inbound_payment` in an invoice creation utility. Note that if the error type of `create_inbound_payment` ever changed, we'd be forced to update the invoice utility's callsite to handle the new error commit 846487555556d8465c5b7b811f976e78f265c48f Author: Valentine Wallace <vwallace@protonmail.com> Date: Mon Nov 29 19:59:18 2021 -0500 Drop need to store pending inbound payments and replace payment_secret with encrypted metadata See docs on `inbound_payment::verify` for details Also add min_value checks to all create_inbound_payment* methods commit 1d516a6fc505783ce752f0289322ce73ae10bcf4 Author: Valentine Wallace <vwallace@protonmail.com> Date: Mon Nov 29 19:36:59 2021 -0500 Add get_single_block to chacha20 module In the next commit, we'll want to get a single block from a chacha stream that takes a 16-byte nonce. commit 063b7583c13e17e339fc9f3083ca3d54e2d2d050 Author: Valentine Wallace <vwallace@protonmail.com> Date: Mon Nov 22 16:53:18 2021 -0500 Macro-ize checking that the total value of an MPP's parts is sane This DRY-ed code will be used in upcoming commits when we stop storing inbound payment data commit 6dd1ec1feda39ee87f0b2cff7168751d827e0a43 Author: Valentine Wallace <vwallace@protonmail.com> Date: Mon Nov 29 12:50:47 2021 -0500 Add get_inbound_payment_key_material to KeysInterface This will allow us to retrieve key material for encrypting/decrypting inbound payment info, in upcoming commits commit cec8ce0fcbfb3073791d4427d8b155ab9ad5ba22 Merge: 6e27ca03 73e8dc41 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Thu Dec 16 23:04:00 2021 +0000 Merge pull request #1212 from TheBlueMatt/2021-12-timeout-graph Add a method to prune stale channels from NetworkGraph commit 73e8dc41a6c277b001587d88426f0c0872f21a0f Author: Matt Corallo <git@bluematt.me> Date: Wed Dec 15 18:59:15 2021 +0000 Automatically prune NetworkGraph of stale channels hourly in BP commit cd43ff4a5ed47cf67a3c553007b36de3e38b476b Author: Matt Corallo <git@bluematt.me> Date: Tue Dec 14 01:33:37 2021 +0000 Reject channel_update messages with timestamps too old or new Because we time out channel info that is older than two weeks now, we should also reject new channel info that is older than two weeks, in addition to rejecting future channel info. commit 4677e14c007a5453613afc20b088cbe938251226 Author: Matt Corallo <git@bluematt.me> Date: Fri Dec 10 06:46:29 2021 +0000 Add a method to prune stale channels from `NetworkGraph` We define "stale" as "haven't heard an updated channel_update in two weeks", as described in BOLT 7. We also filter out stale channels at write-time for `std` users, as we can look up the current time. commit c575429639a388ef3d5fc2b036163914bc947c57 Author: Matt Corallo <git@bluematt.me> Date: Fri Dec 10 05:57:30 2021 +0000 Drop `allow_wallclock_use` feature in favor of simply using `std` Fixes #1147. commit 11d644824efc78624330a7acbc96eabf9d481e89 Author: Matt Corallo <git@bluematt.me> Date: Wed Dec 15 22:38:13 2021 +0000 DRY up network_graph tests substantially with message creation fns commit 6e27ca0316ee161ae328394606d708b1d4fd8ab0 Merge: 064d7099 01ef55ae Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Thu Dec 16 17:44:21 2021 +0000 Merge pull request #1214 from TheBlueMatt/2021-12-fix-tokio-msrv-bump commit 01ef55ae86d9fce4b1741c542b8a0d77081a3163 Author: Matt Corallo <git@bluematt.me> Date: Thu Dec 16 02:21:48 2021 +0000 Build no-std on 1.47 now that core2 supports it commit 144145e4f93e7d34c8034dceb920f367aee0ae4a Author: Matt Corallo <git@bluematt.me> Date: Thu Dec 16 02:10:09 2021 +0000 Pin tokio to 1.14.0 in CI for older rustc's commit 064d709910df70403e84176804c9dcb137b73572 Merge: 88a86d12 e88c7210 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Wed Dec 15 23:32:30 2021 +0000 Merge pull request #1148 from dunxen/2021-11-new-onion-errors Add mpp_timeout and invalid_onion_payload descriptions commit 88a86d126f1bab0af8270a6388843dcc4a354e70 Merge: 54114c9d 0c34737e Author: Conor Okus <conor.okus@hotmail.co.uk> Date: Wed Dec 15 16:15:21 2021 +0000 Merge pull request #1211 from ConorOkus/2021-11-add-max-conversion Update docs to add max return value to conversion list commit 54114c9d858fba00d0885d06419f4650629bf6c7 Merge: 09714e6f 05d7a33a Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Wed Dec 15 04:58:46 2021 +0000 Merge pull request #1202 from TheBlueMatt/2021-12-fix-retries-races Fix payment retry races and inform users when a payment fails commit 05d7a33a581076fbea1e9d626c40e0c3c44031b1 Author: Matt Corallo <git@bluematt.me> Date: Mon Dec 13 17:41:36 2021 +0000 Make attempting to retry a succeeded payment an APIError, not Route This is symmetric with the new failure once a payment is abandoned. commit 3086bd8c8e3f4c1d2c5fa99279f516817ce05587 Author: Matt Corallo <git@bluematt.me> Date: Sat Dec 4 23:41:37 2021 +0000 Use `Event::PaymentFailed` in `InvoicePayer` to remove retry count This finally fixes the bug described in the previous commits where we retry a payment after its retry count has expired due to early removal of the payment from the retry count tracking map. A test is also added which demonstrates the bug in previous versions and which passes now. Fixes #1164. commit 7782d0a1ef9fb4e9d26f78a042da16939708d697 Author: Matt Corallo <git@bluematt.me> Date: Fri Dec 10 00:28:24 2021 +0000 Expose an event when a payment has failed and retries complete When a payment fails, a payer needs to know when they can consider a payment as fully-failed, and when only some of the HTLCs in the payment have failed. This isn't possible with the current event scheme, as discovered recently and as described in the previous commit. This adds a new event which describes when a payment is fully and irrevocably failed, generating it only after the payment has expired or been marked as expired with `ChannelManager::mark_retries_exceeded` *and* all HTLCs for it have failed. With this, a payer can more simply deduce when a payment has failed and use that to remove payment state or finalize a payment failure. commit 0b3240ee6a6e1baeafaed9e766997098be588358 Author: Matt Corallo <git@bluematt.me> Date: Fri Dec 3 19:57:37 2021 +0000 Add a variant to `PendingOutboundPayment` for retries-exceeded When a payer gives up trying to retry a payment, they don't know for sure what the current state of the event queue is. Specifically, they cannot be sure that there are not multiple additional `PaymentPathFailed` or even `PaymentSuccess` events pending which they will see later. Thus, they have a very hard time identifying whether a payment has truly failed (and informing the UI of that fact) or if it is still pending. See [1] for more information. In order to avoid this mess, we will resolve it here by having the payer give `ChannelManager` a bit more information - when they have given up on a payment - and using that to generate a `PaymentFailed` event when all paths have failed. This commit adds the neccessary storage and changes for the new state inside `ChannelManager` and a public method to mark a payment as failed, the next few commits will add the new `Event` and use the new features in our `PaymentRetrier`. [1] https://github.com/lightningdevkit/rust-lightning/issues/1164 commit 8c9615e8d6be6d4e3ff2d861a8a30266217672b2 Author: Matt Corallo <git@bluematt.me> Date: Sat Dec 4 23:41:01 2021 +0000 DRY up payment failure macros in functional_test_utils ... with a more extensible expectation-checking framework for them. commit e88c7210f81d5c09d9f60f7f9d6946de4f18146f Author: Duncan Dean <duncangleeddean@gmail.com> Date: Sat Oct 30 10:25:44 2021 +0200 Add mpp_timeout and invalid_onion_payload descriptions & handling commit 0c34737e3512e7e254a93ce6fa44d35843a594d5 Author: Conor Okus <cokus@cokus-macbookpro.local> Date: Wed Dec 8 17:15:57 2021 +0000 Updates Fee estimator docs to include a max return value for get_est_sat_per_1000_weight Updates docs to include a max return value for get_est_sat_per_1000_weight Adds max to both conversions Additional detail commit 09714e6fe286b8cd5863b616748362b76c2fbdf0 Merge: 61518f97 0fe2aef0 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Thu Dec 9 18:21:55 2021 +0000 Merge pull request #1169 from TheBlueMatt/2021-11-fix-update-announcements Fix announcements of our own gossip commit 0fe2aef0e6f12838e05521d3f38a51e7c03e5380 Author: Matt Corallo <git@bluematt.me> Date: Thu Dec 9 03:49:50 2021 +0000 Add a comment describing the timestamp field's use commit 61518f97212fd6a4c40cdd62a3ee5ab7a15d1971 Merge: 3cf1b15b 02a9f92e Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Wed Dec 8 02:24:32 2021 +0000 Merge pull request #1203 from lightning-signer/2021-12-value-to-self Getter for the total channel balance commit 3cf1b15b787cdbb3160e2ca15c63df8edd01433a Merge: cd4dc39a 4fc7bcee Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Wed Dec 8 01:01:34 2021 +0000 Merge pull request #1210 from TheBlueMatt/2021-12-new-codecod-upload Upgrade to codecov uploader v2 commit 4fc7bcee5849bf362a7fa1052817d5b6f378e463 Author: Matt Corallo <git@bluematt.me> Date: Tue Dec 7 19:17:57 2021 +0000 Upgrade to codecov uploader v2 Some time ago codecov stopped supporting their old v1 uploader, and it seems they've now finally turned it off, so we aren't getting any coverage reports anymore. Hopefully upgrading is pretty trivial. commit cd4dc39a8c4732bea1a3221617f86e34dfb7efb8 Merge: cd9cd47f d5e70ad1 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue Dec 7 20:11:47 2021 +0000 Merge pull request #1208 from TheBlueMatt/2021-12-less-force-close Reduce force-closures with user fee estimators which round poorly commit cd9cd47f686c0ac6543e05fd23fe67d74407c409 Merge: 3ec529d7 8f4a22fe Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue Dec 7 19:28:29 2021 +0000 Merge pull request #1205 from TheBlueMatt/2021-12-new-feature-bit Support the channel_type feature bit. commit d5e70ad185137d2b23744aeaaa0116a0b5b80372 Author: Matt Corallo <git@bluematt.me> Date: Tue Dec 7 17:23:42 2021 +0000 Reduce force-closures with user fee estimators which round poorly See comment for more commit 8f4a22fe310a0a1643a82a5d3e17617da000b912 Author: Matt Corallo <git@bluematt.me> Date: Mon Dec 6 00:18:59 2021 +0000 Support the `channel_type` feature bit. Note that this feature bit does absolutely nothing. We signal it (as we already support channel type negotiation), but do not bother to look to see if peers support it, as we don't care - we simply look for the TLV entry and deduce if a peer supports channel type negotiation from that. The only behavioral change at all here is that we don't barf if a peer sets channel type negotiation to required via the feature bit (instead of failing the channel at open-time), but of course no implementations do this, and likely won't for some time (if ever - you can simply fail channels with unknown types later, and there's no reason to refuse connections, really). As defined in https://github.com/lightning/bolts/pull/906 commit 02a9f92ea4f345b99e0d77b37018b23cb63a0516 Author: Devrandom <c1.devrandom@niftybox.net> Date: Sun Dec 5 12:42:25 2021 +0100 Getter for the total channel balance The existing balance getters subtract reserve, this one does not. commit 3ec529d7cc06e15c0d55161e30b931ac115fac95 Merge: 9c6961ea c453d041 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue Dec 7 00:42:02 2021 +0000 Merge pull request #1201 from jkczyz/2021-12-idempotent-channelmanager Ensure ChannelManager methods are idempotent commit c453d04137997f8ca2f79ae123b4915062e2f369 Author: Jeffrey Czyz <jkczyz@gmail.com> Date: Fri Dec 3 13:04:58 2021 -0600 Ensure ChannelManager methods are idempotent During event handling, ChannelManager methods may need to be called as indicated in the Event documentation. Ensure that these calls are idempotent for the same event rather than panicking. This allows users to persist events for later handling without needing to worry about processing the same event twice (e.g., if ChannelManager is not persisted but the events were, the restarted ChannelManager would return some of the same events). commit 9c6961ea8463580810a591474fcae819b0433b16 Merge: d47aebca 361639de Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon Dec 6 22:49:18 2021 +0000 Merge pull request #1204 from TheBlueMatt/2021-12-no-torv2 Remove OnionV2 parsing support commit d47aebca38ca8bfbd06f57052ed2712121593d6f Merge: ea892865 1b88f163 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon Dec 6 19:35:58 2021 +0000 Merge pull request #1130 from TheBlueMatt/2021-10-mon-fail-after-conf Always return failure in update_monitor after funding spend commit 1b88f1638e5d743dc827b5f0e1ccd35f00b69907 Author: Matt Corallo <git@bluematt.me> Date: Sun Nov 28 17:13:38 2021 +0000 Add trivial test of monitor update failure during block connection commit 9fe0cf19f6dba5895bcb21c7158c04fbc90c1fb6 Author: Matt Corallo <git@bluematt.me> Date: Tue Oct 19 23:44:29 2021 +0000 Add a test for MonitorEvent holding when they complete out-of-order commit fa62775f9d2306051f0134a9d6d0183e9f246623 Author: Matt Corallo <git@bluematt.me> Date: Sun Oct 17 21:24:39 2021 +0000 Add a simple test for ChainMonitor MonitorUpdate-holding behavior commit 6bcb270ae10472a1dd28a3211e80257b1a2b1fd1 Author: Matt Corallo <git@bluematt.me> Date: Sun Oct 17 21:24:53 2021 +0000 Make APIError debug output more clear by including the variant commit 87b687962237c3bfc596a0963b1ef2bc2e9ba6f7 Author: Matt Corallo <git@bluematt.me> Date: Mon Nov 29 21:36:12 2021 +0000 Drop `MonitorUpdateErr` in favor of opaque errors. We don't expect users to ever change behavior based on the string contained in a `MonitorUpdateErr`, except log it, so there's little reason to not just log it ourselves and return a `()` for errors. We do so here, simplifying the callsite in `ChainMonitor` as well. commit 25542b8157e95e362e097b73a366da3f8bfe962d Author: Matt Corallo <git@bluematt.me> Date: Sun Oct 17 21:28:50 2021 +0000 Always return failure in `update_monitor` after funding spend Previously, monitor updates were allowed freely even after a funding-spend transaction confirmed. This would allow a race condition where we could receive a payment (including the counterparty revoking their broadcasted state!) and accept it without recourse as long as the ChannelMonitor receives the block first, the full commitment update dance occurs after the block is connected, and before the ChannelManager receives the block. Obviously this is an incredibly contrived race given the counterparty would be risking their full channel balance for it, but its worth fixing nonetheless as it makes the potential ChannelMonitor states simpler to reason about. The test in this commit also tests the behavior changed in the previous commit. commit aa5e5f967375eb31d8b0d0ed7f20ef31242693f7 Author: Matt Corallo <git@bluematt.me> Date: Fri Jul 23 15:49:24 2021 +0000 Define public getters for all feature flags There's not a ton of reason not to do this, and moving it to the macro removes some code. commit 361639decf9c98ff43d228415e3cab5955e2b973 Author: Matt Corallo <git@bluematt.me> Date: Mon Dec 6 00:12:35 2021 +0000 Remove OnionV2 parsing support OnionV2s don't (really) work on Tor anymore anyway, and the field is set for removal in the BOLTs [1]. Sadly because of the way addresses are parsed we have to continue to understand that type 3 addresses are 12 bytes long. Thus, for simplicity we keep the `OnionV2` enum variant around and just make it an opaque 12 bytes, with the documentation updated to note the deprecation. [1] https://github.com/lightning/bolts/pull/940 commit ea89286569c95c4eb9a692f9aaf9f1e647d012ef Merge: a3e4af0b 857b4c08 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Fri Dec 3 21:04:41 2021 +0000 Merge pull request #1197 from jkczyz/2021-11-score-successful-payment-path Score successful payment paths commit 857b4c08a5870025a3aeb5e606972ef375d4bbea Author: Jeffrey Czyz <jkczyz@gmail.com> Date: Tue Nov 30 22:57:32 2021 -0600 Fix shift overflow in Scorer::channel_penalty_msat An unchecked shift of more than 64 bits on u64 values causes a shift overflow panic. This may happen if a channel is penalized only once and (1) is not successfully routed through and (2) after 64 or more half life decays. Use a checked shift to prevent this from happening. commit d28d6a5403682d2d1539bf73552a4490a2309578 Author: Jeffrey Czyz <jkczyz@gmail.com> Date: Tue Nov 30 18:27:08 2021 -0600 Decay channel failure penalty upon success If a payment failed to route through a channel, a penalty is applied to the channel in the future when finding a route. This penalty decays over time. Immediately decay the penalty by one half life when a payment is successfully routed through the channel. commit c36bf9249978dce2b72b3b67d01e22c6c8d78431 Author: Jeffrey Czyz <jkczyz@gmail.com> Date: Tue Nov 30 17:16:05 2021 -0600 Score successful payment paths Expand the Score trait with a payment_path_successful function for scoring successful payment paths. Called by InvoicePayer's EventHandler implementation when processing PaymentPathSuccessful events. May be used by Score implementations to revert any channel penalties that were applied by calls to payment_path_failed. commit a3e4af0bb800a7fbc9d7908a41706d211eeda835 Merge: 0cdea66b ef246ed7 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Wed Dec 1 20:14:14 2021 +0000 Merge pull request #1196 from TheBlueMatt/2021-11-bad-rustdoc Fix compilation in `payment` rustdoc examples commit ef246ed7862efc8c9be42f2c27eecb43d22e6b97 Author: Matt Corallo <git@bluematt.me> Date: Tue Nov 30 21:01:39 2021 +0000 Fix compilation in `payment` rustdoc examples The samples were not valid rust, but previous versions of rustc had a bug where they were accepted anyway. Latest rustc beta no longer accepts these. commit 0cdea66b0e73d45b2a137eed145a294b7d713cc4 Merge: 9fcc626e e62bd9d1 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue Nov 30 15:35:27 2021 +0000 Merge pull request #1195 from TheBlueMatt/2021-11-chanman-read-regression Fix regression when reading `Event::PaymentReceived` in some cases commit e62bd9d137e8e82ad38b820c7bd34b229b241fe9 Author: Matt Corallo <git@bluematt.me> Date: Mon Nov 29 20:05:35 2021 +0000 Fix regression when reading `Event::PaymentReceived` in some cases For some reason rustc was deciding on a type for the `Option` being deserialized for us as `_user_payment_id`. This really, really, absolutely should have been a compile failure - the type (with methods called on it!) was ambiguous! Instead, rustc seems to have been defaulting to `Option<()>`, causing us to read zero of the eight bytes in the `user_payment_id` field, which returns an `Err(InvalidValue)` error as TLVs must always be read fully. This should likely be reported to rustc as its definitely a bug, but I cannot seem to cause the same error on any kinda of vaguely-minimized version of the same code. Found by `chanmon_consistency` fuzz target. commit 9fcc626ee4d23a276fc8dd87ddb2538f4d5565f9 Merge: e9774aeb 25f4a54a Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon Nov 29 21:02:36 2021 +0000 Merge pull request #1163 from TheBlueMatt/2021-11-support-insecure-counterparty Explicitly support counterparty setting 0 channel reserve commit 25f4a54a2bdda91232a62034befc3c6d20611058 Author: Matt Corallo <git@bluematt.me> Date: Tue Nov 9 21:25:33 2021 +0000 Explicitly support counterparty setting 0 channel reserve A peer providing a channel_reserve_satoshis of 0 (or less than our dust limit) is insecure, but only for them. Because some LSPs do it with some level of trust of the clients (for a substantial UX improvement), we explicitly allow it. Because its unlikely to happen often in normal testing, we test it explicitly here. commit e9774aeb2eaf27dccd2d3d4422b65040995bdc9b Merge: 1a743672 f118bb77 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon Nov 29 16:56:23 2021 +0000 Merge pull request #1189 from TheBlueMatt/2021-11-trivial-impls Derive `Clone` and friends on additional public structs commit f118bb776a5d242916f836e538511cd70f6d01a4 Author: Matt Corallo <git@bluematt.me> Date: Tue Oct 19 06:20:12 2021 +0000 Implement Clone for InvalidShutdownScript Users hopefully shouldn't have much of a reason to use this, but the bindings may need it to ensure no leaking pointers over an ffi. commit 04d0cca87293f5a47812d19920686755892f8446 Author: Matt Corallo <git@bluematt.me> Date: Tue Oct 19 06:19:28 2021 +0000 Implement Clone, Hash, PartialEq for ClosingTransaction This is a public struct intended to be used as an object by users, so it should likely have common implementations, given they're trivial. commit 37c6c18789151b2a8c7c3b7e4c1d98f7f86e906a Author: Matt Corallo <git@bluematt.me> Date: Sun Oct 17 21:26:04 2021 +0000 Continue after a single failure in `ChannelMonitor::update_monitor` `ChannelMonitorUpdate`s may contain multiple updates, including, eg a payment preimage after a commitment transaction update. While such updates are generally not generated today, we shouldn't return early out of the update loop, causing us to miss any updates after an earlier update fails. commit 1ce922c631aac879b3569047a8f17e2a5df5e76a Author: Matt Corallo <git@bluematt.me> Date: Sun Oct 17 21:23:51 2021 +0000 Prefer fully-specified paths in test macros This avoids macros being context-specific use-dependent. commit 63698ecbbfec96998257424e0b09224182a81239 Author: Matt Corallo <git@bluematt.me> Date: Wed Oct 20 01:35:01 2021 +0000 Allow missing-docs on test-only macros commit 1a743672b92a65e3d401a11001354f3b514bf956 Merge: 937403ed 3539f270 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Wed Nov 24 20:03:14 2021 +0000 Merge pull request #1184 from TheBlueMatt/2021-11-c-bindings-tweaks C Bindings Compatibility Tweaks commit 3539f270c47ec8b525bac65fce2b85c94eb55be9 Author: Matt Corallo <git@bluematt.me> Date: Mon Nov 22 18:00:08 2021 +0000 Seal `scoring::Time` and only use `Instant` or `Eternity` publicly `scoring::Time` exists in part to make testing the passage of time in `Scorer` practical. To allow no-std users to provide a time source it was exposed as a trait as well. However, it seems somewhat unlikely that a no-std user is going to have a use for providing their own time source (otherwise they wouldn't be a no-std user), and likely they won't have a graph in memory either. `scoring::Time` as currently written is also exceptionally hard to write C bindings for - the C bindings trait mappings relies on the ability to construct trait implementations at runtime with function pointers (i.e. `dyn Trait`s). `scoring::Time`, on the other hand, is a supertrait of `core::ops::Sub` which requires a `sub` method which takes a type parameter and returns a type parameter. Both of which aren't practical in bindings, especially given the `Sub::Output` associated type is not bound by any trait bounds at all (implying we cannot simply map the `sub` function to return an opaque trait object). Thus, for simplicity, we here simply seal `scoring::Time` and make it effectively-private, ensuring the bindings don't need to bother with it. commit a173ded03f84193f8da14301e81fdec419c5e441 Author: Matt Corallo <git@bluematt.me> Date: Mon Nov 22 03:27:17 2021 +0000 Make `Score : Writeable` in c_bindings and impl on `LockedScore` Ultimately we likely need to wrap the locked `Score` in a struct that exposes writeable somehow, but because all traits have to be fully concretized for C bindings we'll still need `Writeable` on all `Score` in order to expose `Writeable` on the locked score. Otherwise, we'll only have a `LockedScore` with a `Score` visible that only has the `Score` methods, never the original type. commit 937403ed780f63a8a48fe1939f9af23b12d04f9c Merge: ef86a3e2 4831de41 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Wed Nov 24 16:31:32 2021 +0000 Merge pull request #1186 from TheBlueMatt/2021-11-fix-log-select Fix the `max_level_trace` feature commit 4831de41ec40f842148358592da8c1ded7e2d523 Author: Matt Corallo <git@bluematt.me> Date: Tue Nov 23 23:03:34 2021 +0000 Test all log-limiting features in CI commit 31e592bedf0899dc425036d1518dd83fb0161609 Author: Matt Corallo <git@bluematt.me> Date: Tue Nov 23 23:03:13 2021 +0000 Fix compilation with the max_level_trace feature commit d2ac683f4e6267b1ae8997a97b1c1dec1cd540c2 Author: Matt Corallo <git@bluematt.me> Date: Tue Nov 16 20:55:10 2021 +0000 Add a comment describing `update_time_counter` and when its updated commit 391fbfbe1abbd011395542f5f0f96dbc57b8655e Author: Matt Corallo <git@bluematt.me> Date: Sat Nov 13 01:54:54 2021 +0000 Re-broadcast our own gossip even if its same as the last broadcast Even if our gossip hasn't changed, we should be willing to re-broadcast it to our peers. All our peers may have been disconnected the last time we broadcasted it. commit 8f89371bae42d127f28b4362322682fe22718175 Author: Matt Corallo <git@bluematt.me> Date: Sat Nov 13 01:06:09 2021 +0000 Update `ChannelUpdate::timestamp` when channels are dis-/en-abled We update the `Channel::update_time_counter` field (which is copied into `ChannelUpdate::timestamp`) only when the channel is initialized or closes, and when a new block is connected. However, if a peer disconnects or reconnects, we may wish to generate `ChannelUpdate` updates in between new blocks. In such a case, we need to make sure the `timestamp` field is newer than any previous updates' `timestamp` fields, which we do here by simply incrementing it when the channel status is changed. As a side effect of this we have to update `test_background_processor` to ensure it eventually succeeds even if the serialization of the `ChannelManager` changes after the test begins. commit 74828d243567af448cec5f09eb2d6a8eeed3ca48 Author: Matt Corallo <git@bluematt.me> Date: Sat Nov 13 00:27:05 2021 +0000 Separate ChannelAnnouncement and ChannelUpdate broadcast conditions When a `ChannelUpdate` message is generated for broadcast as a part of a `BroadcastChannelAnnouncement` event, it may be newer than our previous `ChannelUpdate` and need to be broadcast. However, if the `ChannelAnnouncement` had already been seen we wouldn't re-broadcast either message as the `handle_channel_announcement` call would fail, short-circuiting the condition to broadcast both. Instead, we split the broadcast of each message as well as the conditional so that we always attempt to handle each message and update our local graph state, then broadcast the message if its update was processed successfully. commit f69311ccff517f01211415461db666db22290232 Author: Matt Corallo <git@bluematt.me> Date: Tue Nov 9 21:12:30 2021 +0000 Store holder channel reserve and max-htlc-in-flight explicitly Previously, `holder_selected_channel_reserve_satoshis` and `holder_max_htlc_value_in_flight_msat` were constant functions of the channel value satoshis. However, in the future we may allow allow users to specify it. In order to do so, we'll need to track them explicitly, including serializing them as appropriate. We go ahead and do so here, in part as it will make testing different counterparty-selected channel reserve values easier. commit ef86a3e20952a96f7b646a9a4154aa5dc220147a Merge: 19191b45 13e4fd58 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue Nov 23 20:46:38 2021 +0000 Merge pull request #1162 from TheBlueMatt/2021-11-fix-accept-chan-checks Correct initial commitment tx fee affordability checks on open commit 19191b450c8f07e7b703ec83eabaa0d268757bd7 Merge: 2b789578 2c4f16d5 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue Nov 23 20:39:28 2021 +0000 Merge pull request #1178 from jkczyz/2021-11-payment-path-successful Generate PaymentPathSuccessful event for each path commit 2b78957888b86760bb16325a23302d9c979b5602 Merge: ba50dd57 530abc5e Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue Nov 23 19:37:09 2021 +0000 Merge pull request #1176 from lightning-signer/2021-11-htlc-anchors Add anchors support for HTLCs commit 2c4f16d5e39e6d4e90d1b9836cceaad11810fbaa Author: Jeffrey Czyz <jkczyz@gmail.com> Date: Thu Nov 18 16:24:14 2021 -0600 Generate PaymentPathSuccessful event for each path A single PaymentSent event is generated when a payment is fulfilled. This is occurs when the preimage is revealed on the first claimed HTLC. For subsequent HTLCs, the event is not generated. In order to score channels involved with a successful payments, the scorer must be notified of each successful path involved in the payment. Add a PaymentPathSuccessful event for this purpose. Generate it whenever a part is removed from a pending outbound payment. This avoids duplicate events when reconnecting to a peer. commit 530abc5efd96da693316f2c45c6169c1ee7064e6 Author: Ken Sedgwick <ken@bonsai.com> Date: Tue Nov 16 11:40:27 2021 -0800 Add test vectors for get_htlc_redeemscript wrt anchors commit 50d81220df0d1f73541f39863657ca9bde193afc Author: Ken Sedgwick <ken@bonsai.com> Date: Tue Nov 16 09:27:33 2021 -0800 Adjust HTLC_{SUCCESS,TIMEOUT}_TX_WEIGHT when anchors used commit 6c36e011a80cd3d02db8591fb9a19111dceecb86 Author: Ken Sedgwick <ken@bonsai.com> Date: Mon Nov 15 19:39:39 2021 -0800 Add anchor support to build_htlc_transaction commit c077f36b4b231b05b9c6bfadd5ef0c017b17eaa0 Author: Ken Sedgwick <ken@bonsai.com> Date: Mon Nov 15 18:50:57 2021 -0800 Increase visibility of anchor related methods commit 3efcbab5d42d5c2cd7d37ce18a8bcca211569bb8 Author: Ken Sedgwick <ken@bonsai.com> Date: Mon Nov 15 18:03:46 2021 -0800 Add anchor support to commitment HTLC outputs commit 016eb96fc7170bdbab238292d3cc9338c2a66eb9 Author: Matt Corallo <git@bluematt.me> Date: Sun Nov 21 22:29:48 2021 +0000 Support `logger::Record` in C by String-ing the fmt::Arguments This adds a new (non-feature) cfg argument `c_bindings` which will be set when building C bindings. With this, we can (slightly) tweak behavior and API based on whether we are being built for Rust or C users. Ideally we'd never need this, but as long as we can keep the API consistent-enough to avoid material code drift, this gives us a cheap way of doing the "right" thing for both C and Rust when the two are in tension. We also move lightning-background-processor to support the same MSRV as the main lightning crate, instead of only lightning-net-tokio's MSRV. commit 13e4fd586ed0c8439dcada150c4fffa33fa67db9 Author: Matt Corallo <git@bluematt.me> Date: Wed Nov 10 04:40:33 2021 +0000 Test fixed channel reserve checks on channel open commit 940ef05371221138a930b1afc1243cb1cdf63873 Author: Matt Corallo <git@bluematt.me> Date: Wed Nov 10 01:36:26 2021 +0000 Correct initial commitment tx fee affordability checks on open Previously, we would reject inbound channels if the funder wasn't able to meet our channel reserve on their first commitment transaction only if they also failed to push enough to us for us to not meet their initial channel reserve as well. There's not a lot of reason to care about us meeting their reserve, however - its largely expected that they may not push enough to us in the initial open to meet it, and its not actually our problem if they don't. Further, we used our own fee, instead of the channel's actual fee, to calculate fee affordability of the initial commitment transaction. We resolve both issues here, rewriting the combined affordability check conditionals in inbound channel open handling and adding a fee affordability check for outbound channels as well. The prior code may have allowed a counterparty to start the channel with "no punishment" states - violating the reason for the reserve threshold. commit 1d30e06893ff6e19fbbe472ea74ef150f296cbfc Author: Matt Corallo <git@bluematt.me> Date: Wed Nov 10 03:44:04 2021 +0000 Rewrite test_update_fee_that_funder_cannot_afford to avoid magic Instead of magic hard-coded constants, its better for tests to derive the values used so that they change if constants are changed and so that it is easier to re-derive constants in the future as needed. commit a33d3b98d7c6c4c786e53f5c904cb8620ec54e24 Author: Matt Corallo <git@bluematt.me> Date: Mon Nov 15 23:22:08 2021 +0000 Make Channel::commit_tx_fee_msat static and take fee explicitly This may avoid risk of bugs in the future as it requires the caller to think about the fee being used, not just blindly use the current (committed) channel feerate. commit ba50dd57868a78d00ed1a3c7a9089cf39d3ca235 Merge: 22398853 c3c0e602 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon Nov 22 22:45:51 2021 +0000 Merge pull request #1054 from ariard/2021-08-check-outbound-feerate Check for outbound feerate update affordability before sending commit 22398853c97966f42c709669fff3c63486d82993 Merge: 3cb3d18e 1180b633 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon Nov 22 21:55:06 2021 +0000 Merge pull request #1168 from TheBlueMatt/2021-11-mpp-routing-fixes Fix MPP routefinding when we first collect 95% of payment value commit c3c0e60226c594f8a5b949b424e74078fd7a24ce Author: Antoine Riard <dev@ariard.me> Date: Sun Nov 21 21:42:58 2021 -0500 Check outbound update_fee affordance incremented with holding cell HTLCs commit 1180b633b409029b45c5dbe31b698eaabc79f81a Author: Matt Corallo <git@bluematt.me> Date: Mon Nov 8 03:17:34 2021 +0000 Fix MPP routefinding when we first collect 95% of payment value See comment in new test for more details. commit 3cb3d18e1d3a7ab8f1ecaa3923faaf7a6887b062 Merge: 58539b84 3b4b74bc Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon Nov 22 18:58:56 2021 +0000 Merge pull request #1145 from tnull/add_gossip_log_level Introduce GOSSIP log level to PeerHandler commit 3b4b74bc6643fbc87e252ad346263fdc22048e51 Author: Elias Rohrer <elias.rohrer@tu-berlin.de> Date: Mon Nov 22 18:19:08 2021 +0100 Add a new log-level for gossip messages. commit 58539b8440e62dbfe54085949ab6246f4f93cba5 Merge: 1f170dba a4822e5b Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon Nov 22 16:40:37 2021 +0000 Merge pull request #1180 from valentinewallace/2021-11-remove-user-pmt-id Remove user_payment_id commit 1f170dba02c45b061116bbb5f1f82a329b6c9a89 Merge: 293e5f21 dea1310c Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon Nov 22 15:41:19 2021 +0000 Merge pull request #1182 from TheBlueMatt/2021-11-fix-txid-log Trivial Logging Fixes commit efd9ad22fc7d7c3ab131a7fc08b2aa1e1f46c1c0 Author: Antoine Riard <dev@ariard.me> Date: Thu Nov 18 21:23:41 2021 -0500 Introduce CommitmentStats commit 40f48def109c61d6b569e03113820ee042837d1b Author: Antoine Riard <dev@ariard.me> Date: Mon Sep 27 20:14:06 2021 -0400 Re-add `test_max_dust_htlc_exposure` commit dea1310c55171f03a23dc8d76b0f5cda666a93bf Author: Matt Corallo <git@bluematt.me> Date: Sat Nov 20 22:49:11 2021 +0000 Ensure current channel state is logged for all channels on startup commit 0b072834ab92081bba4ec907b39f1aa78953016c Author: Matt Corallo <git@bluematt.me> Date: Sat Nov 20 22:27:10 2021 +0000 Correct txid logging to reverse bytes. We also take this opportunity to log the channel being closed when one is closed by an on-chain spend of the funding output. commit 293e5f21ffd286a2037efcf52a025205e624e744 Merge: 8d886ee9 e81ec4a5 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Sat Nov 20 03:26:24 2021 +0000 Merge pull request #1027 from TheBlueMatt/2021-07-check-dust Check all outputs meet the dust threshold in `check_spends!()` commit ab11f450b67628284e80c28bce2b7d1e9033e97e Author: Antoine Riard <dev@ariard.me> Date: Sat Aug 21 18:52:05 2021 -0400 Check we won't overflow `max_dust_htlc_exposure_msat` at outbound feerate update commit a4822e5b27b7bffbae2e5337709c75f7c18dc800 Author: Valentine Wallace <vwallace@protonmail.com> Date: Fri Nov 12 14:48:39 2021 -0500 Remove user_payment_id In upcoming commits, we'll be making the payment secret and payment hash/preimage derivable from info about the payment + a node secret. This means we don't need to store any info about incoming payments and can eventually get rid of the channelmanager::pending_inbound_payments map. commit e81ec4a5adc1b4775ff650a7cd962ab4ea69e30f Author: Matt Corallo <git@bluematt.me> Date: Sat Jul 31 18:33:57 2021 +0000 Check all outputs meet the dust threshold in check_spends!() commit 9c1c7c496c34d274bfd66f04810676d215d2e904 Author: Matt Corallo <git@bluematt.me> Date: Sat Jul 31 22:19:45 2021 +0000 Limit minimum output size to the dust limit when RBF-bumping commit 31975c5994c8daca5b69c7c3b7d5466409ae8c2d Author: Antoine Riard <dev@ariard.me> Date: Sat Aug 21 18:05:51 2021 -0400 Cancel the outbound feerate update if above what we can afford commit ee7c5b572b84bfa0a2d88c72ed7032297869b130 Author: Antoine Riard <dev@ariard.me> Date: Tue Oct 19 13:29:50 2021 -0400 Introduce new helper commit_tx_fee_sat commit 8d886ee92439fdda911fd43887cd7e39d1b41c75 Merge: 2b4ca9e9 e7b2bca1 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Wed Nov 17 19:06:19 2021 +0000 Merge pull request #1173 from tnull/add_accept_inbound_channels_option Add 'accept_inbound_channels' config option. commit e7b2bca1d6b58ff9e2ccc3c0ca7d3c9afadb67e2 Author: Elias Rohrer <elias.rohrer@tu-berlin.de> Date: Wed Nov 17 18:54:47 2021 +0100 Add 'accept_inbound_channels' config option. commit 2b4ca9e9c53ee676620932754165bcc5e2308153 Merge: 77948dbc 35829214 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Wed Nov 17 17:28:36 2021 +0000 Merge pull request #1083 from TheBlueMatt/2021-09-funding-timeout Automatically close channels that go unfunded for 2016 blocks commit 77948dbcd7b167ff4386f1b9de13bd2d2aa97032 Merge: e1ad422c 42ebf774 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue Nov 16 22:10:42 2021 +0000 Merge pull request #1166 from TheBlueMatt/2021-11-chan-size-scoring commit 358292141a02ae52bdd54f44c63404267b5150db Author: Matt Corallo <git@bluematt.me> Date: Sun Sep 19 23:49:57 2021 +0000 Automatically close channels that go unfunded for 2016 blocks As recommended by BOLT 2 added in https://github.com/lightningnetwork/lightning-rfc/pull/839 commit b288a2739afa18c5aedaacb00ed809c98be47187 Author: Matt Corallo <git@bluematt.me> Date: Thu Sep 30 21:35:40 2021 +0000 Return `ClosureReason` from `Channel` chain update methods This fixes a few `ClosureReason`s and allows us to have finer-grained user-visible errors when a channel closes due to an on-chain event. commit 42ebf774155632b5656fd5820eb8c28d0003d9b6 Author: Matt Corallo <git@bluematt.me> Date: Fri Nov 12 15:52:59 2021 +0000 Move `Score` into a `scoring` module instead of a top-level module Traits in top-level modules is somewhat confusing - generally top-level modules are just organizational modules and don't contain things themselves, instead placing traits and structs in sub-modules. Further, its incredibly awkward to have a `scorer` sub-module, but only have a single struct in it, with the relevant trait it is the only implementation of somewhere else. Not having `Score` in the `scorer` sub-module is further confusing because it's the only module anywhere that references scoring at all. commit 9bec35ddde1750359c47b0a83f820ab7287cfd03 Author: Matt Corallo <git@bluematt.me> Date: Fri Nov 12 04:16:23 2021 +0000 Penalize large HTLCs relative to channels in default `Scorer` Sending HTLCs which are any greater than a very small fraction of the channel size tend to fail at a much higher rate. Thus, by default we start applying a penalty at only 1/8th the channel size and increase it linearly as the amount reaches the channel's capacity, 20 msat per 1024th of the channel capacity. commit 8dc7cfab3af7e26cddd8c65a913d5ee021907442 Author: Matt Corallo <git@bluematt.me> Date: Fri Nov 12 03:52:58 2021 +0000 Provide `Score` the HTLC amount and channel capacity This should allow `Score` implementations to make substantially better decisions, including of the form "willing to pay X to avoid routing over this channel which may have a high failure rate". commit e1ad422c1b9bb114f20411777faaee750c670ec5 Merge: 4a3139d2 592bfd7c Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue Nov 16 20:56:40 2021 +0000 Merge pull request #1160 from jkczyz/2011-11-spontaneous-invoice-payer Retries for spontaneous payments commit 592bfd7c58e73eaa5cfd5032d982883878510037 Author: Jeffrey Czyz <jkczyz@gmail.com> Date: Thu Nov 11 21:47:59 2021 -0600 Add PaymentHash parameter to Router::find_route Implementations of Router may need the payment hash in order to look up pre-computed routes from a probe for a given payment. Add a PaymentHash parameter to Router::find_route to allow for this. commit 756b3051573ed9b4e04cc2227af93c45fe0d8104 Author: Jeffrey Czyz <jkczyz@gmail.com> Date: Mon Nov 15 22:47:40 2021 -0600 Test retrying payment on partial send failure Add some test coverage for when InvoicePayer retries within pay_invoice because the Payer returned a partial failure. commit ab9576201d358881583a0327104d798b7f3e4fb0 Author: Jeffrey Czyz <jkczyz@gmail.com> Date: Tue Nov 9 18:07:23 2021 -0600 Replace expect_value_msat with expect_send Modify all InvoicePayer unit tests to use expect_send instead of expect_value_msat, since the former can discern whether the send was for an invoice, spontaneous payment, or a retry. Updates tests to set payer expectations if they weren't already and assert these before returning a failure. commit b4c7370d26a23ccf044b3d0e8b4c344c743433c9 Author: Jeffrey Czyz <jkczyz@gmail.com> Date: Tue Nov 9 17:32:37 2021 -0600 Support spontaneous payments in InvoicePayer InvoicePayer handles retries not only when handling PaymentPathFailed events but also for some types of PaymentSendFailure on the initial send. Expand InvoicePayer's interface with a pay_pubkey function for spontaneous (keysend) payments. Add a send_spontaneous_payment function to the Payer trait to support this and implement it for ChannelManager. commit c9ce344d56991ffa49c1867d5041bc136cedbece Author: Jeffrey Czyz <jkczyz@gmail.com> Date: Tue Nov 9 09:37:34 2021 -0600 Refactor InvoicePayer for spontaneous payments To support spontaneous payments, InvoicePayer's sending logic must be invoice-agnostic. Refactor InvoicePayer::pay_invoice_internal such that invoice-specific code is in pay_invoice_using_amount and the remaining logic is in pay_internal. Further refactor the code's payment_cache locking such that it is accessed consistently when needed, and tidy up the code a bit. commit 4a3139d24df6b7e60161816b2272f0b0e2a622f9 Merge: 4d6c2624 a44587d9 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue Nov 16 18:18:01 2021 +0000 Merge pull request #1161 from TheBlueMatt/2021-11-fix-chan-type-ser Correct Channel type serialization logic commit a44587d9aabe45b5ad419996ec7dbe3e5ddd724b Author: Matt Corallo <git@bluematt.me> Date: Tue Nov 9 21:22:47 2021 +0000 Correct Channel type serialization logic Currently, we write out the Channel's `ChannelTypeFeatures` as an odd type, implying clients which don't understand the `ChannelTypeFeatures` field can simply ignore it. This is obviously nonsense if the channel type is some future version - the client needs to fail to deserialize as it doesn't understand the channel's type. We adapt the serialization logic here to only write out the `ChannelTypeFeatures` field if it is something other than only-static-remote-key, and simply consider that "default" (as it is the only supported type today). Then, we write out the channel type as an even TLV, implying clients which do not understand it must fail to read the `Channel`. Note that we do not need to bother reserving the TLV type no longer written as it never appeared in a release (merged post-0.0.103). commit 4d6c26248d85abe9a3c8aeefe31b4ebafd3b5bee Merge: 4bb81ff5 5e998cce Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue Nov 16 16:18:20 2021 +0000 Merge pull request #1119 from TheBlueMatt/2021-10-less-aggressive-htlc-timeouts Be less aggressive in outbound HTLC CLTV timeout checks commit 5e998cce6ba4511df483cf893cd19b51a0ac0214 Author: Matt Corallo <git@bluematt.me> Date: Wed Oct 13 04:19:13 2021 +0000 Be less aggressive in outbound HTLC CLTV timeout checks We currently assume our counterparty is naive and misconfigured and may force-close a channel to get an HTLC we just forwarded them. There shouldn't be any reason to do this - we don't have any such bug, and we shouldn't start by assuming our counterparties are buggy. Worse, this results in refusing to forward payments today, failing HTLCs for largely no reason. Instead, we keep a fairly conservative check, but not one which will fail HTLC forwarding spuriously - testing only that the HTLC doesn't expire for a few blocks from now. Fixes #1114. commit 4bb81ff5942749077613827d6807b64230ecbcd5 Merge: 1beccf18 119841a2 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Sat Nov 13 00:23:28 2021 +0000 Merge pull request #1131 from TheBlueMatt/2021-10-upstream-dust Use upstream rust-bitcoin's dust calculation instead of our own commit 119841a24365174c0744fbe8916225c0c576f0d6 Author: Matt Corallo <git@bluematt.me> Date: Mon Oct 11 17:22:08 2021 +0000 Use upstream rust-bitcoin's dust calculation instead of our own Not only does this move to common code, but it fixes handling of all output types except for a few trivial cases. commit 1beccf188d157cc3147d348a348ad8d2e5d5fe59 Merge: 081ce7c8 0ec13f61 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Fri Nov 12 17:59:18 2021 +0000 Merge pull request #1143 from TheBlueMatt/2021-10-no-payment-id-leaks Fix a minor memory leak on PermanentFailure mon errs when sending commit 081ce7c843713a42803097a6d2dcf3631778beda Merge: 7c4dfad4 7dd8bd70 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Fri Nov 12 15:22:41 2021 +0000 Merge pull request #1165 from lightning-signer/2021-11-fix-anchors Fix countersignatory (to_remote) output redeemscript when anchors enabled commit 7dd8bd7068fdaab3c83e152b304086e1c363e999 Author: Ken Sedgwick <ken@bonsai.com> Date: Thu Nov 11 12:01:44 2021 -0800 Renamed script_for_p2wpkh to get_p2wpkh_redeemscript to match convention commit 1366d305312a591b0f889919a7ba41b903fa1973 Author: Ken Sedgwick <ken@bonsai.com> Date: Wed Nov 10 00:09:24 2021 -0800 Fix to_remote output redeemscript when anchors enabled commit 0ec13f611bbbee831e37416f052c97d924b23ec0 Author: Matt Corallo <git@bluematt.me> Date: Tue Oct 26 21:40:14 2021 +0000 Fix a minor memory leak on PermanentFailure mon errs when sending If we send a payment and fail to update the first-hop channel state with a `PermanentFailure` ChannelMonitorUpdateErr, we would have an entry in our pending payments map, but possibly not return the PaymentId back to the user to retry the payment, leading to a (rare and relatively minor) memory leak. commit 7c4dfad4fe97c9d8bc37c38e15b9dab95f27e37a Merge: 6f053e48 8e96f6b9 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Thu Nov 11 15:47:37 2021 +0000 Merge pull request #1105 from TheBlueMatt/2021-10-log-persist-time Log before+after ChannelMonitor/Manager updates for visibility commit 8e96f6b92efb2a04e957629f281bdb491f62dc9a Author: Matt Corallo <git@bluematt.me> Date: Mon Oct 4 03:11:36 2021 +0000 Log before+after ChannelMonitor/Manager updates for visibility I realized on my own node that I don't have any visibility into how long a monitor or manager persistence call takes, potentially blocking other operations. This makes it much more clear by adding a relevant log_trace!() print immediately before and immediately after persistence. commit 6f053e48b021d516b562a449c91f977397502e73 Merge: d2f401a7 b57ed798 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon Nov 8 23:56:47 2021 +0000 Merge pull request #1158 from jkczyz/2021-11-scorer-tests Scorer unit tests commit b57ed7982f735c91171dfcf1d0dde5c607dba702 Author: Jeffrey Czyz <jkczyz@gmail.com> Date: Thu Nov 4 16:55:01 2021 -0500 Add unit tests for Scorer Test basic and channel failure penalties, including after a (de-)serialization round trip. commit 2a7d9c8ddd08bcc205eec223a0f35730271de072 Author: Jeffrey Czyz <jkczyz@gmail.com> Date: Thu Nov 4 13:58:11 2021 -0500 Add SinceEpoch time to test Scorer hermetically In order to test Scorer hermetically, sleeps must be avoided. Add a SinceEpoch abstraction for manually advancing time. Implement the Time trait for SinceEpoch so that it can be used with ScorerUsingTime in tests. commit d2f401a79e03a0816301fe25a089ad61cc70e52e Merge: c0bbd4d9 7df087a3 Author: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Fri Nov 5 15:45:21 2021 +0000 Merge pull request #1154 from TheBlueMatt/2021-11-103-doc-tweaks 0.0.103 CHANGELOG tweaks from Jeff commit 7df087a36af69bd94edb5368f21ceb8d47c721b4 Author: Matt Corallo <git@bluematt.me> Date: Wed Nov 3 16:51:56 2021 +0000 Add note about PaymentId fields to 0.0.103 changelog commit 31cd00ebf5cc1e3eb0efb09a…

Add methods to count total fees and total amount in a Route #999 * Added `get_total_fees` method to route, to calculate all the fees paid accross each path. * Added `get_total_amount` method to route, to calculate the total of actual amounts paid in each path. Update docs to specify where process events is called Add Event::ChannelClosed generation at channel shutdown Rename MonitorEvent::CommitmentTxBroadcasted to CommitmentTxConfirmed Extend MsgHandleErrInternal with a new chan_id field Option<[u8; 32]> This field is used in next commit to generate appropriate ChannelClosed event at `handle_error()` processing. Add ChannelClosed generation at cooperative/force-close/error processing When we detect a channel `is_shutdown()` or call on it `force_shutdown()`, we notify the user with a Event::ChannelClosed informing about the id and closure reason. Add `pending_events` deadlock detection in `handle_error` Bump HTTP read timeout to match reality of Bitcoin Core blocking Fix future unknown `Event` variant backwards compatibility In 8ffc2d1742ff1171a87b0410b21cbbd557ff8247, in 0.0.100, we added a backwards compatibility feature to the reading of `Event`s - if the type was unknown and odd, we'd simply ignore the event and treat it as no event. However, we failed to read the length-prefixed TLV stream when doing so, resulting in us reading some of the skipped-event data as the next event or other data in the ChannelManager. We fix this by reading the varint length prefix written, then skipping that many bytes when we come across an unknown odd event type. Fix a panic in Route's new fee-calculation methods and clean up This addresses Val's feedback on the new Route fee- and amount-calculation methods, including fixing the panic she identified and cleaning up various docs and comments. Adds Transaction to lighting-block-sync::convert Includes disclaimer in docs, see https://github.com/rust-bitcoin/rust-lightning/pull/1061#issuecomment-911960862 Rename PaymentFailed -> PaymentPathFailed Since we don't want to imply to users that a payment has completely failed when it really has just partially failed Add path field to PaymentPathFailed event Fix windows-only test failure added in #997 This is a trivial bugfix to add a missing test updated required in PR 997. Move trait bounds on `wire::Type` from use to the trait itself `wire::Type` is only (publicly) used as the `CustomMessage` associated type in `CustomMessageReader`, where it has additional trait bounds on `Debug` and `Writeable`. The documentation for `Type` even mentions that you need to implement `Writeable` because this is the one place it is used. To make this more clear, we move the type bounds onto the trait itself and not on the associated type. This is also the only practical way to build C bindings for `Type` as we cannot have a concrete, single, `Type` struct in C which only optionally implements various subtraits, at least not without runtime checking of the type bounds. Make `ChainMonitor::get_claimable_balances` take a slice of refs For the same reason as `get_route`, a slice of objects isn't practical to map to bindings - the objects in the bindings space are structs with a pointer and some additional metadata. Thus, to create a slice of them, we'd need to take ownership of the objects behind the pointer, place them into a slace, and then restore them to the pointer. This would be a lot of memory copying and marshalling, not to mention wouldn't be thread-safe, which the same function otherwise would be if we used a slice of references instead of a slice of objects. Use Infallible for the unconstructable default custom message type When we landed custom messages, we used the empty tuple for the custom message type for `IgnoringMessageHandler`. This was fine, except that we also implemented `Writeable` to panic when writing a `()`. Later, we added support for anchor output construction in CommitmentTransaction, signified by setting a field to `Some(())`, which is serialized as-is. This causes us to panic when writing a `CommitmentTransaction` with `opt_anchors` set. Note that we never set it inside of LDK, but downstream users may. Instead, we implement `Writeable` to write nothing for `()` and use `core::convert::Infallible` for the default custom message type as it is, appropriately, unconstructable. This also makes it easier to implement various things in bindings, as we can always assume `Infallible`-conversion logic is unreachable. Drop redundant generic bounds when the trait requires the bounds Make method time on trait impl explitit to help bindings generator Associated types in C bindings is somewhat of a misnomer - we concretize each trait to a single struct. Thus, different trait implementations must still have the same type, which defeats the point of associated types. In this particular case, however, we can reasonably special-case the `Infallible` type, as an instance of it existing implies something has gone horribly wrong. In order to help our bindings code figure out how to do so when referencing a parent trait's associated type, we specify the explicit type in the implementation method signature. Update CHANGELOG for 0.0.101 Bump Crate versions to 0.0.101 (and invoice to 0.9) Make `NetworkGraph` Clone-able again There isn't a lot of user-utility for cloning `NetworkGraph` directly (its a rather large struct, and there probably isn't a lot of reason to have *multiple* `NetworkGraph`s). Thus, when locks were pushed down into it, the `Clone`-ability of it was dropped as well. Sadly, mapping the Java memory model onto: * `Read`-ing a `NetworkGraph`, creating a Java-owned `NetworkGraph` object that the JVM will destruct for us, * Passing it to a `NetGraphMsgHandler`, which now expects to own the `NetworkGraph`, including destructing it, isn't really practical without adding a clone in between. Given this, and the fact that there's nothing inherently wrong with clone-ing a `NetworkGraph`, we simply re-add `Clone` here. Drop broken test that is unfixable due to being undocumented This should be reverted at some point, but the test is deficient and breaks on later changes that are important to land ASAP. Increase our default/minimum dust limit to 354 sat/vbytes 330 sat/vbyte, the current value, is not sufficient to ensure a future segwit script longer than 32 bytes meets the dust limit if used for a shutdown script. Thus, we can either check the value on shutdown or we can simply require segwit outputs and require a dust value of no less than 354 sat/vbyte. We swap the minimum dust value to 354 sat/vbyte here, requiring segwit scripts in a future commit. See https://github.com/lightningnetwork/lightning-rfc/issues/905 Reduce the maximum allowed counterparty dust limit to 546 sat/vbyte 546 sat/vbyte is the current default dust limit on most implementations, matching the network dust limit for P2SH outputs. Implementations don't currently appear to send any larger dust limits, and allowing a larger dust limit implies higher payment failure risk, so we'd like to be as tight as we can here. Require user cooperative close payout scripts to be Segwit There is little reason for users to be paying out to non-Segwit scripts when closing channels at this point. Given we will soon, in rare cases, force-close during shutdown when a counterparty closes to a non-Segwit script, we should also require it of our own users. Force-close channels if closing transactions may be non-standard If a counterparty (or an old channel of ours) uses a non-segwit script for their cooperative close payout, they may include an output which is unbroadcastable due to not meeting the network dust limit. Here we check for this condition, force-closing the channel instead if we find an output in the closing transaction which does not meet the limit. Rename MIN_DUST_LIMIT_SATOSHIS constant to disambiguate chan vs P2P While channel and P2P network dust limits are related, they're ultimately two different things, and thus their constant names should reference that. Regenerate PendingHTLCsForwardable on reload instead of serializing When we are prepared to forward HTLCs, we generate a PendingHTLCsForwardable event with a time in the future when the user should tell us to forward. This provides some basic batching of forward events, improving privacy slightly. After we generate the event, we expect users to spawn a timer in the background and let us know when it finishes. However, if the user shuts down before the timer fires, the user will restart and have no idea that HTLCs are waiting to be forwarded/received. To fix this, instead of serializing PendingHTLCsForwardable events to disk while they're pending (before the user starts the timer), we simply regenerate them when a ChannelManager is deserialized with HTLCs pending. Fixes #1042 Don't apply monitor updates after watch_channel PermFail The full stack fuzzer found an unreachable panic where we receive a FundingSigned with a duplicate channel outpoint. Update Watch docs to disallow dup channel outpoints on watch_channel Rename MppId to PaymentId Leftover from previous PR Jeff feedback. Useful in upcoming commits as we'll expose this to users for payment retries Return PaymentId from send_*payment functions Used in upcoming commits for retries Refactor send_payment internals for retries We want to reuse send_payment internal functions for retries, so some need to now be parameterized by PaymentId to avoid generating a new PaymentId on retry Refactor send_payment internals for retries 2 Retrying a partial payment means send_payment_internal needs to be parameterized by a total payment amount, else 'HTLC values do not match' errors channelmanager: Add retry data to pending_outbound_payments Add method to retry payments Don't remove failed payments when all paths fail This is because we want the ability to retry completely failed payments. Upcoming commits will remove these payments on timeout to prevent DoS issues Also test that this removal allows retrying single-path payments Expire outbound payments after 3 blocks if no parts are pending Correct step number in `get_route` Consider many first-hop paths to the same counterparty in routing Previously we'd simply overwritten "the" first hop path to each counterparty when routing, however this results in us ignoring all channels except the last one in the `ChannelDetails` list per counterparty. f readability improvements from val Update Event::PaymentReceived docs since we require payment secret Users no longer need to verify the amounts of received payments as the payment secret will protect us against the probing attacks such verification was intended to fix. Move tests of payment retries into a new module Move pending payment tracking to after the new HTLC flies If we attempt to send a payment, but the HTLC cannot be send due to local channel limits, we'll provide the user an error but end up with an entry in our pending payment map. This will result in a memory leak as we'll never reclaim the pending payment map entry. Correct error returned when `retry_payment` doesn't have a payment Add payment_hash to PaymentSent #999 Replace PublicKey with [u8; 33] in NetworkGraph Adds DiscardFunding event During the event of a channel close, if the funding transaction is yet to be broadcasted then a DiscardFunding event is issued along with the ChannelClose event. Use local channel state when constructing routes in test macro This is a bit more realistic and needed to route over non-public channels. Fix loop label shadowing warning Remove special case for onion error expiry_too_far With channel scoring and payment retries, it is no longer necessary to have expiry_too_far imply a payment failure. Pass hop index in construct_onion_keys_callback This simplifies failing route hop calculation, which will be useful for later identifying the failing hop for PaymentFailed events. Clean up fee_insufficient computation Add failing short channel id to PaymentPathFailed This will be useful for scoring channels when a payment fails. Expose ReadOnlyNetworkGraph::get_addresses to C by cloning result We cannot expose ReadOnlyNetworkGraph::get_addresses as is in C as it returns a list of references to an enum, which the bindings dont support. Instead, we simply clone the result so that it doesn't contain references. Speed up test_timer_tick_called Fix unused variable warnings in fuzzer Replace get_route with get_route_and_payment_hash The interface for get_route will change to take a scorer. Using get_route_and_payment_hash whenever possible allows for keeping the scorer inside get_route_and_payment_hash rather than at every call site. Replace get_route with get_route_and_payment_hash wherever possible. Additionally, update get_route_and_payment_hash to use the known invoice features and the sending node's logger. Move mpp_failure test to payment_tests.rs Move `Persist` trait to chainmonitor as that's the only reference Move ChannelMonitorUpdateErr to chain as it is a chain::Watch val Make `ChainMonitor::monitors` private and expose monitor via getter Exposing a `RwLock<HashMap<>>` directly was always a bit strange, and in upcoming changes we'd like to change the internal datastructure in `ChainMonitor`. Further, the use of `RwLock` and `HashMap` meant we weren't able to expose the ChannelMonitors themselves to users in bindings, leaving a bindings/rust API gap. Thus, we take this opportunity go expose ChannelMonitors directly via a wrapper, hiding the internals of `ChainMonitor` behind getters. We also update tests to use the new API. Simplify channelmonitor tests which use chain::Watch and Persister test_simple_monitor_permanent_update_fail and test_simple_monitor_temporary_update_fail both have a mode where they use either chain::Watch or persister to return errors. As we won't be doing any returns directly from the chain::Watch wrapper in a coming commit, the chain::Watch-return form of the test will no longer make sense. Handle Persister returning TemporaryFailure for new channels Previously, if a Persister returned a TemporaryFailure error when we tried to persist a new channel, the ChainMonitor wouldn't track the new ChannelMonitor at all, generating a PermanentFailure later when the updating is restored. This fixes that by correctly storing the ChannelMonitor on TemporaryFailures, allowing later update restoration to happen normally. This is (indirectly) tested in the next commit where we use Persister to return all monitor-update errors. Use Persister to return errors in tests not chain::Watch As ChainMonitor will need to see those errors in a coming PR, we need to return errors via Persister so that our ChainMonitor chain::Watch implementation sees them. Use Persister to return errors in fuzzers not chain::Watch Fixed 'Advancing Bitcoin' video URL. Add channel scoring to get_route Failed payments may be retried, but calling get_route may return a Route with the same failing path. Add a routing::Score trait used to parameterize get_route, which it calls to determine how much a channel should be penalized in terms of msats willing to pay to avoid the channel. Also, add a Scorer struct that implements routing::Score with a constant constant penalty. Subsequent changes will allow for more robust scoring by feeding back payment path success and failure to the scorer via event handling. Return the temporary channel id in success from `create_channel` This makes it more practical for users to track channels prior to funding, especially if the channel fails because the peer rejects it for a parameter mismatch. Include the user channel id counter in Event::ChannelClosed This makes it more practical for users to track channels using their own IDs, especially across funding. Rename create_channel param to user_channel_id to standardize it Add CHANGELOG entries for 0.0.102 Bump crate versions to 0.0.102 and lightning-invoice 0.10 Simplify prefers_shorter_route_with_higher_fees In order to make the scoring tests easier to read, only check the relevant RouteHop fields. The remaining fields are tested elsewhere. Expand the test to show the path used without scoring. Add source and target nodes to routing::Score Expand routing::Score::channel_penalty_msat to include the source and target node ids of the channel. This allows scorers to avoid certain nodes altogether if desired. Move MonitorEvent serialization to TLV-enum-upgradable from custom Move the two-AtomicUsize counter in peer_handler to a util struct We also take this opportunity to drop byte_utils::le64_to_array, as our MSRV now supports the native to_le_bytes() call. Move ChannelManager::monitor_updated to a MonitorEvent In the next commit we'll need ChainMonitor to "see" when a monitor persistence completes, which means `monitor_updated` needs to move to `ChainMonitor`. The simplest way to then communicate that information to `ChannelManager` is via `MonitorEvet`s, which seems to line up ok, even if they're now constructed by multiple different places. Use an opaque type to describe monitor updates in Persist In the next commit, we'll be originating monitor updates both from the ChainMonitor and from the ChannelManager, making simple sequential update IDs impossible. Further, the existing async monitor update API was somewhat hard to work with - instead of being able to generate monitor_updated callbacks whenever a persistence process finishes, you had to ensure you only did so at least once all previous updates had also been persisted. Here we eat the complexity for the user by moving to an opaque type for monitor updates, tracking which updates are in-flight for the user and only generating monitor-persisted events once all pending updates have been committed. Persist `ChannelMonitor`s after new blocks are connected This resolves several user complaints (and issues in the sample node) where startup is substantially delayed as we're always waiting for the chain data to sync. Further, in an upcoming PR, we'll be reloading pending payments from ChannelMonitors on restart, at which point we'll need the change here which avoids handling events until after the user has confirmed the `ChannelMonitor` has been persisted to disk. It will avoid a race where we * send a payment/HTLC (persisting the monitor to disk with the HTLC pending), * force-close the channel, removing the channel entry from the ChannelManager entirely, * persist the ChannelManager, * connect a block which contains a fulfill of the HTLC, generating a claim event, * handle the claim event while the `ChannelMonitor` is being persisted, * persist the ChannelManager (before the CHannelMonitor is persisted fully), * restart, reloading the HTLC as a pending payment in the ChannelManager, which now has no references to it except from the ChannelMonitor which still has the pending HTLC, * replay the block connection, generating a duplicate PaymentSent event. Update test_dup_htlc_onchain_fails_on_reload for new persist API ChannelMonitors now require that they be re-persisted before MonitorEvents be provided to the ChannelManager, the exact thing that test_dup_htlc_onchain_fails_on_reload was testing for when it *didn't* happen. As such, test_dup_htlc_onchain_fails_on_reload is now testing that we bahve correctly when the API guarantees are not met, something we don't need to do. Here, we adapt it to test the new API requirements through ChainMonitor's calls to the Persist trait instead. Always release `MonitorEvent`s to `ChannelManager` after 3 blocks If we have a `ChannelMonitor` update from an on-chain event which returns a `TemporaryFailure`, we block `MonitorEvent`s from that `ChannelMonitor` until the update is persisted. This prevents duplicate payment send events to the user after payments get reloaded from monitors on restart. However, if the event being avoided isn't going to generate a PaymentSent, but instead result in us claiming an HTLC from an upstream channel (ie the HTLC was forwarded), then the result of a user delaying the event is that we delay getting our money, not a duplicate event. Because user persistence may take an arbitrary amount of time, we need to bound the amount of time we can possibly wait to return events, which we do here by bounding it to 3 blocks. Thanks to Val for catching this in review. Clarify the contexts in which persist_new_channel may be called Its somewhat confusing that `persist_new_channel` is called on startup for an existing channel in common deployments, so we call it out explicitly. Make `Channel::revoke_and_ack`'s return tuple a struct This substantially improves readability at the callsite and in the function. Make `Channel::monitor_updating_restored`'s return tuple a struct This improves readability at the callsite and in the function. Implement `HashMap` read for `MaybeReadable` values This allows us to read a `HashMap` that has values which may be skipped if they are some backwards-compatibility type. We also take this opportunity to fail deserialization if keys are duplicated. Inform ChannelManager when fulfilled HTLCs are finalized When an HTLC has been failed, we track it up until the point there exists no broadcastable commitment transaction which has the HTLC present, at which point Channel returns the HTLCSource back to the ChannelManager, which fails the HTLC backwards appropriately. When an HTLC is fulfilled, however, we fulfill on the backwards path immediately. This is great for claiming upstream HTLCs, but when we want to track pending payments, we need to ensure we can check with ChannelMonitor data to rebuild pending payments. In order to do so, we need an event similar to the HTLC failure event, but for fulfills instead. Specifically, if we force-close a channel, we remove its off-chain `Channel` object entirely, at which point, on reload, we may notice HTLC(s) which are not present in our pending payments map (as they may have received a payment preimage, but not fully committed to it). Thus, we'd conclude we still have a retryable payment, which is untrue. This commit does so, informing the ChannelManager via a new return element where appropriate of the HTLCSource corresponding to the failed HTLC. Track payments after they resolve until all HTLCs are finalized In the next commit, we will reload lost pending payments from ChannelMonitors during restart. However, in order to avoid re-adding pending payments which have already been fulfilled, we must ensure that we do not fully remove pending payments until all HTLCs for the payment have been fully removed from their ChannelMonitors. We do so here, introducing a new PendingOutboundPayment variant called `Completed` which only tracks the set of pending HTLCs. Rename payment object vars to refer to payments and not session IDs Add PaymentSecrets to HTLCSource::OutboundRoute objects Reload pending payments from ChannelMonitor HTLC data on reload If we go to send a payment, add the HTLC(s) to the channel(s), commit the ChannelMonitor updates to disk, and then crash, we'll come back up with no pending payments but HTLC(s) ready to be claim/failed. This makes it rather impractical to write a payment sender/retryer, as you cannot guarantee atomicity - you cannot guarantee you'll have retry data persisted even if the HTLC(s) are actually pending. Because ChannelMonitors are *the* atomically-persisted data in LDK, we lean on their current HTLC data to figure out what HTLC(s) are a part of an outbound payment, rebuilding the pending payments list on reload. Add some basic test coverage of monitor payment data reloading Move test_dup_htlc_onchain_fails_on_reload to payment_tests test_dup_htlc_onchain_fails_on_reload is now more of a payment_test than a functional_test, testing for handling of pending payments. Add a test of an HTLC being fulfilled and then later failed Peers probably shouldn't do this, but if they want to give us free money, we should take it and not generate any spurious events. Define Payee abstraction for use in get_route A payee can be identified by a pubkey and optionally have an associated set of invoice features and route hints. Use this in get_route instead of three separate parameters. This may be included in PaymentPathFailed later to use when finding a new route. Remove outdated line from get_route docs Include PaymentPathRetry data in PaymentPathFailed When a payment path fails, it may be retried. Typically, this means re-computing the route after updating the NetworkGraph and channel scores in order to avoid the failing hop. The last hop in PaymentPathFailed's path field contains the pubkey, amount, and CLTV values needed to pass to get_route. However, it does not contain the payee's features and route hints from the invoice. Include the entire set of parameters in PaymentPathRetry and add it to the PaymentPathFailed event. Add a get_retry_route wrapper around get_route that takes PaymentPathRetry. This allows an EventHandler to retry failed payment paths using the payee's route hints and features. Use option TLV decoding for short_channel_id Using ignorable TLV decoding is only applicable for an Option containing an enum, but short_channel_id is an Option<u64>. Use option TLV encoding instead. Make `Payee::pubkey` pub. `Payee` is expected to be used by users to get routes for payment retries, potentially with their own router. Thus, its helpful if it is pub, even if it is redundant with the last hop in the `path` field in `Events::PaymentPathFailed`. Copy `Payee` into `Route`s to provide them to `ChannelManager` Store `Payee` information in `HTLCSource::OutboundRoute`. This stores and tracks HTLC payee information with HTLCSource info, allowing us to provide it back to the user if the HTLC fails and ensuring persistence by keeping it with the HTLC itself as it passes between Channel and ChannelMonitor. Expose log_bytes! macro for use in other crates Needed to log PaymentHash in the lightning-invoice crate when retrying payments. Add PaymentId to PaymentSent event The payment_hash may not uniquely identify the payment if it has been reused. Include the payment_id in PaymentSent events so it can correlated with the send_payment call. Add PaymentId to PaymentPathFailed event The PaymentId is needed when retrying payments. Include it in the PaymentPathFailed event so it can be used in that manner. Rewrite Invoice's interface in terms of msats InvoiceBuilder's interface was changed recently to work in terms of msats. Update Invoice's interface to return the amount in msats, too, and make amount_pico_btc private. Unify route finding methods An upcoming Router interface will be used for finding a Route both when initially sending a payment and also when retrying failed payment paths. Unify the three varieties of get_route so the interface can consist of a single method implemented by the new `find_route` method. Give get_route pub(crate) visibility so it can still be used in tests. Add InvoicePayer for retrying failed payments When a payment fails, it's useful to retry the payment once the network graph and channel scores are updated. InvoicePayer is a utility for making payments which will retry any failed payment paths for a payment up to a configured number of total attempts. It is parameterized by a Payer and Router for ease of customization and testing. Implement EventHandler for InvoicePayer as a decorator that intercepts PaymentPathFailed events and retries that payment using the parameters from the event. It delegates to the decorated EventHandler after retries have been exhausted and for other events. Support paying zero-value invoices Fail payment retry if Invoice is expired According to BOLT 11: - after the `timestamp` plus `expiry` has passed - SHOULD NOT attempt a payment Add a convenience method for checking if an Invoice has expired, and use it to short-circuit payment retries. Implement Payer and Router for lightning crate Implements Payer for ChannelManager and Rotuer for find_route, which can be used to parameterize InvoicePayer when needing payment retries. Add a utility trait in `router` to get the fees along a given path Pass the failing/succeeding `Path` to PendingOutboundPayment meths This will make the next commit much simpler Track the amount spent on fees as payments are retried Especially once we merge the `InvoicePayer` logic soon, we'll want to expose the total fee paid in the `PaymentSent` event. Correct send-bounding logic in `TestRoutingMessageHandler` The `cmp::min` appeared to confused `end` for a count. Add `PeerManager::disconnect_all_peers` to avoid complexity in BP In the coming commits simply calling `timer_tick_occurred` will no longer disconnect all peers, so its helpful to have a utility method. Constify the ratio in buf limits between forward and init sync msgs Util-ify enqueueing an encoded message in peer_handler This marginally simplifies coming commits. Give peers which are sending us messages longer to respond to ping See comment for rationale. Give peers one timer tick to finish handshake before disconnecting This ensures we don't let a hung connection stick around forever if the peer never completes the initial handshake. This also resolves a race where, on receiving a second connection from a peer, we may reset their_node_id to None to prevent sending messages even though the `channel_encryptor` `is_ready_for_encryption()`. Sending pings only checks the `channel_encryptor` status, not `their_node_id` resulting in an `unwrap` on `None` in `enqueue_message`. Log peer public key more thoroughly when logging in peer_handler Notify scorer of failing payment path and channel Upon receiving a PaymentPathFailed event, the failing payment may be retried on a different path. To avoid using the channel responsible for the failure, a scorer should be notified of the failure before being used to find a new route. Add a payment_path_failed method to routing::Score and call it in InvoicePayer's event handler. Introduce a LockableScore parameterization to InvoicePayer so the scorer is locked only once before calling find_route. Penalize failed channels in Scorer As payments fail, the channel responsible for the failure may be penalized. Implement Scorer::payment_path_failed to penalize the failed channel using a configured penalty. As time passes, the penalty is reduced using exponential decay, though penalties will accumulate if the channel continues to fail. The decay interval is also configurable. Test InvoicePayer in BackgroundProcessor Proof of concept showing InvoicePayer can be used with an Arc<ChannelManager> passed to BackgroundProcessor. Likely do not need to merge this commit. Move PaymentId to a [u8; 32] in bindings as for other hash objects This should allow us to fix https://github.com/lightningdevkit/ldk-garbagecollected/issues/52 Provide payment retry data when an MPP payment failed partially This will allow `InvoicePayer` to properly retry payments that only partially failed to send. Expand `InvoicePayer` documentation somewhat to clarify edge-cases Dont unwrap `RouteParameter::expiry_time` as users can set it Users can provide anything they want as `RouteParameters` so we shouldn't assume any fields are set any particular way, including `expiry_time` set at all. Rewrite InvoicePayer retry to correctly handle MPP partial failures This rewrites a good chunk of the retry logic in `InvoicePayer` to address two issues: * it was not considering the return value of `send_payment` (and `retry_payment`) may indicate a failure on some paths but not others, * it was not considering that more failures may still come later when removing elements from the retry count map. This could result in us seeing an MPP-partial-failure, failing to retry, removing the retries count entry, and then retrying other parts, potentially forever. Add an integration test for InvoicePayer paying when one part fails This tests the multi-part-single-failure-immediately fixes in the previous commit. Add integration test for InvoicePayerretry on an immediate failure Check for invoice expiry in InvoicePayer before we send any HTLCs Parameterize NetGraphMsgHandler with NetworkGraph NetworkGraph is owned by NetGraphMsgHandler, but DefaultRouter requires a reference to it. Introduce shared ownership to NetGraphMsgHandler so that both can use the same NetworkGraph. Make NetGraphMsgHandler::network_graph private Since NetworkGraph has shared ownership, NetGraphMsgHandler does not need to expose its field. Clarify Scorer docs around penalizing channels Refactor channel failure penalty logic Move channel failure penalty logic into a ChannelFailure abstraction. This encapsulates the logic for accumulating penalties and decaying them over time. It also is responsible for the no-std behavior. This cleans up Scorer and will make it easier to serialize it. Parameterize Scorer by a Time trait Scorer uses time to determine how much to penalize a channel after a failure occurs. Parameterizing it by time cleans up the code such that no-std support is in a single AlwaysPresent struct, which implements the Time trait. Time is implemented for std::time::Instant when std is available. This parameterization also allows for deterministic testing since a clock could be devised to advance forward as needed. Implement (de)serialization for Scorer Scorer should be serialized to retain penalty data between restarts. Implement (de)serialization for Scorer by serializing last failure times as duration since the UNIX epoch. For no-std, the zero-Duration is used. Remove trailing ;s from macro calls to silence new rustc warnings Rename Payee::new to Payee::from_node_id to clarify it somewhat This also differentiates it from the bindings default-constructed `new` method which is constructed when all fields are exposed and of mappable types. Make payment_path_failed path type bindings-mappable The bindings don't currently support passing `Vec`s of objects which it mappes as "opaque types". This is because it will require clones to convert its own list of references to Rust's list of objects. In the near future we should resolve this limitation, allowing us to revert this (and make `find_route`'s method signature similarly cleaner), but for now we must avoid `Vec<OpaqueType>`. Remove now-unused import in routing/mod.rs Add `(C-not exported)` tag to a `Payee` modifier with move semantics This matches the other `Payee` move-modifier functions. Add `(C-not exported)` tags as required in tuple types This prepares us for C bindings auto-exporting tuple type fields. Tweak serialization of ScorerUsingTime for better forward compat Update CHANGELOG for 0.0.103 Bump crate versions to 0.0.103/invoice 0.11 Fix `cargo doc` on older rustc Apparently at least rustc 1.48 doesn't support `Self` in doc links, so we make it explicit. Add a ChannelTypeFeatures features object for the new channel_type Its semantics are somewhat different from existing features, however not enough to merit a different struct entirely. Specifically, it only supports required features (if you send a channel_type, the counterparty has to accept it wholesale or try again, it cannot select only a subset of the flags) and it is serialized differently (only appearing in TLVs). Support de/ser of the new channel_type field in open_channel Support send/recv'ing the new channel_type field in open_channel This implements the channel type negotiation, though as we currently only support channels with only static_remotekey set, it doesn't implement the negotiation explicitly. 0.0.103 CHANGELOG tweaks from Jeff Add note about PaymentId fields to 0.0.103 changelog Add SinceEpoch time to test Scorer hermetically In order to test Scorer hermetically, sleeps must be avoided. Add a SinceEpoch abstraction for manually advancing time. Implement the Time trait for SinceEpoch so that it can be used with ScorerUsingTime in tests. Add unit tests for Scorer Test basic and channel failure penalties, including after a (de-)serialization round trip. Log before+after ChannelMonitor/Manager updates for visibility I realized on my own node that I don't have any visibility into how long a monitor or manager persistence call takes, potentially blocking other operations. This makes it much more clear by adding a relevant log_trace!() print immediately before and immediately after persistence. Fix to_remote output redeemscript when anchors enabled Renamed script_for_p2wpkh to get_p2wpkh_redeemscript to match convention Fix a minor memory leak on PermanentFailure mon errs when sending If we send a payment and fail to update the first-hop channel state with a `PermanentFailure` ChannelMonitorUpdateErr, we would have an entry in our pending payments map, but possibly not return the PaymentId back to the user to retry the payment, leading to a (rare and relatively minor) memory leak. Use upstream rust-bitcoin's dust calculation instead of our own Not only does this move to common code, but it fixes handling of all output types except for a few trivial cases. Be less aggressive in outbound HTLC CLTV timeout checks We currently assume our counterparty is naive and misconfigured and may force-close a channel to get an HTLC we just forwarded them. There shouldn't be any reason to do this - we don't have any such bug, and we shouldn't start by assuming our counterparties are buggy. Worse, this results in refusing to forward payments today, failing HTLCs for largely no reason. Instead, we keep a fairly conservative check, but not one which will fail HTLC forwarding spuriously - testing only that the HTLC doesn't expire for a few blocks from now. Fixes #1114. Correct Channel type serialization logic Currently, we write out the Channel's `ChannelTypeFeatures` as an odd type, implying clients which don't understand the `ChannelTypeFeatures` field can simply ignore it. This is obviously nonsense if the channel type is some future version - the client needs to fail to deserialize as it doesn't understand the channel's type. We adapt the serialization logic here to only write out the `ChannelTypeFeatures` field if it is something other than only-static-remote-key, and simply consider that "default" (as it is the only supported type today). Then, we write out the channel type as an even TLV, implying clients which do not understand it must fail to read the `Channel`. Note that we do not need to bother reserving the TLV type no longer written as it never appeared in a release (merged post-0.0.103). Refactor InvoicePayer for spontaneous payments To support spontaneous payments, InvoicePayer's sending logic must be invoice-agnostic. Refactor InvoicePayer::pay_invoice_internal such that invoice-specific code is in pay_invoice_using_amount and the remaining logic is in pay_internal. Further refactor the code's payment_cache locking such that it is accessed consistently when needed, and tidy up the code a bit. Support spontaneous payments in InvoicePayer InvoicePayer handles retries not only when handling PaymentPathFailed events but also for some types of PaymentSendFailure on the initial send. Expand InvoicePayer's interface with a pay_pubkey function for spontaneous (keysend) payments. Add a send_spontaneous_payment function to the Payer trait to support this and implement it for ChannelManager. Replace expect_value_msat with expect_send Modify all InvoicePayer unit tests to use expect_send instead of expect_value_msat, since the former can discern whether the send was for an invoice, spontaneous payment, or a retry. Updates tests to set payer expectations if they weren't already and assert these before returning a failure. Test retrying payment on partial send failure Add some test coverage for when InvoicePayer retries within pay_invoice because the Payer returned a partial failure. Add PaymentHash parameter to Router::find_route Implementations of Router may need the payment hash in order to look up pre-computed routes from a probe for a given payment. Add a PaymentHash parameter to Router::find_route to allow for this. Provide `Score` the HTLC amount and channel capacity This should allow `Score` implementations to make substantially better decisions, including of the form "willing to pay X to avoid routing over this channel which may have a high failure rate". Penalize large HTLCs relative to channels in default `Scorer` Sending HTLCs which are any greater than a very small fraction of the channel size tend to fail at a much higher rate. Thus, by default we start applying a penalty at only 1/8th the channel size and increase it linearly as the amount reaches the channel's capacity, 20 msat per 1024th of the channel capacity. Move `Score` into a `scoring` module instead of a top-level module Traits in top-level modules is somewhat confusing - generally top-level modules are just organizational modules and don't contain things themselves, instead placing traits and structs in sub-modules. Further, its incredibly awkward to have a `scorer` sub-module, but only have a single struct in it, with the relevant trait it is the only implementation of somewhere else. Not having `Score` in the `scorer` sub-module is further confusing because it's the only module anywhere that references scoring at all. Return `ClosureReason` from `Channel` chain update methods This fixes a few `ClosureReason`s and allows us to have finer-grained user-visible errors when a channel closes due to an on-chain event. Automatically close channels that go unfunded for 2016 blocks As recommended by BOLT 2 added in https://github.com/lightningnetwork/lightning-rfc/pull/839 Add 'accept_inbound_channels' config option. Limit minimum output size to the dust limit when RBF-bumping Check all outputs meet the dust threshold in check_spends!() Correct txid logging to reverse bytes. We also take this opportunity to log the channel being closed when one is closed by an on-chain spend of the funding output. Ensure current channel state is logged for all channels on startup Remove user_payment_id In upcoming commits, we'll be making the payment secret and payment hash/preimage derivable from info about the payment + a node secret. This means we don't need to store any info about incoming payments and can eventually get rid of the channelmanager::pending_inbound_payments map. Add a new log-level for gossip messages. Fix MPP routefinding when we first collect 95% of payment value See comment in new test for more details. Introduce new helper commit_tx_fee_sat Cancel the outbound feerate update if above what we can afford Check we won't overflow `max_dust_htlc_exposure_msat` at outbound feerate update Re-add `test_max_dust_htlc_exposure` Introduce CommitmentStats Check outbound update_fee affordance incremented with holding cell HTLCs Add anchor support to commitment HTLC outputs Increase visibility of anchor related methods Add anchor support to build_htlc_transaction Adjust HTLC_{SUCCESS,TIMEOUT}_TX_WEIGHT when anchors used Add test vectors for get_htlc_redeemscript wrt anchors Generate PaymentPathSuccessful event for each path A single PaymentSent event is generated when a payment is fulfilled. This is occurs when the preimage is revealed on the first claimed HTLC. For subsequent HTLCs, the event is not generated. In order to score channels involved with a successful payments, the scorer must be notified of each successful path involved in the payment. Add a PaymentPathSuccessful event for this purpose. Generate it whenever a part is removed from a pending outbound payment. This avoids duplicate events when reconnecting to a peer. Make Channel::commit_tx_fee_msat static and take fee explicitly This may avoid risk of bugs in the future as it requires the caller to think about the fee being used, not just blindly use the current (committed) channel feerate. Rewrite test_update_fee_that_funder_cannot_afford to avoid magic Instead of magic hard-coded constants, its better for tests to derive the values used so that they change if constants are changed and so that it is easier to re-derive constants in the future as needed. Correct initial commitment tx fee affordability checks on open Previously, we would reject inbound channels if the funder wasn't able to meet our channel reserve on their first commitment transaction only if they also failed to push enough to us for us to not meet their initial channel reserve as well. There's not a lot of reason to care about us meeting their reserve, however - its largely expected that they may not push enough to us in the initial open to meet it, and its not actually our problem if they don't. Further, we used our own fee, instead of the channel's actual fee, to calculate fee affordability of the initial commitment transaction. We resolve both issues here, rewriting the combined affordability check conditionals in inbound channel open handling and adding a fee affordability check for outbound channels as well. The prior code may have allowed a counterparty to start the channel with "no punishment" states - violating the reason for the reserve threshold. Test fixed channel reserve checks on channel open Fix compilation with the max_level_trace feature Test all log-limiting features in CI Support `logger::Record` in C by String-ing the fmt::Arguments This adds a new (non-feature) cfg argument `c_bindings` which will be set when building C bindings. With this, we can (slightly) tweak behavior and API based on whether we are being built for Rust or C users. Ideally we'd never need this, but as long as we can keep the API consistent-enough to avoid material code drift, this gives us a cheap way of doing the "right" thing for both C and Rust when the two are in tension. We also move lightning-background-processor to support the same MSRV as the main lightning crate, instead of only lightning-net-tokio's MSRV. Make `Score : Writeable` in c_bindings and impl on `LockedScore` Ultimately we likely need to wrap the locked `Score` in a struct that exposes writeable somehow, but because all traits have to be fully concretized for C bindings we'll still need `Writeable` on all `Score` in order to expose `Writeable` on the locked score. Otherwise, we'll only have a `LockedScore` with a `Score` visible that only has the `Score` methods, never the original type. Seal `scoring::Time` and only use `Instant` or `Eternity` publicly `scoring::Time` exists in part to make testing the passage of time in `Scorer` practical. To allow no-std users to provide a time source it was exposed as a trait as well. However, it seems somewhat unlikely that a no-std user is going to have a use for providing their own time source (otherwise they wouldn't be a no-std user), and likely they won't have a graph in memory either. `scoring::Time` as currently written is also exceptionally hard to write C bindings for - the C bindings trait mappings relies on the ability to construct trait implementations at runtime with function pointers (i.e. `dyn Trait`s). `scoring::Time`, on the other hand, is a supertrait of `core::ops::Sub` which requires a `sub` method which takes a type parameter and returns a type parameter. Both of which aren't practical in bindings, especially given the `Sub::Output` associated type is not bound by any trait bounds at all (implying we cannot simply map the `sub` function to return an opaque trait object). Thus, for simplicity, we here simply seal `scoring::Time` and make it effectively-private, ensuring the bindings don't need to bother with it. Implement Clone, Hash, PartialEq for ClosingTransaction This is a public struct intended to be used as an object by users, so it should likely have common implementations, given they're trivial. Implement Clone for InvalidShutdownScript Users hopefully shouldn't have much of a reason to use this, but the bindings may need it to ensure no leaking pointers over an ffi. Store holder channel reserve and max-htlc-in-flight explicitly Previously, `holder_selected_channel_reserve_satoshis` and `holder_max_htlc_value_in_flight_msat` were constant functions of the channel value satoshis. However, in the future we may allow allow users to specify it. In order to do so, we'll need to track them explicitly, including serializing them as appropriate. We go ahead and do so here, in part as it will make testing different counterparty-selected channel reserve values easier. Explicitly support counterparty setting 0 channel reserve A peer providing a channel_reserve_satoshis of 0 (or less than our dust limit) is insecure, but only for them. Because some LSPs do it with some level of trust of the clients (for a substantial UX improvement), we explicitly allow it. Because its unlikely to happen often in normal testing, we test it explicitly here. Fix regression when reading `Event::PaymentReceived` in some cases For some reason rustc was deciding on a type for the `Option` being deserialized for us as `_user_payment_id`. This really, really, absolutely should have been a compile failure - the type (with methods called on it!) was ambiguous! Instead, rustc seems to have been defaulting to `Option<()>`, causing us to read zero of the eight bytes in the `user_payment_id` field, which returns an `Err(InvalidValue)` error as TLVs must always be read fully. This should likely be reported to rustc as its definitely a bug, but I cannot seem to cause the same error on any kinda of vaguely-minimized version of the same code. Found by `chanmon_consistency` fuzz target. Fix compilation in `payment` rustdoc examples The samples were not valid rust, but previous versions of rustc had a bug where they were accepted anyway. Latest rustc beta no longer accepts these. Score successful payment paths Expand the Score trait with a payment_path_successful function for scoring successful payment paths. Called by InvoicePayer's EventHandler implementation when processing PaymentPathSuccessful events. May be used by Score implementations to revert any channel penalties that were applied by calls to payment_path_failed. Decay channel failure penalty upon success If a payment failed to route through a channel, a penalty is applied to the channel in the future when finding a route. This penalty decays over time. Immediately decay the penalty by one half life when a payment is successfully routed through the channel. Fix shift overflow in Scorer::channel_penalty_msat An unchecked shift of more than 64 bits on u64 values causes a shift overflow panic. This may happen if a channel is penalized only once and (1) is not successfully routed through and (2) after 64 or more half life decays. Use a checked shift to prevent this from happening. Allow missing-docs on test-only macros Prefer fully-specified paths in test macros This avoids macros being context-specific use-dependent. Continue after a single failure in `ChannelMonitor::update_monitor` `ChannelMonitorUpdate`s may contain multiple updates, including, eg a payment preimage after a commitment transaction update. While such updates are generally not generated today, we shouldn't return early out of the update loop, causing us to miss any updates after an earlier update fails. Always return failure in `update_monitor` after funding spend Previously, monitor updates were allowed freely even after a funding-spend transaction confirmed. This would allow a race condition where we could receive a payment (including the counterparty revoking their broadcasted state!) and accept it without recourse as long as the ChannelMonitor receives the block first, the full commitment update dance occurs after the block is connected, and before the ChannelManager receives the block. Obviously this is an incredibly contrived race given the counterparty would be risking their full channel balance for it, but its worth fixing nonetheless as it makes the potential ChannelMonitor states simpler to reason about. The test in this commit also tests the behavior changed in the previous commit. Drop `MonitorUpdateErr` in favor of opaque errors. We don't expect users to ever change behavior based on the string contained in a `MonitorUpdateErr`, except log it, so there's little reason to not just log it ourselves and return a `()` for errors. We do so here, simplifying the callsite in `ChainMonitor` as well. Make APIError debug output more clear by including the variant Add a simple test for ChainMonitor MonitorUpdate-holding behavior Add a test for MonitorEvent holding when they complete out-of-order Add trivial test of monitor update failure during block connection Remove OnionV2 parsing support OnionV2s don't (really) work on Tor anymore anyway, and the field is set for removal in the BOLTs [1]. Sadly because of the way addresses are parsed we have to continue to understand that type 3 addresses are 12 bytes long. Thus, for simplicity we keep the `OnionV2` enum variant around and just make it an opaque 12 bytes, with the documentation updated to note the deprecation. [1] https://github.com/lightning/bolts/pull/940 Ensure ChannelManager methods are idempotent During event handling, ChannelManager methods may need to be called as indicated in the Event documentation. Ensure that these calls are idempotent for the same event rather than panicking. This allows users to persist events for later handling without needing to worry about processing the same event twice (e.g., if ChannelManager is not persisted but the events were, the restarted ChannelManager would return some of the same events). Define public getters for all feature flags There's not a ton of reason not to do this, and moving it to the macro removes some code. Support the `channel_type` feature bit. Note that this feature bit does absolutely nothing. We signal it (as we already support channel type negotiation), but do not bother to look to see if peers support it, as we don't care - we simply look for the TLV entry and deduce if a peer supports channel type negotiation from that. The only behavioral change at all here is that we don't barf if a peer sets channel type negotiation to required via the feature bit (instead of failing the channel at open-time), but of course no implementations do this, and likely won't for some time (if ever - you can simply fail channels with unknown types later, and there's no reason to refuse connections, really). As defined in https://github.com/lightning/bolts/pull/906 Reduce force-closures with user fee estimators which round poorly See comment for more Upgrade to codecov uploader v2 Some time ago codecov stopped supporting their old v1 uploader, and it seems they've now finally turned it off, so we aren't getting any coverage reports anymore. Hopefully upgrading is pretty trivial. Getter for the total channel balance The existing balance getters subtract reserve, this one does not. Separate ChannelAnnouncement and ChannelUpdate broadcast conditions When a `ChannelUpdate` message is generated for broadcast as a part of a `BroadcastChannelAnnouncement` event, it may be newer than our previous `ChannelUpdate` and need to be broadcast. However, if the `ChannelAnnouncement` had already been seen we wouldn't re-broadcast either message as the `handle_channel_announcement` call would fail, short-circuiting the condition to broadcast both. Instead, we split the broadcast of each message as well as the conditional so that we always attempt to handle each message and update our local graph state, then broadcast the message if its update was processed successfully. Update `ChannelUpdate::timestamp` when channels are dis-/en-abled We update the `Channel::update_time_counter` field (which is copied into `ChannelUpdate::timestamp`) only when the channel is initialized or closes, and when a new block is connected. However, if a peer disconnects or reconnects, we may wish to generate `ChannelUpdate` updates in between new blocks. In such a case, we need to make sure the `timestamp` field is newer than any previous updates' `timestamp` fields, which we do here by simply incrementing it when the channel status is changed. As a side effect of this we have to update `test_background_processor` to ensure it eventually succeeds even if the serialization of the `ChannelManager` changes after the test begins. Re-broadcast our own gossip even if its same as the last broadcast Even if our gossip hasn't changed, we should be willing to re-broadcast it to our peers. All our peers may have been disconnected the last time we broadcasted it. Add a comment describing `update_time_counter` and when its updated Add a comment describing the timestamp field's use DRY up payment failure macros in functional_test_utils ... with a more extensible expectation-checking framework for them. Add a variant to `PendingOutboundPayment` for retries-exceeded When a payer gives up trying to retry a payment, they don't know for sure what the current state of the event queue is. Specifically, they cannot be sure that there are not multiple additional `PaymentPathFailed` or even `PaymentSuccess` events pending which they will see later. Thus, they have a very hard time identifying whether a payment has truly failed (and informing the UI of that fact) or if it is still pending. See [1] for more information. In order to avoid this mess, we will resolve it here by having the payer give `ChannelManager` a bit more information - when they have given up on a payment - and using that to generate a `PaymentFailed` event when all paths have failed. This commit adds the neccessary storage and changes for the new state inside `ChannelManager` and a public method to mark a payment as failed, the next few commits will add the new `Event` and use the new features in our `PaymentRetrier`. [1] https://github.com/lightningdevkit/rust-lightning/issues/1164 Expose an event when a payment has failed and retries complete When a payment fails, a payer needs to know when they can consider a payment as fully-failed, and when only some of the HTLCs in the payment have failed. This isn't possible with the current event scheme, as discovered recently and as described in the previous commit. This adds a new event which describes when a payment is fully and irrevocably failed, generating it only after the payment has expired or been marked as expired with `ChannelManager::mark_retries_exceeded` *and* all HTLCs for it have failed. With this, a payer can more simply deduce when a payment has failed and use that to remove payment state or finalize a payment failure. Use `Event::PaymentFailed` in `InvoicePayer` to remove retry count This finally fixes the bug described in the previous commits where we retry a payment after its retry count has expired due to early removal of the payment from the retry count tracking map. A test is also added which demonstrates the bug in previous versions and which passes now. Fixes #1164. Make attempting to retry a succeeded payment an APIError, not Route This is symmetric with the new failure once a payment is abandoned. Updates Fee estimator docs to include a max return value for get_est_sat_per_1000_weight Updates docs to include a max return value for get_est_sat_per_1000_weight Adds max to both conversions Additional detail Add mpp_timeout and invalid_onion_payload descriptions & handling Pin tokio to 1.14.0 in CI for older rustc's Build no-std on 1.47 now that core2 supports it DRY up network_graph tests substantially with message creation fns Drop `allow_wallclock_use` feature in favor of simply using `std` Fixes #1147. Add a method to prune stale channels from `NetworkGraph` We define "stale" as "haven't heard an updated channel_update in two weeks", as described in BOLT 7. We also filter out stale channels at write-time for `std` users, as we can look up the current time. Reject channel_update messages with timestamps too old or new Because we time out channel info that is older than two weeks now, we should also reject new channel info that is older than two weeks, in addition to rejecting future channel info. Automatically prune NetworkGraph of stale channels hourly in BP Add get_inbound_payment_key_material to KeysInterface This will allow us to retrieve key material for encrypting/decrypting inbound payment info, in upcoming commits Macro-ize checking that the total value of an MPP's parts is sane This DRY-ed code will be used in upcoming commits when we stop storing inbound payment data Add get_single_block to chacha20 module In the next commit, we'll want to get a single block from a chacha stream that takes a 16-byte nonce. Drop need to store pending inbound payments and replace payment_secret with encrypted metadata See docs on `inbound_payment::verify` for details Also add min_value checks to all create_inbound_payment* methods Add new invoice CreationError::InvalidAmount for use in checking `create_inbound_payment` in an invoice creation utility. Note that if the error type of `create_inbound_payment` ever changed, we'd be forced to update the invoice utility's callsite to handle the new error Salt inbound payment ExpandedKey Leftover feedback from #1177 create_inbound_payment: warn about dup hashes Leftover feedback from #1177 inbound_payment: DRY verify method for use in getting preimage in the next commit(s) inbound_payment: Add utility to get payment preimage given hash/secret User-requested feature Remove trailing whitespace in the FeeEstimator interface Update CHANGELOG for 0.0.104 Bump versions to 0.0.104/invoice 0.12 Swap around generic argument ordering in DefaultRouter for bindings The bindings generation really should support default generic types in where clauses, but currently does not. To avoid needing to add support during the current release process, we simply swap around the arguments to move them to the first <> instead of the where. Add a constructor to MultiThreadedLockableScore ...as otherwise the struct is rather useless. Add a C-not exported tag to `NetGraphMsgHandler.network_graph` Swap around generic argument ordering in InvoicePayer for bindings The bindings generation really should support generic bounds other than Deref::Target in where clauses, but currently does not. To avoid needing to add support during the current release process, we simply swap around the arguments to move them to the first <> instead of the where. update repo name to use lightningdevkit Log gossip rejections due to stale channel_updates at GOSSIP level This further reduces noise at the TRACE level during initial gossip sync. Support building `cfg=c_bindings` with `no-std` This will be needed for JavaScript bindings eventually. Adapt lightning-invoice to no_std Do not turn on bech32/std by default for lightning-invoice Add a new `WarningMessage` message to send and receive warnings Handle sending and receiving warning messages Send warning messages when appropriate in gossip handling pipeline Convert `shutdown` invalid script checks to warning messages As required by the warning messages PR, we should simply warn our counterparty in this case and let them try again, continuing to try to use the channel until they tell us otherwise. Send `warning` instead of `error` when we incounter bogus gossip Rely on Error/Warning message data lengths being correct In https://github.com/lightning/bolts/pull/950, the (somewhat strange) requirement that error messages be handled even if the length field is set larger than the size of the package was removed. Here we change the code to drop the special handling for this, opting to just fail to read the message if the length is incorrect. Set the SigHashType of remote htlc signatures w/ anchors to SinglePlusAnyoneCanPay Isolated channelmonitor weight unit tests and added anchor loops make WEIGHT{_REVOKED,}_{OFFERED,RECEIVED}_HTLC functions with opt_anchors parameter Add unit test coverage for package::weight_{offered,received}_htlc Convert COMMITMENT_TX_BASE_WEIGHT to anchor-aware function Convert HTLC_{SUCCESS,TIMEOUT}_TX_WEIGHT to anchor-aware functions Debit funder's output to cover anchors Add anchor tests to outbound_commitment_test Set opt_anchors for calls to CommitmentTransaction::new_with_auxiliary_htlc_data Fixed comment on weight_received_htlc Make lockorder consistent in channelmanager This resolves a lockorder inversion in `ChannelManager::finalize_claims` where `pending_outbound_payments` is locked after `pending_events`, opposite of, for example, the lockorder in `ChannelManager::fail_htlc_backwards_internal` where `pending_outbound_payments` is locked at the top of the `HTLCSource::OutboundRoute` handling and then `pending_events` is locked at the end. Fix some (non-bug) lock-order-inversions in tests Check lockorders in tests with a trivial lockorder tracker Fix build errors Create script using p2wsh for comparison Using p2wpkh for generating the payment script spendable_outputs sanity check Fix unused return warning for 'transmute_copy'. Create dependabot.yml Signed-off-by:…

valentinewallace force-pushed the 2021-11-derivable-payment-secret branch from 6ffa650 to 51cc639 Compare November 18, 2021 17:13

TheBlueMatt reviewed Nov 18, 2021

View reviewed changes

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

jkczyz self-requested a review November 18, 2021 21:13

valentinewallace force-pushed the 2021-11-derivable-payment-secret branch 4 times, most recently from d30fcf5 to 20f79b3 Compare November 19, 2021 23:37

TheBlueMatt reviewed Nov 21, 2021

View reviewed changes

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

valentinewallace force-pushed the 2021-11-derivable-payment-secret branch 2 times, most recently from 64ecc17 to 82bc52c Compare November 22, 2021 23:05

TheBlueMatt reviewed Nov 23, 2021

View reviewed changes

valentinewallace force-pushed the 2021-11-derivable-payment-secret branch 2 times, most recently from e52ef48 to 07801f9 Compare November 24, 2021 16:26

valentinewallace marked this pull request as ready for review November 24, 2021 16:29

valentinewallace force-pushed the 2021-11-derivable-payment-secret branch from 07801f9 to a47eb4d Compare November 24, 2021 16:40

TheBlueMatt reviewed Nov 26, 2021

View reviewed changes

TheBlueMatt added the Seeking Code Review label Nov 29, 2021

valentinewallace force-pushed the 2021-11-derivable-payment-secret branch from a47eb4d to 38740ee Compare November 30, 2021 02:47

TheBlueMatt added the Optech Make Me Famous label Nov 30, 2021

valentinewallace force-pushed the 2021-11-derivable-payment-secret branch 2 times, most recently from e51c623 to 369fbda Compare December 1, 2021 21:52

valentinewallace mentioned this pull request Dec 1, 2021

Add support for phantom node payments #1199

Merged

TheBlueMatt reviewed Dec 6, 2021

View reviewed changes

lightning/src/chain/keysinterface.rs Outdated Show resolved Hide resolved

lightning/src/chain/keysinterface.rs Outdated Show resolved Hide resolved

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

jkczyz reviewed Dec 6, 2021

View reviewed changes

valentinewallace force-pushed the 2021-11-derivable-payment-secret branch 2 times, most recently from 73a8df6 to 61107f8 Compare December 7, 2021 01:10

valentinewallace force-pushed the 2021-11-derivable-payment-secret branch from 5055595 to 0fbb69f Compare December 15, 2021 20:24

jkczyz reviewed Dec 15, 2021

View reviewed changes

valentinewallace force-pushed the 2021-11-derivable-payment-secret branch 3 times, most recently from 019d701 to 2eb6f7f Compare December 16, 2021 17:45

jkczyz previously approved these changes Dec 16, 2021

View reviewed changes

valentinewallace dismissed jkczyz’s stale review via fa5aa8e December 16, 2021 21:11

valentinewallace force-pushed the 2021-11-derivable-payment-secret branch from 2eb6f7f to fa5aa8e Compare December 16, 2021 21:11

jkczyz previously approved these changes Dec 16, 2021

View reviewed changes

valentinewallace added 5 commits December 16, 2021 15:30

Add get_inbound_payment_key_material to KeysInterface

6dd1ec1

This will allow us to retrieve key material for encrypting/decrypting inbound payment info, in upcoming commits

Macro-ize checking that the total value of an MPP's parts is sane

063b758

This DRY-ed code will be used in upcoming commits when we stop storing inbound payment data

Add get_single_block to chacha20 module

1d516a6

In the next commit, we'll want to get a single block from a chacha stream that takes a 16-byte nonce.

Drop need to store pending inbound payments

8464875

and replace payment_secret with encrypted metadata See docs on `inbound_payment::verify` for details Also add min_value checks to all create_inbound_payment* methods

Add new invoice CreationError::InvalidAmount for use in checking `cre…

d41499a

…ate_inbound_payment` in an invoice creation utility. Note that if the error type of `create_inbound_payment` ever changed, we'd be forced to update the invoice utility's callsite to handle the new error

valentinewallace dismissed jkczyz’s stale review via d41499a December 16, 2021 23:32

valentinewallace force-pushed the 2021-11-derivable-payment-secret branch from fa5aa8e to d41499a Compare December 16, 2021 23:32

TheBlueMatt approved these changes Dec 16, 2021

View reviewed changes

TheBlueMatt reviewed Dec 16, 2021

View reviewed changes

jkczyz approved these changes Dec 17, 2021

View reviewed changes

TheBlueMatt merged commit 86d5944 into lightningdevkit:main Dec 17, 2021

valentinewallace added a commit to valentinewallace/rust-lightning that referenced this pull request Dec 17, 2021

Salt inbound payment ExpandedKey

6e0820c

Leftover feedback from lightningdevkit#1177

valentinewallace added a commit to valentinewallace/rust-lightning that referenced this pull request Dec 17, 2021

create_inbound_payment: warn about dup hashes

d20239b

Leftover feedback from lightningdevkit#1177

This was referenced Dec 17, 2021

#1177 Followup #1215

Merged

Add utility for retrieving an LDK payment preimage #1216

Merged

TheBlueMatt mentioned this pull request Apr 29, 2022

invoices: implement statelss invoice functinoality lightningnetwork/lnd#6413

Open

Stop storing pending inbound payment data; instead make it derivable on receive #1177

Stop storing pending inbound payment data; instead make it derivable on receive #1177

Uh oh!

Conversation

valentinewallace commented Nov 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

valentinewallace commented Nov 30, 2021

Uh oh!

TheBlueMatt commented Nov 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt commented Nov 30, 2021

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

valentinewallace commented Nov 18, 2021 •

edited

Loading

codecov bot commented Nov 18, 2021 •

edited

Loading

TheBlueMatt commented Nov 30, 2021 •

edited

Loading