-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chainntnfs+lntest: fix TestInterfaces
#8923
chainntnfs+lntest: fix TestInterfaces
#8923
Conversation
Important Review skippedAuto reviews are limited to specific labels. Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, LGTM 🎉
@@ -59,6 +59,12 @@ func NewMiner(t *testing.T, netParams *chaincfg.Params, extraArgs []string, | |||
t.Fatalf("unable to set up backend node: %v", err) | |||
} | |||
|
|||
// Next mine enough blocks in order for segwit and the CSV package | |||
// soft-fork to activate. | |||
numBlocks := netParams.MinerConfirmationWindow*2 + 17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did these numbers come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
partially from the itest where we mine blocks to activate it - then it's retry -> fail cycle to find the minimal blocks. Since it's not used anywhere else so I didn't make it into a constant.
Hmm, I wonder if we need to increase some timeouts in other tests that use the Here's an example:
|
This commit upgrades the test to always use a segwit v0 witness program when creating testing txns.
ede526c
to
48a0efe
Compare
Since it says |
Thanks! But of course the tests don't fail anymore now 🙈 Should I re-trigger the CI to run the unit tests again? |
Cool I'd prefer we merge this and investigate it in a new PR - have a trivial PR cooking locally to update the github action versions, we can then figure it out there, or whenever we see the flakes again in other PRs. |
This flake has popped up multiple times,
Turns out it's failing here, where we rescan blocks to find the relevant spending tx,
lnd/chainntnfs/txnotifier.go
Lines 1310 to 1315 in 6dea864
It's also easy to reproduce - need to run it multiple times tho,
And the relevant logs,
Since we don't ever allow pre-segwit utxos to be used, we now update the tests to always create segwit v0 txns.