I use this in #14912 in order to mock a hardware wallet. The functional first generates a signed PSBT which it writes to a file. The test node then calls a mock signer python script, which reads that file. The latter scripts gets it cwd from bitcoind.

Perhaps the tmp root dir of the functional tests runner is a better choice? cc @jnewbery

Why would you want to do this? Imo you can't rely a user is doing the same. I'd prefer to pass in the absulute paths from outside or specify with a flag that it should be run from relative to the datadir

@MarcoFalke this only affects the test suite. The mock signer script I created isn't itself part of the test suite, so it has no idea where everything lives on the system.

Note that the real thing (HWI) doesn't care where it's called from. It doesn't need to know where Bitcoin Core lives.

The latter scripts gets it cwd from bitcoind.

Why? Is that a requirement?

If you call that script from bitcoind, you might as well pass the datadir to the script?

@MarcoFalke that would involve something like this:

bitcoind -signer="/.../mock.py --mock_cwd=/tmp/.../node1/..".

That --mock_cwd=/.../ argument would then be passed along to all calls to mock.py and concatenated with the other arguments. That would probably work, but then it deviates from the way -signer should normally be used.

You can pass the boost::process::start_dir as datadir in runCommandParseJSON?

You can pass the boost::process::start_dir as datadir in runCommandParseJSON?

I don't want to modify the real world behaviour of runCommandParseJSON(). That should just use the cwd of however bitcoind was started, and that generally shouldn't be the datadir.

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #15419 (qa: Always refresh rotten cache to be out of ibd by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

I simplified it: it now merely sets the default cwd to the temp dir. Tests that need to override this can pass cwd= via args* to node.start(), which wallet_multiwallet.py already does.

It's now actually a bug fix: try creating a directory wallets in master and then run wallet_multiwallet.py; it will fail.

It's now actually a bug fix: try creating a directory wallets in master and then run wallet_multiwallet.py; it will fail.

Please elaborate. this should not happen

Before this PR the test suite would launch nodes with the cwd set to wherever you're launching the test suite from. wallet_multiwallet.py then does the following:

self.nodes[0].assert_start_raises_init_error(['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist')
self.nodes[0].assert_start_raises_init_error(['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir())
self.nodes[0].assert_start_raises_init_error(['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir())

That first test will fail if the directory you launch the test from contains a wallets directory.

This PR fixes that by setting cwd for each node to the temp directory. The second and third command show how to override that default.

The existing behavior of assert_start_raises_init_error is to pass extra arguments like cwd on to the subprocess call, which is a bit of a hack IMO, but this PR doesn't touch that.

utACK 2cb2f54

@MarcoFalke I suppose you've retracted your NACK above?

utACK e3e1a56

@MarcoFalke I suppose you've retracted your NACK above?

Yes, this is a bugfix as pointed out by @Sjors in an earlier comment

utACK e3e1a56