Related to #31144 (review)
Summary
WriteBatch
always returns true
- the errors are handled by throwing dbwrapper_error
instead.
Context
This boolean return value of the Write
methods is confusing because it’s inconsistent with CDBWrapper::Read
, which catches exceptions and returns a boolean to indicate success/failure. It’s bad that Read
returns and Write
throws - but it’s a lot worse that Write
advertises a return value when it actually communicates errors through exceptions.
Solution
This PR removes the constant return values from write methods and inlines true
at their call sites. Many upstream methods had boolean return values only because they were propagating these constants - those have been cleaned up as well.
Methods that returned a constant true
value that now return void
:
CDBWrapper::WriteBatch
,CDBWrapper::Write
,CDBWrapper::Erase
TxIndex::DB::WriteTxs
BlockTreeDB::WriteReindexing
,BlockTreeDB::WriteBatchSync
,BlockTreeDB::WriteFlag
BlockManager::WriteBlockIndexDB
Note
CCoinsView::BatchWrite
(and transitively CCoinsViewCache::Flush
& CCoinsViewCache::Sync
) were intentionally not changed here. While all implementations return true
, the base CCoinsView::BatchWrite
returns false
. Changing this would cause coins_view
tests to fail with:
terminating due to uncaught exception of type std::logic_error: Not all unspent flagged entries were cleared
We can fix that in a follow-up PR.