feat: update Bitcoin Core to v31.1 - #199
Conversation
RonDeBanc
left a comment
There was a problem hiding this comment.
Review — APPROVE
The Bitcoin Core 29.2 → 31.1 update checks out.
Verification performed
- Confirmed v31.1 is an official Bitcoin Core release and uses the documented
bitcoincore.org/bin/bitcoin-core-31.1/distribution path. - Downloaded the official
SHA256SUMSandSHA256SUMS.asc; verification in a fresh GnuPG home produced 11 valid signatures and 0 bad signatures. - Confirmed all three JoininBox-supported artifacts are published and reachable:
arm-linux-gnueabihfaarch64-linux-gnux86_64-linux-gnu
- Downloaded the x86_64 archive; its SHA-256 exactly matched the signed manifest and its layout contains
bitcoin-31.1/bin/bitcoin-cliandbitcoindas expected by the installer. - Reviewed Bitcoin Core 30/31 compatibility changes. JoininBox and current JoinMarket do not use the removed
settxfee,maxorphantx, ornetwork=torinterfaces.onlynet=onion, proxy configuration, and the RPCs used here remain supported. JoininBox already selects descriptor-wallet configuration for Core ≥30. bitcoin-core-version.batsplusrepository-contracts.bats: 8/8 passed locally.bash -nandgit diff --checkpassed.- All current GitHub checks pass, including amd64 and arm64 image builds. The amd64 log confirms signed-manifest verification, binary checksum verification, and extraction under
bitcoin-31.1.
Non-blocking notes
-
The test title “all v31.1 Linux architectures published upstream remain mapped” overstates its scope: upstream also publishes powerpc64 and riscv64. The implementation correctly checks JoininBox’s supported matrix; please consider renaming it to:
all JoininBox-supported v31.1 Linux architectures remain mapped -
The image-build logs verify download, signature/checksum, and extraction, but do not show a
bitcoindstartup/RPC smoke test. A futurebitcoind -versionplus isolatedgetnetworkinfosmoke test would improve confidence for major-version jumps. An optional local daemon smoke test was not authorized, so I did not run one. -
Existing installer trust weaknesses—mutable builder-key retrieval, no explicitly pinned signer/quorum policy, and an unanchored manifest grep—predate this version bump and are not regressions in #199. They are worth separate hardening work.
No merge-blocking issue found.
Primary review model: Kimi K3 (kimi-k3, Kimi Coding provider)
Independent source/artifact/GitHub verification: GPT-5.6 Sol (gpt-5.6-sol, OpenAI Codex provider)
|
Addressed the review notes in
The focused suite is now 10/10 locally ( The mutable builder-key source and explicit signer/quorum policy require a broader trust-model change and are intentionally left for separate hardening work rather than folded into this release bump. |
RonDeBanc
left a comment
There was a problem hiding this comment.
Fresh-delta re-review — APPROVE
Re-reviewed 44e8ffa31 → 481c2bf80.
The follow-up correctly addresses the previous review notes and strengthens the installer:
awk '$2 == name'now selects the exact artifact filename from the signed manifest rather than accepting a case-insensitive substring match.sha256sum "${binaryName}"is quoted, preventing word splitting and pathname expansion.- The post-install check now requires the literal expected prefix
Bitcoin Core version v${bitcoinVersion}instead of accepting any Bitcoin Core version. - The architecture-test title now accurately refers to the JoininBox-supported matrix rather than every Linux artifact published upstream.
- New contract tests pin exact manifest selection and version-specific post-install verification.
Verification on the current head:
- focused Bats plus repository contracts: 10/10 passed;
bash -n: passed;git diff --check: passed;- all GitHub checks pass, including amd64 and arm64 image builds.
The literal source assertions are implementation-sensitive, but that is acceptable here because they deliberately protect security-relevant supply-chain contracts.
Non-blocking: the PR body still reports eight passing tests; the documented command now runs ten. Please update that count when convenient.
No merge-blocking issue found.
Primary re-review model: Kimi K3 (kimi-k3, Kimi Coding provider)
Independent code/test/GitHub verification: GPT-5.6 Sol (gpt-5.6-sol, OpenAI Codex provider)
Summary
SHA256SUMS.ascverification before accepting the binary checksumUpstream artifact verification
Verified against the official Bitcoin Core v31.1 manifest at
bitcoincore.org:bitcoin-31.1-aarch64-linux-gnu.tar.gzbitcoin-31.1-arm-linux-gnueabihf.tar.gzbitcoin-31.1-x86_64-linux-gnu.tar.gzNo hardcoded checksum changes are required: the installer downloads the signed upstream manifest and selects the checksum matching the derived platform artifact.
Tests
bats tests/bitcoin-core-version.bats tests/repository-contracts.bats— 8 passing testsgit diff --check