aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2021-11-09Merge bitcoin/bitcoin#23414: wallet: Fix comment grammar in bdb.hW. J. van der Laan
bd9c6ade46c8c29bea3fd5df62a35efadb77ac89 wallet: Fixed Grammatical error in bdb.h (zealsham) Pull request description: A comment in bdb.h file in the wallet directory contains a grammatical error that makes the underlying code harder to reason about . The comment which says `/** Indicate the a new database user has began using the database. */` should actually be `/** indicate that a new database user has began using the database. */`. The former is quite confusing , and leaves you wondering what "the a new database user " is refering to . This pull request thus provides value to the bitcoin codebase by improving readability . Top commit has no ACKs. Tree-SHA512: db07cda39f89ac344be3497c884a8c6e4b48276afae18e931a9a8e5732c58eed20645ccd18d6b1212c763f64b1300477c1de26a00b5b3b24e6141ffae3658ca7
2021-11-09wallet: Fixed Grammatical error in bdb.hzealsham
Wallet: Fixed Grammatical error in bdb.h
2021-11-09fix `XOnlyPubKey::IsFullyValid` comment referenceSebastian Falbesoner
The method name `CreatePayToContract` doesn't exist, very likely it was a (local) working title that was renamed to `CreateTapTweak` later. Also mention `CheckTapTweak`.
2021-11-09Remove `gArgs` from `wallet.h` and `wallet.cpp`Kiminuo
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
2021-11-08Merge bitcoin/bitcoin#23077: Full CJDNS supportW. J. van der Laan
420695c1933e2b9c6e594fcd8885f1c261e435cf contrib: recognize CJDNS seeds as such (Vasil Dimov) f9c28330a0e77ed077f342e4669e855b3e6b20a1 net: take the first 4 random bits from CJDNS addresses in GetGroup() (Vasil Dimov) 29ff79c0a2a95abf50b78dd2be6ead2abeeaec9f net: relay CJDNS addresses even if we are not connected to CJDNS (Vasil Dimov) d96f8d304c872b21070245c1b6aacc8b1f5da697 net: don't skip CJDNS from GetNetworkNames() (Vasil Dimov) c2d751abbae3811adaf856b1dd1b71b33e54d315 net: take CJDNS into account in CNetAddr::GetReachabilityFrom() (Vasil Dimov) 9b43b3b257a00f777538fcc6e2550702055a1488 test: extend feature_proxy.py to test CJDNS (Vasil Dimov) 508eb258fd569cabda6fe15699f911fd627e0c56 test: remove default argument of feature_proxy.py:node_test() (Vasil Dimov) 6387f397b323b0fb4ca303fe418550f5465147c6 net: recognize CJDNS addresses as such (Vasil Dimov) e6890fcb440245c9a24ded0b7af46267453433f1 net: don't skip CJDNS from GetNetworksInfo() (Vasil Dimov) e9d90d3c11cee8ea70056f69afaa548cee898f40 net: introduce a new config option to enable CJDNS (Vasil Dimov) 78f456c57677e6a3a839426e211078ddf0b3e194 net: recognize CJDNS from ParseNetwork() (Vasil Dimov) de01e312b333b65b09c8dc72f0cea6295ab8e43f net: use -proxy for connecting to the CJDNS network (Vasil Dimov) aedd02ef2750329019d5698b14b17d67c5a563ad net: make it possible to connect to CJDNS addresses (Vasil Dimov) Pull request description: CJDNS overview ===== CJDNS is like a distributed, shared VPN with multiple entry points where every participant can reach any other participant. All participants use addresses from the `fc00::/8` network (reserved IPv6 range). Installation and configuration is done outside of applications, similarly to VPN (either in the host/OS or on the network router). Motivation ===== Even without this PR it is possible to connect two Bitcoin Core nodes through CJDNS manually by using e.g. `-addnode` in environments where CJDNS is set up. However, this PR is necessary for address relay to work properly and automatic connections to be made to CJDNS peers. I.e. to make CJDNS a first class citizen network like IPv4, IPv6, Tor and I2P. Considerations ===== An address from the `fc00::/8` network, could mean two things: 1. Part of a local network, as defined in RFC 4193. Like `10.0.0.0/8`. Bitcoin Core could be running on a machine with such address and have peers with those (e.g. in a local network), but those addresses are not relayed to other peers because they are not globally routable on the internet. 2. Part of the CJDNS network. This is like Tor or I2P - if we have connectivity to that network then we could reach such peers and we do relay them to other peers. So, Bitcoin Core needs to be able to tell which one is it when it encounters a bare `fc00::/8` address, e.g. from `-externalip=` or by looking up the machine's own addresses. Thus a new config option is introduced `-cjdnsreacable`: * `-cjdnsreacable=0`: it is assumed a `fc00::/8` address is a private IPv6 (1.) * `-cjdnsreacable=1`: it is assumed a `fc00::/8` address is a CJDNS one (2.) After setting up CJDNS outside of Bitcoin Core, a node operator only needs to enable this option. Addresses from P2P relay/gossip don't need that because they are properly tagged as IPv6 or as CJDNS. For testing ===== ``` [fc32:17ea:e415:c3bf:9808:149d:b5a2:c9aa]:8333 [fc68:7026:cb27:b014:5910:e609:dcdb:22a2]:8333 [fcb3:dc50:e1ae:7998:7dc0:7fa6:4582:8e46]:8333 [fcc7:be49:ccd1:dc91:3125:f0da:457d:8ce]:8333 [fcf2:d9e:3a25:4eef:8f84:251b:1b4d:c596]:8333 ``` ACKs for top commit: dunxen: ACK 420695c jonatack: re-ACK 420695c1933e2b9c6e594fcd8885f1c261e435cf per `git range-diff 23ae793 4fbff39 420695c` laanwj: Code review ACK 420695c1933e2b9c6e594fcd8885f1c261e435cf Tree-SHA512: 21559886271aa84671d52b120fa3fa5a50fdcf0fcb26e5b32049c56fab0d606438d19dd366a9c8ce612d3894237ae6d552ead3338b326487e3534399b88a317a
2021-11-08Merge bitcoin/bitcoin#23409: refactor: Take Span in SetSeedW. J. van der Laan
fa93ef5a8aeae36304c792697a78af2d07fd9f41 refactor: Take Span in SetSeed (MarcoFalke) Pull request description: This makes calling code less verbose and less fragile. Also, by adding the CKey::data() member function, it is now possible to call HexStr() with a CKey object. ACKs for top commit: sipa: utACK fa93ef5a8aeae36304c792697a78af2d07fd9f41 laanwj: Code review ACK fa93ef5a8aeae36304c792697a78af2d07fd9f41 theStack: Code-review ACK fa93ef5a8aeae36304c792697a78af2d07fd9f41 Tree-SHA512: 73fb999320719ad4b9ab5544018a7a083d140545c2807ee3582ecf7f441040a30b5157e85790b6b840af82f002a7faf30bd8162ebba5caaf2067391c43dc7e25
2021-11-08Merge bitcoin/bitcoin#23439: test: Open streams_test_tmp file in temporary ↵MarcoFalke
folder a04350b86c14a8038117aa013952b4e2b5f91cb0 Open streams_test_tmp file in temporary folder (Martin Leitner-Ankerl) Pull request description: The tests `streams_tests/streams_buffered_file` and `streams_tests/streams_buffered_file_rand` did not use a the temporary directory provided by `BasicTestingSetup`, so it was not possible to execute multiple of them in parallel. This fixes that. To reproduce, run ```sh parallel --halt now,fail=1 './src/test/test_bitcoin --run_test=streams_tests/streams_buffered_file_rand' -- ::: {1..1000} ``` This executes the test 1000 times, one job per CPU. It works on that commit but mergebase fails quickly. ACKs for top commit: theStack: Tested ACK a04350b86c14a8038117aa013952b4e2b5f91cb0 Tree-SHA512: 705b127e99a0fdbf33362283cc94036ac3aeff364a5e75b0f0a2490114bbaa44cf310ac13d43c254a4b51b72d4e856e9ee584904ce56b2f496b92d995ac7dc24
2021-11-08build: Fix `make apk` if ccache enabledHennadii Stepanov
2021-11-05Merge bitcoin/bitcoin#23334: fuzz: Descriptor walletMarcoFalke
11115169a14d0d0be5b7b1c3f6fdc9673a9098d9 ci: Build fuzz with libsqlite3-dev (MarcoFalke) fa7c6efca66627e4c76adecc824f96da220af69c fuzz: Add wallet fuzz test (MarcoFalke) fa59d2ce5b8d6fe8c610f170a13675c756aef58f refactor: Use local args instead of global gArgs in CWallet::Create (MarcoFalke) fadb44606f26a80daf4320eee046c9572e85fe3e build: Inline FUZZ_SUITE_LDFLAGS_COMMON (MarcoFalke) Pull request description: Initial sketch to fuzz descriptor wallets. Can be improved in the future. ACKs for top commit: mjdietzx: Code review ACK 1111516 Tree-SHA512: b1d2f24504d1ed5f3c6a031210f04c27c13d4e15576c4acbf50ded37ac45f7b7a5c7553e91d81d4a06e9ea73b3d745a552218d3ef3b2932fa5325a8331b0d3fd
2021-11-04[test] call CheckPackage for package sanitization checksglozow
Makes the test more minimal. We're just trying to test that our package sanitization logic is correct. Now that this code lives in its own function (rather than inside of AcceptMultipleTransactions), there's no need to call ProcessNewPackage to test this.
2021-11-04MOVEONLY: move package unit tests to their own fileglozow
2021-11-04MOVEONLY: mempool checks to their own functionsglozow
No change in behavior, because package transactions would not be going through the rbf logic in PreChecks anyway (BIP125 is currently disabled for package acceptance, see ATMPArgs). We draw the line here because each individual transaction in package validation still goes through all PreChecks. For example, checking that one's own conflicts and dependencies are disjoint (a consensus check) and individual transaction mempool ancestor/descendant limits.
2021-11-04scripted-diff: clean up MemPoolAccept aliasesglozow
The aliases are leftover from a previous MOVEONLY refactor - they are unnecessary and removing them reduces the diff for splitting out mempool Checks from PreChecks, making RBF variables MemPoolAccept-wide, etc. -BEGIN VERIFY SCRIPT- unalias() { sed -i "s:\<$1\>:$2:g" src/validation.cpp; sed -i "/$2 = $2/d" src/validation.cpp; } unalias nModifiedFees ws.m_modified_fees unalias nConflictingFees ws.m_conflicting_fees unalias nConflictingSize ws.m_conflicting_size unalias setConflicts ws.m_conflicts unalias allConflicting ws.m_all_conflicting unalias setAncestors ws.m_ancestors -END VERIFY SCRIPT-
2021-11-04Open streams_test_tmp file in temporary folderMartin Leitner-Ankerl
The tests `streams_tests/streams_buffered_file` and `streams_tests/streams_buffered_file_rand` did not use a the temporary directory provided by `BasicTestingSetup`, so it was not possible to execute multiple of them in parallel. This fixes that. To reproduce, run ```sh parallel --halt now,fail=1 './src/test/test_bitcoin --run_test=streams_tests/streams_buffered_file_rand' -- ::: {1..1000} ``` This executes the test 1000 times, one job per CPU. It works on that commit but mergebase fails quickly.
2021-11-04document workspace membersglozow
2021-11-04[validation] cache iterators to mempool conflictsglozow
2021-11-05Merge bitcoin/bitcoin#22949: fee: Round up fee calculation to avoid a lower ↵Samuel Dobson
than expected feerate 80dc829be7f8c3914074b85bb4c125baba18cb2c tests: Calculate fees more similarly to CFeeRate::GetFee (Andrew Chow) ce2cc44afd51f3df4ee7f14ea05b8da229183923 tests: Test for assertion when feerate is rounded down (Andrew Chow) 0fbaef9676a1dcb84bcf95afd8d994831ab327b6 fees: Always round up fee calculated from a feerate (Andrew Chow) Pull request description: When calculating the fee for a feerate, it is possible that the final calculation will have fractional satoshis. Currently those are ignored via truncation which results in the absolute fee being rounded down. Rounding down is problematic because it results in a feerate that is slightly lower than the feerate represented by the `CFeeRate` object. A slightly lower feerate particularly causes issues for coin selection as it can trigger an assertion error. To avoid potentially underpaying the feerate (and the assertion), always round up the calculated fee. A test is added for the assertion, along with a comment explaining what happens. It is unlikely that a user can trigger this as it requires a very specific set of rounding errors to occur as well as the transaction not needing any change and being right on the lower bound of the exact match window. However I was able to trigger the assertion while running coin selection simulations, albeit after thousands of transactions and with some weird feerates. ACKs for top commit: ryanofsky: Code review ACK 80dc829be7f8c3914074b85bb4c125baba18cb2c promag: Tested ACK 80dc829be7f8c3914074b85bb4c125baba18cb2c. lsilva01: tACK 80dc829 meshcollider: utACK 80dc829be7f8c3914074b85bb4c125baba18cb2c Tree-SHA512: fe26684c60f236cab48ea6a4600c141ce766dbe59504ec77595dcbd7fd0b34559acc617007f4f499c9155d8fda0a336954413410ba862b19c765c0cfac79d642
2021-11-03[validation] Always call mempool.check() after processing a new transactionJohn Newbery
CTxMemPool::check() will carry out internal consistency checks 1/n times, where n is set by the `-checkmempool` configuration option. By default, mempool consistency checks are disabled entirely on mainnet. Therefore, this change has no effect on mainnet nodes running with default configuration. It simply removes the responsibility to trigger mempool consistency checks from net_processing.
2021-11-03[refactor] Don't call AcceptToMemoryPool() from outside validation.cppJohn Newbery
2021-11-03[validation] Add CChainState::ProcessTransaction()John Newbery
This just calls through to AcceptToMemoryPool() internally, and is currently unused. Also add a new transaction validation failure reason TX_NO_MEMPOOL to indicate that there is no mempool.
2021-11-03[logging/documentation] Remove reference to AcceptToMemoryPool from error stringJohn Newbery
User-facing error messages should not leak internal implementation details like function names. Update the MEMPOOL_REJECTED error string from "Transaction rejected by AcceptToMemoryPool" to the more generic "Transaction rejected by mempool". Also update the MEMPOOL_ERROR error message from "AcceptToMemoryPool failed" to the more precise "Mempool internal error" since this error indicates and internal (e.g. logic/hardware/etc) failure, and not a transaction rejection.
2021-11-03[validation] Remove comment about AcceptToMemoryPool()John Newbery
"This logic is not necessary for memory pool transactions, as AcceptToMemoryPool already refuses previously-known transaction ids entirely." refers to the logic at https://github.com/bitcoin/bitcoin/blob/a206b0ea12eb4606b93323268fc81a4f1f952531/src/main.cpp#L484-L486, which was later removed in commit 450cbb0944cd20a06ce806e6679a1f4c83c50db2.
2021-11-03net: take the first 4 random bits from CJDNS addresses in GetGroup()Vasil Dimov
CJDNS addresses start with constant 8 bits, so in order to account for the first 4 random ones, we must take the first 12. Otherwise the entire CJDNS network will belong to one group.
2021-11-03net: relay CJDNS addresses even if we are not connected to CJDNSVasil Dimov
This will help with propagation, so that multi-homed nodes can learn CJDNS addresses outside of the CJDNS network.
2021-11-03net: don't skip CJDNS from GetNetworkNames()Vasil Dimov
2021-11-03net: take CJDNS into account in CNetAddr::GetReachabilityFrom()Vasil Dimov
This way `GetLocal()` will pick our CJDNS address for a CJDNS peer.
2021-11-03net: recognize CJDNS addresses as suchVasil Dimov
In some cases addresses come from an external source as a string or as a `struct sockaddr_in6`, without a tag to tell whether it is a private IPv6 or a CJDNS address. In those cases interpret the address as a CJDNS address instead of an IPv6 address if `-cjdnsreachable` is set and the seemingly-IPv6-address belongs to `fc00::/8`. Those external sources are: * `-externalip=` * `-bind=` * UPnP * `getifaddrs(3)` (called through `-discover`) * `addnode` * `connect` * incoming connections (returned by `accept(2)`)
2021-11-03net: don't skip CJDNS from GetNetworksInfo()Vasil Dimov
2021-11-03net: introduce a new config option to enable CJDNSVasil Dimov
CJDNS is set up in the host OS, outside of the application. When the routing is configured properly then connecting to fc00::/8 results in connecting to the CJDNS network. Introduce an option so that Bitcoin Core knows whether this is the case.
2021-11-03net: recognize CJDNS from ParseNetwork()Vasil Dimov
This allows to use "cjdns" as an argument to the `getnodeaddresses` RPC and to the `-onlynet=` parameter.
2021-11-03net: use -proxy for connecting to the CJDNS networkVasil Dimov
If `-proxy` is given, then also use it for connecting to the CJDNS network.
2021-11-03net: make it possible to connect to CJDNS addressesVasil Dimov
Connecting to CJDNS addresses works without a proxy, just like connecting to an IPv6 address. Thus adapt `CService::GetSockAddr()` to retrieve the `struct sockaddr*` even for `CService::IsCJDNS()` objects.
2021-11-03[test] Don't set bypass_limits to true in txvalidation_tests.cppJohn Newbery
AcceptToMemoryPool() is called for an invalid coinbase transaction, so setting bypass_limits to true or false has no impact on the test. The only way that changing bypass_limits from true to false could change the result would be to change the outcome to INVALID(TX_MEMPOOL_POLICY). Since the ATMP call in this test results in INVALID(TX_CONSENSUS) both before and after this change, there is no change in behavior.
2021-11-03[test] Don't set bypass_limits to true in txvalidationcache_tests.cppJohn Newbery
AcceptToMemoryPool() is called for transactions with fees above minRelayTxFee and with the mempool not full, so setting bypass_limits to true or false has no impact on the test. The only way that changing bypass_limits from true to false could change the result would be to change the outcome to INVALID(TX_MEMPOOL_POLICY). Since all the ATMP calls in this test result in VALID both before and after this change, there is no change in behavior.
2021-11-03Exit early for an empty vChecks in CCheckQueue::AddHennadii Stepanov
2021-11-03Avoid excessive lock contention in CCheckQueue::AddHennadii Stepanov
2021-11-03wallet: Make GetOldestKeyPoolTime return nullopt for blank walletsHennadii Stepanov
This change suppress the "keypoololdest" field in the getwalletinfo RPC response for blank descriptor wallets.
2021-11-03wallet, refactor: Make GetOldestKeyPoolTime return type std::optionalHennadii Stepanov
This change gets rid of the magic number 0 in the DescriptorScriptPubKeyMan::GetOldestKeyPoolTime() function. No behavior change.
2021-11-03Merge bitcoin/bitcoin#23211: refactor: move `update_*` structs from ↵MarcoFalke
txmempool.h to .cpp file 65aaf9495d19ea3fb875228a7e14aab6c1f2986d refactor: move `update_*` structs from txmempool.h to .cpp file (Sebastian Falbesoner) 9947ce62626c05bd186ae8a4864aa382f673ec1a refactor: use const reference for parents in `CTxMemPool::UpdateAncestorsOf` (Sebastian Falbesoner) Pull request description: These helpers are exclusively used in txmempool.cpp, hence they should also be moved there. The PR also contains a commit which fixes const-correctness for parents in `CTxMemPool::UpdateAncestorsOf` and declares them as reference to avoid a copy. ACKs for top commit: promag: Code review ACK 65aaf9495d19ea3fb875228a7e14aab6c1f2986d. Verified move-only commit locally. Tree-SHA512: 7ce29f3ba0e68b5355001f27725b00f6d54cc993015356eb40b61b8cdd17db49b980f4c3d798c8e0c940d245dc3a72c474bb9ff3c0ee971ead450786076812c2
2021-11-02Merge bitcoin/bitcoin#23223: Disable lock contention logging in checkqueue_testsMarcoFalke
6ae9f1cf9604227e9dfda1f6c91fc711d154362e Disable lock contention logging in checkqueue_tests (Jon Atack) Pull request description: This patch disables lock contention logging in the checkqueue_tests as some of these tests are designed to be heavily contested to trigger race conditions or other issues. This created very large log files when run with DEBUG_LOCKCONTENTION defined (up to v22) or with lock logging enabled by default in current master. Examples running the following command: ``` $ ./src/test/test_bitcoin -t checkqueue_tests/test_CheckQueue_Correct_Random -- DEBUG_LOG_OUT > testlog.txt -rw-r--r-- 87042178 Oct 8 12:41 testlog-with-DEBUG_LOCKCONTENTION-at-v22-run1.txt -rw-r--r-- 73879896 Oct 8 12:42 testlog-with-DEBUG_LOCKCONTENTION-at-v22-run2.txt -rw-r--r-- 65150518 Oct 8 12:51 testlog-with-DEBUG_LOCKCONTENTION-at-bb9f76a-run1.txt -rw-r--r-- 65774554 Oct 8 12:52 testlog-with-DEBUG_LOCKCONTENTION-at-bb9f76a-run2.txt -rw-r--r-- 73493309 Oct 8 13:00 testlog-current-master-at-991753e-run1.txt -rw-r--r-- 65616977 Oct 8 13:01 testlog-current-master-at-991753e-run2.txt -rw-r--r-- 5093 Oct 8 13:04 testlog-with-this-commit-run1.txt -rw-r--r-- 5093 Oct 8 13:05 testlog-with-this-commit-run2.txt ``` Resolves #23167. ACKs for top commit: vasild: ACK 6ae9f1cf9604227e9dfda1f6c91fc711d154362e Tree-SHA512: b16812ed60c58a1cf40c04ebeca9197ac076b2415f71673ac7bb5b7960a1ff80ba2c909345ad221c7689b0562d17f63a32a629f5d6dbcf0e57130bf5760388c1
2021-11-02Adds verification step to Schnorr and ECDSA signingamadeuszpawlik
As defined in BIP340, a verification step should be executed after `secp256k1_schnorrsig_sign` to ensure that a potentially corrupted signature isn't used; using corrupted signatures could reveal information about the private key used. This applies to ECSDA as well. Additionally clears schnorr signature if signing failed.
2021-11-02Merge bitcoin/bitcoin#22735: [net] Don't return an optional from ↵MarcoFalke
TransportDeserializer::GetMessage() f3e451bebfe2e2d8de901d8ac29c064a51d3b746 [net] Replace GetID() with id in TransportDeserializer constructor (Troy Giorshev) 8c96008ab18075abca03bff6b3675643825a21ca [net] Don't return an optional from TransportDeserializer::GetMessage() (Troy Giorshev) Pull request description: Also, access mapRecvBytesPerMsgCmd with `at()` not `find()`. This throws an error if COMMAND_OTHER doesn't exist, which should never happen. `find()` instead just accessed the last element, which could make debugging more difficult. Resolves review comments from PR19107: - https://github.com/bitcoin/bitcoin/pull/19107#discussion_r478718436 - https://github.com/bitcoin/bitcoin/pull/19107#discussion_r478714497 ACKs for top commit: theStack: Code-review ACK f3e451bebfe2e2d8de901d8ac29c064a51d3b746 ryanofsky: Code review ACK f3e451bebfe2e2d8de901d8ac29c064a51d3b746. Changes since last review in https://github.com/bitcoin/bitcoin/pull/20364#pullrequestreview-534369904 were simplifying by dropping the third commit, rebasing, and cleaning up some style & comments in the first commit. Tree-SHA512: 37de4b25646116e45eba50206e82ed215b0d9942d4847a172c104da4ed76ea4cee29a6fb119f3c34106a9b384263c576cb8671d452965a468f358d4a3fa3c003
2021-11-02rpc: move fees object to match helpjosibake
2021-11-02rpc: deprecate fee fields from mempool entriesjosibake
Unless `-deprecatedrpc=fees` is passed, top level fee fields are no longer returned for mempool entries. Add instructions to field help on how to access deprecated fields, update help text for readability, and include units. This is important to help avoid any confusion as users move from deprecated fields to the fee fields object (credit: jonatack). This affects `getmempoolentry`, `getrawmempool`, `getmempoolancestors`, and `getmempooldescendants` Modify `test/functional/mempool_packages.py` and `test/functional/rpc_fundrawtransaction.py` tests to no longer use deprecated fields. Co-authored-by: jonatack <jon@atack.com>
2021-11-01Merge bitcoin/bitcoin#23403: test: Fix segfault in the ↵MarcoFalke
psbt_wallet_tests/psbt_updater_test 68018e4c3e76f7e5bebf5f90ffd972c7bf01e0a0 test: Avoid excessive locking of `cs_wallet` (Hennadii Stepanov) 7986faf2e09ea85b1d4564ce910f07a4c4de8685 test: Fix segfault in the psbt_wallet_tests/psbt_updater_test (Hennadii Stepanov) Pull request description: The dcd6eeb64adb2b532f5003cbb86ba65b3c08a87b commit (bitcoin/bitcoin#23288) introduced an intermittent failure in the `psbt_wallet_tests/psbt_updater_test` unit test. See bitcoin/bitcoin#23368. The test failure can be easily made reproducible with the following patch: ```diff --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -57,6 +57,8 @@ void CScheduler::serviceQueue() Function f = taskQueue.begin()->second; taskQueue.erase(taskQueue.begin()); + UninterruptibleSleep(100ms); + { // Unlock before calling f, so it can reschedule itself or another task // without deadlocking: ``` This PR implements an idea which was mentioned in the [comment](https://github.com/bitcoin/bitcoin/issues/23368#issuecomment-953796339): > Yes, as I said before this looks like a race where the wallet is deleted before stopping the scheduler: [#23368 (comment)](https://github.com/bitcoin/bitcoin/issues/23368#issuecomment-952808824) > > IIRC, the order should be: > > * stop scheduler > > * delete wallet > > * delete scheduler The second commit introduces a refactoring with no behavior change. Fixes bitcoin/bitcoin#23368. ACKs for top commit: mjdietzx: Code review ACK 68018e4c3e76f7e5bebf5f90ffd972c7bf01e0a0 Tree-SHA512: d9103f6252aab807453628159dec243bc543a2595eecaa04ec761dca3c2370085592c55d6f50967d69a4ac6e8b5827eec30dd9b025132c99b0bb9aa5911ad915
2021-11-01refactor: Take Span in SetSeedMarcoFalke
This makes calling code less verbose and less fragile. Also, by adding the CKey::data() member function, it is now possible to call HexStr() with a CKey object.
2021-11-01fuzz: Rework ConsumeScriptMarcoFalke
This should make it easier for the fuzz engine to explore multisig code paths. See discussion in https://github.com/bitcoin/bitcoin/issues/23105 The downside is that all fuzz inputs that use ConsumeScript are now invalidated and need to be re-generated. Another downside may be that most multisig scripts from ConsumeScript are using likely not fully valid pubkeys.
2021-11-01Merge bitcoin/bitcoin#22766: refactor: Clarify and disable unused ↵fanquake
ArgsManager flags c5d7e34bd9a4ad752c5ec88032420e2e90ab17ab scripted-diff: disable unimplemented ArgsManager BOOL/INT/STRING flags (Russell Yanofsky) b8c069b7a952e326d2d974cc671889d1a3b38aa4 refactor: Add explicit DISALLOW_NEGATION ArgsManager flag to clarify flag usage (Russell Yanofsky) 26a50ab322614bceb5bc62e2c282f83e5987bad8 refactor: Split InterpretOption into Interpret{Key,Value} functions (Russell Yanofsky) Pull request description: This is preparation for #16545 or another PR implementing type validation for ArgsManager settings. It fixes misleading usages of existing flags, prevents flags from being similarly misused in the future, and allows validation logic to be added without breaking backwards compatibility. --- Currently, ALLOW_{INT|BOOL|STRING} flags don't do any real validation, so current uses of these flags are misleading and will also break backwards compatibility whenever these flags are implemented in a future PR (draft PR is #16545). An additional complication is that while these flags don't do any real settings validation, they do affect whether setting negation syntax is allowed. Fix this mess by disabling ALLOW_{INT|BOOL|STRING} flags until they are implemented, and adding an unambiguous DISALLOW_NEGATION flag. This is done in three commits, with the first commit cleaning up some code, the second commit adding the DISALLOW_NEGATION flag, and the next commit disabling the ALLOW_{INT|BOOL|STRING} flags. None of the changes affect behavior in any way. ACKs for top commit: ajtowns: utACK c5d7e34bd9a4ad752c5ec88032420e2e90ab17ab promag: Code review ACK c5d7e34bd9a4ad752c5ec88032420e2e90ab17ab, which as the new argument `-legacy`. Tree-SHA512: cad0e06361e8cc584eb07b0a1f8b469e3beea18abb458c4e43d9d16e9f301b12ebf1d1d426a407fbd96f99724ad6c0eae5be05c713881da7c55e0e08044674eb
2021-11-01Merge bitcoin/bitcoin#23380: addrman: Fix AddrMan::Add() return semantics ↵fanquake
and logging 61ec0539b26a902a41a2602187a71f9dba3c6935 [MOVEONLY] reorder functions in addrman_impl.h and addrman.cpp (John Newbery) 2095df7b7bfcb9ab0c5710a93112f7f341e753c9 [addrman] Add Add_() inner function, fix Add() return semantics (John Newbery) 2658eb6d68460272deefb3fcc653b03f6ec6e7cf [addrman] Rename Add_() to AddSingle() (John Newbery) e58598e833d5737900fe3c4369e26f2a08166892 [addrman] Add doxygen comment to AddrMan::Add() (John Newbery) Pull request description: Previously, Add() would return true if the function created a new AddressInfo object, even if that object could not be successfully entered into the new table and was deleted. That would happen if the new table position was already taken and the existing entry could not be removed. Instead, return true if the new AddressInfo object is successfully entered into the new table. This fixes a bug in the "Added %i addresses" log, which would not always accurately log how many addresses had been added. ACKs for top commit: naumenkogs: ACK 61ec0539b26a902a41a2602187a71f9dba3c6935 mzumsande: ACK 61ec0539b26a902a41a2602187a71f9dba3c6935 shaavan: ACK 61ec0539b26a902a41a2602187a71f9dba3c6935 Tree-SHA512: 276f1e8297d4b6d411d05d06ffc7c176f6290a784da039926ab6c471a8ed8e9159ab4f56c893b1285737ae292954930f0d28012d89dfb3f2f825d7df41016feb
2021-10-31test: Avoid excessive locking of `cs_wallet`Hennadii Stepanov
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-17 18:16:46 +0000