Skip to content

scripts+GitHub: use bitcoind v30.0#10689

Open
ziggie1984 wants to merge 3 commits intolightningnetwork:masterfrom
ziggie1984:bump-bitcoind-v30
Open

scripts+GitHub: use bitcoind v30.0#10689
ziggie1984 wants to merge 3 commits intolightningnetwork:masterfrom
ziggie1984:bump-bitcoind-v30

Conversation

@ziggie1984
Copy link
Copy Markdown
Collaborator

@ziggie1984 ziggie1984 commented Mar 29, 2026

Bumps the pinned bitcoind version used in CI integration tests from v29 to v30.0, the current stable release of Bitcoin Core.

@gemini-code-assist
Copy link
Copy Markdown

gemini-code-assist Bot commented Mar 29, 2026

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

saubyk
saubyk approved these changes Mar 29, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@saubyk saubyk left a comment

Choose a reason for hiding this comment

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

Ack

@pinheadmz
Copy link
Copy Markdown
Contributor

pinheadmz commented Mar 30, 2026

Following this.
Note that v29.1 lowered a few default policy limits.

@yyforyongyu yyforyongyu mentioned this pull request Apr 7, 2026
Merged
@saubyk saubyk added this to v0.21 Apr 7, 2026
@saubyk saubyk moved this to In progress in v0.21 Apr 7, 2026
@saubyk saubyk moved this from In progress to In review in v0.21 Apr 7, 2026
@ziggie1984 ziggie1984 moved this from In review to In progress in v0.21 Apr 8, 2026
@saubyk saubyk added this to the v0.21.0 milestone Apr 9, 2026
@saubyk saubyk moved this from In progress to In review in v0.21 Apr 9, 2026
@Roasbeef
Copy link
Copy Markdown
Member

Roasbeef commented Apr 14, 2026

All bitcoind itests failing after that bump.

@github-actions github-actions Bot added the severity-low Best-effort review label Apr 16, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 16, 2026

PR Severity: LOW

Automated classification | 4 files | 183 lines changed

All changed files fall into the LOW severity category:

