test: added test coverage to loadtxoutset could not open file

[kevkevinpal]

The functional test coverage did not cover the rpc error of "Couldn't open file..." for loadtxoutset and this test adds coverage for it

This adds coverage to this line
https://github.com/bitcoin/bitcoin/blob/master/src/rpc/blockchain.cpp#L2777

[DrahtBot]

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	maflcko, rkrux, alfonsoromanz, tdb3, davidgumberg

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #29996 (test: Assumeutxo: import snapshot in a node with a divergent chain by alfonsoromanz)
• #29428 (test: Assumeutxo: snapshots with less work should not be loaded by hernanmarino)

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.

[maflcko]

ACK ee67bba

[rkrux]

ACK ee67bba

Make successful, unit and functional tests successful.
Short and sweet!

[alfonsoromanz]

ACK ee67bba. Code looks good to me. I also ran test/functional/feature_assumeutxo.py to make sure all tests passes, including this one.

[tdb3]

ACK for ee67bba

Thank you for increasing test coverage.
Ran a sanity check that created the .../invalid/path file from dumptxoutset and the test failed (as expected).
Left a minor nit.

[tdb3]

Minor nit:
Style guidelines prefer f'{}'. (https://github.com/bitcoin/bitcoin/blob/master/test/functional/README.md#style-guidelines)

e.g.

diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py
index a65a11a9b5..47f87297f3 100755
--- a/test/functional/feature_assumeutxo.py
+++ b/test/functional/feature_assumeutxo.py
@@ -156,7 +156,7 @@ class AssumeutxoTest(BitcoinTestFramework):
         self.log.info("Test bitcoind should fail when file path is invalid.")
         node = self.nodes[0]
         path = node.datadir_path / node.chain / "invalid" / "path"
-        assert_raises_rpc_error(-8, "Couldn't open file {} for reading.".format(path), node.loadtxoutset, path)
+        assert_raises_rpc_error(-8, f"Couldn't open file {path} for reading.", node.loadtxoutset, path)
 
     def run_test(self):
         """

[maflcko]

With 5 ACKs this nit does not seem worth it to address at this point. Can do in one of the other pulls that touch this file, in a follow-up?

[davidgumberg]

LGTM ACK ee67bba

This PR improves coverage of the loadtxoutset rpc, I tested by removing the error throw from blockchain.cpp and verifying that the test complains:

 FILE* file{fsbridge::fopen(path, "rb")};
 AutoFile afile{file};
 if (afile.IsNull()) {
-    throw JSONRPCError(
-        RPC_INVALID_PARAMETER,
-        "Couldn't open file " + path.utf8string() + " for reading.");
 }

+1 to @tdb3 style nit above

I thought for a bit about whether or not we should also check files with invalid path names or bad permissions, but I think this check sufficiently demonstrates that if a file passed to loadtxoutset cannot be opened for any reason at all, an error will be thrown.