aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2022-03-05fs: Make compatible with boost 1.78Andrew Chow
Github-Pull: #24104 Rebased-From: dc5d6b0d4793ca978f71f69ef7d6b818794676c2
2022-02-15fees: Always round up fee calculated from a feerateAndrew Chow
When calculating the fee for a given tx size from a fee rate, we should always round up to the next satoshi. Otherwise, if we round down (via truncation), the calculated fee may result in a fee with a feerate slightly less than targeted. This is particularly important for coin selection as a slightly lower feerate than expected can result in a variety of issues. Github-Pull: #22949 Rebased-From: 0fbaef9676a1dcb84bcf95afd8d994831ab327b6
2022-02-15wallet: fix segfault by avoiding invalid default-ctored ↵Sebastian Falbesoner
`external_spk_managers` entry In the method `CWallet::LoadActiveScriptPubKeyMan`, the map `external_spk_managers` (or `internal_spk_managers`, if parameter `internal` is false) is accessed via std::map::operator[], which means that a default-ctored entry is created with a null-pointer as value, if the key doesn't exist. As soon as this value is dereferenced, a segmentation fault occurs, e.g. in `CWallet::KeypoolCountExternalKeys`. The bevaviour can be reproduced by the following steps (starting with empty regtest datadir): $ ./src/bitcoind -regtest -daemon $ ./src/bitcoin-cli -regtest -named createwallet_name=wallet descriptors=true blank=true $ cat regtest-descriptors.txt [ { "desc": "tr([e4445899/49'/1'/0']tprv8ZgxMBicQKsPd8jCeBWsYLEoWxbVgzJDatJ7XkwQ6G3uF4FsHuaziHQ5JZAW4K515nj6kVVwPaNWZSMEcR7aFCwL4tQqTcaoprMKTTtm6Zg/1/*)#mr3llm7f", "timestamp": 1634652324, "active": true, "internal": true, "range": [ 0, 999 ], "next": 0 } ] $ ./src/bitcoin-cli -regtest importdescriptors "$(cat regtest-descriptors.txt)" [ { "success": true } ] $ ./src/bitcoin-cli -regtest getwalletinfo error: timeout on transient error: Could not connect to the server 127.0.0.1:18443 (error code 1 - "EOF reached") Bug reported by Josef Vondrlik (josef-v). Github-Pull: #23333 Rebased-From: 6911ab95f19d2b1f60f2d0b2f3961fa6639d4f31
2022-02-15refactor: include a missing <limits> header in fs.cppJoan Karadimov
... needed for std::numeric_limits<T>::max on WIN32 Github-Pull: #23335 Rebased-From: 077a875d94b51e3c87381133657be98989c8643e
2022-02-15consensus: don't call GetBlockPos in ReadBlockFromDisk without lockJon Atack
Github-Pull: #22895 Rebased-From: 350e034e64d175f3db4c85ddca42e76e279912f6
2022-02-15the result of CWallet::IsHDEnabled() was initialized with true.Saibato
But in case of no keys or a blank hd wallet the iterator would be skipped and not set to false but true, since the loop would be not entered. That had resulted in a wrong return and subsequent false HD and watch-only icon display in gui when reloading a wallet after closing. Update src/wallet/wallet.cpp Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Github-Pull: #22781 Rebased-From: 8733a8e84c4b2e484f6ed6159fcf5f29a360d42e
2022-02-15system: skip trying to set the locale on NetBSDfanquake
Just treat it the same as the other BSDs. Fixes #17379. Github-Pull: #22390 Rebased-From: fdd71448e78f442ffd93a3a3398a5062eaba9f1b
2022-02-15Fix (inverse) meaning of -persistmempoolMarcoFalke
Github-Pull: #23061 Rebased-From: faff17bbde6dcb1482a6210bc48b3192603a446f
2021-08-26Merge bitcoin/bitcoin#22667: [22.x] qt: Pre-rc3 translations updateW. J. van der Laan
aa254a01c1d4d853143e0460a8d6ddc481c9785c qt: Pre-rc3 translations update (Hennadii Stepanov) Pull request description: A regularly updated PR with new translations fetched from Transifex.com. ACKs for top commit: laanwj: Sanity-check (did not review any specific translations) ACK aa254a01c1d4d853143e0460a8d6ddc481c9785c Tree-SHA512: bb380b1afb8af7895ac2de5ccdf489b9a73b6d47334d5eb5608370034795c064b5978106814ada96aaa5abd45a04901e6fd749bddc23149fb4a11dd4e6efe846
2021-08-26Fix build with Boost 1.77.0Rafael Sadowski
BOOST_FILESYSTEM_C_STR changed to accept the path as an argument Github-Pull: bitcoin/bitcoin#22713 Rebased-From: acb7aad27ec8a184808aa7905887e3b2c5d54e9c
2021-08-23qt: Handle new added plurals in bitcoin_en.tsHennadii Stepanov
This step was missed. See translation_process.md Github-Pull: bitcoin-core/gui#406 Rebased-From: 2b3d8f3dde383a53f29b7e7ee53ea364d4ef8938
2021-08-23qt: Pre-rc3 translations updateHennadii Stepanov
2021-08-20clientversion: No suffix #if CLIENT_VERSION_IS_RELEASECarl Dong
Previously, building from a release source tarball would result in a version string like v22.0.0-<commithash>, but we expect just v22.0.0. This commit solves this problem. Also use PACKAGE_VERSION instead of reconstructing it. Github-Pull: bitcoin/bitcoin#22685 Rebased-From: 5100deee5822795d385570a380d3c117d05d851d
2021-08-20wallet: Assert that enough was selected to cover the feesAndrew Chow
When the fee is not subtracted from the outputs, the amount that has been reserved for the fee (change_and_fee - change_amount) must be enough to cover the fee that is needed. It would be a bug to not do so, so use an assert to make this obvious if such a situation were to occur. Github-Pull: bitcoin/bitcoin#22686 Rebased-From: d9262324e80da32725e21d4e0fbfee56f25490b1
2021-08-20wallet: Use GetSelectionAmount for target value calculationsAndrew Chow
For target value calculations, GetSelectionAmount should be used, not m_effective_value or m_value. Specifically, ApproximateBestSubset mistakenly uses m_value when calculating whether the target value has been met. This has been changed to use GetSelectionAmount. Github-Pull: bitcoin/bitcoin#22686 Rebased-From: 2de222c40198d3d528668d78cc52e2ce3fa96765
2021-08-20gui: ensure external signer option remains disabled without signersAndrew Chow
When no external signers are available, the option to enable external signers should always be disabled. However the encrypt wallet checkbox can erroneously re-enable the external signer checkbox. To avoid this, CreateWalletDialog now stores whether signers were available during setSigners so that future calls to external_signer_checkbox->setEnabled can account for whether signers are available. Github-Pull: bitcoin-core/gui#396 Rebased-From: a9b9ca82daefc77ee3c884d3f250460d7cf734a5
2021-08-20qt: Fix regression in "Encrypt Wallet" menu itemHennadii Stepanov
Adding a new item to the m_wallet_selector must follow the establishment of signal-slot connections. Github-Pull: bitcoin-core/gui#393 Rebased-From: d54d94959869b0c363939163b99ba0475751dcb6
2021-08-20consensus/params: simplify ValidDeployment check to avoid gcc warningAnthony Towns
Github-Pull: bitcoin/bitcoin#22597 Rebased-From: 059171009b0138555f311cedc2553015ff618323
2021-08-02Merge bitcoin/bitcoin#22534: [22.x] rc2 backportsW. J. van der Laan
739d19053b152c8f6a5d70461a9a1b93549f135c doc: add info to i2p.md about IBD time and multiple networks (Jon Atack) cc8838ce981a7e6345aa07318d2d857420d6a0de contrib, p2p: update I2P hardcoded seeds (Jon Atack) cd57bb1a6626e0820ae2456cd3b71c140cf83403 guix: Ensure EPOCH_SOURCE_DATE does not include GPG information (Andrew Chow) 219900a1236ec056d24ccefa97af119d12e14303 guix: Remove extra \r from all.SHA256SUMS line ending (Andrew Chow) 38d18c01e25d3a103697c120a50b366414876370 guix, doc: Add a note that codesigners need to rebuild after tagging (Andrew Chow) aa9b6aba0302a3c7345f8e6d73a1868083f87874 guix: Allow changing the base manifest in guix-verify (Andrew Chow) 056e47d88748062ef6d4b2f3d3dbf93d9cadca14 guix: Make all.SHA256SUMS rather than codesigned.SHA256SUMS (Andrew Chow) 8f1e3b31b2b4ba024b2adca31a061bbbd2a1378f script, doc: guix touchups (jonatack) 3bbfc1b8e0660a03c7b63eaf2fc8834b499aa811 Updated Readme, Corrected the codesign typo (h) 34f9f88bc95cca04d3a5b71127ea6425a6e3b762 guix/build: Remove vestigial SKIPATTEST.TAG (Carl Dong) 9e52a30ebd0eb29c4068791fbf12d62091ada116 guix/INSTALL: Misc fixups (Carl Dong) 45e0f3d608f707807eadc2e80fc9f2d4f9230a01 guix: Silence getent(1) invocation (Carl Dong) Pull request description: Currently backports #22511. We can collect up further backports and merge prior to rc2. ACKs for top commit: laanwj: ACK 739d19053b152c8f6a5d70461a9a1b93549f135c Tree-SHA512: 8fc795ee56b7757ff405636a2811bd606ea33ba1160f3f1ea42e0e1478ce8211bb60bf7b16a673b932db40a24b76d47c54e703bf2775d3b9385d9b080183b433
2021-08-02contrib, p2p: update I2P hardcoded seedsJon Atack
Github-Pull: #22589 Rebased-From: 2962640c49cf38f76345e45e63045a8f0eed5c61
2021-07-31qt: Pre-rc2 translations updateHennadii Stepanov
2021-07-21qt: Pre-rc2 translations updateW. J. van der Laan
We forgot this for rc1. Thanks to Hebasto for fixing the import script. Tree-SHA512: 127d0989dabf95867f4542e7f3134ef5d5045418b1411582772d60759e16a5090e30c83bffbbc44b2a496ba830a66bdb0d8ba0d2def43f3462a4f15edf64953a
2021-07-20Merge bitcoin/bitcoin#22499: Update assumed chain paramsfanquake
eeddd1c8fa96cf546b0bf92063cefa4fd8c6b415 Update assumed chain params (Sriram) Pull request description: Update the relevant variables in `src/chainparams.cpp` for `mainnet`, `testnet`, and `signet` as given [here](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-branch-off). To review this PR, check out [this guide](https://github.com/fanquake/core-review/blob/master/update-assumevalid.md). Note: added a 10% overhead to the base value of `mainnet` in `m_assumed_blockchain_size` ACKs for top commit: MarcoFalke: ACK eeddd1c8fa96cf546b0bf92063cefa4fd8c6b415, checked against my node 🌮 bfolkens: ACK eeddd1c - checked against `mainnet` achow101: Code Review ACK eeddd1c8fa96cf546b0bf92063cefa4fd8c6b415 0xB10C: ACK mainnet, testnet, and signet eeddd1c8fa96cf546b0bf92063cefa4fd8c6b415 jamesob: ACK eeddd1c8fa96cf546b0bf92063cefa4fd8c6b415 ([`jamesob/ackr/22499.1.sriramdvt.update_assumed_chain_par`](https://github.com/jamesob/bitcoin/tree/ackr/22499.1.sriramdvt.update_assumed_chain_par)) darosior: ACK eeddd1c8fa96cf546b0bf92063cefa4fd8c6b415 mainnet and testnet Tree-SHA512: 0ab19d2acc6a854c6aa38fba199d61c68cec40f005d1d54341ea32b59aae9b7d1aabfd21d7c0bc79f54be99d3e71d1d727196cab88f370259fd2c6e002d3e43c
2021-07-20Merge bitcoin/bitcoin#22492: wallet: Reorder locks in dumpwallet to avoid ↵MarcoFalke
lock order assertion 9b85a5e2f7e003ca8621feaac9bdd304d19081b4 tests: Test for dumpwallet lock order issue (Andrew Chow) 25d99e6511d8c43b2025a89bcd8295de755346a7 Reorder dumpwallet so that cs_main functions go first (Andrew Chow) Pull request description: When a wallet is loaded which has an unconfirmed transaction in the mempool, it will end up establishing the lock order of cs_wallet -> cs_main -> cs_KeyStore. If `dumpwallet` is used on this wallet, then a lock order of cs_wallet -> cs_KeyStore -> cs_main will be used, which causes a lock order assertion. This PR fixes this by reordering `dumpwallet` and `GetKeyBirthTimes` (only used by `dumpwallet`). Specifically, in both functions, the function calls which lock cs_main are done prior to locking cs_KeyStore. This avoids the lock order issue. Additionally, I have added a test case to `wallet_dump.py`. Of course testing this requires `--enable-debug`. Fixes #22489 ACKs for top commit: MarcoFalke: review ACK 9b85a5e2f7e003ca8621feaac9bdd304d19081b4 🎰 ryanofsky: Code review ACK 9b85a5e2f7e003ca8621feaac9bdd304d19081b4. Nice to reduce lock scope, and good test! prayank23: tACK https://github.com/bitcoin/bitcoin/pull/22492/commits/9b85a5e2f7e003ca8621feaac9bdd304d19081b4 lsilva01: Tested ACK https://github.com/bitcoin/bitcoin/pull/22492/commits/9b85a5e2f7e003ca8621feaac9bdd304d19081b4 under the same conditions reported in issue #22489 and the `dumpwallet` command completed successfully. Tree-SHA512: d370a8f415ad64ee6a538ff419155837bcdbb167e3831b06572562289239028c6b46d80b23d227286afe875d9351f3377574ed831549ea426fb926af0e19c755
2021-07-20Merge bitcoin/bitcoin#22261: [p2p/mempool] Two small fixes to node broadcast ↵fanquake
logic 5a77abd4e657458852875a07692898982f4b1db5 [style] Clean up BroadcastTransaction() (John Newbery) 7282d4c0363ab5152baa34af626cb49afbfddc32 [test] Allow rebroadcast for same-txid-different-wtxid transactions (glozow) cd48372b67d961fe661990a2c6d3cc3d91478924 [mempool] Allow rebroadcast for same-txid-different-wtxid transactions (John Newbery) 847b6ed48d7bacec9024618922e9b339d2d97676 [test] Test transactions are not re-added to unbroadcast set (Duncan Dean) 2837a9f1eaa2c6bf402d1d9891d9aa84c4a56033 [mempool] Only add a transaction to the unbroadcast set when it's added to the mempool (John Newbery) Pull request description: 1. Only add a transaction to the unbroadcast set when it's added to the mempool Currently, if BroadcastTransaction() is called to rebroadcast a transaction (e.g. by ResendWalletTransactions()), then we add the transaction to the unbroadcast set. That transaction has already been broadcast in the past, so peers are unlikely to request it again, meaning RemoveUnbroadcastTx() won't be called and it won't be removed from m_unbroadcast_txids. Net processing will therefore continue to attempt rebroadcast for the transaction every 10-15 minutes. This will most likely continue until the node connects to a new peer which hasn't yet seen the transaction (or perhaps indefinitely). Fix by only adding the transaction to the broadcast set when it's added to the mempool. 2. Allow rebroadcast for same-txid-different-wtxid transactions There is some slightly unexpected behaviour when: - there is already transaction in the mempool (the "mempool tx") - BroadcastTransaction() is called for a transaction with the same txid as the mempool transaction but a different witness (the "new tx") Prior to this commit, if BroadcastTransaction() is called with relay=true, then it'll call RelayTransaction() using the txid/wtxid of the new tx, not the txid/wtxid of the mempool tx. For wtxid relay peers, in SendMessages(), the wtxid of the new tx will be taken from setInventoryTxToSend, but will then be filtered out from the vector of wtxids to announce, since m_mempool.info() won't find the transaction (the mempool contains the mempool tx, which has a different wtxid from the new tx). Fix this by calling RelayTransaction() with the wtxid of the mempool transaction in this case. The third commit is a comment/whitespace only change to tidy up the BroadcastTransaction() function. ACKs for top commit: duncandean: reACK 5a77abd naumenkogs: ACK 5a77abd4e657458852875a07692898982f4b1db5 theStack: re-ACK 5a77abd4e657458852875a07692898982f4b1db5 lsilva01: re-ACK https://github.com/bitcoin/bitcoin/pull/22261/commits/5a77abd4e657458852875a07692898982f4b1db5 Tree-SHA512: d1a46d32a9f975220e5b432ff6633fac9be01ea41925b4958395b8d641680500dc44476b12d18852e5b674d2d87e4d0160b4483e45d3d149176bdff9f4dc8516
2021-07-20Merge bitcoin/bitcoin#22096: p2p: AddrFetch - don't disconnect on ↵fanquake
self-announcements 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 test: Add functional test for AddrFetch connections (Martin Zumsande) c34ad3309f93979b274a37de013502b05d25fad8 net, rpc: Enable AddrFetch connections for functional testing (Martin Zumsande) 533500d9072b7d5a36a6491784bdeb9247e91fb0 p2p: Add timeout for AddrFetch peers (Martin Zumsande) b6c5d1e450dde6a54bd785504c923adfb45c7060 p2p: AddrFetch - don't disconnect on self-announcements (Martin Zumsande) Pull request description: AddrFetch connections (old name: oneshots) are intended to be short-lived connections on which we ask a peer for addresses via `getaddr` and disconnect after receiving them. This is done by disconnecting after receiving the first `addr`. However, it is no longer working as intended, because nowadays, the first `addr` a typical bitcoin core node sends is its self-announcement. So we'll disconnect before the peer gets a chance to answer our `getaddr`. I checked that this affects both `-seednode` peers specified manually, and DNS seeds when AddrFetch is used as a fallback if DNS doesn't work for us. The current behavior of getting peers via AddrFetch when starting with an empty addrman would be to connect to the peer, receive its self-announcement and add it to addrman, disconnect, reconnect to the same peer again as a full outbound (no other addresses in addrman) and then receive more `addr`. This is silly and not in line with AddrFetch peer being intended to be short-lived peers.  Fix this by only disconnecting after receiving an `addr` message of size > 1. [Edit] As per review discussion, this PR now also adds a timeout after which we disconnect if we haven't received any suitable `addr`, and a functional test. ACKs for top commit: amitiuttarwar: reACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 naumenkogs: ACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 jnewbery: ACK 5730a43703 Tree-SHA512: 8a81234f37e827705138eb254223f7f3b3bf44a06cb02126fc7990b0d231b9bd8f07d38d185cc30d55bf35548a6fdc286b69602498d875b937e7c58332158bf9
2021-07-20Merge bitcoin/bitcoin#22502: scripted-diff: Revert "fuzz: Add Temporary ↵fanquake
debug assert for oss-fuzz issue" facd56750c8a6aee88eeef75d8c8233778d35757 scripted-diff: Revert "fuzz: Add Temporary debug assert for oss-fuzz issue" (MarcoFalke) Pull request description: No longer needed, as it wouldn't help to debug this issue. See https://github.com/bitcoin/bitcoin/pull/22472#issuecomment-882692900 ACKs for top commit: fanquake: ACK facd56750c8a6aee88eeef75d8c8233778d35757 Tree-SHA512: 13352b3529c43d6e65ab127134b32158d3072dc2fbbb326fea9adfeada5a8610d0477ea75748b8b68e7abb3b9869a989df3a3169e92bdd458053d64bae6ed379
2021-07-20Merge bitcoin/bitcoin#22497: scripted-diff: remove ResetI2PPorts() (revert ↵fanquake
e0a2b390c14) d4b67c8ebc2bb7488bcaaccc3a801cdef1cf1678 scripted-diff: remove ResetI2PPorts() (revert e0a2b390c14) (Vasil Dimov) Pull request description: `CAddrMan::ResetI2PPorts()` was temporary. Remove it: * it has partially achieved its goal: probably ran on about half of the I2P nodes * it is hackish, deemed risky and two bugs where found in it: https://github.com/bitcoin/bitcoin/issues/22467 https://github.com/bitcoin/bitcoin/issues/22470 -BEGIN VERIFY SCRIPT- git show e0a2b390c144e123e2fc8a289fdff36815476964 |git apply -R -END VERIFY SCRIPT- Fixes https://github.com/bitcoin/bitcoin/issues/22467 Fixes https://github.com/bitcoin/bitcoin/issues/22470 ACKs for top commit: laanwj: ACK d4b67c8ebc2bb7488bcaaccc3a801cdef1cf1678 MarcoFalke: review ACK d4b67c8ebc2bb7488bcaaccc3a801cdef1cf1678 😲 jonatack: ACK d4b67c8ebc2bb7488bcaaccc3a801cdef1cf1678 per IRC discussions https://www.erisian.com.au/bitcoin-core-dev/log-2021-07-16.html#l-212 and https://www.erisian.com.au/bitcoin-core-dev/log-2021-07-19.html#l-210 Tree-SHA512: 60d8f0ea0f66a8fcedfcb9c8944a419b974b15509b54ddfeec58db49ae9418e6916df712bba3fbd6b29497d85f7951fb9aa2e48eb9c59f88d09435685bd00b4c
2021-07-19scripted-diff: Revert "fuzz: Add Temporary debug assert for oss-fuzz issue"MarcoFalke
-BEGIN VERIFY SCRIPT- git show faf1af58f85da74f94c6b5f6910c7faf7b47cc88 | git apply --reverse -END VERIFY SCRIPT-
2021-07-19Reorder dumpwallet so that cs_main functions go firstAndrew Chow
DEBUG_LOCKORDER expects cs_wallet, cs_main, and cs_KeyStore to be acquired in that order. However dumpwallet would take these in the order cs_wallet, cs_KeyStore, cs_main. So when configured with `--enable-debug`, it is possible to hit the lock order assertion when using dumpwallet. To fix this, cs_wallet and cs_KeyStore are no longer locked at the same time. Instead cs_wallet will be locked first. Then the functions which lock cs_main will be run. Lastly cs_KeyStore will be locked afterwards. This avoids the lock order issue. Furthermore, since GetKeyBirthTimes (only used by dumpwallet) also uses a function that locks cs_main, and itself also locks cs_KeyStore, the same reordering is done here.
2021-07-19Update assumed chain paramsSriram
Note: 10% overhead to the base value of `mainnet` in `m_assumed_blockchain_size`
2021-07-19scripted-diff: remove ResetI2PPorts() (revert e0a2b390c14)Vasil Dimov
`CAddrMan::ResetI2PPorts()` was temporary. Remove it: * it has partially achieved its goal: probably ran on about half of the I2P nodes * it is hackish, deemed risky and two bugs where found in it https://github.com/bitcoin/bitcoin/issues/22467 https://github.com/bitcoin/bitcoin/issues/22470 -BEGIN VERIFY SCRIPT- git show e0a2b390c144e123e2fc8a289fdff36815476964 |git apply -R -END VERIFY SCRIPT- Fixes https://github.com/bitcoin/bitcoin/issues/22467 Fixes https://github.com/bitcoin/bitcoin/issues/22470
2021-07-19Merge bitcoin/bitcoin#22455: addrman: detect on-disk corrupted nNew and ↵MarcoFalke
nTried during unserialization 816f29eab296ebec2da8f8606ad618609e3ba228 addrman: detect on-disk corrupted nNew and nTried during unserialization (Vasil Dimov) Pull request description: Negative `nNew` or `nTried` are not possible during normal operation. So, if we read such values during unserialize, report addrman corruption. Fixes https://github.com/bitcoin/bitcoin/issues/22450 ACKs for top commit: MarcoFalke: cr ACK 816f29eab296ebec2da8f8606ad618609e3ba228 jonatack: ACK 816f29eab296ebec2da8f8606ad618609e3ba228 lsilva01: Code Review ACK https://github.com/bitcoin/bitcoin/pull/22455/commits/816f29eab296ebec2da8f8606ad618609e3ba228. This change provides a more accurate description of the error. Tree-SHA512: 01bdd72d2d86a0ef770a319fee995fd1e147b24a8db84ddb8cd121688e7f94fed73fddc0084758e7183c4f8d08e971f0b1b224f5adb10928a5aa4dbbc8709d74
2021-07-19Merge bitcoin/bitcoin#22387: Rate limit the processing of rumoured addressesW. J. van der Laan
a4bcd687c934d47aa3922334e97e579caf5f8124 Improve tests using statistics (John Newbery) f424d601e1b6870e20bc60f5ccba36d2e210377b Add logging and addr rate limiting statistics (Pieter Wuille) b4ece8a1cda69cc268d39d21bba59c51fa2fb9ed Functional tests for addr rate limiting (Pieter Wuille) 5648138f5949013331c017c740646e2f4013bc24 Randomize the order of addr processing (Pieter Wuille) 0d64b8f709b4655d8702f810d4876cd8d96ded82 Rate limit the processing of incoming addr messages (Pieter Wuille) Pull request description: The rate at which IP addresses are rumoured (through ADDR and ADDRV2 messages) on the network seems to vary from 0 for some non-participating nodes, to 0.005-0.025 addr/s for recent Bitcoin Core nodes. However, the current codebase will happily accept and process an effectively unbounded rate from attackers. There are measures to limit the influence attackers can have on the addrman database (bucket restrictions based on source IPs), but still - there is no need to permit them to feed us addresses at a rate that's orders of magnitude larger than what is common on the network today, especially as it will cause us to spam our peers too. This PR implements a [token bucket](https://en.wikipedia.org/wiki/Token_bucket) based rate limiter, allowing an average of 0.1 addr/s per connection, with bursts up to 1000 addresses at once. Whitelisted peers as well as responses to GETADDR requests are exempt from the limit. New connections start with 1 token, so as to not interfere with the common practice of peers' self-announcement. ACKs for top commit: laanwj: ACK a4bcd687c934d47aa3922334e97e579caf5f8124 vasild: ACK a4bcd687c934d47aa3922334e97e579caf5f8124 jnewbery: ACK a4bcd687c934d47aa3922334e97e579caf5f8124 jonatack: ACK a4bcd687c934d47aa3922334e97e579caf5f8124 Tree-SHA512: b757de76ad78a53035b622944c4213b29b3b55d3d98bf23585afa84bfba10808299d858649f92269a16abfa75eb4366ea047eae3216f7e2f6d3c455782a16bea
2021-07-18Merge bitcoin/bitcoin#22421: Make IsSegWitOutput return true for taproot outputsSamuel Dobson
8465978f235e2e43feb5dabe2a4d61026343b6ab Make IsSegWitOutput return true for taproot outputs (Pieter Wuille) Pull request description: This fixes a bug: currently `utxoupdatepsbt` will not fill in UTXO data for PSBTs spending taproot outputs. ACKs for top commit: achow101: Code Review ACK 8465978f235e2e43feb5dabe2a4d61026343b6ab jonatack: ACK 8465978f235e2e43feb5dabe2a4d61026343b6ab meshcollider: utACK 8465978f235e2e43feb5dabe2a4d61026343b6ab Tree-SHA512: 2f8f873450bef4b5a4ce5962a231297b386c6b1445e69ce5f36ab28eca7343be3a11bc09c38534b0f75e6f99ba15d78d3ba5d484f6c63e5a9775e1f3f55a74e0
2021-07-18Merge bitcoin/bitcoin#22445: fuzz: Move implementations of non-template fuzz ↵MarcoFalke
helpers from util.h to util.cpp a2aca207b1ad00ec05d7533dbd75bbff830e1d75 Move implementations of non-template fuzz helpers (Sriram) Pull request description: There are 78 cpp files that include `util.h` (`grep -iIr "#include <test/fuzz/util.h>" src/test/fuzz | wc -l`). Modifying the implementation of a fuzz helper in `src/test/fuzz/util.h` will cause all fuzz tests to be recompiled. Keeping the declarations of these non-template fuzz helpers in `util.h` and moving their implementations to `util.cpp` will skip the redundant recompilation of all the fuzz tests, and builds these helpers only once in `util.cpp`. Functions moved from `util.h` to `util.cpp`: - `ConsumeTxMemPoolEntry` - `ContainsSpentInput` - `ConsumeNetAddr` - Methods of `FuzzedFileProvider::(open, read, write, seek, close)` ACKs for top commit: MarcoFalke: review ACK a2aca207b1ad00ec05d7533dbd75bbff830e1d75 🍂 Tree-SHA512: e7037ebb86d0fc56048e4f3d8733eefc21da11683b09d2b22926bda410719628d89c52ddd9b4c18aa243607a66fdb4d13a63e62ca010e66b3ec9174fd18107f0
2021-07-18Merge bitcoin/bitcoin#22461: wallet: Change ScriptPubKeyMan::Upgrade default ↵Samuel Dobson
to True 5012a7912ee9fa35bc417cb073eebffd85f36c6c Test that descriptor wallet upgrade does nothing (Andrew Chow) 48bd7d3b7737656052d2c745ed40c7f6670842cf Change ScriptPubKeyMan::Upgrade to default to return true (Andrew Chow) Pull request description: When adding a new ScriptPubKeyMan, it's likely that there will be nothing for `Upgrade` to do. If it is called (via `upgradewallet`), then it should do nothing, successfully. This PR changes the default `ScriptPubKeyMan::Upgrade` function so that it returns a success instead of failure when doing nothing. Fixes #22460 ACKs for top commit: jonatack: ACK 5012a7912ee9fa35bc417cb073eebffd85f36c6c meshcollider: utACK 5012a7912ee9fa35bc417cb073eebffd85f36c6c Tree-SHA512: 578c6521e997f7bb5cc44be2cfe9e0a760b6bd4aa301026a6b8b3282e8757473e4cb9f68b2e79dacdc2b42dddae718450072e0a38817df205dfea177a74d7f3d
2021-07-18Merge bitcoin/bitcoin#22234: build: Mark print-% target as phony.fanquake
fb7be92b094477131140b58a4e3ae98366b93e76 Mark print-% target as phony. (Dmitry Goncharov) Pull request description: .PHONY does not take patterns (such as print-%) as prerequisites. Have print-% depend on force and mark force as phony. This change ensures print-% rule works even when there is a file that matches the target. ``` $ # on master $ make print-host host=x86_64-pc-linux-gnu $ touch print-host $ make print-host make: 'print-host' is up to date. $ $ git co mark_print_as_phony Switched to branch 'mark_print_as_phony' $ make print-host host=x86_64-pc-linux-gnu $ touch force $ make print-host host=x86_64-pc-linux-gnu ``` ACKs for top commit: hebasto: ACK fb7be92b094477131140b58a4e3ae98366b93e76, tested on Linux Mint 20.2 (x86_64). Tree-SHA512: b89ae66aa8c7aa6a7ab5f0956f9eb3b3ef9d56994b60dc2a97d498d4c1bba537845c190723e8a10310280b1b35df2cd935cc30aeb76735cac2dc621ad7823772
2021-07-18Merge bitcoin/bitcoin#21430: build: Add -Werror=implicit-fallthrough compile ↵fanquake
flag 3c4c8e79baf02af97ba1502189f649b04ef2198d build: Add -Werror=implicit-fallthrough compile flag (Hennadii Stepanov) 014110c47d94ece6e3e655cdbf02ed8c91c7a5cf Use C++17 [[fallthrough]] attribute, and drop -Wno-implicit-fallthrough (Hennadii Stepanov) Pull request description: ACKs for top commit: fanquake: ACK 3c4c8e79baf02af97ba1502189f649b04ef2198d - looks ok to me now. Checked that warnings occur in our code & leveldb by removing a `[[fallthrough]]` or `FALLTHROUGH_INTENDED`. jarolrod: ACK 3c4c8e79baf02af97ba1502189f649b04ef2198d theStack: ACK 3c4c8e79baf02af97ba1502189f649b04ef2198d Tree-SHA512: 4dce91f0f26b8a3de09bd92bb3d7e1995e078e3a8b3ff861c4fbf6c0b32b2327d063633b07b89c4aa94a1141d7f78d46d9d43ab8df865273e342693ad30645b6
2021-07-15Make IsSegWitOutput return true for taproot outputsPieter Wuille
2021-07-15bench: fix 32-bit narrowing warning in bench/peer_eviction.cppJon Atack
2021-07-15Add logging and addr rate limiting statisticsPieter Wuille
Includes logging improvements by Vasil Dimov and John Newbery.
2021-07-15Randomize the order of addr processingPieter Wuille
2021-07-15Rate limit the processing of incoming addr messagesPieter Wuille
While limitations on the influence of attackers on addrman already exist (affected buckets are restricted to a subset based on incoming IP / network group), there is no reason to permit them to let them feed us addresses at more than a multiple of the normal network rate. This commit introduces a "token bucket" rate limiter for the processing of addresses in incoming ADDR and ADDRV2 messages. Every connection gets an associated token bucket. Processing an address in an ADDR or ADDRV2 message from non-whitelisted peers consumes a token from the bucket. If the bucket is empty, the address is ignored (it is not forwarded or processed). The token counter increases at a rate of 0.1 tokens per second, and will accrue up to a maximum of 1000 tokens (the maximum we accept in a single ADDR or ADDRV2). When a GETADDR is sent to a peer, it immediately gets 1000 additional tokens, as we actively desire many addresses from such peers (this may temporarily cause the token count to exceed 1000). The rate limit of 0.1 addr/s was chosen based on observation of honest nodes on the network. Activity in general from most nodes is either 0, or up to a maximum around 0.025 addr/s for recent Bitcoin Core nodes. A few (self-identified, through subver) crawler nodes occasionally exceed 0.1 addr/s.
2021-07-15Change ScriptPubKeyMan::Upgrade to default to return trueAndrew Chow
If a ScriptPubKeyMan does not implement Upgrade, then using upgraewallet will fail unexpectedly. By changing the default to return true, then this error can be avoided. This is still correct because a successful upgrade can be that nothing happened.
2021-07-15Merge bitcoin/bitcoin#22211: net: relay I2P addresses even if not reachable ↵W. J. van der Laan
(by us) 7593b06bd1262f438bf34769ecc00e9c22328e56 test: ensure I2P addresses are relayed (Vasil Dimov) e7468139a1dddd4946bc70697ec38816b3fa8f9b test: make CAddress in functional tests comparable (Vasil Dimov) 33e211d2a442e4555ff3401f92af4ee2f7552568 test: implement ser/unser of I2P addresses in functional tests (Vasil Dimov) 86742811ce3662789ac85334008090a3b54babe3 test: use NODE_* constants instead of magic numbers (Vasil Dimov) ba45f0270815d54ae3290efc16324c2ff1984565 net: relay I2P addresses even if not reachable (by us) (Vasil Dimov) Pull request description: Nodes that can reach the I2P network (have set `-i2psam=`) will relay I2P addresses even without this patch. However, nodes that can't reach the I2P network will not. This was done as a precaution in https://github.com/bitcoin/bitcoin/pull/20119 before anybody could connect to I2P because then, for sure, it would have been useless. Now, however, we have I2P support and a bunch of I2P nodes, so get all nodes on the network to relay I2P addresses to help with propagation, similarly to what we do with Tor addresses. ACKs for top commit: jonatack: ACK 7593b06bd1262f438bf34769ecc00e9c22328e56 naumenkogs: ACK 7593b06bd1262f438bf34769ecc00e9c22328e56. laanwj: Code review ACK 7593b06bd1262f438bf34769ecc00e9c22328e56 kristapsk: ACK 7593b06bd1262f438bf34769ecc00e9c22328e56. Code looks correct, tested that functional test suite passes and also that `test/functional/p2p_addrv2_replay.py` fails if I undo changes in `IsRelayable()`. Tree-SHA512: c9feec4a9546cc06bc2fec6d74f999a3c0abd3d15b7c421c21fcf2d610eb94611489e33d61bdcd5a4f42041a6d84aa892f7ae293b0d4f755309a8560b113b735
2021-07-15Merge bitcoin/bitcoin#22284: p2p, refactor: performance improvements to ↵W. J. van der Laan
ProtectEvictionCandidatesByRatio() b1d905c225e87a4a289c0cd3593c6c21cea3fba7 p2p: earlier continuation when no remaining eviction candidates (Vasil Dimov) c9e8d8f9b168dec2bc7b845da38449e96708cf8e p2p: process more candidates per protection iteration (Jon Atack) 02e411ec456af80d1da76085a814c68bb3aca6de p2p: iterate eviction protection only on networks having candidates (Jon Atack) 5adb06457403f8c1d874e9c6748ecbb78ef8fa2b bench: add peer eviction protection benchmarks (Jon Atack) 566357f8f7471f74729297868917aa32f6d3c390 refactor: move GetRandomNodeEvictionCandidates() to test utilities (Jon Atack) Pull request description: This follow-up to #21261 improves `ProtectEvictionCandidatesByRatio()` for better performance. Benchmarks are added; the performance improvement is between 2x and 5x for the benchmarked cases (CPU 2.50GHz, Turbo off, performance mode, Debian Clang 11 non-debug build). ``` $ ./src/bench/bench_bitcoin -filter="EvictionProtection*.*" ``` The refactored code is well-covered by existing unit tests and also a fuzzer. - `$ ./src/test/test_bitcoin -t net_peer_eviction_tests` - `$ FUZZ=node_eviction ./src/test/fuzz/fuzz ../qa-assets/fuzz_seed_corpus/node_eviction` ACKs for top commit: klementtan: Tested and code review ACK b1d905c2. vasild: ACK b1d905c225e87a4a289c0cd3593c6c21cea3fba7 jarolrod: ACK b1d905c225e87a4a289c0cd3593c6c21cea3fba7 Tree-SHA512: a3a6607b9ea2fec138da9780c03f63e177b6712091c5a3ddc3804b896a7585216446310280791f5e20cc023d02d2f03a4139237e12b5c1d7f2a1fa1011610e96
2021-07-15addrman: detect on-disk corrupted nNew and nTried during unserializationVasil Dimov
Negative `nNew` or `nTried` are not possible during normal operation. So, if we read such values during unserialize, report addrman corruption. Fixes https://github.com/bitcoin/bitcoin/issues/22450
2021-07-15Merge bitcoin/bitcoin#22415: Make m_mempool optional in CChainStateMarcoFalke
ceb7b35a39145717e2d9d356fd382bd1f95d2a5a refactor: move UpdateTip into CChainState (James O'Beirne) 4abf0779d6594e97222279110c328b75b5f3db7b refactor: no mempool arg to GetCoinsCacheSizeState (James O'Beirne) 46e3efd1e4ae2f058ecfffdaee7e882c4305eb35 refactor: move UpdateMempoolForReorg into CChainState (James O'Beirne) 617661703ac29e0744f21de74501d033fdc53ff6 validation: make CChainState::m_mempool optional (James O'Beirne) Pull request description: Make `CChainState::m_mempool` optional by making it a pointer instead of a reference. This will allow a simplification to assumeutxo semantics (see https://github.com/bitcoin/bitcoin/pull/15606#pullrequestreview-692965905) and help facilitate the `-nomempool` option. ACKs for top commit: jnewbery: ACK ceb7b35a39145717e2d9d356fd382bd1f95d2a5a naumenkogs: ACK ceb7b35a39145717e2d9d356fd382bd1f95d2a5a ryanofsky: Code review ACK ceb7b35a39145717e2d9d356fd382bd1f95d2a5a (just minor style and test tweaks since last review) lsilva01: Code review ACK and tested on Signet ACK https://github.com/bitcoin/bitcoin/pull/22415/commits/ceb7b35a39145717e2d9d356fd382bd1f95d2a5a MarcoFalke: review ACK ceb7b35a39145717e2d9d356fd382bd1f95d2a5a 😌 Tree-SHA512: cc445ad33439d5918cacf80a6354eea8f3d33bb7719573ed5b970fad1a0dab410bcd70be44c862b8aba1b71263b82d79876688c553e339362653dfb3d8ec81e6
2021-07-15Merge bitcoin/bitcoin#22385: refactor: Use DeploymentEnabled to hide VB ↵MarcoFalke
deployments fa5658ed077bfb02b6281d642dc649abdb99b6ee Use DeploymentEnabled to hide VB deployments (MarcoFalke) fa11fecf0dac44846a08e1b325547641f2eca957 doc: Move buried deployment doc to the enum that enumerates them (MarcoFalke) Pull request description: Plus a doc commit. ACKs for top commit: jnewbery: utACK fa5658ed077bfb02b6281d642dc649abdb99b6ee ajtowns: utACK fa5658ed077bfb02b6281d642dc649abdb99b6ee Tree-SHA512: 2aeceee0674feb603d76656eff40695b7d7305de309f837bbb6a8c1dbb1d0b962b741f06ab7b9a8b1dbd1964c9c0c9aa5dc9588fd8e6d896e620b69e08eedbaa