lnd: fix missing RPC permissions when bitcoind is pruned

[erikarvstedt]

This branch includes some required commits from #559.

I've confirmed that lnd requires extra bitcoin RPC commands (via pruned_block_dispatcher.go) when bitcoind is pruned. This PR fixes the missing RPC permissions.

@ekimber, does this PR fix #562 for you?

[ekimber]

This branch includes some required commits from #559.

I've confirmed that lnd requires extra bitcoin RPC commands (via pruned_block_dispatcher.go) when bitcoind is pruned. This PR fixes the missing RPC permissions.

@ekimber, does this PR fix #562 for you?

Nice, this is working for me

[jonasnick]

Pinging @seberm, could you have a look at the commits affecting shellcheck?

[erikarvstedt]

The shellcheck-services commits fix code that I had originally added. They are technically unrelated to shellcheck and only deal with the NixOS module system.

• test/shellcheck-services: simplify accessing service definitions is a refactor with zero behavior changes (except for the performance boost).
• test/shellcheck-services: fix error by excluding unavailable services fixes an error with service detection.
  Previously, unavailable services defined via systemd.service.foo = mkIf false ...; (like clightning-rest-migrate-datadir) were not ignored, which caused an error when accessing their config.

[jonasnick]

The lnd commit looks good to me.

[jonasnick]

I wonder at what point it would make sense to abstract this. "privileged", "joinmarket-ob-watcher", "btcpayserver" all set the same options modulo user and rpcwhitelist.

[erikarvstedt]

Yes, I have a branch from 2021 which implements such an abstraction (by adding options passwordFile.{user,group} to bitcoind.rpc.users.<name>).
I guess I deemed that its benefits didn't warrant the increase in implicitness and the extra review burden. But if you're open to it, I'll dust it off and publish it.

[jonasnick]

Ah well if you already came to that conclusion no need to dust it off now.

[seberm]

Pinging @seberm, could you have a look at the commits affecting shellcheck?

Hello @jonasnick, the code seems fine to me.

I also rechecked the shellcheck still works as expected (made shellcheck erros into preStart scripts of nix-bitcoin services):

...
X="my test"
touch $X
cat $X

As you can see, it's working fine when running:

./run-tests.sh -s default
...
/nix/store/fx7d1yzp7370adpdw2wxi5kh6mh66nzy-unit-script-lnd-pre-start/bin/lnd-pre-start

In /nix/store/fx7d1yzp7370adpdw2wxi5kh6mh66nzy-unit-script-lnd-pre-start/bin/lnd-pre-start line 11:
touch $X
      ^-- SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
touch "$X"


In /nix/store/fx7d1yzp7370adpdw2wxi5kh6mh66nzy-unit-script-lnd-pre-start/bin/lnd-pre-start line 12:
cat $X
    ^-- SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
cat "$X"

But the test is not failing, when I add a vm argument:

./run-tests.sh -s default vm

Is this expected behavior?

Thanks.

[erikarvstedt]

Is this expected behavior?

Yes, service shellcheck is only run for the VM test that's run as a Nix build.
The other test methods are for debugging runtime behavior and should build as quickly as possible.

[jonasnick]

ACK 67949a0

Thanks @seberm