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

Add btcdctrl & generatetoaddress RPC #2257

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

linden
Copy link

@linden linden commented Sep 30, 2024

This PR adds btcdctrl, a process controller wrapping btcd and providing typed configuration on par with the cli. Easing both production and testing with sensible defaults, making btcd incredibly easy to run programmatically.

btcdctrl embeds an RPC client, both to ensure btcd is ready when Start is called and to lower the amount of boilerplate needed to start and connect. This somewhat overlaps with integration/rpctest but covers a larger amount of use-cases while overall becoming a simpler implementation.

I've also added generatetoaddress, an RPC method which makes testing much simpler - as you no longer have to run an additional wallet for managing funds. I'm happy to split this into another PR, maintainers choice.

Note: integration/rpctest has a race condition with the port allocation, an empty port is found then only used somewhat late in the start-up procedure - has been an issue for me, in more complex testing environments when multiple processes are looking for empty ports at the same time. I've fixed that here by using port :0 which allows the OS to allocate a free port.

removes the need to run dedicated wallet when testing on simnet/regtest.
makes it possible to import `config` into another package, `params` & `log` moved to avoid circular imports.
`btcdctrl` makes it easy to programmatically run `btcd` - with a defined configuration and sensible defaults, it eases testing and managing `btcd` in production.
@Roasbeef
Copy link
Member

Roasbeef commented Sep 30, 2024

This PR adds btcdctrl, a process controller wrapping btcd and providing typed configuration on par with the cli. Easing both production and testing with sensible defaults, making btcd incredibly easy to run programmatically.

What does this accomplish that btcctl or rpclient don't?

Can you explain the use case a bit more?

@Roasbeef
Copy link
Member

Re the current approach to gain a free port: https://github.com/btcsuite/btcd/pull/2071/files#r1431328709

I think we can make this approach and the normal ":0" approach both available via a config.

@linden
Copy link
Author

linden commented Sep 30, 2024

What does this accomplish that btcctl or rpclient don't?

Can you explain the use case a bit more?

@Roasbeef Sure, happy to. Our goal is to make running btcd simple, this materializes as a few things:

  1. Configuration: Define the flags, makes it easy to avoid type errors and spelling mistakes as they'll be caught at compile-time.
  2. Less Boilerplate: Ease the amount of boilerplate involved in starting up btcd and connecting via rpcclient, just 3 lines of code.
  3. Standards: we've also implemented identical packages for lnd and tapd, making it easy to run all 3 at once under the same interface.

Our use cases so far have been, unit testing for our wallet and managing tapd/btcd/lnd in a single process in production (i.e. ensure btcd is ready before launching lnd).

Re the current approach to gain a free port: https://github.com/btcsuite/btcd/pull/2071/files#r1431328709

I think we can make this approach and the normal ":0" approach both available via a config.

I agree, although we'd have to listen on port :0 inside the process itself to avoid another process binding to it in the meantime. I'd opt to replace the current implementation, as I'm unable to think of an advantage of :0 - @guggero's comment is only relevant if we're binding to the port before the process.

1. killing a process then waiting for it can return an error depending on state, instead just exit the loop.
2. websockets hang indefinitely every 20 tests or so, use HTTP POST instead.
3. handle the RPC/P2P message being the last line in the log.
was missed by tests as this only appears on long-lived processes, eventually the pipe will run out of space causing `btcd` to hang.
the client could exist but with an error case, this should be handled.
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.

2 participants