Making bitcoin backend commands chainable hooks

[andrewtoth]

It would be useful for plugins to be able to chain bitcoin backend commands.

For many plugin use cases, bcli is sufficient for 4 of the 5 commands. For instance, continue using bitcoin-cli but use a different sendrawtransaction implementation to send over tor. Or use bitcoin-cli, but substitute estimatefees to use mempool.space fee estimation.

However, it's not possible to substitute a single command, and instead bcli must be disabled completely. This forces the plugin author to rewrite the entire bcli functionality when all they care about is one command. With chaining, a plugin can be added to override just a single command. There could also be multiple plugins overriding the same command, and deferring to other plugins as fallbacks.

In #3354 there's a lot of discussion about implementing the commands as hooks. Moving the commands to hooks at this point will break many plugins. Could we make the rpc commands chainable, or check for both hooks and command registrations?

See also #3709.

[vincenzopalazzo]

This forces the plugin author to rewrite the entire bcli functionality when all they care about is one command. With chaining, a plugin can be added to override just a single command. There could also be multiple plugins overriding the same command and deferring to other plugins as fallbacks.

In my knowledge, the hook doesn't override the command itself, but the results of the command will pass also to the RPC command (before or after) run the command like sendrawtransaction. I'm misunderstanding the doc in some way?

For many plugin use cases, bcli is sufficient for 4 of the 5 commands. For instance, continue using bitcoin-cli but use a different sendrawtransaction implementation to send over tor. Or use bitcoin-cli, but substitute estimatefees to use mempool.space fee estimation.

Concept ack, I agree that some time have some custom behavior it is useful especially with bcli

[ZmnSCPxj]

Changing an RPC command should be doable by the rpc_command hook. You look at the method and check if it is one of the commands you are replacing, and if so, send back a return response. Just make sure that you never trigger an RPC command from rpc_command hook handling; return a value first. E.g. what you want can be done by a plugin registering on rpc_command, then if it sees a method of estimatefees, do an HTTPS request to mempool.space and return the values as expected in a return response.

CLBOSS uses rpc_command to monitor for sendpay attempts by the pay plugin, for example. I expect (but do not know for sure) that the backend code in lightningd should still pass through rpc_command hook.

Ah, #3709 proves me wrong, sorry. I think they should get onto rpc_command hook. By being commands, client code can also use the backend commands too, which might be useful in some cases (e.g. plugins do not need to be configured with Bitcoin RPC credentials separately, they just use the backend commands and have lightningd use its configured RPC credentials).

[andrewtoth]

In my knowledge, the hook doesn't override the command itself, but the results of the command will pass also to the RPC command (before or after) run the command like sendrawtransaction. I'm misunderstanding the doc in some way?

@vincenzopalazzo the rpc_command hook does indeed allow overriding the effects of the command. The hook is called before the command hits lightningd, and the plugin can then decide to respond to the command itself with custom behavior or pass the command to lightningd to perform the default behavior.

[andrewtoth]

I think they should get onto rpc_command hook.

@ZmnSCPxj but as #3709 says, they are not user commands, so shouldn't be passed into rpc_command. Since they are internal, I think they should get their own hooks. Since there are 5 of them, perhaps we can make a bitcoin_backend hook, so we don't confuse behavior from user commands and internal ones?

For instance, calling lightning-cli sendrawtransaction <txhex> does indeed call the rpc_command hook with "method": "sendrawtransaction". Calling lightning-cli withdraw <addr> <sats> does not, even though sendrawtransaction rpc is called from lightningd to the plugin internally.

[ZmnSCPxj]

I do not really see the point of separating "user commands". Ideally the RPC bus should be agnostic as to the source of the command, meaning rpc_command should see them whether it was from an external program or from lightningd. Consider that plugins issue "user commands" in this model. Nothing in lightningd design prevents any part of its code from eventually being moved into plugins (pay and autoclean were previously not plugins, but part of lightningd). In principle, even the onchain wallet can be in a plugin, rather than part of lightningd, and in that case it would issue commands that would look like "user commands" as well. So I think this "user commands" is a false dichotomy. Something needs a service from some piece of code, whether the client is human or another part of the lightningd codebase should be immaterial.

The CLBOSS example is instructive. It uses rpc_command to monitor sendpay attempts performed by the pay plugin so it can get statistics on the usefulness of its peers. If the pay command were part of lightningd and used an internal bus that did not pass through the rpc_command hook then CLBOSS would not be able to do that monitoring. And in principle it does not matter what triggered the sendpay, whether it is our builtin pay implementation or an external replacement pay that uses mincostflow and has ponies and rainbows. I think this "user commands" distinction is worse than useless.

[andrewtoth]

@ZmnSCPxj thanks for that insight. Investigating this further, it seems that commands sent from an external program or a plugin call rpc_command. When lightningd is sending directly to a plugin, rpc_command is not called.

Moving the 5 backend commands to instead go through the incoming request dispatch would solve this issue, and let any plugin handle only a specific backend command via rpc_command hook instead of disabling bcli completely. 👍

[andrewtoth]

Looking into this further, I don't think grouping the bitcoin backend commands with the rpc_command hook makes architectural sense. The rpc_command hook is triggered by commands coming into lightningd from another process, while the bitcoin backend commands are triggered internally by the lightningd process. Also, the rpc_command hook has options to return a custom error or return a replacement rpc command to execute which don't really make sense here.

I think this would be better accomplished with a new hook type, named something like bitcoin_backend, that passes the method name. It can either return a custom response, or defer to lightningd normal behavior which is to just call the method on the plugin that registered it.

[ZmnSCPxj]

Looking into this further, I don't think grouping the bitcoin backend commands with the rpc_command hook makes architectural sense.

I think it does --- in principle the entirety of lightningd sans the actual RPC bus can be put in plugins. That includes blockchain monitoring and onchain fund management. In that case, everything is a plugin, or in principle should be (consider that we even have rudimentary support for plugins storing data into the database, and with a better interface the DB can be moved to a plugin, too: you could emulate compare-and-exchange and have an interface which takes as argument 0 or more SELECT queries (the "compare") and 1 or more updating queries (the "exchange"), and if all the SELECT queries all result in 1 or more rows, only then will the updating queries be applied, to ensure atomicity of DB updates, for instance). Whether the behavior is inside the lightningd process or not should not be material at all; rpc_command should be triggered.

[andrewtoth]

I ended up creating a plugin that allows optionally compiling methods to replace bcli. This solves the issue for me, but it is unfortunately not built-in and bcli must be disabled. https://github.com/andrewtoth/rust-bcli