lntest: disable bitcoind v2 P2P transport in itests#10661
lntest: disable bitcoind v2 P2P transport in itests#10661ellemouton wants to merge 1 commit intolightningnetwork:masterfrom
Conversation
bitcoind v29 attempts a v2 P2P handshake when connecting to the btcd miner, but btcd doesn't support v2 transport. The handshake times out after 30s before falling back to v1, which consumes the entire DefaultTimeout budget and causes flakes in tests that rely on timely block propagation after reconnecting (e.g. open_channel_reorg_test). Add -v2transport=0 to both the itest chain backend and the bitcoind miner backend, matching what the unit test backend already does.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a persistent flakiness in integration tests by mitigating a P2P transport incompatibility between Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request addresses a flaky integration test by disabling bitcoind's v2 P2P transport, which causes a 30-second handshake timeout when connecting to a btcd miner. The fix involves adding the -v2transport=0 flag to the bitcoind command arguments in the itest chain backend and the bitcoind miner backend. The changes are correct, well-commented, and consistent with existing test setups. The added comments clearly explain the reason for the change and include a TODO for future removal. The implementation is straightforward and I have no further suggestions.
Summary
Fixes a flaky itest (
open_channel_reorg_test) caused by bitcoind v29'sv2 P2P transport handshake timing out when connecting to the btcd miner.
btcd does not support v2 P2P transport. When bitcoind (the chain backend)
connects to the btcd miner, it first attempts a v2 handshake which btcd
silently ignores. After 30 seconds, bitcoind gives up and retries with
v1, which succeeds immediately. This 30-second delay consumes the entire
DefaultTimeoutforWaitForNodeBlockHeight, causing a race thatresults in flaky test failures.
The unit test backend (
lntest/unittest/backend.go) already disables v2transport with
-v2transport=0for the same reason. This PR applies thesame fix to:
lntest/bitcoind_common.go)lntest/miner/bitcoind_miner.go)Log analysis
Example failing build
The bitcoind chain backend log shows the issue clearly:
The v2 handshake hung for exactly 30s (08:16:48 → 08:17:18), then the v1
fallback connected instantly. Meanwhile,
WaitForNodeBlockHeightstartedat ~08:16:50 with a 30s timeout and expired at 08:17:20 — just 1 second
after the reorg blocks finally started flowing.
Test plan
go build -tags=bitcoind ./lntest/...compilesgo build ./lntest/miner/...compiles