aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
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-03test: extend feature_proxy.py to test CJDNSVasil Dimov
2021-11-03test: remove default argument of feature_proxy.py:node_test()Vasil Dimov
The default bool argument makes it harder to read because the last but one argument is also bool. Pass all of them as named arguments to increase readability. Another bool argument will be added to indicate whether to test CJDNS. Co-authored-by: Jon Atack <jon@atack.com>
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-03Merge bitcoin/bitcoin#23154: doc: add assumeutxo notesMarcoFalke
9ab440199d5c888363a42c957433d0e46cd0d2ff doc: add assumeutxo notes (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: #15606) --- Adds some notes on assumeutxo design. Related: https://github.com/bitcoin/bitcoin/pull/21526#discussion_r715558994 ACKs for top commit: ariard: ACK 9ab4401 naumenkogs: ACK 9ab4401 michaelfolkson: ACK 9ab440199d5c888363a42c957433d0e46cd0d2ff fjahr: ACK 9ab440199d5c888363a42c957433d0e46cd0d2ff Tree-SHA512: 2fca8373b78701754957d12bc43ce18aa6928507965448741cb4e8c56589ad61d261f8542e348094fc9631d46ee6a7afee75c965c0db993fc816758569137b74
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-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-02Merge bitcoin/bitcoin#23410: doc: Add output script descriptors BIPs 380..386MarcoFalke
c02a674e97b61f675c94341fca1bf232070e6991 doc: Add output script descriptors BIPs 380..386 (Hennadii Stepanov) Pull request description: BIPs 380..385 are implemented as of v0.17.0. BIP 386 is implemented as of v22.0. ACKs for top commit: sipa: ACK c02a674e97b61f675c94341fca1bf232070e6991 jarolrod: ACK c02a674e97b61f675c94341fca1bf232070e6991 shaavan: ACK c02a674e97b61f675c94341fca1bf232070e6991 Tree-SHA512: 40f0252d3aad08c61a8e1476d26a590dbcf3f9d66c1f8315d15d13feb17288cc25b9c75df5b938f77695eafaba847dacc0020a880ba6034a511e7c9b7f40fd8f
2021-11-01doc: Add output script descriptors BIPs 380..386Hennadii Stepanov
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-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
2021-10-31test: Fix segfault in the psbt_wallet_tests/psbt_updater_testHennadii Stepanov
The bug was introduced in dcd6eeb64adb2b532f5003cbb86ba65b3c08a87b.
2021-10-30Merge bitcoin/bitcoin#23385: refactor: get wallet path relative to wallet_dirfanquake
9ba7c44265a47880585e39d0167d057ba935ff16 refactor: get wallet path relative to wallet_dir (Michael Dietz) Pull request description: Now that boost has been updated > 1.60 (see #22320), we can simplify how we get wallet path relative to wallet_dir by using: `boost::filesystem::lexically_relative`, removing a TODO. Test coverage comes from `test/functional/wallet_multiwallet.py` I first tried this in #20265 which was my first attempted PR, and funny enough exactly 1 year later I'm opening this one to hopefully finally close this. ACKs for top commit: ryanofsky: Code review ACK 9ba7c44265a47880585e39d0167d057ba935ff16. Basically this same code change is made in #20744 commit b70c84348ac7a8e427a1183f894c73e52c734529, so this PR helps simplify that one lsilva01: Code Review ACK 9ba7c44 Tree-SHA512: 6ccb91a18bcb52c3ae0c789a94a18fb5be7db7769fd1121552d63f259fbd32b50c3dcf169cec0b02f978321db3bc60eb4b881b8327e9764f32e700236e0d8a35
2021-10-29refactor: get wallet path relative to wallet_dirMichael Dietz
Now that boost has been updated > 1.60, we can simplify how we get wallet path relative to wallet_dir by using: `boost::filesystem::lexically_relative`
2021-10-29Merge bitcoin/bitcoin#23354: Introduce new V4 format addrmanMarcoFalke
d891ae768185b464cae476c16c74c365372d4a3c Introduce new V4 format addrman (Pieter Wuille) Pull request description: #23306 effectively changed the on-disk format in an incompatible way: old deserializers cannot deal with multiple entries for the same IP. Introduce a `V4_MULTIPORT` format, and increment the compatibility base, so that old versions correctly recognize it as an incompatible future version, rather than corruption. ACKs for top commit: naumenkogs: ACK d891ae768185b464cae476c16c74c365372d4a3c ajtowns: utACK d891ae768185b464cae476c16c74c365372d4a3c vasild: ACK d891ae768185b464cae476c16c74c365372d4a3c Tree-SHA512: de2153beb59152504ee0656dd0cc0b879b09136eb07e3ce0426d2fea778adfabacebbce5cf1a9a65dc99ad4e99cda42ab26743fe672fb82a9fbfec49c4cccb4d
2021-10-29Merge bitcoin/bitcoin#23375: test: MiniWallet: more deterministic coin ↵MarcoFalke
selection for coinbase UTXOs (oldest first) d2c4904ef707e2023ceb8dfbfe61a92c4060e100 test: MiniWallet: more deterministic coin selection for coinbase UTXOs (oldest first) (Sebastian Falbesoner) Pull request description: The coin selection strategy for MiniWallet is quite straight-forward: simply pick a single UTXO with the largest value: https://github.com/bitcoin/bitcoin/blob/ab25ef8c7f767258d5fe44f53b35ad8bd51ed5cd/test/functional/test_framework/wallet.py#L173-L174 If there are several candidates with the same value, however, it is not clear which one is taken. This can be particularly problematic for coinbase outputs with fixed block subsidy, since spending could lead to a `bad-txns-premature-spend-of-coinbase` reject if an UTXO from a too-recent block is picked. Introduce block height as second criteria (saved in `self._utxos` in the methods `generate(...)` and `rescan_utxos(...)`), in order to avoid potential issues with coinbases that are not matured yet. If there is a tie between coinbase UTXOs and non-coinbase UTXOs (the latter are added via `scan_tx(...)`), prefer the non-coinbase UTXOs, since those don't need to mature. The issue came up while refactoring the test rpc_blockchain.py, see https://github.com/bitcoin/bitcoin/pull/23371#discussion_r737401936 (PR #23371). ACKs for top commit: MarcoFalke: review ACK d2c4904ef707e2023ceb8dfbfe61a92c4060e100 shaavan: ACK d2c4904ef707e2023ceb8dfbfe61a92c4060e100 Tree-SHA512: 15d67b42fb8b77fd53022ea2ab8a6ed2b615567f3ce73bab16c06bfcb687c1a04dcb0360d0c2287c526b604cd3ac5eef7b14ce46fc31e23047ce1a3290027306
2021-10-29Merge bitcoin/bitcoin#22972: test: fix misleading fee unit in mempool_limit.pyMarcoFalke
2600db6c36c11bf49a0a113ee2e2274406ade61c test: fix misleading fee unit in mempool_limit.py (Sebastian Falbesoner) Pull request description: The PR is a follow-up to #22543. The helper `send_large_txs` in its current interface has a fee_rate parameter, implying that it would create a transaction with exactly that rate. Unfortunately, this fee rate is only passed to MiniWallet's `create_self_transfer` method, which can't know that we append several tx outputs after, increasing the tx's vsize and decreasing it's fee rate accordingly. In our case, the fee rate is off by several orders of magnitude, as the tx's vsize changes changes from 96 to 67552 vbytes (>700x), i.e. the value passed to this function is neither really a fee rate nor an absolute fee, but something in-between, which is very confusing. It was suggested to simply in-line this helper as it's currently only used in this single test (https://github.com/bitcoin/bitcoin/pull/22543#discussion_r701685136, https://github.com/bitcoin/bitcoin/pull/22543#issuecomment-918986896), but I could imagine that this helper may also become useful for other tests and may be moved to a library (e.g. wallet.py) in the future. Clarify the interface by passing an absolute fee that is deducted in the end (and also verified, via testmempoolaccept) and also describe how we come up with the value passed. On master, the comment says that the fee rate needs to increased "massively"; this word is also removed because the fee rate only needs to be higher for the test to succeed. ACKs for top commit: stratospher: ACK 2600db6. Tree-SHA512: 0bfacc3fa87603970d86c1d0186e51511f6c20c64b0559e19e7e12a68647f79dcb4f436000dee718fd832ce6a68e3bbacacb29145e0287811f1cb03d2f316843
2021-10-29Merge bitcoin/bitcoin#22787: refactor: actual immutable pointingMarcoFalke
54011e7aa274bdc1b921440cc8b4623aa1e0d89e refactor: use CWallet const shared pointers when possible (Karl-Johan Alm) 96461989a2de737151bc4fb216221bf49cb53ce6 refactor: const shared_ptrs (Karl-Johan Alm) Pull request description: ```C++ const std::shared_ptr<CWallet> wallet = x; ``` means we can not do `wallet = y`, but we can totally do `wallet->DestructiveOperation()`, contrary to what that line looks like. This PR * introduces a new convention: always use const shared pointers to `CWallet`s (even when we mutate the pointed-to thing) * uses `const shared_ptr<const CWallet>` everywhere where wallets are not modified In the future, this should preferably apply to all shared pointers, not limited to just `CWallet`s. Both of these serve the same purpose: to dispell the misconception that `const shared_ptr<X>` immutates `X`. It doesn't, and it's dangerous to leave this misconception as is, for obvious reasons. ACKs for top commit: theStack: re-ACK 54011e7aa274bdc1b921440cc8b4623aa1e0d89e Tree-SHA512: 3bf4062fc821751be30770c6b4ead10a016847970f155a0a5156f304347d221b9830840030c2fbfba8cd1e282f4eda45f5b4107fe6df8138afdcb6c2e95a2836
2021-10-28[MOVEONLY] reorder functions in addrman_impl.h and addrman.cppJohn Newbery
Keep the internal {Function}_() functions grouped together. Review with `git diff --color-moved=dimmed-zebra`
2021-10-28[addrman] Add Add_() inner function, fix Add() return semanticsJohn Newbery
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. p2p_addrv2_relay.py and p2p_addr_relay.py need to be updated since they were incorrectly asserting on the buggy log (assuming that addresses are added to addrman, when there could in fact be new table position collisions that prevent some of those address records from being added).
2021-10-28[addrman] Rename Add_() to AddSingle()John Newbery
2021-10-28[addrman] Add doxygen comment to AddrMan::Add()John Newbery
Does not document the return value since we're going to fix the semantics in a future commit.
2021-10-27test: MiniWallet: more deterministic coin selection for coinbase UTXOs ↵Sebastian Falbesoner
(oldest first) The coin selection strategy for MiniWallet is quite straight-forward: simply pick a single UTXO with the largest value: self._utxos = sorted(self._utxos, key=lambda k: k['value']) utxo_to_spend = utxo_to_spend or self._utxos.pop() If there are several candidates with the same value, however, it is not clear which one is taken. This can be particularly problematic for coinbase outputs with fixed block subsidy, since spending could lead to a 'bad-txns-premature-spend-of-coinbase' reject if an UTXO from a too-recent block is picked. Introduce block height as second criteria (saved in self._utxos in the methods generate(...) and rescan_utxos(...)), in order to avoid potential issues with coinbases that are not matured yet.
2021-10-27Merge bitcoin/bitcoin#23305: test: refactor: add `script_util` helper for ↵MarcoFalke
creating bare multisig scripts 4718897ce3a7c728ff7aebbadabcc8ed7a0b8d6e test: add script_util helper for creating bare multisig scripts (Sebastian Falbesoner) Pull request description: This PR is a follow-up to #22363 and #23118 and introduces a helper `keys_to_multisig_script` for creating bare multisig outputs in the form of ``` OP_K PubKey1 PubKey2 ... PubKeyN OP_N OP_CHECKMULTISIG ``` The function takes a list of pubkeys (both hex- and byte-strings are accepted due to the `script_util.check_key` helper being used internally) and optionally a threshold _k_. If no threshold is passed, a n-of-n multisig output is created, with _n_ being the number of passed pubkeys. ACKs for top commit: shaavan: utACK 4718897ce3a7c728ff7aebbadabcc8ed7a0b8d6e rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/23305/commits/4718897ce3a7c728ff7aebbadabcc8ed7a0b8d6e Tree-SHA512: b452d8a75b0d17316b66ac4ed4c6893fe59c7c417719931d4cd3955161f59afca43503cd09b83a35b5a252a122eb3f0fbb9da9f0e7c944cf8da572a02219ed9d
2021-10-26Merge bitcoin/bitcoin#23006: multiprocess: Add new bitcoin-gui, bitcoin-qt, ↵MarcoFalke
bitcoin-wallet init implementations d5f985e51f2863fda0c0d632e836eba42a5c3e15 multiprocess: Add new bitcoin-gui, bitcoin-qt, bitcoin-wallet init implementations (Russell Yanofsky) Pull request description: Add separate `interfaces::Init` subclasses for `bitcoin-wallet`, `bitcoin-gui`, and `bitcoin-qt` binaries instead of sharing `bitcoind` and `bitcoin-node` init subclasses in different binaries. After this, the new init subclasses can be customized in #10102, so node and wallet code is dropped from the `bitcoin-gui` binary and wallet code is dropped from into the `bitcoin-node` binary. --- This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). ACKs for top commit: lsilva01: reACK d5f985e hebasto: re-ACK d5f985e51f2863fda0c0d632e836eba42a5c3e15, only suggested changes since my [previous](https://github.com/bitcoin/bitcoin/pull/23006#pullrequestreview-787537444) review. Tree-SHA512: 6784210bd9ce3a6fbc66852680d0e9bc513c072dc538aeb7f48cb6a41580d3f567ccef04f975ee767a714a4b05d4d87273e94a79abda1b9c25d5ac4bbe752006
2021-10-26test: add script_util helper for creating bare multisig scriptsSebastian Falbesoner
2021-10-26Merge bitcoin/bitcoin#23332: doc: Fix CWalletTx::Confirmation docMarcoFalke
fa8fef6ef2287cd36ae14fbf10e852ddef7e62ac doc: Fix CWalletTx::Confirmation doc (MarcoFalke) Pull request description: Follow-up to: * commit 700c42b85d20e624bef4228eef062c93084efab5, which replaced pIndex with block_hash in AddToWalletIfInvolvingMe. * commit 9700fcb47feca9d78e005b8d18b41148c8f6b25f, which replaced posInBlock with confirm.nIndex. ACKs for top commit: laanwj: Code review ACK fa8fef6ef2287cd36ae14fbf10e852ddef7e62ac ryanofsky: Code review ACK fa8fef6ef2287cd36ae14fbf10e852ddef7e62ac Tree-SHA512: e7643b8e9200b8ebab3eda30435ad77006568a03476006013c9ac42c826c84e42e29bcdefc6f443fe4d55e76e903bfed5aa7a51f831665001c204634b6f06508
2021-10-25Introduce new V4 format addrmanPieter Wuille
92617b7a758c0425330fba4b886296730567927c effectively changed the on-disk format in an incompatible way: old deserializers cannot deal with multiple entries for the same IP. Introduce a V4_MULTIPORT format, and increment the compatibility base, so that old versions correctly recognize it as an incompatible future version.
2021-10-25scripted-diff: disable unimplemented ArgsManager BOOL/INT/STRING flagsRussell Yanofsky
This commit does not change behavior in any way. See previous commit for complete rationale, but these flags are being disabled because they aren't implemented and will otherwise break backwards compatibility when they are implemented. -BEGIN VERIFY SCRIPT- sed -i 's:\(ALLOW_.*\) \(//!< unimplemented\):// \1\2:' src/util/system.h sed -i '/DISALLOW_NEGATION.*scripted-diff/d' src/util/system.cpp git grep -l 'ArgsManager::ALLOW_\(INT\|STRING\)' | xargs sed -i 's/ArgsManager::ALLOW_\(INT\|STRING\)/ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION/g' git grep -l 'ALLOW_BOOL' -- ':!src/util/system.h' | xargs sed -i 's/ALLOW_BOOL/ALLOW_ANY/g' -END VERIFY SCRIPT-
2021-10-25Merge bitcoin/bitcoin#23306: Make AddrMan support multiple ports per IPMarcoFalke
92617b7a758c0425330fba4b886296730567927c Make AddrMan support multiple ports per IP (Pieter Wuille) Pull request description: For a long part of Bitcoin's history, this codebase has aggressively avoided making automatic connections to anything but nodes running on port 8333. I'd like to propose changing that, and this is a first PR necessary for that. The folklore justification (eventually actually added as a comment to the codebase in #20668) is that this is to prevent the Bitcoin P2P network from being leveraged to perform a DoS attack on other services, if their IP/port would get rumoured. It appears, at least the current network scale - and probably significantly larger - that the impact is very low at best (see calculations by vasild in https://github.com/bitcoin/bitcoin/issues/5150#issuecomment-853888909 e.g.). Another possible justification would be a risk that treating different IP:port combinations separately would help perform Eclipse attacks (by an attacker rumouring their own IP with many ports). This concern is (a) no different than what is possible with IPv6 (where large ranges of IP addresses are very cheaply available), and (b) already hopefully sufficiently addressed by addrman's design (which limits access through based selected based on network groups). And this policy has downsides too; in particular, a fixed port is easy to detect, and a very obvious sign a Bitcoin node is running there. One obstacle in moving away from a default port that is the fact that addrman is currently restricted to a single entry per IP address. If ports are no longer expected to be generally always the default one, we need to deal with the case where conflicting information is relayed. It turns out there is a very natural solution to this: treat (IP,port) combination exactly as we're treating IPs now; this automatically means that the same IP may appear with multiple ports, simply because those would be distinct entries. Given that indexing into addrman's bucket _already_ uses the port number, the only change required is making all addrman lookup be (IP,port) (aka `CService`) based, rather than IP (aka `CNetAddr`) based. This PR doesn't include any change to the actual outbound connection preference logic, as perhaps that's something that we want to phase in more gradually. ACKs for top commit: jnewbery: Code review ACK 92617b7a758c0425330fba4b886296730567927c naumenkogs: ACK 92617b7a758c0425330fba4b886296730567927c ajtowns: ACK 92617b7a758c0425330fba4b886296730567927c vasild: ACK 92617b7a758c0425330fba4b886296730567927c Tree-SHA512: 9eef06ce97a8b54a3f05fb8acf6941f253a9a5e0be8ce383dd05c44bb567cea243b74ee5667178e7497f6df2db93adab97ac66edbc37c883fd8ec840ee69a33f
2021-10-25refactor: Add explicit DISALLOW_NEGATION ArgsManager flag to clarify flag usageRussell Yanofsky
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 two commits, with this commit adding the DISALLOW_NEGATION flag, and the next commit disabling the ALLOW_{INT|BOOL|STRING} flags.
2021-10-25refactor: Split InterpretOption into Interpret{Key,Value} functionsRussell Yanofsky
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2021-10-25Merge bitcoin/bitcoin#23312: tests: reduce feature_segwit.py usage of the ↵MarcoFalke
legacy wallet e9ade032f3283971025943d750b1305bc8da56fc tests: Add feature_segwit.py --descriptors to test_runner.py (Andrew Chow) ae6cbcc90926b65099ed8747e7a13a4aefba787a tests: restrict feature_segwit legacy wallet import tests (Andrew Chow) 1d13c44a4cb02b0acef6c6a108410cfc7eb20f71 tests: Use descriptors for feature_segwit multisig setup (Andrew Chow) Pull request description: `feature_segwit.py` has tests for some legacy wallet behavior, but otherwise does not really need the legacy wallet. Those parts now require `--legacy-wallet` to be provided (as with other legacy wallet tests). Other parts of the test are changed to work with descriptor wallets as well. ACKs for top commit: lsilva01: tACK e9ade03 Tree-SHA512: 4ae76a4d2a8d318e7f8ad4c984748e3cdd67ed00359fcd798a08492ed9399b1d01be88c9ebea3d6c996fbe190cf41708a15c98f088f0cb5c47d6d00fff258944
2021-10-25Merge bitcoin/bitcoin#23311: wallet: Use PACKAGE_NAME to mention our softwareMarcoFalke
da791c7f66243080177f92ce5f38c49305da63dc wallet: Use PACKAGE_NAME to mention our software (Hennadii Stepanov) Pull request description: This PR replaces "bitcoin" and "bitcoind" with `PACKAGE_NAME` in wallet log and error messages. ACKs for top commit: jonatack: ACK da791c7f66243080177f92ce5f38c49305da63dc lsilva01: Tested ACK da791c7 on Ubuntu 20.04. brunoerg: tACK da791c7f66243080177f92ce5f38c49305da63dc stratospher: Tested ACK da791c7 shaavan: ACK da791c7f66243080177f92ce5f38c49305da63dc Tree-SHA512: c613446d9c8c3f85e6f5fec77382c9bc17746a853c89e72e1a3a79bf355d7bd9e455bbde8f9e02a894d225a67029c732cdc68ec8c58ac8237dde27d39dae8be7
2021-10-25Merge bitcoin/bitcoin#23157: txmempool -/-> validation 1/2: improve ↵MarcoFalke
performance of check() and remove dependency on validation 082c5bf099c64e3d27abe9b68a71ce500b693e7e [refactor] pass coinsview and height to check() (glozow) ed6115f1eae0eb4669601106a9aaff078a2f3a74 [mempool] simplify some check() logic (glozow) 9e8d7ad5d9cc4b013826daead9cee09aad539401 [validation/mempool] use Spend/AddCoin instead of UpdateCoins (glozow) 09d18916afb0ecae90700d4befd9d5dc52767970 MOVEONLY: remove single-use helper func CheckInputsAndUpdateCoins (glozow) e8639ec26aaf4de3fae280963434bf1cf2017b6f [mempool] remove now-unnecessary code (glozow) 54c6f3c1da01090aee9691a2c2bee0984a054ce8 [mempool] speed up check() by using coins cache and iterating in topo order (glozow) 30e240f65e69c6dffcd033afc63895345bd51f53 [bench] Benchmark CTxMemPool::check() (glozow) cb1407196fba648aa75504e3ab3d46aa0181563a [refactor/bench] make mempool_stress bench reusable and parameterizable (glozow) Pull request description: Remove the txmempool <-> validation circular dependency by removing txmempool's dependency on validation. There are two functions in txmempool that need validation right now: `check()` and `removeForReorg()`. This PR removes the dependencies in `check()`. This PR also improves the performance of `CTxMemPool::check()` by walking through the entries exactly once, in ascending ancestorcount order, which guarantees that we see parents before children. ACKs for top commit: jnewbery: reACK 082c5bf099c64e3d27abe9b68a71ce500b693e7e GeneFerneau: tACK [082c5bf](https://github.com/bitcoin/bitcoin/pull/23157/commits/082c5bf099c64e3d27abe9b68a71ce500b693e7e) rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/23157/commits/082c5bf099c64e3d27abe9b68a71ce500b693e7e theStack: Code-review ACK 082c5bf099c64e3d27abe9b68a71ce500b693e7e Tree-SHA512: 40ac622af1627b5c3e6abb4f0f035d833265a8c5e8dc88faf5354875dfb5137f137825e54bbd2a2668ed37b145c5d02285f776402629f58596e51853a9a79d29
2021-10-25Merge bitcoin/bitcoin#22711: test: check for specific block reject reasons ↵MarcoFalke
in p2p_segwit.py 45827fd718d51acffdbeb3bb12d1eb96e3fa8bc0 test: check for block reject reasons in p2p_segwit.py [2/2] (Sebastian Falbesoner) 4eb532ff8b5ea9338e6cb1b927abaa43f8e3c94e test: check for block reject reasons in p2p_segwit.py [1/2] (Sebastian Falbesoner) b1488c4dcecb9dda9d7c583c1c9e1bc0852c79b2 test: fix reference to block processing test in p2p_segwit.py (Sebastian Falbesoner) Pull request description: In the test `p2p_segwit.py`, there are many instances where we send a segwit block to a node with the expectation that it is rejected. For this purpose, the helper function `test_witness_block` exists which allows also to check for a specific reject `reason` that is asserted in the debug log: https://github.com/bitcoin/bitcoin/blob/502d22ceed1f90ed41336260f8eb428d3acaf514/test/functional/p2p_segwit.py#L119-L120 This PR aims to increase the precision of the tests by adding the expected reject reasons to all `test_witness_block` call instances (found via `grep`ing after `test_witness_block(.*accepted=False`). For some blocks that are rejected due to failed script verification, the exact reason is only shown in the debug log if parallel script verification is disabled. For this reason, the addition of the reasons is split up in two commits: * first, all instances are tackled that are not subject to script verification, i.e. it doesn't matter whether parallel script verification is enabled or not (e.g. `bad-blk-weight`, `bad-witness-merkle-match`...) * then, we explicitely set `-par=1` to only use one script thread, and add the remaining reasons (`non-mandatory-script-verify-flag` with the more specific reason in parantheses) ACKs for top commit: stratospher: tested ACK 45827fd. Tree-SHA512: 00f31867f11d48b38a42b1e79a1303bda1c797ccf3d8c73e6107d70b062604d51ee2a3f2251e7f068dfafdaf09469d27dfee438d9bc9ebaef7febc4b6ef90a95
2021-10-25Merge bitcoin/bitcoin#23338: tests: speed up coinselector_testsMarcoFalke
a52f1d13409e4ef46277596ec13fa8b421fa1329 walletdb: Use SQLiteDatabase for mock wallet databases (Andrew Chow) a78c2298080f173d0266e708267458a72eb2f600 tests: Place into mapWallet in coinselector_tests (Andrew Chow) Pull request description: #23288 changed coinselector_tests to use `DescriptorScriptPubKeyMan`, but it also ended up significantly slowing down the test, from 4 seconds to over 1 minute. It appears that the source of this slow down is with `CWallet::AddToWallet`, and primarily due to writing data to the mock wallet database. Because the only thing that is actually needed is for the created transaction to be placed into `CWallet::mapWallet`, this PR removes the call to `AddToWallet` and just places the transaction into `mapWallet` directly. This reduces the test time to 5 seconds. To speed things up further, `CreateMockWalletDatabase` is changed to make a `SQLiteDatabase` instead of a `BerkeleyDatabase`. This is safe because there are no tests that require a specific mock database type. ACKs for top commit: brunoerg: tACK a52f1d13409e4ef46277596ec13fa8b421fa1329 lsilva01: tACK a52f1d1. Performed 74.36% better on Ubuntu 20.04 (VM, 12 MB, 8vCPU). glozow: utACK a52f1d13409e4ef46277596ec13fa8b421fa1329 Tree-SHA512: da77936bfd2e816d2e71703567b9389d0ee79f3a4a690802ffe3469df5bed371b296cb822b897f625654dab9436d91fd6bc02364a518a47d746e487d70a72595
2021-10-25refactor: use CWallet const shared pointers when possibleKarl-Johan Alm
While const shared_ptr<X> gives us an immutable shared pointer to a mutable X (we can't set it to some other X later), shared_ptr<const X> gives us a shared pointer to an immutable X. Importantly, we can recast shared_ptr<X> into shared_ptr<const X>, but not the other way around. We do this for two reasons: because it makes the code safer to guarantee the wallet is not modified, and because it further dispells the misconception that const shared_ptr<X> gives immutability to X.
2021-10-25refactor: const shared_ptrsKarl-Johan Alm
Introduce convention to use const shared pointers everywhere, unless the shared pointer is modified at some point, which it very rarely is. We want this convention, as it helps alleviate the misconception that a const shared pointer somehow results in a pointer to an immutable object, which is false.
2021-10-23Merge bitcoin/bitcoin#23215: ci: Add vcpkg tools cachefanquake
f778845d972578e7abdced64ec6129d809c816f6 ci: Add vcpkg tools cache (Hennadii Stepanov) Pull request description: On master (02feae54a53ff654f7cecf17ed467edcd612cd0c) vcpkg downloads some tools that are used internally: ``` ... A suitable version of cmake was not found (required v3.20.0). Downloading portable cmake v3.20.0... Downloading cmake... https://github.com/Kitware/CMake/releases/download/v3.20.2/cmake-3.20.2-windows-i386.zip -> C:\Users\ContainerAdministrator\AppData\Local\Temp\vcpkg\downloads\cmake-3.20.2-windows-i386.zip Extracting cmake... A suitable version of 7zip was not found (required v18.1.0). Downloading portable 7zip v18.1.0... Downloading 7zip... https://www.nuget.org/api/v2/package/7-Zip.CommandLine/18.1.0 -> C:\Users\ContainerAdministrator\AppData\Local\Temp\vcpkg\downloads\7-zip.commandline.18.1.0.nupkg Extracting 7zip... A suitable version of nuget was not found (required v5.5.0). Downloading portable nuget v5.5.0... Downloading nuget... https://dist.nuget.org/win-x86-commandline/v5.5.1/nuget.exe -> C:\Users\ContainerAdministrator\AppData\Local\Temp\vcpkg\downloads\22ea847d-nuget.exe Detecting compiler hash for triplet x64-windows-static... A suitable version of powershell-core was not found (required v7.1.0). Downloading portable powershell-core v7.1.0... Downloading powershell-core... https://github.com/PowerShell/PowerShell/releases/download/v7.1.3/PowerShell-7.1.3-win-x86.zip -> C:\Users\ContainerAdministrator\AppData\Local\Temp\vcpkg\downloads\PowerShell-7.1.3-win-x86.zip Extracting powershell-core... ... ``` If any of downloads above fails the whole CI task fails (see #23107). The most recent failure examples in the master branch: - https://github.com/bitcoin/bitcoin/commit/c001da306b29c46ef1e7421002c3aba3ff5ed514, [log](https://api.cirrus-ci.com/v1/task/5182966176415744/logs/build.log) - https://github.com/bitcoin/bitcoin/commit/12ff8993bcc7315f4a9b69bf16def980bb0e5197, [log](https://api.cirrus-ci.com/v1/task/5367684129882112/logs/build.log) This PR adds vcpkg tools cache. Closes #23107. --- Note for reviewers. Some patches from the initial PR were split into separated PRs: #23310 and #23329. Therefore, a discussion here could be outdated or irrelevant until the recent [push](https://github.com/bitcoin/bitcoin/pull/23215#issuecomment-949556531). ACKs for top commit: sipsorcery: ACK f778845d972578e7abdced64ec6129d809c816f6. Tree-SHA512: 201f4e4d38c00cbec6aaeca4f3e1b60f1c65289fb68b82cead47f37f6af5ac42dd539cf8ed3566f5bd9afe4a7762d42bbabb316d58f47352d4e7bfc2214806fe
2021-10-22walletdb: Use SQLiteDatabase for mock wallet databasesAndrew Chow
Default to SQLiteDatabase instead of BerkeleyDatabase for CreateDummyWalletDatabase. Most tests already use descriptor wallets and the mock db doesn't really matter for tests. The tests where it does matter will make the db directly.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-02 20:30:17 +0000