Skip to content

Probing backround service follow-up.#1004

Open
randomlogin wants to merge 1 commit into
lightningdevkit:mainfrom
randomlogin:probing-service-follow-up
Open

Probing backround service follow-up.#1004
randomlogin wants to merge 1 commit into
lightningdevkit:mainfrom
randomlogin:probing-service-follow-up

Conversation

@randomlogin

Copy link
Copy Markdown
Contributor

Closes: #975

  1. Move probing code in builder before the memory leak checker.

  2. Prevent potential overflow due to public channel info fee values.

  3. Account for aggregated fee potentially being bigger than constructed probing path's max htlc bound (doing second pass once we constructed path with fees).

@ldk-reviews-bot

ldk-reviews-bot commented Jul 24, 2026

Copy link
Copy Markdown

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-reviews-bot
ldk-reviews-bot requested a review from tnull July 24, 2026 11:17

@tnull tnull 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.

One comment otherwise LGTM

Comment thread src/probing.rs
.checked_mul(channel_update_info.fees.proportional_millionths as u64)
.map(|v| v / 1_000_000)?;
let fee = (channel_update_info.fees.base_msat as u64).checked_add(proportional_fee)?;
forwarded = forwarded.checked_add(fee)?;

@tnull tnull Jul 24, 2026

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.

Codex:

  • [P2] Check the HTLC bound before adding the current hop’s fee — /home/tnull/worktrees/ldk-node/pr-1004-latest-20260724/src/probing.rs:698

    forwarded initially represents the amount sent through next_channel, including only downstream fees. The code adds the current node’s fee and then compares against that channel’s bounds. That fee is collected on the preceding channel and must not count toward
    next_channel.htlc_maximum_msat.

    For example, if B→C has a 1,000,000-msat maximum and B charges 1,000 msat, delivering exactly 1,000,000 msat over B→C is valid. This code checks 1,001,000 and rejects the path.

    BOLT 7 defines the maximum as what the origin node will send through that channel, while its fee is calculated from amount_to_forward. BOLT 7 channel updates and HTLC fees (https://github.com/lightning/bolts/blob/master/07-routing-gossip.md#htlc-fees)

    The bound check should occur before calculating and adding fee.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Move probing code in builder before the memory leak checker.

Prevent potential overflow due to public channel info fee values.

Account aggregated fee potentially being bigger than constructed porbing
path's max htlc bound.
@randomlogin
randomlogin force-pushed the probing-service-follow-up branch from 415befb to 5dec4e7 Compare July 24, 2026 13:16
@randomlogin
randomlogin requested a review from tnull July 24, 2026 13:17
Sign up for free to 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.

Probing service follow-ups

3 participants