Files: .github/workflows/main.yml (CI/CD config, .github/*), itest/lnd_bump_fee.go (integration test), itest/lnd_funding_test.go (integration test), itest/lnd_sweep_test.go (integration test).

Severity bump check: After excluding itest/* test files, only 1 non-test file remains with 2 lines changed. No bump triggered (thresholds: >20 files or >500 lines).

This PR is a test/CI maintenance change warranting best-effort review only.

To override, add a severity-override-{critical,high,medium,low} label.
<!-- pr-severity-bot -->

yyforyongyu
yyforyongyu requested changes Apr 17, 2026
View reviewed changes
Comment thread itest/lnd_bump_fee.go

// assertFeeRateGreater is a helper closure that asserts the fee rate
// of the pending sweep tx is greater than the expected fee rate.
assertFeeRateGreater := func(expected uint64) {
Copy link
Copy Markdown
Member

@yyforyongyu yyforyongyu Apr 17, 2026

Choose a reason for hiding this comment

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

why do we remove this?

Comment thread itest/lnd_funding_test.go Outdated
unconfirmedUtxos := ht.GetUTXOsUnconfirmed(carol, "")
mempoolTxid := mempoolTxids[0].String()

// The sweeper output may be RBF-replaced, which can leave multiple
Copy link
Copy Markdown
Member

@yyforyongyu yyforyongyu Apr 17, 2026

Choose a reason for hiding this comment

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

i don't think that's the case, maybe cc is hallucinating.

@github-actions github-actions Bot added severity-critical Requires expert review - security/consensus critical and removed severity-low Best-effort review labels Apr 20, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 20, 2026

⚠️ Severity changed: severity-low → severity-critical (files changed since last classification)

🔴 PR Severity: CRITICAL

Highest-severity package match | 10 files | 357 lines changed

🔴 Critical (2 files)
  • lnwallet/mock.go - wallet mock in lnwallet/* package (wallet operations, channel funding, signing)
  • lnwallet/wallet.go - core wallet implementation in lnwallet/* package
🟢 Low (8 files — excluded from severity determination)
  • itest/lnd_bump_fee.go - integration test
  • itest/lnd_channel_force_close_test.go - integration test
  • itest/lnd_funding_test.go - integration test
  • itest/lnd_recovery_test.go - integration test
  • itest/lnd_sweep_test.go - integration test
  • lnwallet/wallet_test.go - test file (*_test.go)
  • .github/workflows/main.yml - CI/CD configuration
  • scripts/install_bitcoind.sh - scripts/tools

Analysis

This PR touches lnwallet/wallet.go and lnwallet/mock.go, both in the lnwallet/* package which covers wallet operations, channel funding, signing, and commitment transactions. This package is classified as CRITICAL and requires expert review.

The bulk of changes (8 files) are in integration tests (itest/*), a test file, CI config, and a script — all LOW severity. These are excluded from the file/line counting for severity bump purposes (4 non-excluded files, ~30 non-excluded lines changed), so no severity bump was triggered. The highest-severity file (lnwallet/*) determines the overall classification.

To override, add a severity-override-{critical,high,medium,low} label.
<!-- pr-severity-bot -->

@ziggie1984 ziggie1984 force-pushed the bump-bitcoind-v30 branch 2 times, most recently from cd14ff6 to 219b54b Compare April 21, 2026 13:51
@ziggie1984
testing: allow patch versions of bitcoin core
82af67b
@ziggie1984
lntest: pin pre-v30 mempool policy defaults in itest bitcoind
7bdbe60 
Bitcoind v30 lowered the default minrelaytxfee and incrementalrelayfee
from 1000 sat/kvB (1 sat/vB) to 100 sat/kvB. The itest suite was
written against the old defaults and the lower values cascade into:

  - integer sat/vByte assertions losing precision below 1 sat/vB, and
  - RBF bump thresholds that alter sweeper/bumpfee replacement timing.

Pin the old defaults in the itest bitcoind backend so the existing
tests keep passing without per-test adaptation. Running against the
new defaults is still worth doing, but that is a separate exercise
that should not be bundled with the v30 version bump.
@github-actions github-actions Bot added severity-low Best-effort review and removed severity-critical Requires expert review - security/consensus critical labels Apr 21, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 21, 2026

⚠️ Severity changed: severity-critical → severity-low (files changed since last classification)

🟢 PR Severity: LOW

Automated classification | 3 files | 26 lines changed

🟢 Low (3 files)
  • .github/workflows/main.yml - CI/CD workflow configuration (.github/*)
  • lntest/bitcoind_common.go - Test infrastructure file (lntest/*)
  • scripts/install_bitcoind.sh - Build/install script (scripts/*)

Analysis

All three files in this PR fall into the LOW severity category:

  • .github/workflows/main.yml is a GitHub Actions CI/CD configuration file, classified as LOW under .github/*.
  • lntest/bitcoind_common.go is part of the test infrastructure (lntest/*), classified as LOW.
  • scripts/install_bitcoind.sh is a utility script (scripts/*), classified as LOW.

No severity bump conditions apply: only 2 non-test/non-CI files with 15 lines changed (well under the 20-file and 500-line thresholds). This PR appears to update the bitcoind installation script and related test/CI configuration — purely infrastructure changes with no impact on core Lightning Network logic.

To override, add a severity-override-{critical,high,medium,low} label.
<!-- pr-severity-bot -->

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

Reviewers

@yyforyongyu yyforyongyu yyforyongyu requested changes

@saubyk saubyk saubyk approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

@ziggie1984 ziggie1984

Labels

no-changelog severity-low Best-effort review

Projects

v0.21
Status: In review

Milestone

v0.21.0

Development

Successfully merging this pull request may close these issues.

5 participants

@ziggie1984 @pinheadmz @Roasbeef @yyforyongyu @saubyk