Skip to content
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

update nixpkgs #610

Merged
merged 12 commits into from Jun 2, 2023
Merged

update nixpkgs #610

merged 12 commits into from Jun 2, 2023

Conversation

jonasnick
Copy link
Member

@jonasnick jonasnick commented May 22, 2023

bitcoind: 24.0.1 -> 25.0
btcpayserver: 1.7.12 -> 1.9.3
clightning: 23.02.2 -> 23.05
nbxplorer: 2.3.62 -> 2.3.63

This is (EDIT: was) a draft because the clightning update breaks clboss (see ZmnSCPxj/clboss#164 and ZmnSCPxj/clboss#162) and other plugins. I think it would make sense for us to deprecate clboss since it's currently unmaintained and may stopped working properly already with 23.02.

@alumbigi
Copy link

+1 to deprecating clboss

@erikarvstedt
Copy link
Collaborator

@jonasnick jonasnick marked this pull request as ready for review May 28, 2023 10:01
@jonasnick
Copy link
Member Author

Added lightningd/plugins#451 for prometheus

@prusnak
Copy link
Contributor

prusnak commented May 28, 2023

You can update nixpkgs again to pick up bitcoin 25.0

@jonasnick
Copy link
Member Author

I had just checked. It's not yet in nixpkgs-unstable.

jonasnick and others added 4 commits May 29, 2023 06:41
@jonasnick
Copy link
Member Author

updated nixpkgs again for bitcoind 25

@erikarvstedt
Copy link
Collaborator

erikarvstedt commented May 29, 2023

bitcoind 25.0 bug

Test backups fails when run locally:
When the test calls systemctl stop bitcoind, bitcoind hangs indefinitely during exiting.
The last log message printed by bitcoind is net thread exit or msghand thread exit.

This issue might be caused by a race condition in bitcoind 25.0.
It doesn't happen on our slow CI nodes.
It also doesn't happen when adding a 1 sec delay before running each test in the test runner.

I'd recommend to skip version 25.0 for now.
I'll look into this bug later this week.

How to continue

Here's a fixed branch based on 9c59b96 (this PR before the last force-push, without bitcoind 25.0).
It adds a clightning-rest update that makes it compatible with the msat clightning API change.

I've scanned the clightning-plugins for further uses of the changed API (besides prometheus), with no results.
So the remaining clightning clients that need to be fixed are:

  • btcpayserver (fixed by v1.10.0, which will probably be released today)
  • spark-wallet (shouldn't we deprecate/remove this?)

@fanquake
Copy link

This issue might be caused by a race condition in bitcoind 0.25.

Could you report this upstream. We haven't yet had any reports of similar issues with v25.0.

@erikarvstedt
Copy link
Collaborator

Could you report this upstream

Yes, sure, as soon as I've gathered some more data.

@erikarvstedt
Copy link
Collaborator

erikarvstedt commented May 29, 2023

After rebooting my test host system, the bitcoind shutdown bug has disappeared.
Before that, I could repro it consistently.
FYI, the backtrace of the hanging thread was:

#0  0x00007f7ec4df97d5 in __futex_abstimed_wait_common ()
   from target:/nix/store/vnwdak3n1w2jjil119j65k8mw1z23p84-glibc-2.35-224/lib/libc.so.6
#1  0x00007f7ec4dfc202 in pthread_cond_wait@@GLIBC_2.3.2 ()
   from target:/nix/store/vnwdak3n1w2jjil119j65k8mw1z23p84-glibc-2.35-224/lib/libc.so.6
#2  0x00007f7ec559e940 in std::condition_variable::wait(std::unique_lock<std::mutex>&) ()
   from target:/nix/store/6plx60y4x4q2lfp6n7190kaihyxr7m1w-gcc-11.3.0-lib/lib/../lib64/libstdc++.so.6
#3  0x00005595f8913510 in std::condition_variable::wait<StopHTTPServer()::<lambda()> > (__p=..., __lock=..., this=<optimized out>)
    at /nix/store/wy4ywjsch9q2hj5lphqjdg9p2kf7w0ls-gcc-11.3.0/include/c++/11.3.0/condition_variable:103
#4  StopHTTPServer () at httpserver.cpp:484
#5  0x00005595f85c5b38 in Shutdown (node=...) at init.cpp:247
#6  0x00005595f85afe0c in AppInit (node=..., argc=argc@entry=2,
    argv=argv@entry=0x7ffd83be2048) at bitcoind.cpp:242
#7  0x00005595f85b0176 in main (argc=2, argv=0x7ffd83be2048)
    at bitcoind.cpp:266

(bitcoind version v25.0, built with pkgs.enableDebugging pkgs.bitcoind, implying -ggdb -Og)

This PR

Here's my fixed branch, based on bitcoind 25.0.

@jonasnick
Copy link
Member Author

Added @erikarvstedt's commits and removed spark-wallet.

@jonasnick
Copy link
Member Author

@seberm do you have an idea why the trustedcoin test scenario fails?

@erikarvstedt
Copy link
Collaborator

Actually, the bitcoind 25.0 shutdown bug happens in our CI tests.
The bitcoind process is eventually killed by systemd, causing a >30 min test duration.
The test succeeds because we're currently not checking for a clean exit.

The bug has already been reported in bitcoin/bitcoin#27722, including a fix (Crypt-iQ/bitcoin@4fd7adb). I've added a comment.

I think we should move back to bitcoind 24.1.

@jonasnick
Copy link
Member Author

Ok, I added a commit that downgrades bitcoind to 24.1. I would suggest we use that one until there's a reviewed PR for bitcoin/bitcoin#27722 that we can backport to 25.0.

@erikarvstedt
Copy link
Collaborator

erikarvstedt commented Jun 1, 2023

Fixed branch containing a RTL update (for msat compatibility).

It's currently running live at https://nixbitcoin.org

Can you fix my commit msg?
bitcoind: update module to v23.05 ->
bitcoind: update module to v25.0

Edit: Here's a squashed version of the above branch which also includes the commit msg renaming.

Copy link
Collaborator

@erikarvstedt erikarvstedt left a comment

Choose a reason for hiding this comment

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

ACK af87d59

@jonasnick jonasnick merged commit e3190b2 into fort-nix:master Jun 2, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants