-
Notifications
You must be signed in to change notification settings - Fork 718
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
Refactor block file logic + util.h/cpp moved to util/system.h #2336
Refactor block file logic + util.h/cpp moved to util/system.h #2336
Conversation
c121a5f
to
5629d03
Compare
5629d03
to
9529162
Compare
Rebased after #2326 merge, ready to go. |
9529162
to
894890a
Compare
rebased on master so this work gets more test coverage. |
3a901d6
to
c4a798c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. ACK c4a798c
b9ec374
to
2e35dea
Compare
rebased due conflicts with a recently merged PR, ready to go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 2e35dea after rebase
2e35dea
to
b1a4b50
Compare
rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-re-utACK b1a4b50 after rebase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from introducing a couple compiler warnings that can be fixed directly here, or in a small followup PR,
ACK b1a4b50
src/validation.cpp
Outdated
} else | ||
return state.Error("out of disk space"); | ||
bool out_of_space; | ||
size_t bytes_allocated = BlockFileSeq().Allocate(pos, nAddSize, out_of_space); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bytes_allocated
is never used, because we don't have block pruning yet. This introduces a compiler warning that can be squelched by simply re-writing the line as follows until block pruning is introduced.
/*size_t bytes_allocated = */BlockFileSeq().Allocate(pos, nAddSize, out_of_space);
src/validation.cpp
Outdated
} else | ||
return state.Error("out of disk space"); | ||
bool out_of_space; | ||
size_t bytes_allocated = UndoFileSeq().Allocate(pos, nAddSize, out_of_space); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
/*size_t bytes_allocated = */UndoFileSeq().Allocate(pos, nAddSize, out_of_space);
-BEGIN VERIFY SCRIPT- sed -i 's/CDiskBlockPos/FlatFilePos/g' $(git ls-files 'src/*.h' 'src/*.cpp') -END VERIFY SCRIPT-
b1a4b50
to
3e48fc1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 3e48fc1
Merging.. |
…r_basic_data 23d31db [BUG][Tests] Properly set temporary datadir for dbwrapper_basic_data (random-zebra) Pull request description: Few cases left out in #2336 ACKs for top commit: Fuzzbawls: ACK 23d31db furszy: ACK 23d31db and merging Tree-SHA512: fac4fdd14b028520a473b234faac9e9472869ff1e97948c31af126554bc959ef6b920979e62036bb61189728635801367dc1fe35c8fdac828633fe95bc38d073
This is part of the block logic refactoring and encapsulation work coming from upstream (work started in #2326 and #2328 due the possible blocks db corruption issue).
Needed for two future works: (1) faster block validation using a new cache structure and (2) the future possible custom BIP157 (new sync protocol that supersedes BIP37 for light clients -- pending research needed after finishing v6.0 priorities --)
Essentially, it's encapsulating the sequenced files open/read/write functionalities inside a new
flatfile
module, and extending the unit test coverage to validate its correct behavior.Adapted the following PRs:
util.h/cpp
files mv toutil/system.h
&util/system.cpp
.