Please add this change to the release notes for the next release.

This dramatically increases the complexity of this module. Who is the maintainer? Are containers not an adequate way to run multiple instances of this service on the same machine?

@mmahut @prusnak is the added complexity "worth it"?

@aanderse hmm, good point, but I don't consider this to be a major rewrite, at least running both testnet and mainnet networks at the same time is a standard feature that has been omitted in the previous module. I like the rewrite, the previous code was a mess (understandable, if we consider what a mess the upstream bitcoin node is).

@1000101 as mentioned, please note this change in the release notes and if possible, add yourself as maintainer (I guess you are fine with it given you added yourself to the test?).

@mmahut thanks for feedback.

This dramatically increases the complexity of this module. Who is the maintainer? Are containers not an adequate way to run multiple instances of this service on the same machine?

@mmahut @prusnak is the added complexity "worth it"?

@aanderse It may look like it, but it's just a rewrite. In fact, everything will work out of box (as previously), all you have to do is name the instance and that's it. I've updated some of stuff I found not working properly but other than that, I kept it compatible with the original service (apart from the instance name).

There was no maintainer before, but it's a good point, I'll add myself.

@aanderse hmm, good point, but I don't consider this to be a major rewrite, at least running both testnet and mainnet networks at the same time is a standard feature that has been omitted in the previous module. I like the rewrite, the previous code was a mess (understandable, if we consider what a mess the upstream bitcoin node is).

@1000101 as mentioned, please note this change in the release notes and if possible, add yourself as maintainer (I guess you are fine with it given you added yourself to the test?).

@mmahut yeap, will do

@GrahamcOfBorg test bitcoind

Looks good, thank you.

This changes the bitcoind data directory, correct? That means that if there's already a running instance which is using /var/lib/bitcoind/ as the data directory, this change would ignore that and bitcoind would start doing an initial sync.

There's also the issue of changing the bitcoind user, since existing files would be owned by the original bitcoind user.

You may need to use NixOS stateVersion (and accompaning release notes) to prevent this change from applying automatically to users which already run bitcoind.

Given this is breaking the way this service is configured I think just mentioning it in the release notes is enough, given this change will not redefine the directory nor user out of the box without users intervention.

But you have a good point that it would be useful if the release notes did mention this change, just in case. @1000101 do you mind opening an addendum?

To use this new multi-instance config with an existing bitcoind data directory, the user would have to change from something like this:

  services.bitcoind = {
    enable = true;
    extraConfig = "...";
    ...
  };

To something like this:

  services.bitcoind.mainnet = {
    enable = true;
    dataDir = "/var/lib/bitcoind";
    user = "bitcoin";
    extraConfig = "...";
    ...
  };

The key settings are:

• dataDir - to continue using the same data directory
• user - to continue using the same user so that bitcoind maintains access to its files.

This changes the bitcoind data directory, correct? That means that if there's already a running instance which is using /var/lib/bitcoind/ as the data directory, this change would ignore that and bitcoind would start doing an initial sync.

There's also the issue of changing the bitcoind user, since existing files would be owned by the original bitcoind user.

The directory and user may/will change by default, but you have the option to set them manually to the original (desired) values. However, you are absolutely right and I should mention this in the release notes (incompatibility section).

You may need to use NixOS stateVersion (and accompaning release notes) to prevent this change from applying automatically to users which already run bitcoind.

Since it is a breaking change it will not automatically change, however, I definitely need to add the points you mention in the release notes.

To use this new multi-instance config with an existing bitcoind data directory, the user would have to change from something like this:

  services.bitcoind = {
    enable = true;
    extraConfig = "...";
    ...
  };

To something like this:

  services.bitcoind.mainnet = {
    enable = true;
    dataDir = "/var/lib/bitcoind";
    user = "bitcoin";
    extraConfig = "...";
    ...
  };

The key settings are:

* `dataDir` - to continue using the same data directory

* `user` - to continue using the same user so that bitcoind maintains access to its files.

Thank you so much, I'll amend it.

If DynamicUser changes the user ID then I don't recommend using it (for users already using bitcoind) because then bitcoind may not be able to access the files in it's data directory.

DynamicUser keeps all data under StateDirectory owned by the user the service runs by, even when that user changes.