#36151 makeseeds: fix off-by-one in field count check

full analysis

https://github.com/bitcoin/bitcoin/pull/36151 · aman21-droid · +22/-1 in 1 files, 2 commits · labels: Scripts and tools

Goal

  • Prevent seed generation tooling from crashing on malformed crawler output during release preparation

Fixes an off-by-one check in `contrib/seeds/makeseeds.py` where lines with exactly 11 fields bypassed the length check but triggered an `IndexError` when accessing `sline[11]`. The check is updated to require at least 12 fields, and unit tests are added for `parseline`.

Problem: When parsing crawler outputs to generate static seeds for releases, a truncated or malformed input line containing exactly 11 whitespace-separated fields crashed `makeseeds.py` with an unhandled exception rather than being skipped.

Category: Tools and scripts (#7 of 22)

P3 · bug fix

  • P3 because this is a minor fix to release seed generation tooling used only by maintainers
  • Prevents a crash on malformed inputs without altering normal seed generation behavior

Seed generation tools are explicitly noted as affecting real deployments in the tools rubric. However, this is a minor bug fix preventing an unhandled IndexError on malformed crawler output in a script run only by a handful of maintainers during release preparation.

Membership: Modifies `contrib/seeds/makeseeds.py`, which is part of the seed generation tooling under contrib.

Factors: security/stability 0, bug 1, performance 0, user value 1, leverage 0

Reviewability: Ready

  • Ready to review
  • Open question remains on whether to keep or remove the added unit tests

The patch is small and functional; the only open question is whether maintainers want to keep or drop the inline unit tests.

Author status: active, offered to remove tests if maintainers prefer

Resolved concerns:

  • l0rinc suggested framing the test as a characterization commit first, which the author adopted
  • achow101 noted that few people run this script and questioned the need for unit tests; the author offered to remove them if preferred

Agreement: Positive

  • Support and testing for the bug fix (l0rinc)
  • Questioned whether unit tests are worthwhile for a rarely used script (achow101)
  • Author offered to drop the test commit if maintainers prefer

Positive; fix is ACKed by l0rinc, with minor debate over whether unit tests are worthwhile (achow101)

l0rinc tested and ACKed the change. achow101 questioned whether unit tests are needed for a rarely run script, but did not object to the bug fix itself.

  • l0rinc ACKed commit da29409f4b
  • achow101 noted the script is rarely run and questioned test utility
  • author offered to drop the test commit if desired

Review verdicts (DrahtBot): 1

Files

23 lines under test/bench/ci.

  • contrib/seeds/makeseeds.py +22/-1

Card

Fixes an off-by-one field check in makeseeds.py that caused lines with exactly 11 fields to trigger an IndexError when accessing the 12th element. It ensures truncated crawler lines are skipped as originally intended instead of crashing the seed generation process. The script is run only by maintainers preparing release seeds. Review is essentially complete, with one ACK and an author offer to drop the newly added unit tests if maintainers prefer.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T16:23, confidence high, input hash 6b153adc6fc96714