Update configuration.nix

[nixbitcoin]

This PR updates configuration.nix comments to the most current inter-option dependencies and introduces an assertions.nix to verify that all these dependencies are met.

Closes #37

[erikarvstedt]

Awesome 🌟

• trivial fixups
• Remove assertion cfg.recurring-donations.enable -> cfg.clightning.enable, instead add services.clightning.enable = true to recurring-donations.nix.
  In general, service modules should enable services they depend on. Same for spark-wallet, webindex, nanopos, lightning-charge.
• The remaining assertions should be defined directly in the relevant module files.
  • E.g., cfg.electrs.enable -> cfg.bitcoind.prune belongs to electrs.
  • cfg.lnd.enable -> !cfg.clightning.enable has no definite location, but we should move it to lnd.nix
• The prune assertions for lnd and electrs fail when prune = null. I think it's best if we remove the possible null value for bitcoind.prune and set prune = 0 in bitcoind as a default. This is will make the assertions work. We can then also remove prune = 0 in secure-node.nix and the mkForce in configuration.nix (bitcoind.prune = lib.mkForce ).
• Commit titles: Rename configuration: -> example config:

[nixbitcoin]

I squashed your fixup, clarified nix-bitcoin will auto-detect invalid settings in all other comments as well, removed the possible null value for bitcoind.prune, and renamed the commits.

However, I don't think it's a good idea for service modules to enable services they depend on or to define assertions directly in the relevant module files.

• Example1: What would happen if a user wanted to use lightning-charge with a different clightning module named clightning-with-plugins? Nix-bitcoin would try to enable clightning in the background and throw a confusing error.
• Example 2: What would happen if a user tried to enable electrs outside nix-bitcoin with a custom bitcoind module that didn't even have a pruning option?

IMO this reduces modularity and should instead be handled in higher-level nix-bitcoin expressions like configuration.nix.

[erikarvstedt]

Great fixups, thanks!

I'm still convinced we should add the assertions to the corresponding modules (like in NixOS) as they are already tightly coupled:

• Example1: lightning-charge is directly dependent on clightning via the reference to config.services.clightning.dataDir. It would already fail to be compatible with a hypothetic services.clightning-with-plugins.
• Example 2: electrs is referencing the option config.services.bitcoind.dataDirReadableByGroup so it's already dependent on our nix-bitcoin specific bitcoind module.

You could indeed add the cfg.lnd.enable -> !cfg.clightning.enable assertion to modules.nix, because it belongs to more than one service module.

[nixbitcoin]

Thanks for the explanation, I agree now on adding assertions to the modules. I'll work on implementing.

I still am not convinced it's a good idea to cross-enable modules without the users explicit direction. But I'm of course willing to reconsider. Any comment on this @jonasnick ?

[jonasnick]

Imo allowing modules to be interchangeable is only very rarely useful. Instead of using a different clightning module it should be possible to make the existing clightning module more flexible. Moreover, having more logic in the modules themselves simplifies writing and maintaining them which I clearly noticed when @erikarvstedt added the option to set something like nix-bitcoin.secrets.bitcoin-rpcpassword directly in the module vs. the old way of defining all secrets in network.nix.

As for modules implicitly enabling other modules, I'd prefer to not do that to allow users to easily see and opt-in to what software they actually use on their node (for now at least... the number of modules is still manageable).

[nixbitcoin]

Moved assertions to individual modules and split them up into separate commits.

[erikarvstedt]

Fixups.
I've removed redundant antecedents in the assertion conditions and improved the assertion messages.
It's okay, even encouraged, to gather self-contained and related changes in a single commit, to ease reviewing.
assertions: add lnd, clightning exclusivity deserves its own commit because it contains two changes (hunks) in different files, where one change depends on the other.

[nixbitcoin]

Just realized we need to ignore the lnd, clightning exclusivity assertion in order to run the tests, do you know how to do that?

[erikarvstedt]

I've added this to the fixups.

[nixbitcoin]

Had to slightly correct your fixup and added a comment, explaining the ||

[erikarvstedt]

ACK b9559ef

[nixbitcoin]

Changed the HWI assertion to check disablewallet directly, as this is the speicifc option that makes it incompatible.

[jonasnick]

ACK f280d54