aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2022-07-28Merge bitcoin/bitcoin#24584: wallet: avoid mixing different `OutputTypes` ↵Andrew Chow
during coin selection 71d1d13627ccd27319f347e2d8167c8fe8a433f4 test: add unit test for AvailableCoins (josibake) da03cb41a4ce15ebceee7fa4a4fdd2d3602fe284 test: functional test for new coin selection logic (josibake) 438e04845bf3302b7f459a50e88a1b772527f1e6 wallet: run coin selection by `OutputType` (josibake) 77b07072061c59f50c69be29fbcddf0d433e1077 refactor: use CoinsResult struct in SelectCoins (josibake) 2e67291ca3ab2d8f498fa910738ca655fde11c5e refactor: store by OutputType in CoinsResult (josibake) Pull request description: # Concept Following https://github.com/bitcoin/bitcoin/pull/23789, Bitcoin Core wallet will now generate a change address that matches the payment address type. This improves privacy by not revealing which of the outputs is the change at the time of the transaction in scenarios where the input address types differ from the payment address type. However, information about the change can be leaked in a later transaction. This proposal attempts to address that concern. ## Leaking information in a later transaction Consider the following scenario: ![mix input types(1)](https://user-images.githubusercontent.com/7444140/158597086-788339b0-c698-4b60-bd45-9ede4cd3a483.png) 1. Alice has a wallet with bech32 type UTXOs and pays Bob, who gives her a P2SH address 2. Alice's wallet generates a P2SH change output, preserving her privacy in `txid: a` 3. Alice then pays Carol, who gives her a bech32 address 4. Alice's wallet combines the P2SH UTXO with a bech32 UTXO and `txid: b` has two bech32 outputs From a chain analysis perspective, it is reasonable to infer that the P2SH input in `txid: b` was the change from `txid: a`. To avoid leaking information in this scenario, Alice's wallet should avoid picking the P2SH output and instead fund the transaction with only bech32 Outputs. If the payment to Carol can be funded with just the P2SH output, it should be preferred over the bech32 outputs as this will convert the P2SH UTXO to bech32 UTXOs via the payment and change outputs of the new transaction. **TLDR;** Avoid mixing output types, spend non-default `OutputTypes` when it is economical to do so. # Approach `AvailableCoins` now populates a struct, which makes it easier to access coins by `OutputType`. Coin selection tries to find a funding solution by each output type and chooses the most economical by waste metric. If a solution can't be found without mixing, coin selection runs over the entire wallet, allowing mixing, which is the same as the current behavior. I've also added a functional test (`test/functional/wallet_avoid_mixing_output_types.py`) and unit test (`src/wallet/test/availablecoins_tests.cpp`. ACKs for top commit: achow101: re-ACK 71d1d13627ccd27319f347e2d8167c8fe8a433f4 aureleoules: ACK 71d1d13627ccd27319f347e2d8167c8fe8a433f4. Xekyo: reACK 71d1d13627ccd27319f347e2d8167c8fe8a433f4 via `git range-diff master 6530d19 71d1d13` LarryRuane: ACK 71d1d13627ccd27319f347e2d8167c8fe8a433f4 Tree-SHA512: 2e0716efdae5adf5479446fabc731ae81d595131d3b8bade98b64ba323d0e0c6d964a67f8c14c89c428998bda47993fa924f3cfca1529e2bd49eaa4e31b7e426
2022-07-28Merge bitcoin/bitcoin#25674: add unit tests for RBF rules in isolationglozow
c320cddb1b57a9c9911054fc440f7a12aaea61b5 [unit tests] individual RBF Rules in isolation (glozow) Pull request description: Test each RBF rule more thoroughly and in isolation so we're not relying on things like overall mempool acceptance logic, ordering of mempool checks, RPC results, etc. RBF was pretty recently refactored out, so there isn't much unit test coverage. From https://marcofalke.github.io/btc_cov/test_bitcoin.coverage/src/policy/rbf.cpp.gcov.html: ![image](https://user-images.githubusercontent.com/25183001/180783280-6777f4b4-ef95-462a-b414-1a9e268836a6.png) ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/25674/commits/c320cddb1b57a9c9911054fc440f7a12aaea61b5 jonatack: ACK c320cddb1b57a9c9911054fc440f7a12aaea61b5 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25674/commits/c320cddb1b57a9c9911054fc440f7a12aaea61b5 Tree-SHA512: dab555214496255801b9ea92b7bf708bba1ff23edf055c85e29be5eab7d7a863440ee19588aacdce54b2c03feaa4b5963eb159ed89473560bd228737cbfec160
2022-07-28[unit tests] individual RBF Rules in isolationglozow
Test each component of the RBF policy in isolation. Unlike the RBF functional tests, these do not rely on things like RPC results, mempool submission, etc.
2022-07-27test: Drop unused boost workaroundHennadii Stepanov
2022-07-27Merge bitcoin/bitcoin#24697: refactor address relay timefanquake
fa64dd6673767992eb4e0e775fb0afdfd298610d refactor: Use type-safe std::chrono for addrman time (MarcoFalke) fa2ae373f33fa76dc4e435e7cb4778055aa6afd5 Add type-safe AdjustedTime() getter to timedata (MarcoFalke) fa5103a9f5f8559ab005c0b012d3d3a8057d81fb Add ChronoFormatter to serialize (MarcoFalke) fa253d385f9201ea10beacecf3e0e80ff69f3138 util: Add HoursDouble (MarcoFalke) fa21fc60c292ab947b2200e54201440f16230566 scripted-diff: Rename addrman time symbols (MarcoFalke) fa9284c3e9acec4b44b2560256f27b3d78c753e2 refactor: Remove not needed std::max (MacroFake) Pull request description: Those refactors are overlapping with, but otherwise largely unrelated to #24662. ACKs for top commit: naumenkogs: utACK fa64dd6673767992eb4e0e775fb0afdfd298610d dergoegge: Code review ACK fa64dd6673767992eb4e0e775fb0afdfd298610d Tree-SHA512: a50625e78036e7220a11997e6d9b6c6b317cb38ce02b1835fb41cbee2d8bfb1faf29b29d8990be78d6b5e15e9a9d8dec33bf25fa439b47610ef708950969724b
2022-07-26Merge bitcoin/bitcoin#25705: tidy: enable readability-redundant-string-initMacroFake
49168df073d465450b1da4a506ac7ea24fbbb877 tidy: enable readability-redundant-string-init (fanquake) 4ddd746bf9714a209b2f82918a70c4fe81d895c9 refactor: remove unnecessary string initializations (fanquake) Pull request description: Remove unnecessary `std::string` = "" initializations. Enable `readability-redundant-string-init`. See: https://releases.llvm.org/14.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-redundant-string-init.html ACKs for top commit: shaavan: ACK 49168df073d465450b1da4a506ac7ea24fbbb87 Tree-SHA512: 69e72a434908c9166d407551657b310361ae2ef0170f8289cb1c2b8e96a4632be718c0d55cb12af03a3c3d621d9583eced88e5e9d924abb0a8b1a9b36c903d66
2022-07-26Merge bitcoin/bitcoin#24974: refactor: Make FEELER_SLEEP_WINDOW type safe ↵fanquake
(std::chrono) fa74e726c414f5f7a1e63126a69463491f66e0ec refactor: Make FEELER_SLEEP_WINDOW type safe (std::chrono) (MacroFake) fa3b3cb9b5d944d34b1d5ac3e102ac333482a475 Expose underlying clock in CThreadInterrupt (MacroFake) Pull request description: This gets rid of the `value*1000` manual conversion. ACKs for top commit: naumenkogs: utACK fa74e726c414f5f7a1e63126a69463491f66e0ec dergoegge: Code review ACK fa74e726c414f5f7a1e63126a69463491f66e0ec Tree-SHA512: 90409c05c25f0dd2f1c4dead78f707ebfd78b7d84ea4db9fcefd9c4958a1a3338ac657cd9e99eb8b47d52d4485fa3c947dce4ee1559fb56ae65878685e1ed9a3
2022-07-26Merge bitcoin/bitcoin#25694: refactor: Make CTransaction constructor explicitMacroFake
fa2247a9f9754d90ea60f254f6c0ed881c55772b refactor: Make CTransaction constructor explicit (MacroFake) Pull request description: It involves calculating two hashes, so the performance impact should be made explicit. Also, add the module to iwyu. ACKs for top commit: aureleoules: ACK fa2247a9f9754d90ea60f254f6c0ed881c55772b. hebasto: ACK fa2247a9f9754d90ea60f254f6c0ed881c55772b, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: e236c352a472c7edfd4f0319a5a16a59f627b0ab7eb8531b53c75d730a3fa3e990a939978dcd952cd73e647925fc79bfa6d9fd87624bbc3ef180f40f95acef19
2022-07-26Merge bitcoin/bitcoin#25689: fuzz: Remove no-op SetMempoolConstraintsglozow
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
2022-07-26tidy: enable readability-redundant-string-initfanquake
See: https://releases.llvm.org/14.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-redundant-string-init.html
2022-07-26refactor: remove unnecessary string initializationsfanquake
2022-07-26Merge bitcoin/bitcoin#25699: scripted-diff: Replace NullUniValue with ↵fanquake
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
2022-07-26refactor: Use type-safe std::chrono for addrman timeMarcoFalke
2022-07-26Add type-safe AdjustedTime() getter to timedataMarcoFalke
Also, fix includes. The getter will be used in a future commit.
2022-07-26Add ChronoFormatter to serializeMarcoFalke
2022-07-26util: Add HoursDoubleMarcoFalke
2022-07-26scripted-diff: Rename addrman time symbolsMarcoFalke
-BEGIN VERIFY SCRIPT- ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ./src ./test); } ren nLastTry m_last_try ren nLastSuccess m_last_success ren nLastGood m_last_good ren nLastCountAttempt m_last_count_attempt ren nSinceLastTry since_last_try ren nTimePenalty time_penalty ren nUpdateInterval update_interval ren fCurrentlyOnline currently_online -END VERIFY SCRIPT-
2022-07-26refactor: Remove not needed std::maxMacroFake
2022-07-25fix comment spellings from the codespell lintGreg Weber
test/lint/all-lint.py includes the codespell lint
2022-07-25psbt: Fix unsigned integer overflowAurèle Oulès
2022-07-25scripted-diff: Replace NullUniValue with UniValue::VNULLMacroFake
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-
2022-07-25fuzz: refactor: Replace NullUniValue with UniValue{}MacroFake
This is needed for the scripted-diff to compile in the next commit
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-25refactor: Make CTransaction constructor explicitMacroFake
It involves calculating two hashes, so the performance impact should be made explicit. Also, add the module to iwyu.
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-24fuzz: Remove no-op SetMempoolConstraintsMacroFake
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-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-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#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