Skip to content

Make DirectedChannelInfo source and target public - #2870

Merged
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
benthecarman:pub-source-target
Feb 2, 2024
Merged

Make DirectedChannelInfo source and target public#2870
TheBlueMatt merged 1 commit into
lightningdevkit:mainfrom
benthecarman:pub-source-target

Conversation

@benthecarman

Copy link
Copy Markdown
Contributor

We use these params for our HubPreferentialScorer and without these utility functions then we have to manually do this and calculate which is the source and which is the target node.

We use these params for our HubPreferentialScorer and without these
utility functions then we have to manually do this and calculate which
is the source and which is the target node.
@coderabbitai

coderabbitaiBot commented Feb 2, 2024

Copy link
Copy Markdown

Walkthrough

The lightning crate, specifically within its routing module, has been updated to enhance accessibility of certain methods. The DirectedChannelInfo class now features its source() and target() methods as publicly accessible, a shift from their previous pub(super) designation. This change broadens the scope of access, allowing for usage beyond the internal confines of the crate.

Changes

File PathChange Summary
lightning/src/routing/.../gossip.rssource() and target() methods in DirectedChannelInfo made public.

🐇✨
In the code where secrets lie,
A change was made, no need to pry.
From super bounds, we now break free,
source and target, for all to see.
🌟🐾

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository from git and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 3751398 and a931dec.
Files selected for processing (1)
  • lightning/src/routing/gossip.rs (1 hunks)

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5bf58f0) 89.14% compared to head (a931dec) 89.13%.
Report is 7 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@ Coverage Diff @@## main #2870 +/- ##
==========================================
- Coverage 89.14% 89.13% -0.01% 
==========================================
Files 116 116 Lines 93205 93252 +47 Branches 93205 93252 +47 ==========================================
+ Hits 83089 83123 +34 - Misses 7583 7591 +8 - Partials 2533 2538 +5 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@TheBlueMattTheBlueMatt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sure. No real reason not to, though we should consider making it return by ownership, not reference.

