aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2022-07-26refactor: Remove not needed std::maxMacroFake
2022-07-25Merge bitcoin/bitcoin#25349: CBlockIndex/CDiskBlockIndex improvements for ↵MacroFake
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
2022-07-25Merge bitcoin/bitcoin#25611: univalue: Avoid brittle, narrowing and verbose ↵fanquake
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
2022-07-25Merge bitcoin/bitcoin#25693: test: remove unused if statementsMacroFake
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
2022-07-25Merge bitcoin/bitcoin#25691: RPC: Document "asm" and "hex" fields for ↵MacroFake
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
2022-07-25test: remove unused if statementsAurèle Oulès
2022-07-25RPC: Document "asm" and "hex" fields for scriptsLuke Dashjr
2022-07-25Bugfix: RPC/blockchain: Correct type of "value" in getblock docs; add ↵Jon Atack
missing "desc"
2022-07-23Merge bitcoin-core/gui#629: Fix translator comment for Restore Wallet ↵Hennadii Stepanov
`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
2022-07-22gui: Fix translator comment for Restore Wallet QInputDialogw0xlt
This also changes the window title name from `Restore Name` to `Restore Wallet`.
2022-07-22refactor: move CBlockIndex#ToString() from header to implementationJon Atack
which allows dropping tinyformat.h from the header file.
2022-07-22CDiskBlockIndex: rename GetBlockHash() to ConstructBlockHash()Jon Atack
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.
2022-07-22CDiskBlockIndex: remove unused ToString() class memberJon Atack
and mark its inherited CBlockIndex#ToString public interface member as deleted, to disallow calling it in the derived CDiskBlockIndex class.
2022-07-22CBlockIndex: ensure phashBlock is not nullptr before dereferencingJon Atack
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
2022-07-22Merge bitcoin/bitcoin#25668: refactor: Fix iwyu on node/chainstateMacroFake
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
2022-07-22Merge bitcoin/bitcoin#25331: Add HashWriter without ser-type and ser-version ↵fanquake
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
2022-07-22Merge bitcoin/bitcoin#25662: contrib: prune valgrind suppressionsMacroFake
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
2022-07-21refactor: Fix iwyu on node/chainstateMacroFake
2022-07-21Merge bitcoin/bitcoin#22485: doc: BaseIndex sync behavior with empty datadirMacroFake
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
2022-07-21doc: BaseIndex sync behavior with empty datadirJames O'Beirne
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()).
2022-07-21contrib: remove unneeded valgrind suppressionsfanquake
2022-07-21ci: better pin to dwarf4 in valgrind jobfanquake
Use `-gdwarf` and also set CFLAGS. I was seeing Valgrind issues otherwise.
2022-07-20Merge bitcoin/bitcoin#25543: wallet: cleanup cached amount and input mine ↵Andrew Chow
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
2022-07-20Merge bitcoin/bitcoin#25625: test: add test for decoding PSBT with per-input ↵Andrew Chow
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
2022-07-20Merge bitcoin/bitcoin#25493: compat: document code in compat.hMacroFake
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
2022-07-20Use HashWriter where possibleMacroFake
2022-07-20Add HashWriter without ser-type and ser-versionMacroFake
The moved parts can be reviewed with "--color-moved=dimmed-zebra".
2022-07-20Merge bitcoin/bitcoin#25638: refactor: Use chainman() helper consistently in ↵MacroFake
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
2022-07-20compat: document redefining ssize_t when using MSVCfanquake
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
2022-07-20compat: document error-code mappingfanquake
See: https://docs.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2
2022-07-20compat: document sockopt_arg_type definitionfanquake
2022-07-20compat: document S_I* defines when building for Windowsfanquake
2022-07-20compat: extract and document MAX_PATHfanquake
2022-07-20compat: remove unused WSA* definitionsfanquake
2022-07-20compat: document FD_SETSIZE redefinition for WIN32fanquake
2022-07-20refactor: move compat.h into compat/fanquake
2022-07-20Merge bitcoin/bitcoin#25285: Add AutoFile without ser-type and ser-version ↵fanquake
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
2022-07-20Merge bitcoin/bitcoin#25308: refactor: Reduce number of LoadChainstate ↵MacroFake
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
2022-07-19Merge bitcoin/bitcoin#25645: refactor: Remove unused includes from dbwrapper.hfanquake
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
2022-07-19ci: Enable IWYU in src/kernel directoryRyan Ofsky
Suggested https://github.com/bitcoin/bitcoin/pull/25308#discussion_r892505713
2022-07-19refactor: Reduce number of SanityChecks return valuesRyan Ofsky
2022-07-19refactor: Reduce number of LoadChainstate return valuesRussell Yanofsky
2022-07-19refactor: Reduce number of LoadChainstate parametersRussell Yanofsky
2022-07-19Merge bitcoin/bitcoin#25494: indexes: Stop using node internal typesfanquake
7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd indexes, refactor: Remove CChainState use in index CommitInternal method (Ryan Ofsky) ee3a079fab2c33b4186b62ab822753954a4e545f indexes, refactor: Remove CBlockIndex* uses in index Rewind methods (Ryan Ofsky) dc971be0831959e7ee6a6df9e1aa46091351a8fb indexes, refactor: Remove CBlockIndex* uses in index WriteBlock methods (Ryan Ofsky) bef4e405f3de2718dfee279a9abff4daf016da26 indexes, refactor: Remove CBlockIndex* uses in index Init methods (Ryan Ofsky) addb4f2af183a25ce4a6b6485b5b49575a2ba31b indexes, refactor: Remove CBlockIndex* uses in coinstatsindex LookUpOne function (Ryan Ofsky) 33b4d48cfcdf145f49cb2283ac3e2936a4e23fff indexes, refactor: Pass Chain interface instead of CChainState class to indexes (Ryan Ofsky) a0b5b4ae5a24536d333cbce2ea584f2d935c651f interfaces, refactor: Add more block information to block connected notifications (Ryan Ofsky) Pull request description: Start transitioning index code away from using internal node types like `CBlockIndex` and `CChain` so index code is less coupled to node code and index code will later be able to stop locking cs_main and sync without having to deal with validationinterface race conditions, and so new indexes are easier to write and can run as plugins or separate processes. This PR contains the first 7 commits from https://github.com/bitcoin/bitcoin/pull/24230#issuecomment-1165625977 which have been split off for easier review. Previous review comments can be found in #24230 ACKs for top commit: MarcoFalke: ACK 7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd though did not review the last commit 🤼 mzumsande: Code Review ACK 7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd Tree-SHA512: f84ac2eb6dca2c305566ddeb35ea14d0b71c00860c0fd752bbcf1a0188be833d8c2a6ac9d3ef6ab5b46fbd02d7a24cbb8f60cf12464cb8ba208e22287f709989
2022-07-19Merge bitcoin/bitcoin#25513: psbt: Check Taproot tree depth and leaf versionsfanquake
76fb300b63c5c0d01d768510ec69d894820432fa psbt: Check Taproot tree depth and leaf versions (Andrew Chow) Pull request description: Since TaprootBuilder has assertions for the depth and leaf versions, the PSBT decoder should check these values before calling TaprootBuilder::Add so that the assertions are not triggered on malformed taproot trees. Fixes https://github.com/bitcoin/bitcoin/pull/22558#issuecomment-1170935136 ACKs for top commit: Sjors: utACK 76fb300b63c5c0d01d768510ec69d894820432fa sipa: utACK 76fb300b63c5c0d01d768510ec69d894820432fa w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25513/commits/76fb300b63c5c0d01d768510ec69d894820432fa Tree-SHA512: 94b288bc1453d10bce9a8a6389bc866f2c71c76579b7908e22d6b5770ac387086f6221af8597668e62977d4d6861fe2d72ec7b052002a2c36769d056b2e66360
2022-07-19Merge bitcoin/bitcoin#25643: depends: compile FastFixedDtoa with -O1 to fix ↵Andrew Chow
cross-arch reproducibility for arm32 c32fa85909dde8f7d61c2a291564f3f26884e170 depends: modify FastFixedDtoa optimisation flags (fanquake) Pull request description: This fixes a non-determinism issue in the asm produced for this function when cross-compiling on x86_64 and aarch64 for the arm-linux-gnueabihf HOST. Related to #21194. Alternative to #25636. Initial discussion in https://github.com/bitcoin/bitcoin/pull/24615#issuecomment-1080809879. Guix Build (x86_64): ```bash 28ae0ec2874ead334edd1c5dc509344379d82f7058b649c9076992defd7190d7 guix-build-c32fa85909dd/output/arm-linux-gnueabihf/SHA256SUMS.part 48d34073b029c4f62c8e1bd906533610228d5ca0bb5eefea6010dfa7372ba067 guix-build-c32fa85909dd/output/arm-linux-gnueabihf/bitcoin-c32fa85909dd-arm-linux-gnueabihf-debug.tar.gz 850d6e9859e88bcb93ed586bdb0c0bc95a44249d6a0ec1b1d13125cb9dd56413 guix-build-c32fa85909dd/output/arm-linux-gnueabihf/bitcoin-c32fa85909dd-arm-linux-gnueabihf.tar.gz b8bb092b1307684ea4b53d810ac110ec14f29eeab8028d924d1cac7418009b14 guix-build-c32fa85909dd/output/dist-archive/bitcoin-c32fa85909dd.tar.gz ``` Guix Build (arm64): ```bash 28ae0ec2874ead334edd1c5dc509344379d82f7058b649c9076992defd7190d7 guix-build-c32fa85909dd/output/arm-linux-gnueabihf/SHA256SUMS.part 48d34073b029c4f62c8e1bd906533610228d5ca0bb5eefea6010dfa7372ba067 guix-build-c32fa85909dd/output/arm-linux-gnueabihf/bitcoin-c32fa85909dd-arm-linux-gnueabihf-debug.tar.gz 850d6e9859e88bcb93ed586bdb0c0bc95a44249d6a0ec1b1d13125cb9dd56413 guix-build-c32fa85909dd/output/arm-linux-gnueabihf/bitcoin-c32fa85909dd-arm-linux-gnueabihf.tar.gz b8bb092b1307684ea4b53d810ac110ec14f29eeab8028d924d1cac7418009b14 guix-build-c32fa85909dd/output/dist-archive/bitcoin-c32fa85909dd.tar.gz ``` ACKs for top commit: achow101: ACK c32fa85909dde8f7d61c2a291564f3f26884e170 hebasto: ACK c32fa85909dde8f7d61c2a291564f3f26884e170 jarolrod: ACK c32fa85909dde8f7d61c2a291564f3f26884e170 Tree-SHA512: 137d76274b1421247f43e5f040b4b5c42473f94d734498c73ab40e580c47dfecbbf11f1a69c15a87d805d4b8e9ef1fd62cc1b69c0f1614c62ff3cba98b1733e8
2022-07-19test: add test for decoding PSBT with per-input preimage typesSebastian Falbesoner
2022-07-19refactor: move helper `random_bytes` to util librarySebastian Falbesoner
Can be easily reviewed with `--color-moved=dimmed-zebra`.
2022-07-19Merge bitcoin/bitcoin#25639: guix: Drop repetition of option's default valuefanquake
2ade04c0d9f8a18cd6f0c425289a016ad5b1e478 guix: Drop repetition of option's default value (Hennadii Stepanov) Pull request description: Split from bitcoin/bitcoin#25169. Guix builds on `x86_64`: ``` 492efd1debd9a8587754521aca7a7362338eabd1e96fbec21c89c3ba3c2607fd guix-build-2ade04c0d9f8/output/aarch64-linux-gnu/SHA256SUMS.part 22d1b357e984710fd6ebc9b2b636d129376f486039a12c87cbb56e4b9c35d9bc guix-build-2ade04c0d9f8/output/aarch64-linux-gnu/bitcoin-2ade04c0d9f8-aarch64-linux-gnu-debug.tar.gz 067e2efb51abc18afbd95d539cb300d63b7c7289d95e95fd3de889962c5835e9 guix-build-2ade04c0d9f8/output/aarch64-linux-gnu/bitcoin-2ade04c0d9f8-aarch64-linux-gnu.tar.gz 87f1bc63f0d98b6a1df0e5ebf6f89d9d12fe02761af88766d45a78e24a10ccb2 guix-build-2ade04c0d9f8/output/arm-linux-gnueabihf/SHA256SUMS.part 7e3dfcd0ec2d693f77b2711681155592cd00e22bf6bfca05a8efbd1d50225461 guix-build-2ade04c0d9f8/output/arm-linux-gnueabihf/bitcoin-2ade04c0d9f8-arm-linux-gnueabihf-debug.tar.gz c9d887e0839808426d6f9edf38a805ec72a44e759e3012e9b89435e59ba4fc0b guix-build-2ade04c0d9f8/output/arm-linux-gnueabihf/bitcoin-2ade04c0d9f8-arm-linux-gnueabihf.tar.gz f4634a8f9117d94f43ac26121755fc221e88c45d6a8f84c971911ff36bf8a897 guix-build-2ade04c0d9f8/output/arm64-apple-darwin/SHA256SUMS.part 639ccbd374500b6f75fa8968821ec643577846c6495e2d0910f6d9423099f899 guix-build-2ade04c0d9f8/output/arm64-apple-darwin/bitcoin-2ade04c0d9f8-arm64-apple-darwin-unsigned.dmg 728767f11990e5310ef94816ba11176caa4b42c4bea181cf10f3bae8d2cd70fc guix-build-2ade04c0d9f8/output/arm64-apple-darwin/bitcoin-2ade04c0d9f8-arm64-apple-darwin-unsigned.tar.gz dd3913946e02c895e932bfb7d621cb68ed26022d81d6b4ebf3c5927a86b86647 guix-build-2ade04c0d9f8/output/arm64-apple-darwin/bitcoin-2ade04c0d9f8-arm64-apple-darwin.tar.gz 0d69ada990f1a3f9b80d8fe495bb049297c496993b9994e276d97f8aeaecbceb guix-build-2ade04c0d9f8/output/dist-archive/bitcoin-2ade04c0d9f8.tar.gz 5b72d28a5b6eee9b184906efe4b774598a3a9bb24a5af71be72ee20175bcd24c guix-build-2ade04c0d9f8/output/powerpc64-linux-gnu/SHA256SUMS.part 5dc632d75d68cb6a4277c03dc1d0b0dfb64979a9689b20e1132f7f639158d9a7 guix-build-2ade04c0d9f8/output/powerpc64-linux-gnu/bitcoin-2ade04c0d9f8-powerpc64-linux-gnu-debug.tar.gz 32a69218d20f0f7e9dd55eb46e5d0b73aa70fb55718d79964acb4a58ee64109f guix-build-2ade04c0d9f8/output/powerpc64-linux-gnu/bitcoin-2ade04c0d9f8-powerpc64-linux-gnu.tar.gz 15431ebb9ccc413eab68e622c0ac9cc3360df52cd967e3ccba516d6b7bbc9ea1 guix-build-2ade04c0d9f8/output/powerpc64le-linux-gnu/SHA256SUMS.part f6bd41ee2b80ab8e7f78eb3071a8cda943061870d32fa5eefca042a3ef0e65f4 guix-build-2ade04c0d9f8/output/powerpc64le-linux-gnu/bitcoin-2ade04c0d9f8-powerpc64le-linux-gnu-debug.tar.gz bcb07c4d94dcd56fbd8b656bbd003441357eed9a4c6ec4a2ca1784ef8d986ef7 guix-build-2ade04c0d9f8/output/powerpc64le-linux-gnu/bitcoin-2ade04c0d9f8-powerpc64le-linux-gnu.tar.gz 8038401712d0283e4ef5d2933e54647c3505796c5b6b2ef4bb5c1fb6346301b4 guix-build-2ade04c0d9f8/output/riscv64-linux-gnu/SHA256SUMS.part 35df9ff846450ca571f05db1c07de6e06a14a9c7da50b30945231287a97e47f8 guix-build-2ade04c0d9f8/output/riscv64-linux-gnu/bitcoin-2ade04c0d9f8-riscv64-linux-gnu-debug.tar.gz 02d2356bb9492857b51a5c54bfc0e24f39a4e0ee95fb40366afe9250bb3fd60c guix-build-2ade04c0d9f8/output/riscv64-linux-gnu/bitcoin-2ade04c0d9f8-riscv64-linux-gnu.tar.gz 0dc09ec63e36a3cc4ad7151290e1f648aa99b184161831f48c519073f22a20e1 guix-build-2ade04c0d9f8/output/x86_64-apple-darwin/SHA256SUMS.part b5b8dded31154227bbdf30a4d97b695c2495b6c0ede7ff12ebaafafe47a47df0 guix-build-2ade04c0d9f8/output/x86_64-apple-darwin/bitcoin-2ade04c0d9f8-x86_64-apple-darwin-unsigned.dmg 2a9848487b55af9cf2359148f23d4dc5ff62f6adadf612cb0bd3539d9adcbdbe guix-build-2ade04c0d9f8/output/x86_64-apple-darwin/bitcoin-2ade04c0d9f8-x86_64-apple-darwin-unsigned.tar.gz d4d871f7df69eb172e625fd4170aa956c4f7cba92ae167ac0cbee3b0f381ce52 guix-build-2ade04c0d9f8/output/x86_64-apple-darwin/bitcoin-2ade04c0d9f8-x86_64-apple-darwin.tar.gz 5e05167a88a821953cc5028af9d3e0a80d606eaecf37f3fac01f95abc6161cc2 guix-build-2ade04c0d9f8/output/x86_64-linux-gnu/SHA256SUMS.part ab1d889b3e174428cdf71e41784c7778641bc7001abb88382c7b5d8016b5e08e guix-build-2ade04c0d9f8/output/x86_64-linux-gnu/bitcoin-2ade04c0d9f8-x86_64-linux-gnu-debug.tar.gz d196e8bddc633a08d70e61ec3fc5dacbf58195a91b9e2a20ea53c91b09d3b9eb guix-build-2ade04c0d9f8/output/x86_64-linux-gnu/bitcoin-2ade04c0d9f8-x86_64-linux-gnu.tar.gz ed2f15dadcc401d343869f06ed3c709484b126549aa2bd844ad9e37290d0789a guix-build-2ade04c0d9f8/output/x86_64-w64-mingw32/SHA256SUMS.part 319e59326a20bc206d3fc66439f3d246371ca5d9de17e9a18cc9ee8e39ce0e90 guix-build-2ade04c0d9f8/output/x86_64-w64-mingw32/bitcoin-2ade04c0d9f8-win64-debug.zip f955183729fd7eba43b3e4c637998bb6f2b1f30b37b8be13199ae4096a04d85c guix-build-2ade04c0d9f8/output/x86_64-w64-mingw32/bitcoin-2ade04c0d9f8-win64-setup-unsigned.exe 57fbfb0c00fbd8ba5b23ee2b08299c863d1a6ac9f6b49e1a336612b3027f97fb guix-build-2ade04c0d9f8/output/x86_64-w64-mingw32/bitcoin-2ade04c0d9f8-win64-unsigned.tar.gz 0582dd6ad0504c14692c47e721e51dc3b74b5d9e7c0f543b5a5a0965506b5a27 guix-build-2ade04c0d9f8/output/x86_64-w64-mingw32/bitcoin-2ade04c0d9f8-win64.zip ``` ACKs for top commit: fanquake: ACK 2ade04c0d9f8a18cd6f0c425289a016ad5b1e478 Tree-SHA512: 0c8214b884517794e257de31b8d226bc1d28c91deb4db1ce18affd9d792251b8181e65b2c14081d06408c444d9772279ee42cdfac7952b906b3c8e39bba6ce1e
2022-07-19Merge bitcoin/bitcoin#25630: Add symlinks for hardcoded Makefiles in out of ↵fanquake
tree builds 9aeeb75cf91fdfb03206284e900887211dc76ed3 Add symlinks for hardcoded Makefiles in out of tree builds (Pablo Greco) Pull request description: When doing out of tree builds, some hardwired Makefiles are not symlinked, which makes it a bit more uncomfortable to run some instances of make. There's no "real" functionality loss without this patch because the symlinked files are just for quick access to thinks in the main Makefile ACKs for top commit: hebasto: ACK 9aeeb75cf91fdfb03206284e900887211dc76ed3, tested on Ubuntu 22.04. Tree-SHA512: 656f73c387584cee34f66b3f95993267a40b915762949c7a84b73ba2ea8d37b7b5850733377110e0110ed2f7da64e6a5f9b303812080fe7815154dbb40c8a44c
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-18 01:41:55 +0000