pyln: Add a fixture for createwallet

[jsarenik]

When testing against current master (209900), I got errors about a non-existing wallet.

$ pytest tests
...
E           bitcoin.rpc.JSONRPCError: {'code': -32601, 'message': 'Method not found (wallet method is disabled because no wallet is loaded)'}

Changelog-None

[cdecker]

Thanks for catching this so early 👍 I just tested it and it works as advertised, with one minor caveat: test_bitcoind_goes_backwards which restarts the bitcoind instance doesn't create/unlock the wallet after restarting, so maybe we need to add this in the BitcoinD.start() function as well.

[cdecker]

Hm, seems createwallet in BitcoinD.start() chokes if the wallet already exists. We should probably createwallet on first startup, and afterwards use loadwalletfor restarts. We can check if a wallet exists usinglistwallets`.

[jsarenik]

Still reading through git log and figuring out if we are not bending anything and why the error which I noticed appeared now (since createwallet RPC was added back in summer of 2018 as bitcoin/bitcoin#13058)

[jsarenik]

The last related upstream change seems to be bitcoin/bitcoin#15937 (was looking for createwallet in the git log)

[jsarenik]

Since then there were a couple of changes. I am retesting again against today's bitcoin master.

Yes, the same error happens on Bitcoin Core version v0.20.99.0-40aab35e9 but I am starting to guess it is rather bitcoin's bug.

[jsarenik]

I have submitted an issue to bitcoin: bitcoin/bitcoin#20051

[jsarenik]

Ah, too late :) It is in master for some days but I did not find it in the log looking for createwallet. bitcoin/bitcoin#15454

[jsarenik]

so the solution seems to be using -wallet argument when starting bitcoind

[jsarenik]

Adding -wallet works both with current bitcoind from master branch and v0.20.1

[jsarenik]

In the end I set it to wallet="" but feel free to set it to something instead.

[jsarenik]

Ah, and it still may end up being something else. Using -wallet="something" is considered to be a hack and in future versions it may not create a wallet if it does not exist. See bitcoin/bitcoin#20034

[cdecker]

ACK cd5508e

[cdecker]

Still hunting down some unrelated flaky tests, the failures in CI here are unrelated to this PR 👍

[jsarenik]

Just a note: it should be OK in long term when things stay as they are now:

  -wallet=<path>
       Specify wallet database path. Can be specified multiple times to load
       multiple wallets. Path is interpreted relative to <walletdir> if
       it is not absolute, and will be created if it does not exist (as
       a directory containing a wallet.dat file and log files). For
       backwards compatibility this will also accept names of existing
       data files in <walletdir>.)

(from bitcoind -help)

"load" and "will be created if it does not exist" are the important parts of this help message.

[jsarenik]

Still hunting down some unrelated flaky tests, the failures in CI here are unrelated to this PR

As long as it helps ... there is no hurry although someone who tests against current bitcoind may notice it already. Others after release of Bitcoin Core 0.21

[cdecker]

Got Travis working again, rebasing and merging as soon as it passes 👍

[cdecker]

ACK 3df395f

[jsarenik]

Found this in my reflog: https://github.com/jsarenik/lightning/commits/jsn/pyln_fixtures