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

utils and libraries: Make 'blocksdir' always net specific #14409

Merged
merged 2 commits into from Jan 16, 2019

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Oct 5, 2018

The blocks directory is net specific by definition.

Also this prevents the side effect of calling GetBlocksDir(false) in the non-mainnet environment.
Currently a new node creates an unused blocks\ directory in the root of the data directory when -testnet or -regtest is specified.

Refs:

@hebasto hebasto changed the title utils and libraries: Make blockdir always net specific utils and libraries: Make 'blocksdir' always net specific Oct 5, 2018
@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 5, 2018

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #13746 (-masterdatadir for datadir bootstrapping by kallewoof)

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.

@ken2812221
Copy link
Contributor

Concept ACK. This seem to require a release note.

@maflcko
Copy link
Member

maflcko commented Oct 9, 2018

Needs tests before merge

@meshcollider
Copy link
Contributor

Concept ACK, so this only affects the case where the user has explicitly specified a directory with the -blocksdir argument right?

@hebasto
Copy link
Member Author

hebasto commented Oct 12, 2018

@meshcollider

Thank you for your review.

Concept ACK, so this only affects the case where the user has explicitly specified a directory with the -blocksdir argument right?

This PR works regardless of the -blocksdir option.

@sipa
Copy link
Member

sipa commented Oct 12, 2018

Does this risk breaking any existing configurations?

@meshcollider
Copy link
Contributor

meshcollider commented Oct 12, 2018

@sipa that was my concern, the actual behaviour looks unchanged but I think now if you specify a base directory with -blocksdir it will error if the child directory doesn't exist, previously it would just create it? Which could be annoying for users...

@hebasto
Copy link
Member Author

hebasto commented Oct 13, 2018

@meshcollider @sipa

... I think now if you specify a base directory with -blocksdir it will error if the child directory doesn't exist, previously it would just create it?

Test of this PR:

$ rm -rf /home/hebasto/.bitcoin/
$ /home/hebasto/github/bitcoin/src/bitcoind
2018-10-13T09:27:14Z Bitcoin Core version v0.17.99.0-c1a60d4a1 (release build)
...
2018-10-13T09:27:30Z Shutdown: done
$ find /home/hebasto/.bitcoin/ > ~/pr14409-without-blocksdir

$ rm -rf /home/hebasto/.bitcoin/
$ /home/hebasto/github/bitcoin/src/bitcoind -blocksdir=/home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR
2018-10-13T09:33:59Z Bitcoin Core version v0.17.99.0-c1a60d4a1 (release build)
2018-10-13T09:33:59Z InitParameterInteraction: parameter interaction: -whitelistforcerelay=1 -> setting -whitelistrelay=1
2018-10-13T09:33:59Z Error: Specified blocks directory "/home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR" does not exist.
Error: Specified blocks directory "/home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR" does not exist
$ mkdir -p /home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR
$ /home/hebasto/github/bitcoin/src/bitcoind -blocksdir=/home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR
2018-10-13T09:36:37Z Bitcoin Core version v0.17.99.0-c1a60d4a1 (release build)
...
2018-10-13T09:37:00Z Shutdown: done
$ find /home/hebasto/.bitcoin/ > ~/pr14409-with-blocksdir

$ diff ~/pr14409-without-blocksdir ~/pr14409-with-blocksdir 
3a4,8
> /home/hebasto/.bitcoin/TEST_DIR
> /home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR
> /home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR/blocks
> /home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR/blocks/rev00000.dat
> /home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR/blocks/blk00000.dat
12d16
< /home/hebasto/.bitcoin/blocks/rev00000.dat
18d21
< /home/hebasto/.bitcoin/blocks/blk00000.dat

Test of master be99270:

$ rm -rf /home/hebasto/.bitcoin/
$ /home/hebasto/github/bitcoin/src/bitcoind
2018-10-13T10:19:33Z Bitcoin Core version v0.17.99.0-be992701b (release build)
...
2018-10-13T10:19:51Z Shutdown: done
$ find /home/hebasto/.bitcoin/ > ~/master-be992701b-without-blocksdir

$ rm -rf /home/hebasto/.bitcoin/
$ /home/hebasto/github/bitcoin/src/bitcoind -blocksdir=/home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR
2018-10-13T10:22:30Z Bitcoin Core version v0.17.99.0-be992701b (release build)
2018-10-13T10:22:30Z InitParameterInteraction: parameter interaction: -whitelistforcerelay=1 -> setting -whitelistrelay=1
2018-10-13T10:22:30Z Error: Specified blocks directory "/home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR" does not exist.
Error: Specified blocks directory "/home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR" does not exist.
$ mkdir -p /home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR
$ /home/hebasto/github/bitcoin/src/bitcoind -blocksdir=/home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR
2018-10-13T10:23:18Z Bitcoin Core version v0.17.99.0-be992701b (release build)
...
2018-10-13T10:23:35Z Shutdown: done
$ find /home/hebasto/.bitcoin/ > ~/master-be992701b-with-blocksdir

