Skip to content

scripts: validate the first Comments-URI entry - #2233

Open
grandpig wants to merge 1 commit into
bitcoin:masterfrom
grandpig:master
Open

scripts: validate the first Comments-URI entry#2233
grandpig wants to merge 1 commit into
bitcoin:masterfrom
grandpig:master

Conversation

@grandpig

@grandpig grandpig commented Aug 3, 2026

Copy link
Copy Markdown

The Comments-URI validation condition was reversed when the field was made optional.

$found{'Comments-URI'} is incremented only after the current header line has been processed. As a result, the existing condition skipped validation for the first entry and applied it to continuation entries instead.

This allowed a nonstandard first Comments-URI to pass, while rejecting a valid external discussion URI following the standard Comments wiki URI.

Restore the original condition so that only the first entry must match:

https://github.com/bitcoin/bips/wiki/Comments:BIP-NNNN

Additional continuation entries remain unrestricted.

Signed-off-by: grandpig <grandpig@outlook.com>
@murchandamus murchandamus added the CI label Aug 5, 2026
Comment thread scripts/buildtable.pl
}
} elsif ($field eq 'Comments-URI') {
if ($found{'Comments-URI'}) {
if (not $found{'Comments-URI'}) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change makes sense to me. I see that I introduced this mistake in #1820. Thanks for catching that.

It’s not clear to me why we should be adding the second script to our build actions. It seems to be a proof that the bugfix change is correct, but if that’s the case adding it to the build actions going forth seems unnecessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants