init: Remove auto-import of `bootstrap.dat` and associated code #17044

loadblock, not loadpath

fixed

remove "in"

fixed

Can use https://docs.python.org/3/reference/compound_stmts.html#with instead?

And the writing it like this, should make the test shorter in line count. https://github.com/bitcoin/bitcoin/blob/7b701fef58f627956d597817a1f9422edd890cdc/test/functional/test_framework/util.py#L306

Yes, that's much better, implemented that.

What is the point of blocksdir_path (new empty blocks dir), when the whole datadir is cleaned?

I'd even go further and not remove the whole datadir tree. Since that also deletes the debug log for debugging purposes.

You could spin up two nodes, and call setnetworkactive(False) on the second one. Then reboot the second one with the -loadblock arg here.

I was weighing different options initially and thought using just one node might be more economical. But I did not think about the logs. I am now using a second node.

I think you can't run these with the travis user, only with the docker root user.

I.e.

DOCKER_EXEC cp -R ../../contrib/ contrib/

Also, if those files are missing because they are not in the distdir, I'd prefer to add them instead of copying them manually in the ci

Yes, that seems cleaner, doing that instead now

Shouldn't you check the return code?

Also, does the > work on all OS flavors? It seems that appveyor windows is passing, so it seems fine!?

also, please use subprocess instead of os.system to invoke other processes from python

Also, does the > work on all OS flavors? It seems that appveyor windows is passing, so it seems fine!?

I did a brief check and did not find any issues concerning windows. I am on macOS and it's fine.

also, please use subprocess instead of os.system to invoke other processes from python

done

Shouldn't you check the return code?

done

You could remove this check, iterating an empty array is fine.

done

what does this comment mean? GetArgs takes care of the -noloadblock option

Also, why is this code block changed at all?

True, after implementing the feedback this doesn't do anything different aside from using AbsPathForConfigVal() which does not seem to be critical here.

why is this removed? I'd rather add a test for importing blk000.dat files.

@MarcoFalke: I suggested removing that above, because it seemed to imply a non-existing naming convention for the file (might as well load from bootstrap.dat or whatever your file is named).

See for example this test:

diff --git a/test/functional/feature_loadblock.py b/test/functional/feature_loadblock.py
index bf2a4ff61f..a532a83946 100755
--- a/test/functional/feature_loadblock.py
+++ b/test/functional/feature_loadblock.py
@@ -25,7 +25,7 @@ from test_framework.util import assert_equal, wait_until
 class LoadblockTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
-        self.num_nodes = 2
+        self.num_nodes = 3
 
     def run_test(self):
         self.nodes[1].setnetworkactive(state=False)
@@ -71,13 +71,16 @@ class LoadblockTest(BitcoinTestFramework):
         subprocess.run([sys.executable, linearize_data_file, cfg_file],
                        check=True)
 
-        self.log.info("Restart second, unsynced node with bootstrap file")
+        self.log.info("Restart unsynced nodes with bootstrap file or raw block file")
         self.stop_node(1)
         self.start_node(1, ["-loadblock=" + bootstrap_file])
-        wait_until(lambda: self.nodes[1].getblockcount() == 100)
+        self.stop_node(2)
+        self.start_node(2, ["-loadblock=" + os.path.join(blocks_dir, 'blk00000.dat')])
+        for i in [1, 2]:
+            wait_until(lambda: self.nodes[i].getblockcount() == 100)
 
-        assert_equal(self.nodes[1].getblockchaininfo()['blocks'], 100)
-        assert_equal(self.nodes[0].getbestblockhash(), self.nodes[1].getbestblockhash())
+            assert_equal(self.nodes[i].getblockchaininfo()['blocks'], 100)
+            assert_equal(self.nodes[0].getbestblockhash(), self.nodes[i].getbestblockhash())
 
 
 if __name__ == '__main__':

Agree with removing this. blkXXXXX.dat are the block files and the way to (re)index them is -reindex, it has nothing to do with -loadblock.

Isn't the point that you can import the block.dat files from another datadir with -loadblock=blk001.dat -loadblock=blk002.dat -loadblock=...

I don't think that's the point. In that case it's less hassle to copy over all the blk files and do a reindex.

I think the idea behind -loadblock is to import the output from linearize; huge consecutive 'block archives' (there used to be torrents for this). But I might be wrong. I'm not sure what it's used for nowadays.

Oh, so it is multi-arg to support a split-up bootstrap.dat?

Yeah, that was my understanding as well. But I did see it spelled out anywhere explicitly.

Oh, so it is multi-arg to support a split-up bootstrap.dat?

I think so; linearize splits it into 1GB files by default.