Skip to content

fix: let unannounced channels take full-size payments - #120

Open
ovitrif wants to merge 2 commits into
mainfrom
fix/inbound-htlc-in-flight-unannounced
Open

ovitrif wants to merge 2 commits into
mainfrom
fix/inbound-htlc-in-flight-unannounced

Conversation

@ovitrif

@ovitrif ovitrif commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Refs:

Description

  • Sets the maximum inbound HTLC value in flight to 100% of the channel value for nodes that cannot announce channels (no node alias or no listening addresses), so a single incoming payment can use the whole channel instead of 10% of it. Such a node only accepts unannounced channels, and ldk-node already sets 100% for the unannounced channels it opens itself.
  • The limit is part of the channel handshake, so channels opened before this change keep their 10% limit; only new channels benefit.
  • Adds inbound_htlc_in_flight_limit_for_unannounced_node: opens a 1,000,000 sat channel to a node without an alias, pays it 9% (works with and without the change) and 20% of the channel value (fails with PaymentSendingFailed without the change).
  • Adds the unit test inbound_htlc_value_in_flight_limit_follows_announce_ability for the default_user_config branch: no alias and no listening addresses, only one of them missing, and a node that can announce channels (keeps LDK's default).
  • Upstream rust-lightning makes 100% the default for unannounced channels (2867d5c1a, in the 0.3 pre-releases); the fork is on LDK 0.2.0, where it is still 10%.
  • The changelog line goes into the release PR, per the fork's single-section CHANGELOG rule.

Out of Scope

  • src/builder.rs, src/event.rs: the LSPS2 paths keep their own 100% override.
  • Nodes that can announce keep LDK's default for every inbound channel, unannounced ones included; no config option is added.
  • Existing channels: the limit is fixed at channel open, so they stay at 10%.
  • Version bump and release: separate release PR, followed by the bump PRs in the Bitkit apps.
  • Newer LDK defaults 100% for unannounced channels (unannounced_channel_max_inbound_htlc_value_in_flight_percentage); when the fork moves to an LDK release with that option, this override can be removed.
  • The Rust CI workflow (rust.yml, manual dispatch only) fails on main in its UniFFI build, see QA Notes; fixing it is a separate change.

QA Notes

Automated Checks

  • added inbound_htlc_in_flight_limit_for_unannounced_node in tests/integration_tests_rust.rs: fails on main (a payment above 10% of the channel value must be routable: PaymentSendingFailed), passes with the change.
  • added inbound_htlc_value_in_flight_limit_follows_announce_ability in src/config.rs: fails with the default_user_config change removed (left: 10, right: 100), passes with it.
  • ran the steps of rust.yml locally on the rebased head fdb891c1 (stable 1.98.1): cargo fmt --check, cargo build, cargo doc (release and private items) and cargo check --release pass, as do the 120 unit tests with --features uniffi.
  • ran the integration suites with --cfg no_download, with and without --features uniffi: all pass except the tests below. reorg_test and the 110 tests of multi_address_types_tests pass (test_multi_wallet_all_combinations failed once with InvalidSocketAddress in a parallel run and passes alone).
  • not completed locally: splice_channel and the eight channel_full_cycle* tests hang at their splice step. splice_channel hangs the same way on main (6d24c44), so the cause is not this change; these need the hosted run.
  • the hosted Rust CI cannot reach the tests today. build (ubuntu-latest, stable) fails in the step Build with UniFFI support on Rust stable (cargo build --features uniffi with RUSTFLAGS=-D warnings) with 5 errors: four unused imports in src/ffi/types.rs and the deprecated bitcoin::FeeRate::from_sat_per_vb_unchecked used by the FeeRate constructor in bindings/ldk_node.udl. They are warnings in a plain build, which is why the app builds work, and the same 5 errors appear with main's tree; this PR touches none of those files. rust.yml only runs on manual dispatch and no recent merged PR (fix: report on-chain broadcast outcomes #119, fix: restore android release publishing #121) ran it.

Manual Tests

  • Bitkit Android master (2.5.0) on a regtest LND channel of 1,000,000 sats, with a libldk_node built from 0.7.0-rc.66 plus this one-line change swapped in: the channel is accepted with max_htlc_value_in_flight_msat: 1000000000 (was 100000000), and single payments of 110,000 sats (11%) and 500,000 sats (50%) succeed with "Received Instant Bitcoin". Without the change the 11% payment fails.

@ovitrif ovitrif self-assigned this Sep 30, 2026
@ovitrif ovitrif changed the title fix: accept incoming payments above 10% of an unannounced channel's value fix: let unannounced channels take full-size payments Sep 30, 2026
@ovitrif

ovitrif commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 555d5db: dropped the changelog entry from this PR; the release PR adds it in the fork's single section. Dispatched the Rust CI on this branch: https://github.com/synonymdev/ldk-node/actions/runs/36661558748

@ovitrif

ovitrif commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

The Rust CI run on this branch (https://github.com/synonymdev/ldk-node/actions/runs/36661558748) failed in "build (ubuntu-latest, stable)", and fail-fast cancelled the other jobs. The failure is on main too (26664614): with --features uniffi and -D warnings, rustc 1.98.1 rejects four unused imports in src/ffi/types.rs (lines 45, 48-50, 54, 63) and the deprecated bitcoin::FeeRate::from_sat_per_vb_unchecked used by the constructor at bindings/ldk_node.udl:416. This PR touches neither. The same build passes on 1.85.0 and on beta, so the test steps never ran; they need a run once main builds on stable.

…alue

A node that cannot announce channels only accepts unannounced ones, so LDK's default of 10% of the channel value as maximum inbound HTLC value in flight capped every single incoming payment far below the inbound capacity. Set it to 100% for such nodes, as already done for unannounced channels we open ourselves.

The integration test opens a channel to a node without a node alias and pays it 9% and 20% of the channel value; the second payment fails without the change.
@ovitrif
ovitrif force-pushed the fix/inbound-htlc-in-flight-unannounced branch from 555d5db to fdb891c Compare October 1, 2026 20:21
@ovitrif

ovitrif commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed fdb891c: rebased onto main (0 behind, includes #119 and #121) and folded the changelog add/remove pair into the single fix commit; the diff is still src/config.rs and the test only.

On the failed Rust CI run: the failing step is Build with UniFFI support on Rust stable (cargo build --features uniffi with RUSTFLAGS=-D warnings), where rustc 1.98.1 turns four unused imports in src/ffi/types.rs and the deprecated FeeRate::from_sat_per_vb_unchecked into errors. It reproduces locally with the same command and it is unrelated to this change; a plain cargo build --features uniffi only warns, which is why the app builds work. #119 and #121 merged without a rust.yml run, since it is manual dispatch only. Local results for the workflow's steps and tests are in the PR body.

@ovitrif
ovitrif marked this pull request as ready for review October 1, 2026 21:30
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-01T21:35:10.004727Z fdb891c Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fdb891c145

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/config.rs
The default_user_config branch that sets the maximum inbound HTLC value in flight to 100% now has focused unit tests: a node without alias and listening addresses, one without listening addresses, one without alias, and a node that can announce channels, which keeps LDK's default.
@ovitrif

ovitrif commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 4a7e44d: adds the unit test inbound_htlc_value_in_flight_limit_follows_announce_ability in src/config.rs, answering the Codex review on default_user_config. It covers a node without alias and listening addresses, one missing only the listening addresses, one missing only the alias (all 100%) and an announcement-capable node (keeps LDK's default); it fails with the production change removed. No production code changed, still 0 behind main. The integration suite results in the body were taken on fdb891c1, which has the same production code.

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.

1 participant