Age | Commit message (Collapse) | Author |
|
The counter is an optimization over calling `ret.empty()`. It was
suggested that the compiler would realize `cnt` is only `0` on the first
iteration, and not actually emit the check and conditional.
This optimization was actually not triggered at all, since we
incremented `cnt` at the beginning of the first iteration. Fix it by
incrementing at the end instead.
This was reported by Github user "Janus".
|
|
fa57c449cf45a4f1df195970c711bba8f02f3cc6 fuzz: Remove no-op SetMempoolConstraints (MacroFake)
Pull request description:
Now that the mempool no longer uses the args manager (after commit e4e201dfd9a9dbd8e22cac688dbbde16234cd937), there is no point setting the mempool limits after it is constructed.
Fix that by setting them once right before the mempool is constructed.
ACKs for top commit:
dongcarl:
utACK fa57c449cf45a4f1df195970c711bba8f02f3cc6
glozow:
utACK fa57c449cf45a4f1df195970c711bba8f02f3cc6
Tree-SHA512: d236f9cdcee8c2076272b82c97f8a5942f1ecf119ab36edafd42088ef97554592348a61e1fbe504fd52b30301ef0177813042599ad12e8cb95b4a20586c85bb0
|
|
UniValue::VNULL
fa28d0f3c3fe528dae7fd6dc7725219b9bdf0e1b scripted-diff: Replace NullUniValue with UniValue::VNULL (MacroFake)
fa962103e8eb0b078b83943a21831be39e7716c9 fuzz: refactor: Replace NullUniValue with UniValue{} (MacroFake)
Pull request description:
This refactor is needed to disable the (potentially expensive for large json) UniValue copy constructors.
ACKs for top commit:
fanquake:
ACK fa28d0f3c3fe528dae7fd6dc7725219b9bdf0e1b
Tree-SHA512: 7d4204cce0a6fc4ecda96973de77d15b7e4c7caa3e0e890e1f5b9a4b9ace8b240b1f7565d6ab586e168a5fa1201b6c60a924868ef34d6abfbfd8ab7f0f99fbc7
|
|
850b0850ccacc4e4f7e82ce2291a111132eae756 fix comment spellings from the codespell lint (Greg Weber)
Pull request description:
test/lint/all-lint.py includes the codespell lint
ACKs for top commit:
aureleoules:
ACK 850b0850ccacc4e4f7e82ce2291a111132eae756.
Tree-SHA512: bf63690da2652886e705d6594903bab67ff0f35a0e5a5505f063827f5148ebce47681e541cbe0e52396baf1addb25d9fe50e5faa9176456f579a7cd2f1321c44
|
|
test/lint/all-lint.py includes the codespell lint
|
|
4fa79837ad19fada3a3df3fb490617f6ca4606e0 psbt: Fix unsigned integer overflow (Aurèle Oulès)
Pull request description:
Fixes #25692.
This change prevents an unsigned integer overflow during the deserialization of a PSBT.
ACKs for top commit:
achow101:
ACK 4fa79837ad19fada3a3df3fb490617f6ca4606e0
Tree-SHA512: 0863d4d31ada1ba50632b6a66cb4c694c0a15680a90cf9370129cf3db15e3c10e65610b779db047d5a4cc7c920708b728948708e4023e916099c6bfe730f01f9
|
|
|
|
This is required for removing the UniValue copy constructor.
-BEGIN VERIFY SCRIPT-
sed -i 's/return NullUniValue/return UniValue::VNULL/g' $(git grep -l NullUniValue ':(exclude)src/univalue')
-END VERIFY SCRIPT-
|
|
This is needed for the scripted-diff to compile in the next commit
|
|
safety, consistent behavior
3a61fc56a0ad6ed58570350dcfd9ed2d10239b48 refactor: move CBlockIndex#ToString() from header to implementation (Jon Atack)
57865eb51288852c3ce99607eff76c61ae5f5365 CDiskBlockIndex: rename GetBlockHash() to ConstructBlockHash() (Jon Atack)
99e8ec8721a52cd08bdca31f6e926c9c1ce281fb CDiskBlockIndex: remove unused ToString() class member (Jon Atack)
14aeece462b149eaf0d28a37d55cc169df99b2cb CBlockIndex: ensure phashBlock is not nullptr before dereferencing (Jon Atack)
Pull request description:
Fix a few design issues, potential footguns and inconsistent behavior in the CBlockIndex and CDiskBlockIndex classes.
- Ensure phashBlock in `CBlockIndex#GetBlockHash()` is not nullptr before dereferencing and remove a now-redundant assert preceding a GetBlockHash() caller. This protects against UB here, and in case of failure (which would indicate a consensus bug), the debug log will print `bitcoind: chain.h:265: uint256 CBlockIndex::GetBlockHash() const: Assertion 'phashBlock != nullptr' failed. Aborted` instead of `Segmentation fault`.
- Remove the unused `CDiskBlockIndex#ToString()` class member, and mark the inherited `CBlockIndex#ToString()` public interface member as deleted to disallow calling it in the derived CDiskBlockIndex class.
- Rename the `CDiskBlockIndex GetBlockHash()` class member to `ConstructBlockHash()`, which also makes sense as they perform different operations to return a blockhash, and mark the inherited `CBlockIndex#GetBlockHash()` public interface member as deleted to disallow calling it in the derived CDiskBlockIndex class.
- Move `CBlockIndex#ToString()` from header to implementation, which also allows dropping `tinyformat.h` from the header file.
Rationale and discussion regarding the CDiskBlockIndex changes:
Here is a failing test on master that demonstrates the inconsistent behavior of the current design: calling the same inherited public interface functions on the same CDiskBlockIndex object should yield identical behavior, but does not.
```diff
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index 6dc522b421..dac3840f32 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -240,6 +240,15 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup)
const CBlockIndex* tip = chainman.ActiveTip();
BOOST_CHECK_EQUAL(tip->nChainTx, au_data.nChainTx);
+ // CDiskBlockIndex "is a" CBlockIndex, as it publicly inherits from it.
+ // Test that calling the same inherited interface functions on the same
+ // object yields identical behavior.
+ CDiskBlockIndex index{tip};
+ CBlockIndex *pB = &index;
+ CDiskBlockIndex *pD = &index;
+ BOOST_CHECK_EQUAL(pB->GetBlockHash(), pD->GetBlockHash());
+ BOOST_CHECK_EQUAL(pB->ToString(), pD->ToString());
```
(build and run: `$ ./src/test/test_bitcoin -t validation_chainstatemanager_tests`)
The GetBlockHash() test assertion only passes on master because the different methods invoked by the current design happen to return the same result. If one of the two is changed, it fails like the ToString() assertion does.
Redefining inherited non-virtual functions is well-documented as incorrect design to avoid inconsistent behavior (see Scott Meyers, Effective C++, Item 36). Class usage is confusing when the behavior depends on the pointer definition instead of the object definition (static binding happening where dynamic binding was expected). This can lead to unsuspected or hard-to-track bugs.
Outside of critical hot spots, correctness usually comes before optimisation, but the current design dates back to main.cpp and it may possibly have been chosen to avoid the overhead of dynamic dispatch. This solution does the same: the class sizes are unchanged and no vptr or vtbl is added.
There are better designs for doing this that use composition instead of inheritance, or that separate the public interface from the private implementations. One example of the latter would be a non-virtual public interface that calls private virtual implementation methods, i.e. the Template pattern via the Non-Virtual Interface (NVI) idiom.
ACKs for top commit:
vasild:
ACK 3a61fc56a0ad6ed58570350dcfd9ed2d10239b48
Tree-SHA512: 9ff358ab0a6d010b8f053ad8303c6d4d061e62d9c3755a56b9c9f5eab855d02f02bee42acc77dfa0cbf4bb5cb775daa72d675e1560610a29bd285c46faa85ab7
|
|
integral type confusions
fa23c197509f692a815193acc1b50bad2fcbedfe univalue: Avoid narrowing and verbose int constructors (MacroFake)
fa3a9a1e8d9b6dffda772e97c279f3c0af6813f9 rpc: Select int-UniValue constructor for enum value in upgradewallet RPC (MacroFake)
Pull request description:
As UniValue provides several constructors for integral types, the
compiler is unable to select one if the passed type does not exactly
match. This is unintuitive for developers and forces them to write
verbose and brittle code. (Refer to `-Wnarrowing` compiler warning)
For example, there are many places where an unsigned int is cast to a
signed int. While the cast is safe in practice, it is still needlessly
verbose and confusing as the value can never be negative. In fact it
might even be unsafe if the unsigned value is large enough to map to a
negative signed one.
Fix this issue and other (minor) type issues.
ACKs for top commit:
aureleoules:
ACK fa23c197509f692a815193acc1b50bad2fcbedfe.
Tree-SHA512: 7d99b5b90c7d8eed2e3448167255a59e817dd6b8fcfc1b17c69ddefd0db33d1bf4344fbcd8b7f8685b58182c0f572ab9ffa99467afa666ac21843df7ea645033
|
|
7ab43eb8110af74e8f5be029118e19b39fe16125 test: remove unused if statements (Aurèle Oulès)
Pull request description:
This change removes two useless if statements in a functional test.
ACKs for top commit:
furszy:
Straightforward ACK 7ab43eb8,
Tree-SHA512: 56ff472f6f53f82d35dead7181dfefa9e7545dfb989e80fb750062a517f0f3c02882db6daa115f2d844f68fac9ce58170c340cf9c9989368419b02fa7f9790e3
|
|
scripts & fix getblock help
56d92447d0e75f459511bf48e105efae0dffc6b6 RPC: Document "asm" and "hex" fields for scripts (Luke Dashjr)
2cdd4df1406fc3ea892ecc29cd78fcded2ae4e10 Bugfix: RPC/blockchain: Correct type of "value" in getblock docs; add missing "desc" (Jon Atack)
Pull request description:
Inspired by #24718
ACKs for top commit:
kristapsk:
cr utACK 56d92447d0e75f459511bf48e105efae0dffc6b6
Tree-SHA512: 2c6d0291397929f6a76b2d2998789187da123d7bfcace77375331cb81995eb0afd2600286c1e25cf68d16e35bd58706d2f672f63a3febe5e3a556a668f2175a2
|
|
|
|
|
|
missing "desc"
|
|
|
|
`QInputDialog`
9d9a098530df9986039f64b2810b6375b715f196 gui: Fix translator comment for Restore Wallet QInputDialog (w0xlt)
Pull request description:
Fix translator comment for Restore Wallet `QInputDialog`, as suggested in https://github.com/bitcoin-core/gui/pull/471#discussion_r917437779.
This also changes the window title name from `Restore Name` to `Restore Wallet` as it seems clearer.
ACKs for top commit:
shaavan:
reACK 9d9a098530df9986039f64b2810b6375b715f196
Tree-SHA512: 02aec661839215ab1183e4e92fa131671daa986339373a87c0a0e2c5e79a46f362a8846f4a5f6d630a99884a7949031982d13352336bd3f0573625826406dde8
|
|
This also changes the window title name
from `Restore Name` to `Restore Wallet`.
|
|
which allows dropping tinyformat.h from the header file.
|
|
and mark the inherited CBlockIndex#GetBlockHash public interface member
as deleted, to disallow calling it in the derived CDiskBlockIndex class.
Here is a failing test on master demonstrating the inconsistent behavior of the
current design: calling the same inherited public interface functions on the
same CDiskBlockIndex object should yield identical behavior.
```diff
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index 6dc522b421..dac3840f32 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -240,6 +240,15 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_activate_snapshot, TestChain100Setup)
const CBlockIndex* tip = chainman.ActiveTip();
BOOST_CHECK_EQUAL(tip->nChainTx, au_data.nChainTx);
+ // CDiskBlockIndex "is a" CBlockIndex, as it publicly inherits from it.
+ // Test that calling the same inherited interface functions on the same
+ // object yields identical behavior.
+ CDiskBlockIndex index{tip};
+ CBlockIndex *pB = &index;
+ CDiskBlockIndex *pD = &index;
+ BOOST_CHECK_EQUAL(pB->GetBlockHash(), pD->GetBlockHash());
+ BOOST_CHECK_EQUAL(pB->ToString(), pD->ToString());
+
```
The GetBlockHash() test assertion only passes on master because the different
methods invoked by the current design happen to return the same result. If one
of the two is changed, it fails like the ToString() assertion does.
Redefining inherited non-virtual functions is well-documented as incorrect
design to avoid inconsistent behavior (see Scott Meyers, "Effective C++", Item
36). Class usage is confusing when the behavior depends on the pointer
definition instead of the object definition (static binding happening where
dynamic binding was expected). This can lead to unsuspected or hard-to-track
bugs.
Outside of critical hot spots, correctness usually comes before optimisation,
but the current design dates back to main.cpp and it may possibly have been
chosen to avoid the overhead of dynamic dispatch. This solution does the same:
the class sizes are unchanged and no vptr or vtbl is added.
There are better designs for doing this that use composition instead of
inheritance or that separate the public interface from the private
implementations. One example of the latter would be a non-virtual public
interface that calls private virtual implementation methods, i.e. the Template
pattern via the Non-Virtual Interface (NVI) idiom.
|
|
and mark its inherited CBlockIndex#ToString public interface member
as deleted, to disallow calling it in the derived CDiskBlockIndex class.
|
|
and remove a now-redundant assert preceding a GetBlockHash() caller.
This protects against UB here, and in case of failure (which would
indicate a consensus bug), the debug log will print
bitcoind: chain.h:265: uint256 CBlockIndex::GetBlockHash() const: Assertion `phashBlock != nullptr' failed.
Aborted
instead of
Segmentation fault
|
|
fad3c5826e6618fa1d58b9be48972f4ef03a2103 refactor: Fix iwyu on node/chainstate (MacroFake)
Pull request description:
Fix the CI warning on master: https://cirrus-ci.com/task/5398182703136768?logs=ci#L7020
ACKs for top commit:
fanquake:
ACK fad3c5826e6618fa1d58b9be48972f4ef03a2103 - could do chain.h
Tree-SHA512: 94f6ea0b3d9667863a4217b65bd1b9e07c65bdb566378faf0727bae5eb38d2d527ecae0c39efdda740b7ab7c8269141437ffbcb470cca7d559f09b8ee132d101
|
|
and use it where possible
faf9accd662974a69390213fee1b5c6237846b42 Use HashWriter where possible (MacroFake)
faa5425629d35708326b255570c51139aef0c8c4 Add HashWriter without ser-type and ser-version (MacroFake)
Pull request description:
This was done in the context of https://github.com/bitcoin/bitcoin/pull/25284 , but I think it also makes sense standalone.
The basic idea is that serialization type should not be initialized when it is not needed. Same for the serialization version.
So do this here for `HashWriter`. `CHashWriter` remains in places where it is not yet possible.
ACKs for top commit:
sipa:
utACK faf9accd662974a69390213fee1b5c6237846b42
Empact:
utACK https://github.com/bitcoin/bitcoin/pull/25331/commits/faf9accd662974a69390213fee1b5c6237846b42
Tree-SHA512: 544cc712436e49f6e608120bcd3ddc5ea72dd236554ce30fb6cfff34a92d7e67b6e6527336ad0f5b6365e2b2884f4c6508aef775953ccd9312f17752729703f2
|
|
a08c9723f50945edfc82ed9ae1d0106231c2babc contrib: remove unneeded valgrind suppressions (fanquake)
cc5b39e44ec2133df32e059bbe124e33c0a23401 ci: better pin to dwarf4 in valgrind job (fanquake)
Pull request description:
Prune some unneeded suppressions. Running either valgrind job locally these are no-longer needed.
Top commit has no ACKs.
Tree-SHA512: e191f121d545efb428fa1a0ca40f843593dd95e9895313d764364ed1fb409105a0ac264d1a67dc024ee241afa64a193a241d12be9abbe0549a24006fe845bd9c
|
|
|
|
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()).
|
|
|
|
Use `-gdwarf` and also set CFLAGS. I was seeing Valgrind issues otherwise.
|
|
check code
47ea70fbb85fefeb4de9d3142a11596d292eab9b wallet: clean AllInputsMine code, use InputIsMine internally (furszy)
bf310b0e8ce82d52bacceeb47c9f5dbb26885f7e wallet: clean InputIsMine code, use GetWalletTx (furszy)
0cb177263c36118094b7cd3b8f94741c0471ff62 wallet: unify CachedTxGetImmatureCredit and CachedTxGetImmatureWatchOnlyCredit (furszy)
04c6423f7b250ae1e51bb5cd159913e97494fb0e wallet: remove always true 'fUseCache' arg from CachedTxGetAvailableCredit (furszy)
4f0ca9bff6299353f595fe168dce720a96a91c41 wallet: remove always false 'recalculate' arg from GetCachableAmount (furszy)
47b1012677821ce2939e10ba462fbe53ffff17df wallet: remove always true 'fUseCache' from CachedTxGetImmatureWatchOnlyCredit (furszy)
da8f62de2c5561e091ef8073d6950c033f41aabf wallet: remove always true 'fUseCache' from CachedTxGetImmatureCredit (furszy)
Pull request description:
Another wallet's code garbage collector work. Part of the `mapWallet` encapsulation goal.
Focused on the following points:
1) Remove always true `fUseCache` argument from `CachedTxGetImmatureCredit`, `CachedTxGetImmatureWatchOnly` and `CachedTxGetAvailableCredit`.
2) Remove always false `recalculate` argument from `GetCachableAmount`.
3) Merge `CachedTxGetImmatureCredit` and `CachedTxGetImmatureWatchOnlyCredit` as they do share the exact same code.
4) Clean `InputIsMine` method; use `GetWalletTx` instead of access the wallet's map directly.
5) Clean `AllInputsMine` method; use `InputIsMine` instead of duplicate the exact same code internally.
ACKs for top commit:
aureleoules:
re-ACK 47ea70fbb85fefeb4de9d3142a11596d292eab9b
achow101:
ACK 47ea70fbb85fefeb4de9d3142a11596d292eab9b
theStack:
re-ACK 47ea70fbb85fefeb4de9d3142a11596d292eab9b
Tree-SHA512: e9b64b57de7be6165c5e5552e28cd8a03d4736b0a3707d29d129e3a0a3db6a855c2abf47a24917236060835a297b564a97b66d4c8b178d6bdafb93a12a7c0b40
|
|
preimage types
71a751f6c3e8912e1b1cfe388e593309d210e576 test: add test for decoding PSBT with per-input preimage types (Sebastian Falbesoner)
faf43378e223c563b0741c28a4b5406f471c1332 refactor: move helper `random_bytes` to util library (Sebastian Falbesoner)
fdc1ca389646a55c4d9cb2a79feaa69f90b18c67 test: add constants for PSBT key types (BIP 174) (Sebastian Falbesoner)
1b035c03f9fbbdf7a13663a35d75fb2428f44743 refactor: move PSBT(Map) helpers from signet miner to test framework (Sebastian Falbesoner)
7c0dfec2dd9998932d13dd183c3ce4b22bd5851b refactor: move `from_binary` helper from signet miner to test framework (Sebastian Falbesoner)
597a4b35f6e11d0ec5181e0d4d2d8f9bbf59898a scripted-diff: rename `FromBinary` helper to `from_binary` (signet miner) (Sebastian Falbesoner)
Pull request description:
This PR adds missing test coverage for the `decodepsbt` RPC in the case that a PSBT with on of the per-input preimage types (`PSBT_IN_RIPEMD160`, `PSBT_IN_SHA256`, `PSBT_IN_HASH160`, `PSBT_IN_HASH256`; see [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#Specification)) is passed. As preparation, the first four commits move the already existing helpers for (de)serialization of PSBTs and PSBTMaps from the signet miner to the test framework (in a new module `psbt.py`), which should be quite useful for further tests to easily create PSBTs.
ACKs for top commit:
achow101:
ACK 71a751f6c3e8912e1b1cfe388e593309d210e576
Tree-SHA512: 04f2671612d94029da2ac8dc15ff93c4c8fcb73fe0b8cf5970509208564df1f5e32319b53ae998dd6e544d37637a9b75609f27a3685da51f603f6ed0555635fb
|
|
f7dc99244c8e78dbd0196f612690efcc449c37dc compat: document redefining ssize_t when using MSVC (fanquake)
3be7ee750fd0d31d6e995140025e0d18e6aa788e compat: document error-code mapping (fanquake)
3f1d2fb035bf6413c33847326ac5938802cd5860 compat: document sockopt_arg_type definition (fanquake)
fb6db6fb0eb96f96dc331f565acaa8193f285ab2 compat: document S_I* defines when building for Windows (fanquake)
203e682d22a89af23dab21418e841e3b54b136d4 compat: extract and document MAX_PATH (fanquake)
b63ddb7e6d5c0463b4b8888ae015df87a381c0f6 compat: remove unused WSA* definitions (fanquake)
7c3df5e548ee3404d1ad5b47410dd7b6f77258d3 compat: document FD_SETSIZE redefinition for WIN32 (fanquake)
cc7b2fdd70da439c3cf8daef3bb79cf593f1cef3 refactor: move compat.h into compat/ (fanquake)
Pull request description:
Move `compat.h` into `compat/`, and document what is in there.
ACKs for top commit:
vasild:
ACK f7dc99244c8e78dbd0196f612690efcc449c37dc
hebasto:
re-ACK f7dc99244c8e78dbd0196f612690efcc449c37dc
Tree-SHA512: 9e7e90261a97eae7998ef8d140d8ffab504cccf19abb44ca253d8919a067bb01e3fa9876a44194a1a9fb08a4b0489376844f827d8a27aa66c0f99c60ad5d7041
|
|
|
|
The moved parts can be reviewed with "--color-moved=dimmed-zebra".
|
|
ChainImpl
fa32b1bbfd418410c47b2a33c6fa106371288138 refactor: Use chainman() helper consistently in ChainImpl (MacroFake)
Pull request description:
Doing anything else will just lead to more verbose and inconsistent code.
ACKs for top commit:
fanquake:
ACK fa32b1bbfd418410c47b2a33c6fa106371288138 - all instances of `Assert(m_node.chainman)` in node/interfaces replaced with `chainman()`, which is the same thing.
shaavan:
Code Review ACK fa32b1bbfd418410c47b2a33c6fa106371288138
Tree-SHA512: a417680f79c150e4431aa89bc9db79fdf2dd409419081eb243194837b4ab8d16434165393f39a157473802753843e8c5314ad05c569b4e9221ce29a9fd1cefb8
|
|
See:
https://docs.microsoft.com/en-us/windows/win32/winprog/windows-data-types#ssize_t
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html
|
|
See:
https://docs.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2
|
|
|
|
|
|
|
|
|
|
|
|
|
|
and use it where possible
facc2fa7b8a218a0df6a19772e1641ea68dda2e3 Use AutoFile where possible (MacroFake)
6666803c897e4ad27b45cb74e3a9aa74a335f1bf streams: Add AutoFile without ser-type and ser-version (MacroFake)
Pull request description:
This was done in the context of https://github.com/bitcoin/bitcoin/pull/25284 , but I think it also makes sense standalone.
The basic idea is that serialization type should not be initialized when it is not needed. Same for the serialization version.
So do this here for `AutoFile`. `CAutoFile` remains in places where it is not yet possible.
ACKs for top commit:
laanwj:
Code review ACK facc2fa7b8a218a0df6a19772e1641ea68dda2e3
fanquake:
ACK facc2fa7b8a218a0df6a19772e1641ea68dda2e3
Tree-SHA512: d82d024d55af57565ac53d9d1517afafc12b46964effba0332de62a6c77869356fa77f89e6d4834438fff44c45b64fccdf5a1358bfea03e28dfe55013b3c099d
|
|
parameters and return values
1e761a0169ebdbd3b5784af39fc2248b4546eeea ci: Enable IWYU in src/kernel directory (Ryan Ofsky)
6db6552377ad6316626b3ab8605a98f96f22c3d2 refactor: Reduce number of SanityChecks return values (Ryan Ofsky)
b3e7de7ee6efb186efc272855ff1af5d9254b971 refactor: Reduce number of LoadChainstate return values (Russell Yanofsky)
3b91d4b9947adbec74721f538e46c712db22587c refactor: Reduce number of LoadChainstate parameters (Russell Yanofsky)
Pull request description:
Replace long LoadChainstate parameters list with options struct. Replace long list of return values with simpler error strings.
No changes in behavior. Motivation is just to make libbitcoin_kernel API easier to use and more future-proof, and make internal code clearer and more maintainable.
ACKs for top commit:
MarcoFalke:
ACK 1e761a0169ebdbd3b5784af39fc2248b4546eeea 🕚
Tree-SHA512: 86f251ab820ca6664ade87ccac8330f79b0e48e26b98082f022f592ed1380f8eefc3cce260b85d5eea5d2f5f2531602e03d641e579c15684ecd9093b2aebcc58
|
|
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
|
|
Suggested https://github.com/bitcoin/bitcoin/pull/25308#discussion_r892505713
|
|
|