@TheBlueMatt
TheBlueMatt merged commit 4d993c0 into lightningdevkit:mainFeb 2, 2024
@benthecarman
benthecarman deleted the pub-source-target branch February 2, 2024 17:12
PXplod pushed a commit to bitlightlabs/rust-lightning that referenced this pull request Sep 30, 2024
v0.0.123 - May 08, 2024 - "BOLT12 Dust Sweeping"
API Updates
===========
* To reduce risk of force-closures and improve HTLC reliability the default
dust exposure limit has been increased to
`MaxDustHTLCExposure::FeeRateMultiplier(10_000)`. Users with existing
channels might want to consider using
`ChannelManager::update_channel_config` to apply the new default (lightningdevkit#3045).
* `ChainMonitor::archive_fully_resolved_channel_monitors` is now provided to
remove from memory `ChannelMonitor`s that have been fully resolved on-chain
and are now not needed. It uses the new `Persist::archive_persisted_channel`
to inform the storage layer that such a monitor should be archived (lightningdevkit#2964).
* An `OutputSweeper` is now provided which will automatically sweep
`SpendableOutputDescriptor`s, retrying until the sweep confirms (lightningdevkit#2825).
* After initiating an outbound channel, a peer disconnection no longer results
in immediate channel closure. Rather, if the peer is reconnected before the
channel times out LDK will automatically retry opening it (lightningdevkit#2725).
* `PaymentPurpose` now has separate variants for BOLT12 payments, which
include fields from the `invoice_request` as well as the `OfferId` (lightningdevkit#2970).
* `ChannelDetails` now includes a list of in-flight HTLCs (lightningdevkit#2442).
* `Event::PaymentForwarded` now includes `skimmed_fee_msat` (lightningdevkit#2858).
* The `hashbrown` dependency has been upgraded and the use of `ahash` as the
no-std hash table hash function has been removed. As a consequence, LDK's
`Hash{Map,Set}`s no longer feature several constructors when LDK is built
with no-std; see the `util::hash_tables` module instead. On platforms that
`getrandom` supports, setting the `possiblyrandom/getrandom` feature flag
will ensure hash tables are resistant to HashDoS attacks, though the
`possiblyrandom` crate should detect most common platforms (lightningdevkit#2810, lightningdevkit#2891).
* `ChannelMonitor`-originated requests to the `ChannelSigner` can now fail and
be retried using `ChannelMonitor::signer_unblocked` (lightningdevkit#2816).
* `SpendableOutputDescriptor::to_psbt_input` now includes the `witness_script`
where available as well as new proprietary data which can be used to
re-derive some spending keys from the base key (lightningdevkit#2761, lightningdevkit#3004).
* `OutPoint::to_channel_id` has been removed in favor of
`ChannelId::v1_from_funding_outpoint` in preparation for v2 channels with a
different `ChannelId` derivation scheme (lightningdevkit#2797).
* `PeerManager::get_peer_node_ids` has been replaced with `list_peers` and
`peer_by_node_id`, which provide more details (lightningdevkit#2905).
* `Bolt11Invoice::get_payee_pub_key` is now provided (lightningdevkit#2909).
* `Default[Message]Router` now take an `entropy_source` argument (lightningdevkit#2847).
* `ClosureReason::HTLCsTimedOut` has been separated out from
`ClosureReason::HolderForceClosed` as it is the most common case (lightningdevkit#2887).
* `ClosureReason::CooperativeClosure` is now split into
`{Counterparty,Locally}Initiated` variants (lightningdevkit#2863).
* `Event::ChannelPending::channel_type` is now provided (lightningdevkit#2872).
* `PaymentForwarded::{prev,next}_user_channel_id` are now provided (lightningdevkit#2924).
* Channel init messages have been refactored towards V2 channels (lightningdevkit#2871).
* `BumpTransactionEvent` now contains the channel and counterparty (lightningdevkit#2873).
* `util::scid_utils` is now public, with some trivial utilities to examine
short channel ids (lightningdevkit#2694).
* `DirectedChannelInfo::{source,target}` are now public (lightningdevkit#2870).
* Bounds in `lightning-background-processor` were simplified by using
`AChannelManager` (lightningdevkit#2963).
* The `Persist` impl for `KVStore` no longer requires `Sized`, allowing for
the use of `dyn KVStore` as `Persist` (lightningdevkit#2883, lightningdevkit#2976).
* `From<PaymentPreimage>` is now implemented for `PaymentHash` (lightningdevkit#2918).
* `NodeId::from_slice` is now provided (lightningdevkit#2942).
* `ChannelManager` deserialization may now fail with `DangerousValue` when
LDK's persistence API was violated (lightningdevkit#2974).
Bug Fixes
=========
* Excess fees on counterparty commitment transactions are now included in the
dust exposure calculation. This lines behavior up with some cases where
transaction fees can be burnt, making them effectively dust exposure (lightningdevkit#3045).
* `Future`s used as an `std::...::Future` could grow in size unbounded if it
was never woken. For those not using async persistence and using the async
`lightning-background-processor`, this could cause a memory leak in the
`ChainMonitor` (lightningdevkit#2894).
* Inbound channel requests that fail in
`ChannelManager::accept_inbound_channel` would previously have stalled from
the peer's perspective as no `error` message was sent (lightningdevkit#2953).
* Blinded path construction has been tuned to select paths more likely to
succeed, improving BOLT12 payment reliability (lightningdevkit#2911, lightningdevkit#2912).
* After a reorg, `lightning-transaction-sync` could have failed to follow a
transaction that LDK needed information about (lightningdevkit#2946).
* `RecipientOnionFields`' `custom_tlvs` are now propagated to recipients when
paying with blinded paths (lightningdevkit#2975).
* `Event::ChannelClosed` is now properly generated and peers are properly
notified for all channels that as a part of a batch channel open fail to be
funded (lightningdevkit#3029).
* In cases where user event processing is substantially delayed such that we
complete multiple round-trips with our peers before a `PaymentSent` event is
handled and then restart without persisting the `ChannelManager` after having
persisted a `ChannelMonitor[Update]`, on startup we may have `Err`d trying to
deserialize the `ChannelManager` (lightningdevkit#3021).
* If a peer has relatively high latency, `PeerManager` may have failed to
establish a connection (lightningdevkit#2993).
* `ChannelUpdate` messages broadcasted for our own channel closures are now
slightly more robust (lightningdevkit#2731).
* Deserializing malformed BOLT11 invoices may have resulted in an integer
overflow panic in debug builds (lightningdevkit#3032).
* In exceedingly rare cases (no cases of this are known), LDK may have created
an invalid serialization for a `ChannelManager` (lightningdevkit#2998).
* Message processing latency handling BOLT12 payments has been reduced (lightningdevkit#2881).
* Latency in processing `Event::SpendableOutputs` may be reduced (lightningdevkit#3033).
Node Compatibility
==================
* LDK's blinded paths were inconsistent with other implementations in several
ways, which have been addressed (lightningdevkit#2856, lightningdevkit#2936, lightningdevkit#2945).
* LDK's messaging blinded paths now support the latest features which some
nodes may begin relying on soon (lightningdevkit#2961).
* LDK's BOLT12 structs have been updated to support some last-minute changes to
the spec (lightningdevkit#3017, lightningdevkit#3018).
* CLN v24.02 requires the `gossip_queries` feature for all peers, however LDK
by default does not set it for those not using a `P2PGossipSync` (e.g. those
using RGS). This change was reverted in CLN v24.02.2 however for now LDK
always sets the `gossip_queries` feature. This change is expected to be
reverted in a future LDK release (lightningdevkit#2959).
Security
========
0.0.123 fixes a denial-of-service vulnerability which we believe to be reachable
from untrusted input when parsing invalid BOLT11 invoices containing non-ASCII
characters.
* BOLT11 invoices with non-ASCII characters in the human-readable-part may
cause an out-of-bounds read attempt leading to a panic (lightningdevkit#3054). Note that all
BOLT11 invoices containing non-ASCII characters are invalid.
In total, this release features 150 files changed, 19307 insertions, 6306
deletions in 360 commits since 0.0.121 from 17 authors, in alphabetical order:
* Arik Sosman
* Duncan Dean
* Elias Rohrer
* Evan Feenstra
* Jeffrey Czyz
* Keyue Bao
* Matt Corallo
* Orbital
* Sergi Delgado Segura
* Valentine Wallace
* Willem Van Lint
* Wilmer Paulino
* benthecarman
* jbesraa
* olegkubrakov
* optout
* shaavan
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@benthecarman@codecov-commenter@TheBlueMatt