$ diff ~/master-be992701b-without-blocksdir ~/master-be992701b-with-blocksdir 
3a4,8
> /home/hebasto/.bitcoin/TEST_DIR
> /home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR
> /home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR/blocks
> /home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR/blocks/rev00000.dat
> /home/hebasto/.bitcoin/TEST_DIR/TEST_BLOCKS_DIR/blocks/blk00000.dat
12d16
< /home/hebasto/.bitcoin/blocks/rev00000.dat
18d21
< /home/hebasto/.bitcoin/blocks/blk00000.dat

The behaviour is unchanged. If you specify a base directory with -blocksdir and if the child directory doesn't exist it will an error both on master and on this PR.

The blocks directory is net specific by definition.

Also this prevents the side effect of calling GetBlocksDir(false) in the
non-mainnet environment.
@hebasto hebasto force-pushed the 20181005-blockspath-always-netspecific branch from c1a60d4 to c3f1821 Compare November 5, 2018 11:27
@hebasto
Copy link
Member Author

hebasto commented Nov 5, 2018

Rebased.

@laanwj
Copy link
Member

laanwj commented Nov 23, 2018

This absolutely needs testing in a functional test.

Also to make it clear what behavior is failing right now, and which will pass after this.

A new node should not create an unused `blocks` directory in the root of
the data directory when `-testnet` or `-regtest` is specified.
@hebasto
Copy link
Member Author

hebasto commented Nov 30, 2018

@laanwj Thank you for your review.

This absolutely needs testing in a functional test.
Also to make it clear what behavior is failing right now, and which will pass after this.

The existed feature_blocksdir.py functional test has been patched. Currently this patched test fails on master.

@laanwj
Copy link
Member

laanwj commented Dec 13, 2018

utACK e4a0c35

1 similar comment
@maflcko
Copy link
Member

maflcko commented Dec 22, 2018

utACK e4a0c35

@laanwj laanwj merged commit e4a0c35 into bitcoin:master Jan 16, 2019
laanwj added a commit that referenced this pull request Jan 16, 2019
e4a0c35 Improve blocksdir functional test. (Hennadii Stepanov)
c3f1821 Make blockdir always net specific (Hennadii Stepanov)

