CI: Allow ldk-node integration test to target a custom fork/branch

[joostjager]

• Allows the LDK Node integration job to run against a custom ldk-node fork/branch by parsing the PR description for a ldk-node: owner/repo#branch line. Falls back to lightningdevkit/ldk-node at its default branch when not specified or on push events.

• This lets PRs that change LDK's API pair with a corresponding ldk-node branch, avoiding expected build failures that are misleading and can mask other critical job failures on the overall commit status. This masking is more problematic since Restrict CI build matrix to Linux+MSRV for PRs #4410 reduced the PR build matrix to only linux/msrv. That change may trigger unexpected failures on main.

ldk-node: joostjager/ldk-node#rename-liquidities

[ldk-reviews-bot]

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

[joostjager]

Tried multiple approaches, but it turns out to be difficult to get that neutral status :(

[tnull]

but a red icon on the overall commit status is misleading and distracting

Huh, no? That is exactly the purpose. If people tend to ignore it, it rather means we need to make the icons larger.

[joostjager]

Open to suggestions. I don't think a bigger icon is going to change anything. Unless it's a branch protection, I don't think it'll work.

[TheBlueMatt]

I mean isn't the point that breaking LDK Node is a failure, just not one that has to be fixed pre-merge (because its impossible), rather one that needs to be fixed right after (ideally with a PR pre-merge)? I'm not sure that people are ignoring it, but if they are a comment from the bot after merge seems like an appropriate next-step.

[codecov]

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.04%. Comparing base (2647583) to head (fbc9140).

Files with missing lines	Patch %	Lines
lightning/src/routing/scoring.rs	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4413      +/-   ##
==========================================
- Coverage   86.05%   86.04%   -0.02%     
==========================================
  Files         156      156              
  Lines      103384   103384              
  Branches   103384   103384              
==========================================
- Hits        88971    88958      -13     
- Misses      11900    11911      +11     
- Partials     2513     2515       +2     

Flag	Coverage Δ
tests	86.04% <87.50%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[joostjager]

rather one that needs to be fixed right after (ideally with a PR pre-merge)?

I was thinking, maybe it could be a requirement to mention a repo/branch of ldk-node in the PR description that the CI for PRs then picks up and tries to build against. That needs to pass before merge can happen. If no branch is mentioned, CI will just build against ldk-node main.

I'm not sure that people are ignoring it, but if they are a comment from the bot after merge seems like an appropriate next-step.

In https://github.com/lightningdevkit/ldk-node/pull/789/commits, I had to fix quite a few things.

[TheBlueMatt]

In https://github.com/lightningdevkit/ldk-node/pull/789/commits, I had to fix quite a few things.

Then the appropriate solution seems like a comment that tags the PR author post-merge to remind them to go fix ldk node?

[joostjager]

I probably don't need to repeat that I think the best solution is a mono repo.

For the post-merge part, it probably doesn't get any better than tagging.

Gating merge on a building ldk-node branch also seems like a clear win. Helps with verifying the ldk changes too. I've updated this PR to add that.