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

[Net] peers.dat, banlist.dat recreated when missing #7458

Merged
merged 1 commit into from Feb 4, 2016

Conversation

Projects
None yet
3 participants
@kirkalx
Contributor

kirkalx commented Feb 2, 2016

In response to issue #7452. Still leaves an error message for the missing file in the logs though.

My first PR, so be nice please :)

@kirkalx kirkalx changed the title from peers.dat, banlist.dat recreated when missing to [Net] peers.dat, banlist.dat recreated when missing Feb 2, 2016

@kirkalx

This comment has been minimized.

Show comment
Hide comment
@kirkalx

kirkalx Feb 2, 2016

Contributor

Sorry if I've messed this up with the 2nd commit, jumped the gun a little...

Contributor

kirkalx commented Feb 2, 2016

Sorry if I've messed this up with the 2nd commit, jumped the gun a little...

@laanwj laanwj added the P2P label Feb 2, 2016

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Feb 2, 2016

Member

Welcome @kirkalx

The number of commits doesn't matter; better too many than too few, you can always use git rebase -i and force-push to squash commits together, if that results in clearer or more atomic changes. Separating them out is a bit more tricky.

Looks good to me. utACK.

Member

laanwj commented Feb 2, 2016

Welcome @kirkalx

The number of commits doesn't matter; better too many than too few, you can always use git rebase -i and force-push to squash commits together, if that results in clearer or more atomic changes. Separating them out is a bit more tricky.

Looks good to me. utACK.

@kirkalx

This comment has been minimized.

Show comment
Hide comment
@kirkalx

kirkalx Feb 2, 2016

Contributor

Thanks Wladimir. I'll stop making changes now as I obviously need to learn a bit about GitHub.

Contributor

kirkalx commented Feb 2, 2016

Thanks Wladimir. I'll stop making changes now as I obviously need to learn a bit about GitHub.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Feb 3, 2016

Member

Tested ACK
Without the patch: no banlist.dat written
With patch:

$ mkdir /store2/tmp/testbtc4
$ src/bitcoind -datadir=/store2/tmp/testbtc4 -printtoconsole -disablewallet
2016-02-03 12:48:10 ERROR: Read: Failed to open file /store2/tmp/testbtc4/peers.dat
2016-02-03 12:48:10 Invalid or missing peers.dat; recreating
2016-02-03 12:48:11 init message: Loading banlist...
2016-02-03 12:48:11 ERROR: Read: Failed to open file /store2/tmp/testbtc4/banlist.dat
2016-02-03 12:48:11 Invalid or missing banlist.dat; recreating
^C
$ ls  /store2/tmp/testbtc4
banlist.dat  blocks  chainstate  debug.log  fee_estimates.dat  onion_private_key  peers.dat
$ src/bitcoind -datadir=/store2/tmp/testbtc4 -printtoconsole -disablewallet                                                           
2016-02-03 12:50:10 init message: Loading addresses...
2016-02-03 12:50:10 Loaded 141 addresses from peers.dat  1ms
2016-02-03 12:50:10 init message: Loading banlist...

Before merging, can you please squash these changes into one commit? e.g.

$ git rebase -i 196ad69

In the editor, replace the bottom three 'pick' with 'f', then save and exit.
Then, force-push the result to this branch with

git push -f origin master
Member

laanwj commented Feb 3, 2016

Tested ACK
Without the patch: no banlist.dat written
With patch:

$ mkdir /store2/tmp/testbtc4
$ src/bitcoind -datadir=/store2/tmp/testbtc4 -printtoconsole -disablewallet
2016-02-03 12:48:10 ERROR: Read: Failed to open file /store2/tmp/testbtc4/peers.dat
2016-02-03 12:48:10 Invalid or missing peers.dat; recreating
2016-02-03 12:48:11 init message: Loading banlist...
2016-02-03 12:48:11 ERROR: Read: Failed to open file /store2/tmp/testbtc4/banlist.dat
2016-02-03 12:48:11 Invalid or missing banlist.dat; recreating
^C
$ ls  /store2/tmp/testbtc4
banlist.dat  blocks  chainstate  debug.log  fee_estimates.dat  onion_private_key  peers.dat
$ src/bitcoind -datadir=/store2/tmp/testbtc4 -printtoconsole -disablewallet                                                           
2016-02-03 12:50:10 init message: Loading addresses...
2016-02-03 12:50:10 Loaded 141 addresses from peers.dat  1ms
2016-02-03 12:50:10 init message: Loading banlist...

Before merging, can you please squash these changes into one commit? e.g.

$ git rebase -i 196ad69

In the editor, replace the bottom three 'pick' with 'f', then save and exit.
Then, force-push the result to this branch with

git push -f origin master
@kirkalx

This comment has been minimized.

Show comment
Hide comment
@kirkalx

kirkalx Feb 3, 2016

Contributor

Thanks for the help with that Wladimir, seems to have worked. I'll try to come back next week and get rid of the error messages in the logs as well.

Contributor

kirkalx commented Feb 3, 2016

Thanks for the help with that Wladimir, seems to have worked. I'll try to come back next week and get rid of the error messages in the logs as well.

@laanwj laanwj merged commit c77c662 into bitcoin:master Feb 4, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Feb 4, 2016

Merge #7458: [Net] peers.dat, banlist.dat recreated when missing
c77c662 peers.dat, banlist.dat recreated when missing (kirkalx)

luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Feb 12, 2016

@laanwj laanwj removed the Needs backport label Sep 26, 2016

@laanwj laanwj added this to the 0.12.2 milestone Sep 26, 2016

@fanquake fanquake removed the Needs backport label Mar 7, 2018

zkbot added a commit to zcash/zcash that referenced this pull request Apr 5, 2018

Auto merge of #2812 - str4d:2074-store-banlist, r=<try>
banlist.dat: store banlist on disk

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6310
- bitcoin/bitcoin#6315
  - Only the new signal and net changes, no QT
- bitcoin/bitcoin#7350
- bitcoin/bitcoin#7458

Part of #2074.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment