| Age | Commit message (Collapse) | Author |
|---|
|
Deduplicates code in the `FatalError` template function by using
`AbortNode` which does the exact same thing if called without any user
message (i.e. without second parameter specified). The template is still
kept for ease-of-use w.r.t. not having to call `tfm::format(...)` at the
call-side each time, and also to keep the diff minimal.
|
|
The 'm_synced' flag enables 'BlockConnected' events to be processed by
the index. If we set the flag before calling 'CustomInit', we could be
dispatching a block connected event to an uninitialized index child
class.
e.g. BlockFilterIndex, initializes the next filter position
inside 'CustomInit'. So, if `CustomInit` is not called prior receiving
the block event, the index will use 'next_filter_position=0' which
overwrites the first filter in disk.
|
|
This is achieved by letting the index sync thread wait until
reindex-chainstate is finished.
This also disables the pruning check when reindexing the chainstate (which is
incompatible with prune mode) because there would be no chain at this point
in init.
|
|
The index sync code has logic to go back the chain to the forking point, while
also updating index-specific state, which is necessary to prevent
possible corruption of the coinstatsindex.
Also add a test for this (a reorg happens while the index is deactivated)
that would not pass before this change.
|
|
This is a commit in preparation for the next few commits. The functions
are moved to methods to avoid their re-declaration for the purpose of
passing in BlockManager options.
The functions that were now moved into the BlockManager should no longer
use the params as an argument, but instead use the member variable.
In the moved ReadBlockFromDisk and UndoReadFromDisk, change
the function signature to accept a reference to a CBlockIndex instead of
a raw pointer. The pointer is expected to be non-null, so reflect that
in the type.
To allow for the move of functions to BlockManager methods all call
sites require an instantiated BlockManager, or a callback to one.
|
|
This is an extraction of ArgsManager related functions from util/system
into their own common file.
Config file related functions are moved to common/config.cpp.
The background of this commit is an ongoing effort to decouple the
libbitcoinkernel library from the ArgsManager. The ArgsManager belongs
into the common library, since the kernel library should not depend on
it. See doc/design/libraries.md for more information on this rationale.
|
|
error is a low-level function with a sole dependency on LogPrintf, which
is defined in logging.h
The background of this commit is an ongoing effort to decouple the
libbitcoinkernel library from the ArgsManager defined in system.h.
Moving the function out of system.h allows including it from a separate
source file without including the ArgsManager definitions from system.h.
|
|
This is only used in the current PR to avoid ugly
`strprintf(Untranslated("%s:\n%s"), str, MakeUnorderedList(details)`
boilerplate in init code. But in the future the function could be extended and
more widely used to include more details in GUI error messages or display them
in a more readable way, see code comment.
|
|
Add DBParams and DBOptions structs to remove ArgsManager uses from dbwrapper.
To reduce size of this commit, this moves references to gArgs variable out of
dbwrapper.cpp to calling code in txdb.cpp. But these moves are temporary. The
gArgs references in txdb.cpp are moved out to calling code in init.cpp in later
commits.
This commit does not change behavior.
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
- 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7
- 2020: fa0074e2d82928016a43ca408717154a1c70a4db
- 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
Since commit f08c9fb0c6a799e3cb75ca5f763a746471625beb from PR
https://github.com/bitcoin/bitcoin/pull/21726, index
`BlockUntilSyncedToCurrentChain` behavior has been less reliable, and there has
also been a race condition in the `coinstatsindex_initial_sync` unit test.
It seems better for `BlockUntilSyncedToCurrentChain` to actually wait for the
last connected block to be fully processed, than to be able to return before
prune locks are set, so this switches the order of `m_best_block_index =
block;` and `UpdatePruneLock` statements in `SetBestBlockIndex` to make it more
reliable.
Also since commit f08c9fb0c6a799e3cb75ca5f763a746471625beb, there has been a
race condition in the `coinstatsindex_initial_sync` test. Before that commit,
the atomic index best block pointer `m_best_block_index` was updated as the
last step of `BaseIndex::BlockConnected`, so `BlockUntilSyncedToCurrentChain`
could safely be used in tests to wait for the last `BlockConnected`
notification to be finished before stopping and destroying the index. But
after that commit, calling `BlockUntilSyncedToCurrentChain` is no longer
sufficient, and there is a race between the test shutdown code which destroys
the index object and the new code introduced in that commit calling
`AllowPrune()` and `GetName()` on the index object. Reproducibility
instructions for this are in
https://github.com/bitcoin/bitcoin/issues/25365#issuecomment-1259744133
This commit fixes the `coinstatsindex_initial_sync` race condition, even though
it will require an additional change to silence TSAN false positives,
https://github.com/bitcoin/bitcoin/pull/26188, after it is fixed. So this
partially addresses but does not resolve the bug reporting TSAN errors
https://github.com/bitcoin/bitcoin/issues/25365.
There is no known race condition outside of test code currently, because the
bitcoind `Shutdown` function calls `FlushBackgroundCallbacks` not
`BlockUntilSyncedToCurrentChain` to safely shut down.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
Co-authored-by: MacroFake <falke.marco@gmail.com>
|
|
|
|
11780f29e7c3f50fb7717fc98950ece9385d314b doc: BaseIndex sync behavior with empty datadir (James O'Beirne)
Pull request description:
Make a note about a potentially confusing behavior with `BaseIndex::m_synced`;
if the user starts bitcoind with an empty datadir and an index enabled,
BaseIndex will consider itself synced (as a degenerate case). This affects
how indices are built during IBD (relying solely on BlockConnected signals vs.
using ThreadSync()).
ACKs for top commit:
mzumsande:
ACK 11780f29e7c3f50fb7717fc98950ece9385d314b
Tree-SHA512: 0b530379e57c62e05d2ddca7bb8e2c936786fa00678f9eaa1bb3742d957c48f141d46f936734b03f6673d964abc7eb72c1769f1784b9d3563d218e96296b7afd
|
|
Make a note about a potentially confusing behavior with `BaseIndex::m_synced`;
if the user starts bitcoind with an empty datadir and an index enabled,
BaseIndex will consider itself synced (as a degenerate case). This affects
how indices are built during IBD (relying solely on BlockConnected signals vs.
using ThreadSync()).
|
|
faf98aecf876fae0ec6d4d16b7e66f3a35253180 Remove unused includes in rpc/fees.cpp (MacroFake)
1111ddeedf7ea801507db4e23b4737ec183eb19c Remove unused includes from dbwrapper.h (MacroFake)
fa77fdd0475fa15a1a3641c5d5a2bf7ad095aa84 Add missing includes (MacroFake)
fa869ce2c2b906d8b087c4e7a5f1804a74b1c522 Add missing includes to node/chainstate (MacroFake)
Pull request description:
Unused includes are confusing, but also cause unrelated compile errors when the unused includes were to be removed.
Fix that by adding the missing includes where they are needed and then remove them where they are not needed. This is also checked by iwyu.
ACKs for top commit:
hebasto:
ACK faf98aecf876fae0ec6d4d16b7e66f3a35253180, I have reviewed the code and it looks OK, I agree it can be merged.
jarolrod:
Code Review ACK https://github.com/bitcoin/bitcoin/commit/faf98aecf876fae0ec6d4d16b7e66f3a35253180
Tree-SHA512: 75f3c6e6f6ecf8a98233e1a1463c75ca4e0eb3ec341150d274141072fe95413a3c2ec6386d1c527899cc63d43f63f5eb5991509847412773362808ddfb1bb435
|
|
They are needed, otherwise the next commit will not compile
|
|
Replace CommitInternal method with CustomCommit and use interfaces::Chain
instead of CChainState to generate block locator.
This commit does not change behavior in any way, except in the
(m_best_block_index == nullptr) case, which was added recently in
https://github.com/bitcoin/bitcoin/pull/24117 as part of an ongoing attempt to
prevent index corruption if bitcoind is interrupted during startup. New
behavior in that case should be slightly better than the old behavior (skipping
the entire custom+base commit now vs only skipping the base commit previously)
and this might avoid more cases of corruption.
|
|
Replace Rewind method with CustomRewind and pass block hashes and
heights instead of CBlockIndex* pointers
This commit does not change behavior in any way.
|
|
Replace WriteBlock method with CustomAppend and pass BlockInfo struct
instead of CBlockIndex* pointer
This commit does not change behavior in any way.
|
|
Replace overriden index Init() methods that use the best block
CBlockIndex* pointer with pure CustomInit() callbacks that are passed
the block hash and height.
This gets rid of more CBlockIndex* pointer uses so indexes can work
outside the bitcoin-node process. It also simplifies the initialization
call sequence so index implementations are not responsible for
initializing the base class.
There is a slight change in behavior here since now the best block
pointer is loaded and checked before the custom index init functions are
called instead of while they are called.
|
|
Passing abstract Chain interface will let indexes run in separate
processes.
This commit does not change behavior in any way.
|
|
-BEGIN VERIFY SCRIPT-
sed -i "s|node/ui_interface|node/interface_ui|g" $(git grep -l "node/ui_interface" ./src)
git mv src/node/ui_interface.cpp src/node/interface_ui.cpp
git mv src/node/ui_interface.h src/node/interface_ui.h
sed -i "s|BITCOIN_NODE_UI_INTERFACE_H|BITCOIN_NODE_INTERFACE_UI_H|g" src/node/interface_ui.h
-END VERIFY SCRIPT-
|
|
indexing
7171ebc7cbd911fa7ccad732ea7f73bce30928ee index: Don't commit a best block before indexing it during sync (Martin Zumsande)
Pull request description:
This changes the periodic commit of the best block during the index sync phase to use the already indexed predecessor of the current block index, instead of committing the current one that will only be indexed (by calling `WriteBlock()`) after committing the best block.
The previous code would leave the index database in an inconsistent state until the block is actually indexed - if an unclean shutdown happened at just this point in time, the index could get corrupted because at next startup, we'd assume that we have already indexed this block.
ACKs for top commit:
ryanofsky:
Code review ACK 7171ebc7cbd911fa7ccad732ea7f73bce30928ee. Looks great! Just commit message changes since last review
Tree-SHA512: a008de511dd6a1731b7fdf6a90add48d1e53f7f7d6402672adb83e362677fc5b9f5cd021d3111728cb41d73f1b9c2140db79d7e183df0ab359cda8c01b0ef928
|
|
Committing a block prior to indexing would leave the index database
in an inconsistent state until it is indexed, which could corrupt the
index in case of a unclean shutdown. Thus commit its predecessor.
Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
|
|
This refactors the sync variables to use `std::chrono::steady_clock`
as it is best suitable for measuring intervals.
|
|
instead of by pointer, so as to not accept a nullptr.
|
|
instead of a global
|
|
Prior to this change blocks could be pruned up to the last block before the blockfilterindex current best block.
|
|
|
|
Also report an error when coinstatsindex init fails.
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
* 2020: fa0074e2d82928016a43ca408717154a1c70a4db
* 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
fab6d6b2d154893ab422dda87f3535d42c3e06f4 Move pindexBestInvalid to ChainstateManager (MarcoFalke)
facd2137eceacb95e1f71c87ddc704d752b37272 Move m_failed_blocks to ChainstateManager (MarcoFalke)
fa47b5c100f81c65c15b5a6afaf6c91bc0861264 Move AcceptBlockHeader to ChainstateManager (MarcoFalke)
fa3d62cf7b3501a056b34c5458c14d2fe6a55bd7 Move FindForkInGlobalIndex from BlockManager to CChainState (MarcoFalke)
Pull request description:
Move globals or members of the wrong class to the right class.
ACKs for top commit:
naumenkogs:
ACK fab6d6b2d154893ab422dda87f3535d42c3e06f4
Sjors:
ACK fab6d6b2d154893ab422dda87f3535d42c3e06f4
shaavan:
ACK fab6d6b2d154893ab422dda87f3535d42c3e06f4
Tree-SHA512: 926cbdfa22838517497bacb79ed5f521f64117c2aacf96a0176f62831b4713314a32abc0213df5ee067edf63e4a4300f752a26006d36e5aab415bb91209a271f
|
|
The helper was moved in commit b026e318c39f59a06e29f1b25c7f577e01b25ccb,
which also mentioned that it could be moved to CChainState. So do that,
as the functionality is not block-storage related.
This also allows to drop one function argument.
|
|
These were suggested in review of #23365
Co-authored-by: John Newbery <john@johnnewbery.com>
|
|
This allows filters to be reconstructed when the best known block is
the Genesis block without needing to reindex.
It fixes Init errors seen in #23289.
|
|
|
|
|
|
-BEGIN VERIFY SCRIPT-
find_regex='((assert|CHECK_NONFATAL)\(std::addressof|TODO: REVIEW-ONLY)' \
&& git grep -l -E "$find_regex" -- . \
| xargs sed -i -E "/${find_regex}/d"
-END VERIFY SCRIPT-
|
|
|
|
|
|
|
|
792be53d3e9e366b9f6aeee7a1eeb912fa28062e refactor: Replace std::bind with lambdas (Hennadii Stepanov)
a508f718f3e087c96a306399582a85df2e1d53ae refactor: Use appropriate thread constructor (Hennadii Stepanov)
30e44482152488a78f2c495798a75e6f553dc0c8 refactor: Make TraceThread a non-template free function (Hennadii Stepanov)
Pull request description:
This PR does not change behavior.
Its goal is to improve readability and maintainability of the code.
ACKs for top commit:
jnewbery:
utACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
jonatack:
tACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
MarcoFalke:
cr ACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
Tree-SHA512: a03142f04f370f6bc02bd3ddfa870819b51740fcd028772241d68c84087f95a2d78207cbd5edb3f7c636fcf2d76192d9c59873f8f0af451d3b05c0cf9cf234df
|
|
|
|
Lambdas are shorter and more readable.
Changes are limited to std::thread ctor calls only.
|
|
Also it is moved into its own module.
|
|
Can be reviewed with the git options
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
|
|
|
|
|