Pull request description:

  The blocks directory is net specific by definition.

  Also this prevents the side effect of calling `GetBlocksDir(false)` in the non-mainnet environment.
  Currently a new node creates an unused `blocks\` directory in the root of the data directory when `-testnet` or `-regtest` is specified.

  Refs:
  - #12653
  - #12653 (comment) by @laanwj
  - #14595 (comment)

Tree-SHA512: c9957a68a4a200ebd2010823a56db7e61563afedcb7c9828e86b13f3af2990e07854b622c1f3374756f94574acb3ea32de7d2a399eef6c0623f0e11265155627
@hebasto hebasto deleted the 20181005-blockspath-always-netspecific branch January 16, 2019 20:16
@maflcko maflcko added this to the 0.17.2 milestone Feb 9, 2019
This was referenced Feb 19, 2019
hebasto added a commit to hebasto/bitcoin that referenced this pull request Feb 22, 2019
The blocks directory is net specific by definition.

Also this prevents the side effect of calling GetBlocksDir(false) in the
non-mainnet environment.

Github-Pull: bitcoin#14409
Rebased-From: c3f1821
hebasto added a commit to hebasto/bitcoin that referenced this pull request Feb 22, 2019
A new node should not create an unused `blocks` directory in the root of
the data directory when `-testnet` or `-regtest` is specified.

Github-Pull: bitcoin#14409
Rebased-From: e4a0c35
@fanquake
Copy link
Member

Being backported in #15445.

random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request May 1, 2021
… Allow to specify blocks storage directory

b0e0d55 doc: add '-blocksdir' option to release-notes (furszy)
50e66c2 Turn TryCreateDirectory() into TryCreateDirectories() (Marko Bencun)
035d3c2 util: Simplify path argument for CBlockTreeDB ctor (Hennadii Stepanov)
a571d24 Fail if either disk space check fails (Ben Woosley)
e567003 Improve blocksdir functional test. (Hennadii Stepanov)
ff0ae45 Make blockdir always net specific (Hennadii Stepanov)
13a4119 doc: Clarify -blocksdir usage (Daniel McNally)
a4ff899 -blocksdir: keep blockindex leveldb database in datadir (Jonas Schnelli)
3054076 QA: Add -blocksdir test (Jonas Schnelli)
39b8127 Allow to optional specify the directory for the blocks storage (Jonas Schnelli)
a821b0b BugFix: Move TestingSetup::ECCVerifyHandle member to BasicTestingSetup. (furszy)
5844452 Remove unneeded GetTempPath, only use temp directory in unit tests (furszy)
7fc893b [Test] BasicTestingSetup constructor receiving the network param. (furszy)

Pull request description:

  Grouped two different, zero risk, modifications here to not have to create two/three small and straightforward PRs:

  #### 1) Unit Tests:

  * Fixing basic unit test setup issue, was not initializing the libsecp256k1 context. Thus why we have been forced to use the extended unit testing setup for almost every test case (which includes CConnman, scheduler, validation interface, etc.. that are not needed so often).
  * Refactor: `BasicTestingSetup` receiving the network in the constructor.

  #### 2) Back ported the ability to specify a custom directory for the blocks storage.

  > Since the actual block files taking up more and more space, it may be desirable to have them stored in a different location then the data directory (use case: SSD for chainstate, etc., HD for blocks).
  This PR adds a -blocksdir option that allows one to keep the blockfiles external from the data directory (instead of creating symlinks).

  * bitcoin#9895.
  * bitcoin#12653.
  * bitcoin#14364.
  * bitcoin#14409.
  * bitcoin#15124.
  * bitcoin#17059.

ACKs for top commit:
  Fuzzbawls:
    re-ACK b0e0d55
  random-zebra:
    utACK b0e0d55 and merging...

Tree-SHA512: 289e5b826c8fc23a5933a5b30e6c043f82bad827b10f3093cbc5869a9e278d7ad0a325dd19b5ad3c97f23a3dee69f5b0bee3869e486eeed5e8d8342a39919cf5
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 26, 2021
… specific

e4a0c35 Improve blocksdir functional test. (Hennadii Stepanov)
c3f1821 Make blockdir always net specific (Hennadii Stepanov)

Pull request description:

  The blocks directory is net specific by definition.

  Also this prevents the side effect of calling `GetBlocksDir(false)` in the non-mainnet environment.
  Currently a new node creates an unused `blocks\` directory in the root of the data directory when `-testnet` or `-regtest` is specified.

  Refs:
  - bitcoin#12653
  - bitcoin#12653 (comment) by @laanwj
  - bitcoin#14595 (comment)

Tree-SHA512: c9957a68a4a200ebd2010823a56db7e61563afedcb7c9828e86b13f3af2990e07854b622c1f3374756f94574acb3ea32de7d2a399eef6c0623f0e11265155627
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 27, 2021
… specific

e4a0c35 Improve blocksdir functional test. (Hennadii Stepanov)
c3f1821 Make blockdir always net specific (Hennadii Stepanov)

Pull request description:

  The blocks directory is net specific by definition.

  Also this prevents the side effect of calling `GetBlocksDir(false)` in the non-mainnet environment.
  Currently a new node creates an unused `blocks\` directory in the root of the data directory when `-testnet` or `-regtest` is specified.

  Refs:
  - bitcoin#12653
  - bitcoin#12653 (comment) by @laanwj
  - bitcoin#14595 (comment)

Tree-SHA512: c9957a68a4a200ebd2010823a56db7e61563afedcb7c9828e86b13f3af2990e07854b622c1f3374756f94574acb3ea32de7d2a399eef6c0623f0e11265155627
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 28, 2021
… specific

e4a0c35 Improve blocksdir functional test. (Hennadii Stepanov)
c3f1821 Make blockdir always net specific (Hennadii Stepanov)

Pull request description:

  The blocks directory is net specific by definition.

  Also this prevents the side effect of calling `GetBlocksDir(false)` in the non-mainnet environment.
  Currently a new node creates an unused `blocks\` directory in the root of the data directory when `-testnet` or `-regtest` is specified.

  Refs:
  - bitcoin#12653
  - bitcoin#12653 (comment) by @laanwj
  - bitcoin#14595 (comment)

Tree-SHA512: c9957a68a4a200ebd2010823a56db7e61563afedcb7c9828e86b13f3af2990e07854b622c1f3374756f94574acb3ea32de7d2a399eef6c0623f0e11265155627
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants