invoice: fix false BROKEN log in cmp_rr_number when sort self-compares#9153
Open
nGoline wants to merge 1 commit into
Open
invoice: fix false BROKEN log in cmp_rr_number when sort self-compares#9153nGoline wants to merge 1 commit into
nGoline wants to merge 1 commit into
Conversation
Some qsort implementations (notably macOS) call the comparator with the same element against itself (a == b). cmp_rr_number assumed rr_numbers are unique across distinct channels and logged BROKEN on any tie, but when a == b then a->c == b->c and the tie is trivially expected — not a data corruption. Guard the BROKEN log with `if (a->c != b->c)`: equal channel pointers mean the same channel entry; only different channels sharing an rr_number is genuinely broken. This silences the spurious BROKEN log on macOS that caused test_xpay_limited_max_accepted_htlcs to fail with "had BROKEN or That's weird messages". Changelog-Fixed: invoice: no longer spuriously logs BROKEN when macOS qsort compares a routehint candidate to itself. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
nGoline
force-pushed
the
xpay-limited-max-accepted-htlcs
branch
from
0e73ed2 to
a465fb9
Compare
Collaborator
|
I'm okay with this solution if @Lagrang3 is okay with it. When @rustyrussell and I were looking at it last we were considering changing the ccan implementation of asort because there seems to be a bug in it that causes it to be behave differently than the qsort implementation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cmp_rr_number()inlightningd/invoice.clogged BROKEN whenever two candidates had equalrr_numbers, but someqsortimplementations (notably macOS) call the comparator with the same element against itself (a == b)a == b,a->c == b->ctrivially — not a data corruptionif (a->c != b->c): only log BROKEN when genuinely different channels share anrr_numbertest_xpay_limited_max_accepted_htlcsto fail with "had BROKEN or That's weird messages" and exit with return code 1Closes #9035
🤖 Generated with Claude Code