Bolt quote improvements#9044
Open
rustyrussell wants to merge 20 commits intoElementsProject:masterfrom
Open
Conversation
Update BOLT quotes in test and library Python files to match current BOLT text: punctuation (periods to semicolons), capitalization (Lightning on Bitcoin mainnet), fix field ordering in invoice breakdowns, add ... to skip features fields and SHA256 hex details, correct backtick-quoting of field names, and remove incorrect BOLT #7 label from a plain comment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…ker. We're going to get stricter, so "/* BOLT #N to-local output */" or "/* BOLT11 ... */" will upset it. Also remove a stray bare blank line in a BOLT comment block. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…_final_cltv_expiry_delta. The BOLT #7 was updated to use the new default 18 as min_final_cltv_expiry_delta, so update our quote and test, and fix other textual spec changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Some of these are malformed (thus were unchecked!), some are from the stricter interpretation of `...` which won't cross section boundaries. Several BOLT quotes had drifted from the current spec text: - connectd/queries.c: BOLT #7 uses 'full_information' not 'complete'; remove a second quote that referenced query_channel_range but was actually wrong (that requirement is for query_short_channel_ids). - lightningd/dual_open_control.c: channel reserve is 1% rounded down (not just 1%); witness weight check now says SHOULD broadcast rather than MUST fail; RBF quotes simplified to match actual BOLT wording. - openingd/openingd.c: 'The sending node' became 'The sender'; invalid signature response now says MUST send warning/error not just MUST fail the channel. - openingd/dualopend.c: same invalid-signature quote as openingd.c. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mose of these are from the stricter `...` which won't cross section boundaries. The listoffers_done doesn't actually need the ellipsis at all. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Textual updates: the tests are actually correct. The invoice.c change is to cross a section boundary. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The Python tool (copied from lnprototest) handles multiple comment styles. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Splits BOLT files into per-section chunks so wildcards can't inadvertently cross section headers, supports `...`-at-start semantics, and adds make-style `-k` Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…ics. The new check_quotes.py tool will treat `...` at the start of a quote as "immediately follows previous quote in BOLT text". To prepare for that, we change existing quotes which used leading `...` to mean "skip some text": split them into two consecutive BOLT comments (the second starting with `...` to use the existing wildcard match), and add explicit `*...` markers between consecutive BOLT test vector sections which cross `# From` headers. Also remove leading `...` from nonce quotes in cryptomsg.c/handshake.c where the actual BOLT text starts a fresh sentence (no prior quote in file). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> sCo-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
….py, check Rust and Python too Extend check-source-bolt to also check Python and Rust source files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
check_quotes.py gains --coverage=FILE: on each successful match, atomically
appends one line '{bolt} {section_idx} {start} {end}' to FILE using a single
os.write() call so parallel make invocations don't interleave records.
find_quote() and find_quote_immediate() are updated to return match start
positions (needed to record the covered range, not just the end).
bolt-coverage.py reads the coverage file and reports BOLT text not covered
by any source comment. By default it restricts output to Requirements
sections; --all-sections shows every section. --bolt N restricts to
a single BOLT number.
Exit status is 0 if everything in the selected sections is covered, 1 if
anything is uncovered.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prints out a report of uncovered Requirements sections of the BOLTs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Claude messed up about half of these: putting them too far from the appropriate code. After this I did it myself. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Fixed: Protocol: use BOLT4's paranoid advice about doing constant-time error decryption.
…hannels. We can decide to send an HTLC down a preferred channel which leads to the same peer as the one they asked for, but the spec is clear that you shouldn't send the "wrong" channel_update in that case. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Fixed: Protocol: when we send errors, we won't include a `channel_update` if we chose a different channel than the one they told us to.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
BOLT 7 says this, but we don't do it. (Actually, it only says that for certain types, but I've fixed that in lightning/bolts#1331). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
d8fd06f to
a26b96a
Compare
… other chains. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Fixed: Protocol: `gossipd` will now silently ignore gossip for other chains (rather than sending warnings).
This is what was merged in the spec, so update our checks (and bolt quote). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
a26b96a to
fce19f8
Compare
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
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.
One incredibly valuable thing about Core Lightning is we include, and enforce BOLT checks. This PR takes it to the next level:
...at the beginning means "follows the previous quote"...elsewhere means "skip something, but now won't skip over sections.make check-requirements-coverageto show what Requirement sections we don't quoteGreater coverage puts us in a much stronger position when we update to the latest BOLT version at the beginning of a release cycle.
Changelog-None: Users will never see it. Their loss!