aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2021-08-10Add a new RPC command: restorewalletlsilva01
2021-08-10Merge bitcoin/bitcoin#22632: test: Set regtest.BIP66Height = 102 to speed up ā†µW. J. van der Laan
tests fafe896a0b870d85250927bd5374caf73d379468 test: Set regtest.BIP66Height = 102 to speed up tests (MarcoFalke) Pull request description: No need to waste time by forcing creation of more than 1000 blocks to get the benefits of being able to test BIP 66. Also, reducing the height makes it more likely that (third-party) tests are conforming to BIP 66, which is enforced on mainnet for all new blocks. ACKs for top commit: GeneFerneau: Concept + code review ACK [fafe896](https://github.com/bitcoin/bitcoin/pull/22632/commits/fafe896a0b870d85250927bd5374caf73d379468) 0xB10C: crACK fafe896a0b870d85250927bd5374caf73d379468 laanwj: ACK fafe896a0b870d85250927bd5374caf73d379468 Zero-1729: tACK fafe896 kristapsk: ACK fafe896a0b870d85250927bd5374caf73d379468. Full functional test suite showed few second speed incrase on my laptop (although I didn't do proper benchmarking with multiple runs, just single `time ./test/functional/test_runner.py` on current master vs this PR). theStack: Tested ACK fafe896a0b870d85250927bd5374caf73d379468 hg333: tACK https://github.com/bitcoin/bitcoin/commit/fafe896a0b870d85250927bd5374caf73d379468 Tree-SHA512: 4bbee3c8587d612e74a59fde49b6439c1296f2fc27d3a7cf59a35e920f729fdd581c930290bd04def618f81412236676ddb99b4ceb4d80dfb9fd610b128a04b1
2021-08-10Merge bitcoin/bitcoin#22547: cli: Add progress bar for -getinfoSamuel Dobson
b851a92c06cd1d6a40a5527ae39d60ae7fa33fb9 cli: Add progress bar for -getinfo (klementtan) Pull request description: Add a progress bar for the `Verification progress` attribute in `-getinfo` when verification progress `< 99%`. ![image](https://user-images.githubusercontent.com/49265907/127897458-27d8aaa9-7893-4665-9c40-36389a8d9cbb.png) **Motivation**: * Improve user-friendliness of `-getinfo` * Can be useful with `watch -n 1 bitcoin-cli -getinfo`(suggested by theStack [below](https://github.com/bitcoin/bitcoin/pull/22547#issuecomment-887488172)) * The progress bar is only display when are still syncing to tip(verification progress `< 99%`) **Reviewing** If your verification progress is `> 99%` you can restart the verification progress with ```shell $ ./src/bitcoind -reindex $./src/bitcoin-cli -getinfo ``` ACKs for top commit: prayank23: reACK https://github.com/bitcoin/bitcoin/commit/b851a92c06cd1d6a40a5527ae39d60ae7fa33fb9 theStack: re-ACK b851a92c06cd1d6a40a5527ae39d60ae7fa33fb9 šŸ¹ Zero-1729: re-tACK b851a92c06cd1d6a40a5527ae39d60ae7fa33fb9 (re-tested, works as expected šŸ¾) jonatack: ACK b851a92c06cd1d6a40a5527ae39d60ae7fa33fb9 lsilva01: Tested ACK https://github.com/bitcoin/bitcoin/pull/22547/commits/b851a92c06cd1d6a40a5527ae39d60ae7fa33fb9 on mainnet and signet on Ubuntu 20.04. Tree-SHA512: 2046d812e3c4623c6cc3ed4c24f2daaa92ba12cd181fa21626b782743890c2373be3175cff1441a7ba37295b6d5818368deea90d483959875c22f7ad9b601a20
2021-08-09Merge bitcoin/bitcoin#21800: mempool/validation: mempool ancestor/descendant ā†µfanquake
limits for packages accf3d5868460b4b14ab607fd66ac985b086fbb3 [test] mempool package ancestor/descendant limits (glozow) 2b6b26e57c24d2f0abd442c1c33098e3121572ce [test] parameterizable fee for make_chain and create_child_with_parents (glozow) 313c09f7b7beddfdb74c284720d209c81dfdb94f [test] helper function to increase transaction weight (glozow) f8253d69d6f02850995a11eeb71fedc22e6f6575 extract/rename helper functions from rpc_packages.py (glozow) 3cd663a5d33aa7ef87994e452bced7f192d021a0 [policy] ancestor/descendant limits for packages (glozow) c6e016aa139c8363e9b38bbc1ba0dca55700b8a7 [mempool] check ancestor/descendant limits for packages (glozow) f551841d3ec080a2d7a7988c7b35088dff6c5830 [refactor] pass size/count instead of entry to CalculateAncestorsAndCheckLimits (glozow) 97dd1c729d2bbedf9527b914c0cc8267b8a7c21b MOVEONLY: add helper function for calculating ancestors and checking limits (glozow) f95bbf58aaf72aab8a9c5827b1f162f3b8ac38f4 misc package validation doc improvements (glozow) Pull request description: This PR implements a function to calculate mempool ancestors for a package and enforces ancestor/descendant limits on them as a whole. It reuses a portion of `CalculateMemPoolAncestors()`; there's also a small refactor to move the reused code into a generic helper function. Instead of calculating ancestors and descendants on every single transaction in the package and their ancestors, we use a "worst case" heuristic, treating every transaction in the package as each other's ancestor and descendant. This may overestimate everyone's counts, but is still pretty accurate in the our main package use cases, in which at least one of the transactions in the package is directly related to all the others (e.g. 1 parent + 1 child, multiple parents with 1 child, or chains). Note on Terminology: While "package" is often used to describe groups of related transactions _within_ the mempool, here, I only use package to mean the group of not-in-mempool transactions we are currently validating. #### Motivation It would be a potential DoS vector to allow submission of packages to mempool without a proper guard for mempool ancestors/descendants. In general, the purpose of mempool ancestor/descendant limits is to limit the computational complexity of dealing with families during removals and additions. We want to be able to validate multiple transactions on top of the mempool, but also avoid these scenarios: - We underestimate the ancestors/descendants during package validation and end up with extremely complex families in our mempool (potentially a DoS vector). - We expend an unreasonable amount of resources calculating everyone's ancestors and descendants during package validation. ACKs for top commit: JeremyRubin: utACK accf3d5 ariard: ACK accf3d5. Tree-SHA512: 0d18ce4b77398fe872e0b7c2cc66d3aac2135e561b64029584339e1f4de2a6a16ebab3dd5784f376e119cbafc4d50168b28d3bd95d0b3d01158714ade2e3624d
2021-08-09Merge bitcoin/bitcoin#22337: wallet: Use bilingual_str for errorsSamuel Dobson
92993aa5cf37995e65e68dfd6f129ecaf418e01c Change SignTransaction's input_errors to use bilingual_str (Andrew Chow) 171366e89b828a557f8262d9dc14ff7a03f813f7 Use bilingual_str for address fetching functions (Andrew Chow) 9571c69b51115454c6a699be9492024f7b46c2b4 Add bilingual_str::clear() (Andrew Chow) Pull request description: In a couple of places in the wallet, errors are `std::string`. In order for these errors to be translated, change them to use `bilingual_str`. ACKs for top commit: hebasto: re-ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c, only rebased since my [previous](https://github.com/bitcoin/bitcoin/pull/22337#pullrequestreview-694542729) review, verified with klementtan: Code review ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c meshcollider: Code review ACK 92993aa5cf37995e65e68dfd6f129ecaf418e01c Tree-SHA512: 5400e419dd87db8c49b67ed0964de2d44b58010a566ca246f2f0760ed9ef6a9b6f6df7a6adcb211b315b74c727bfe8c7d07eb5690b5922fa5828ceef4c83461f
2021-08-09Merge bitcoin/bitcoin#22359: wallet: Do not set fInMempool in ā†µSamuel Dobson
transactionAddedToMempool when tx is not in the mempool fa6fd3dd6a4e7f30eff5963836aed43fe01af078 wallet: Properly set fInMempool in mempool notifications (MarcoFalke) Pull request description: A wallet method (like bumping the fee) might have set `fInMempool` to false because the transaction was removed from the mempool (See commit fa4e088cbac035b8029a10b492849540150d0622). Avoid setting it back to true (incorrectly) in the validation interface background thread. Fixes #22357 ACKs for top commit: ryanofsky: Code review ACK fa6fd3dd6a4e7f30eff5963836aed43fe01af078. Only change since last review is extending workaround to `transactionRemovedFromMempool`. Since we know this workaround is imperfect and the goal of this PR is mainly to fix CI errors, I would probably be inclined to limit the workaround to as few places as possible where we have seen actual failures, instead of adding the workaround to as many places as possible, where there is some chance it might trigger new failures. But since this workaround is so straightforward and almost looks like a real fix, probably it doesn't matter. meshcollider: utACK fa6fd3dd6a4e7f30eff5963836aed43fe01af078 Tree-SHA512: d690136a577f1f532aa1fee80d3f6600ff7fc61286fbf564a53d7938d5ae52d33f0dbb0fef8b8c041a4970fb424f0b9f1ee7ce791e0ff8354e0000ecc9e22b84
2021-08-09Merge bitcoin/bitcoin#21500: wallet, rpc: add an option to list private ā†µSamuel Dobson
descriptors bb822a7af86897a9b6a5d616f193c258e8e76729 wallet, rpc: add listdescriptors private option (S3RK) Pull request description: Rationale: make it possible to backup your wallet with `listdescriptors` command * The default behaviour is still to show public version * For private version only the root xprv is returned Example use-case: ``` > bitcoin-cli -regtest -named createwallet wallet_name=old descriptors=true > bitcoin-cli -regtest -rpcwallet=old listdescriptors true | jq '.descriptors' > descriptors.txt > bitcoin-cli -regtest -named createwallet wallet_name=new descriptors=true blank=true > bitcoin-cli -regtest -rpcwallet=new importdescriptors "$(cat descriptors.txt)" ``` In case of watch-only wallet without private keys there will be following output: ``` error code: -4 error message: Can't get descriptor string. ``` ACKs for top commit: achow101: re-ACK bb822a7af86897a9b6a5d616f193c258e8e76729 Rspigler: tACK bb822a7af86897a9b6a5d616f193c258e8e76729 jonatack: ACK bb822a7af86897a9b6a5d616f193c258e8e76729 per `git diff 2854ddc bb822a7` prayank23: tACK https://github.com/bitcoin/bitcoin/pull/21500/commits/bb822a7af86897a9b6a5d616f193c258e8e76729 meshcollider: Code review ACK bb822a7af86897a9b6a5d616f193c258e8e76729 Tree-SHA512: f6dddc72a74e5667071ccd77f8dce578382e8e29e7ed6a0834ac2e114a6d3918b59c2f194f4079b3259e13d9ba3b4f405619940c3ecb7a1a0344615aed47c43d
2021-08-07fuzz: Re-enable assert in banman againMarcoFalke
2021-08-06Merge bitcoin-core/gui#396: Ensure external signer option remains disabled ā†µHennadii Stepanov
without signers a9b9ca82daefc77ee3c884d3f250460d7cf734a5 gui: ensure external signer option remains disabled without signers (Andrew Chow) Pull request description: 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. Fixes #395 ACKs for top commit: hebasto: ACK a9b9ca82daefc77ee3c884d3f250460d7cf734a5, tested on Linux Mint 20.2 (Qt 5.12.8). Sjors: tACK a9b9ca82daefc77ee3c884d3f250460d7cf734a5 jarolrod: ACK a9b9ca82daefc77ee3c884d3f250460d7cf734a5 Tree-SHA512: 98951bcadc23fce99a66ea2d367c44360989e888c253845a767e1f7085c594562d0f099de4130f4a078c5072aa7806294097d976ee6407291f3d3c5a4a608b44
2021-08-06Merge bitcoin-core/gui#379: Prompt to reset settings when settings.json ā†µHennadii Stepanov
cannot be read 1ee6d0b01a517893967379677029fb5417978247 gui: Prompt to reset settings when settings.json cannot be read (Russell Yanofsky) Pull request description: Currently the GUI shows confusing error messages when `settings.json` can't be read or written on startup. This causes the unrecoverable read error described in bitcoin/bitcoin#21340 and write error described bitcoin/bitcoin#21974. Current error read message looks like: ![current](https://user-images.githubusercontent.com/7133040/124977362-638ffc80-dffe-11eb-9edd-89135a9bc602.png) This PR tries to clarify the error dialog, and adds an option to just clear the settings and reset them to default: ![new-read-error](https://user-images.githubusercontent.com/7133040/124977636-b669b400-dffe-11eb-8d35-02eda95f48c0.png) ![new-read-details](https://user-images.githubusercontent.com/7133040/124977644-bb2e6800-dffe-11eb-9209-11c1c3d7be40.png) Additionally the PR also shows a slightly better error message when there is an error trying to write the settings file. This error probably should occur less frequently, but it is easy to improve, and it should be good to make the write error consistent with the read error. The new write error dialog looks like: ![new-write-error](https://user-images.githubusercontent.com/7133040/124978016-3bed6400-dfff-11eb-9d79-9b2e9bbc4369.png) ![new-write-details](https://user-images.githubusercontent.com/7133040/124978025-3db72780-dfff-11eb-8df5-741f75a402d9.png) ACKs for top commit: jarolrod: ACK 1ee6d0b01a517893967379677029fb5417978247 Zero-1729: ACK 1ee6d0b01a517893967379677029fb5417978247 hebasto: ACK 1ee6d0b01a517893967379677029fb5417978247, tested on Linux Mint 20.2 (Qt 5.12.8). Tree-SHA512: fb57a0a0d032e3f8219fff49a4de69b4c962bf0b448544ccf9d8d4d45c5bd209e23653d4f13300b9e534b9c03de159498bef1658e95defe3ab6a8ecac57d592c
2021-08-06[policy] ancestor/descendant limits for packagesglozow
2021-08-05gui: 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.
2021-08-05Merge bitcoin/bitcoin#21129: fuzz: check that ser+unser produces the same ā†µMarcoFalke
AddrMan 87651795d8622d354f8e3c481eb868d9433b841c fuzz: check that ser+unser produces the same AddrMan (Vasil Dimov) 6408b24517f3418e2a408071b4c2ce26571f3167 fuzz: move init code to the CAddrManDeterministic constructor (Vasil Dimov) Pull request description: Add a fuzz test that fills addrman with a pile of randomly generated addresses, serializes it to a stream, unserializes the stream to another addrman object and compares the two. Some discussion of this already happened at https://github.com/jnewbery/bitcoin/pull/18. ACKs for top commit: practicalswift: cr ACK 87651795d8622d354f8e3c481eb868d9433b841c jonatack: ACK 87651795d8622d354f8e3c481eb868d9433b841c rebased to current master, reviewed, fuzz build, ran `FUZZ=addrman_serdeser src/test/fuzz/fuzz` Tree-SHA512: 7eda79279f14f2649840bf752e575d7b02cbaad541f74f7254855ebd4a32da988f042d78aa9228983350283bb74dd0c71f51f04c0846889c3ba2f19f01a0c303
2021-08-05[mempool] check ancestor/descendant limits for packagesglozow
When calculating ancestor/descendant counts for transactions in the package, as a heuristic, count every transaction in the package as an ancestor and descendant of every other transaction in the package. This may overestimate, but will not underestimate, the ancestor/descendant counts. This shortcut still produces an accurate count for packages of 1 parent + 1 child.
2021-08-05[refactor] pass size/count instead of entry to CalculateAncestorsAndCheckLimitsglozow
This does not change existing behavior. The ancestor/descendant limits are inclusive of the entries themselves, but CalculateAncestorsAndCheckLimits() does not need access to them.
2021-08-05MOVEONLY: add helper function for calculating ancestors and checking limitsglozow
2021-08-05misc package validation doc improvementsglozow
2021-08-05test: Set regtest.BIP66Height = 102 to speed up testsMarcoFalke
2021-08-05test: Add missing includeMarcoFalke
2021-08-05Merge bitcoin/bitcoin#22277: test: Properly set BIP34 height in ā†µMarcoFalke
CreateNewBlock_validity unit test faa670d3862783017f5cd1491f37648e1875f19f test: Properly set BIP34 height in CreateNewBlock_validity unit test (MarcoFalke) Pull request description: The coinbase scriptSig in this unit test has several issues: * The BIP34 height is not the "first item" as required (See https://github.com/bitcoin/bips/blob/master/bip-0034.mediawiki#specification) * It uses the wrong encoding ( See https://github.com/bitcoin/bitcoin/blob/da69d9965a112c6421fce5649b5a18beb7513526/src/validation.cpp#L3250 ) * It uses the wrong height (off by one) While BIP34 isn't currently enforced in this unit test, this should be fixed to avoid confusion and to promote self-consistency. The change obviously requires new proof of work (`BLOCKINFO`). Also change the block version from `1` to `VERSIONBITS_TOP_BITS`, because this test shouldn't care about the block version and bumping it is required for other changes. ACKs for top commit: theStack: Code review ACK faa670d3862783017f5cd1491f37648e1875f19f Tree-SHA512: 8dbe2d5300a640f3e1817ff048906e60463aca64ba50fec8ee4f18fb1c70e511008755b0b5baba81114a1a6265fdfae9a4b7ae8acadfb2c7ad43223157a0386c
2021-08-05Merge bitcoin/bitcoin#22618: [p2p] Small follow-ups to 21528MarcoFalke
9778b0fec13c047a4c7f3ae425d044da26eadc81 [net_processing] Provide debug error if code assumptions change. (Amiti Uttarwar) aa79c912608fdc77c69dd43653aa87f1f34e01dd [docs] Add release notes for #21528 (Amiti Uttarwar) Pull request description: Adds a release note & addresses [this](https://github.com/bitcoin/bitcoin/pull/21528#discussion_r680963101) review comment to make expectations more explicit. ACKs for top commit: Zero-1729: re-ACK 9778b0fec13c047a4c7f3ae425d044da26eadc81 jonatack: ACK 9778b0fec13c047a4c7f3ae425d044da26eadc81 Tree-SHA512: 9507df5f2746d05c6df8c86b7a19364610ebfafc81af7650be7e68d7536a0685cce9fd2e5f287ef92b6245c584f8875b24a958109ba5bd8acf3c8fc9fd19eef2
2021-08-05Merge bitcoin-core/gui#393: Fix regression in "Encrypt Wallet" menu itemHennadii Stepanov
d54d94959869b0c363939163b99ba0475751dcb6 qt: Fix regression in "Encrypt Wallet" menu item (Hennadii Stepanov) Pull request description: Fix #392. Adding a new item to the `m_wallet_selector` must follow the establishment of a connection between the `WalletView::encryptionStatusChanged` signal and the `BitcoinGUI::updateWalletStatus` slot. This was a regression introduced in https://github.com/bitcoin/bitcoin/commit/20e2e24e90d782219e853ef0676ac66dc6a9de6a (#29). --- An _encrypted_ wallet being auto-loaded at the GUI startup: - on master (eaf09bda4ab21f79f89822d2c6fa3d7a3ce57b0d) ![Screenshot from 2021-08-03 22-38-49](https://user-images.githubusercontent.com/32963518/128075837-cdbb2047-5327-43ea-b2d5-2dcdef67cdc0.png) - with this PR ![Screenshot from 2021-08-03 22-34-58](https://user-images.githubusercontent.com/32963518/128075572-cb727652-ad44-4b85-bf64-edcd19f9dea1.png) ACKs for top commit: achow101: ACK d54d94959869b0c363939163b99ba0475751dcb6 jarolrod: ACK d54d94959869b0c363939163b99ba0475751dcb6 Tree-SHA512: 669615ec8e1517c2f4cdf59bd11a7c85be793ba0dda112361cf95e6c2f0636215fed331d26a86dc9b779a49defae1b248232f98dab449584376c111c288e87bb
2021-08-05Merge bitcoin-core/gui#337: test: Use Regex Search in ApptestsHennadii Stepanov
6969b2bb98a2f44e1b51c905db92ec2e28345078 qt, test: use regex search in apptests (Jarol Rodriguez) d09d1cf1a267b1c5563d8876aa55c4e8f70f0562 qt, test: introduce FindInConsole function (Jarol Rodriguez) Pull request description: This PR refactors our GUI `apptests` so that it uses regex search to find values in our console/qtextedit output regardless if it is in `plaintext`, `html`, or `markdown`. This introduces a new function `FindInConsole` which uses [QRegularExpression](https://doc.qt.io/qt-5/qregularexpression.html) to search the output of the console. The function must be provided with a [perl compatible regex](https://www.debuggex.com/cheatsheet/regex/pcre) pattern which wants to match a single group. The function then returns the matched group. If no match is found, an empty `QString` is returned. We then use this new function in `TestRpcCommand` to find the current `chain` value instead of reading with univalue. This approach can apply to a wider variety of testing scenarios as we can reuse this function to search for values when the console output is exported in a different format than `plaintext`. As an example, A follow up PR will add tests for console resizing and needs to look for the size in `html` tags after exporting the console text with `toHtml()`. ACKs for top commit: hebasto: ACK 6969b2bb98a2f44e1b51c905db92ec2e28345078 ShaMan239: ACK 6969b2bb98a2f44e1b51c905db92ec2e28345078 Tree-SHA512: 4db8bcd4a1acc4539ca64bbd7de572fe7dd6afc3e95108235abfc2891585bc4db3a56a33928fa38e8d44ac87023ce0dee3abcfadfbcd4440e3a21a52fef02536
2021-08-04[net_processing] Provide debug error if code assumptions change.Amiti Uttarwar
Currently, this call to SetupAddressRelay will never return false because of the previous guard that returns early if the peer is not an inbound connection. Rather than implicitly relying on this guarantee, throw an error in the debug build if it ever changes.
2021-08-04[docs] Add release notes for #21528Amiti Uttarwar
And fix a typo in the test.
2021-08-04Merge bitcoin/bitcoin#22621: make ParseOutputType return a ā†µMarcoFalke
std::optional<OutputType> 32fa49a18497a9b8c72e36a72ae96e7b23930223 make ParseOutputType return a std::optional<OutputType> (fanquake) Pull request description: Similar to #22220. Skipped using `auto` here for the same reasons outlined in that PR. ACKs for top commit: jnewbery: utACK 32fa49a18497a9b8c72e36a72ae96e7b23930223 jonatack: Code review ACK 32fa49a18497a9b8c72e36a72ae96e7b23930223 and debian clang 13 debug build is clean / unit tests locally are green MarcoFalke: review ACK 32fa49a18497a9b8c72e36a72ae96e7b23930223 šŸ¢ Tree-SHA512: 7752193117669b800889226185d49d164395697853828f8acb568f07651789bc5b2cddc45555957450353886e46b9a1e13c77a5e730a14c6ee621fabc8dc3d10
2021-08-04fuzz: check that ser+unser produces the same AddrManVasil Dimov
2021-08-04Merge bitcoin/bitcoin#22616: p2p, rpc: address relay fixupsMarcoFalke
5e33f762d44557a1e3f0ff3c280d8a3ab98e3867 p2p, rpc: address relay fixups (Jon Atack) Pull request description: Following review of new changes merged today, move a use of `statestats` in getpeerinfo to within the section guarded by `if (fStateStats)`, e.g. `PeerManagerImpl::GetNodeStateStats` true, and pass an in-param by reference to const. ACKs for top commit: amitiuttarwar: ACK 5e33f762d4 jnewbery: ACK 5e33f762d44557a1e3f0ff3c280d8a3ab98e3867 Tree-SHA512: b42f33c615b14079e2c4e6060209de8707d71b351dd1e11e04a2a6fc12d15747d0c5d9b24850217080fd1ef92e63f96d6925c4badf280b781edd696c349be7d6
2021-08-04Merge bitcoin/bitcoin#22577: Close minor startup race between main and ā†µMarcoFalke
scheduler threads 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942 Close minor startup race between main and scheduler threads (Larry Ruane) Pull request description: This is a low-priority bug fix. The scheduler thread runs `CheckForStaleTipAndEvictPeers()` every 45 seconds (EXTRA_PEER_CHECK_INTERVAL). If its first run happens before the active chain is set up (`CChain::SetTip()`), `bitcoind` will assert: ``` (...) 2021-07-28T22:16:49Z init message: Loading block indexā€¦ bitcoind: validation.cpp:4968: CChainState& ChainstateManager::ActiveChainstate() const: Assertion `m_active_chainstate' failed. Aborted (core dumped) ``` I ran into this while using the debugger to investigate an unrelated problem. Single-stepping through threads with a debugger can cause the relative thread execution timing to be very different than usual. I don't think any automated tests are needed for this PR. I'll give reproduction steps in the next PR comment. ACKs for top commit: MarcoFalke: cr ACK 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942 tryphe: tested ACK 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942 0xB10C: ACK 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942 glozow: code review ACK 703b1e612a4bd4521e20ae21eb8fb7c19f4ef942 - it makes sense to me to start peerman's background tasks here, after `chainstate->LoadChainTip()` and `node.connman->Start()` have been called. Tree-SHA512: 9316ad768cba3b171f62e2eb400e3790af66c47d1886d7965edb38d9710fc8c8f8e4fb38232811c9346732ce311d39f740c5c2aaf5f6ca390ddc48c51a8d633b
2021-08-04make ParseOutputType return a std::optional<OutputType>fanquake
2021-08-04Merge bitcoin/bitcoin#22576: doc: Update high-level addrman descriptionfanquake
036d7eadf5dd0e06e0734a5d78dbe28f4bfaa07f doc: Correct description of CAddrMan::Create() (Amiti Uttarwar) 318176aff1ded36d1fbc5977f288ac3bac1d8712 doc: Update high-level addrman description (Martin Zumsande) Pull request description: The high-level description of `addrman` has outdated information with respect to the eviction behavior, both for the New and Tried tables (at least since #5941) - this has confused me in the past. This PR corrects this and also adds basic info about the bucket size and position. ACKs for top commit: amitiuttarwar: reACK 036d7eadf5dd0e06e0734a5d78dbe28f4bfaa07f jnewbery: ACK 036d7eadf5dd0e06e0734a5d78dbe28f4bfaa07f Tree-SHA512: 3f0635d765f5e580a1fae31187742a833cef66ef2286d40eeb28f2253521260038e16e5f1a65741464a2ddfdbeb5c0f1bc38bf73841e600639033d59c3c534e4
2021-08-03qt: 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.
2021-08-03Merge bitcoin/bitcoin#22609: [GetTransaction] remove unneeded cs_main lock ā†µfanquake
acquire 4a1b2a7ba7f804e656a8cd29d5aa80fcbd40904f [GetTransaction] remove unneeded `cs_main` lock acquire (Sebastian Falbesoner) Pull request description: This PR is a follow-up to #22383. For reading from the mempool, only `mempool.cs` needs to be locked (see [suggestion by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/22383#discussion_r675069128)): https://github.com/bitcoin/bitcoin/blob/b620b2d58a55a88ad21da70cb2000863ef17b651/src/txmempool.h#L554-L558 `CTxMemPool::get()` acquires this lock: https://github.com/bitcoin/bitcoin/blob/b620b2d58a55a88ad21da70cb2000863ef17b651/src/txmempool.cpp#L822-L829 so we don't need to acquire any lock ourselves in `GetTransaction()`, as the other functions called in the remaining parts also don't need to have `cs_main` locked. ACKs for top commit: tryphe: Concept ACK. tested 4a1b2a7ba7f804e656a8cd29d5aa80fcbd40904f but not extensively. jnewbery: Code review ACK 4a1b2a7ba7f804e656a8cd29d5aa80fcbd40904f Tree-SHA512: 60e869f72e65cf72cb144be1900ea7f3d87c12f322756994f6a3ed8cd975230b36c7c90c34b60bbf41f9186f4add36decaac1d4f0d0749fb5451b3938a8aa78c
2021-08-03Merge bitcoin/bitcoin#22597: consensus/params: simplify ValidDeployment ā†µMarcoFalke
check to avoid gcc warning 059171009b0138555f311cedc2553015ff618323 consensus/params: simplify ValidDeployment check to avoid gcc warning (Anthony Towns) Pull request description: Simplifies the ValidDeployment check to only check the upperbound, relying on the lower bound to be trivially true due to enum values starting at the minimum value of the underlying type (which is checked at compile time in deploymentstatus.cpp). Avoids a "comparison always true" warning in some versions of gcc. Fixes #22587 ACKs for top commit: MarcoFalke: review ACK 059171009b0138555f311cedc2553015ff618323 tryphe: retested ACK 059171009b0138555f311cedc2553015ff618323 Tree-SHA512: e53b5d478b46d35ec476d004e3c92803afb874c138dd6ef3848861f4281cc113fe88921bc4ac74fd4decbf318ed776d3f816c3a1185f99dc36a5cfecfff51f7c
2021-08-03p2p, rpc: address relay fixupsJon Atack
2021-08-03Merge bitcoin/bitcoin#16333: test: Set BIP34Height = 2 for regtestMarcoFalke
222290f54388270937cb6c174195717e2214ec0d test: Set BIP34Height = 2 for regtest (MarcoFalke) fac90c55be478f0323eafa1d560ea2c56f04fb23 test: Create all blocks with version 4 or higher (MarcoFalke) Pull request description: BIP34 is active on the current tip of mainnet, so all miners must obey it. It would be nice if it also was active in fresh regtest instances from the earliest time possible. I changed the BIP34 height to `2`, so that the block at height=1 may be used to mine a duplicate coinbase. (Needed to test mainnet behaviour) This pull is done in two commits: * test: Create all blocks with version 4 or higher: Now that BIP34 is activated earlier, we need to create blocks with a higher version number. Just bump it to 4 instead of 2 to avoid having to bump it again later. * test: Set BIP34Height = 2 for regtest: This fixes the BIP34 implementation in the tests (to match the one of the Core codebase) and updates the tests where needed ACKs for top commit: ajtowns: ACK 222290f54388270937cb6c174195717e2214ec0d jonatack: ACK 222290f54388270937cb6c174195717e2214ec0d tested and reviewed rebased to current master 5e213822f86d theStack: Tested ACK 222290f54388270937cb6c174195717e2214ec0d Tree-SHA512: d69c637a62a64b8e87de8c7f0b305823d8f4d115c1852514b923625dbbcf9a4854b5bb3771ff41702ebf47c4c182a4442c6d7c0b9f282c95a34b83e56a73939b
2021-08-03Merge bitcoin/bitcoin#22496: addrman: Remove addrman hotfixesfanquake
65332b1178c75e1f83415bad24918996a1524866 [addrman] Remove RemoveInvalid() (John Newbery) Pull request description: PRs #22179 and #22112 (EDIT: later reverted in #22497) added hotfix code to addrman to remove invalid addresses and mutate the ports of I2P entries after entering into addrman. Those hotfixes included at least two addrman data corruption bugs: - #22467 (Assertion `nRndPos1 < vRandom.size() && nRndPos2 < vRandom.size()' failed) - #22470 (Changing I2P ports in addrman may wronly skip some entries from "new" buckets) Hotfixing addrman is inherently dangerous. There are many members that have implicit assumptions on each others' state, and mutating those directly can lead to violating addrman's internal invariants. Instead of trying to hotfix addrman, just don't insert any invalid addresses. For now, those are addresses which fail `CNetAddr::IsValid()`. ACKs for top commit: sipa: utACK 65332b1178c75e1f83415bad24918996a1524866. I tried to reason through scenarios that could introduce inconsistencies with this code, but can't find any. fanquake: ACK 65332b1178c75e1f83415bad24918996a1524866 - Skipping the addition of invalid addresses (this code was initially added for Tor addrs) rather than adding all the invalids then removing them all when finishing unserializing seems like an improvement. Especially if it can be achieved with less code. Tree-SHA512: 023113764cb475572f15da7bf9824b62b79e10a7e359af2eee59017df354348d2aeed88de0fd4ad7a9f89a0dad10827f99d70af6f1cb20abb0eca2714689c8d7
2021-08-03Merge bitcoin/bitcoin#22448: Update libsecp256k1 subtree to latest upstreamfanquake
c020cbaa5c8e9e61b2b8efd8dc09be743fcd4273 Squashed 'src/secp256k1/' changes from efad3506a8..be8d9c262f (Pieter Wuille) Pull request description: This updates our src/secp256k1 subtree to the lastest upstream master. Notable changes: * New schnorrsig API (https://github.com/bitcoin-core/secp256k1/pull/844), which adds support for variable-length messages (not used in BIP341/342 transaction signing, so not relevant for us, but it changes the API, and makes some other simplifications). Some of our call sites had to be adapted. * Don't use asm optimizations for `gen_context` (https://github.com/bitcoin-core/secp256k1/pull/965). This fixes #22441. * Various testing/CI improvements ACKs for top commit: hebasto: ACK e4ffb44716bb7a7b9f0a5d70ac07058632234370 jonatack: Light ACK e4ffb44716bb7a7b9f0a5d70ac07058632234370 debug built (debian clang 13.0), ran bitcoind node/tests/git-subtree-check.sh, lightly reviewed the diff and API changes fanquake: ACK e4ffb44716bb7a7b9f0a5d70ac07058632234370 Tree-SHA512: 89a5c3019ec010d578e84bcef756d2c679420c5c768bcdece673405c4e10955179c5a1339aafc68b8b74b1e3912e147bf2f392f44f15af73791d93f6537960b3
2021-08-03Merge bitcoin/bitcoin#22098: [test, init] DNS seed querying logicfanquake
82b6f89819e55af26f5264678e0f93052934bcb3 [style] Small style improvements to DNS parameters (Amiti Uttarwar) 4c89e24f64c1dc1a56a3bcb6b5e2b4fb95e8b29f [test] Test the delay before querying DNS seeds (Amiti Uttarwar) 6395c8ed5689ea72e9a1618f14551775246f6361 [test] Test the interactions between -forcednsseed and -dnsseed (Amiti Uttarwar) 6f6b7df6bdcf863af160c0426e3a22028ca8259a [init] Disallow starting up with conflicting paramters for -dnsseed and -forcednsseed (Amiti Uttarwar) 26d0ffe4f2573e0297c9b0e095c2a0868929b08b [test] Test -forcednsseed causes querying DNS seeds (Amiti Uttarwar) 35851450a928ffacca240fadbf1747a42d5ba256 [test] Test the interactions between -connect and -dnsseed (Amiti Uttarwar) 75c05af361552eeecd100cee8cc40d4cd5a3aa27 [test] Test logic to query DNS seeds with block-relay-only connections (Amiti Uttarwar) 9c0871977839c28636eff975748182888299cd2b [test] Introduce test logic to query DNS seeds (Amiti Uttarwar) Pull request description: This PR adds a DNS seed to the regtest chain params to enable testing the DNS seed querying logic of `CConnman::ThreadDNSAddressSeed` and relevant startup parameters. Adds coverage for the changes in #22013 (and then some). The main behavioral change to bitcoind is that this PR disallows starting up with conflicting parameters for `-dnsseed` and `-forcednsseed`. The tests include: * parameter interactions of different combinations of `-connect`, `-dnsseed` and `-forcednsseed` * the delay before querying DNS seeds depending on how many addresses are in the addrman * the behavior of `-forcednsseed` * skipping DNS querying if we have outbound full relay connections & not block-relay-only connections Huge props to mzumsande for identifying the timing technique for testing successful connections before running `ThreadDNSAddressSeed` šŸ™ŒšŸ½ ACKs for top commit: mzumsande: ACK 82b6f89819e55af26f5264678e0f93052934bcb3 jnewbery: reACK 82b6f89819e55af26f5264678e0f93052934bcb3 Tree-SHA512: 9f0c29bfbf99426727e79c0a25606ae09deab91a92e3c5cee7f84c3ca7503a8ac9ab85a85c51841d40b164ef8c991326070f0b2f41d075fb7985df26f6e95d6d
2021-08-03Merge bitcoin/bitcoin#21528: [p2p] Reduce addr blackholesfanquake
3f7250b328b8b2f5d63f323702445ac5c989b73d [test] Use the new endpoint to improve tests (Amiti Uttarwar) 3893da06db1eb622f540605700f8663f8d87b2df [RPC] Add field to getpeerinfo to indicate if addr relay is enabled (Amiti Uttarwar) 0980ca78cd930a00c9985d7f00083a3b8e8be89e [test] Test that we intentionally select addr relay peers. (Amiti Uttarwar) c061599e40dc3d379c10b914765061a7a8449dd7 [net_processing] Remove RelayAddrsWithPeer function (Amiti Uttarwar) 201e4964816f8896cfe7b4f6d8ddbfffe7102f87 [net_processing] Introduce new field to indicate if addr relay is enabled (Amiti Uttarwar) 1d1ef2db7ea0d93c7dab4a9800ec74afa7a019eb [net_processing] Defer initializing m_addr_known (Amiti Uttarwar) 6653fa3328b5608fcceda1c6ea8e68c5d58739ec [test] Update p2p_addr_relay test to prepare (Amiti Uttarwar) 2fcaec7bbb96d6fe72a7e3a5744b0c35c79733e8 [net_processing] Introduce SetupAddressRelay (Amiti Uttarwar) Pull request description: This PR builds on the test refactors extracted into #22306 (first 5 commits). This PR aims to reduce addr blackholes. When we receive an `addr` message that contains 10 or less addresses, we forward them to 1-2 peers. This is the main technique we use for self advertisements, so sending to peers that wouldn't relay would effectively "blackhole" the trickle. Although we cannot prevent this in a malicious case, we can improve it for the normal, honest cases, and reduce the overall likelihood of occurrence. Two known cases where peers would not participate in addr relay are if they have connected to you as a block-relay-only connection, or if they are a light client. This implementation defers initialization of `m_addr_known` until it is needed, then uses its presence to decide if the peer is participating in addr relay. For outbound (not block-relay-only) peers, we initialize the filter before sending the initial self announcement when processing their `version` message. For inbound peers, we initialize the filter if/when we get an addr related message (`ADDR`, `ADDRV2`, `GETADDR`). We do NOT initialize the filter based on a `SENDADDRV2` message. To communicate about these changes beyond bitcoin core & to (try to) ensure that no other software would be disrupted, I have: - Posted to the [mailing list](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-April/018784.html) - Researched other open source clients to confirm compatibility, opened issues in all the projects & documented in https://github.com/bitcoin/bitcoin/pull/21528#issuecomment-809906430. Many have confirmed that this change would not be problematic. - Raised as topic during [bitcoin-core-dev meeting](https://www.erisian.com.au/bitcoin-core-dev/log-2021-03-25.html#l-954) - Raised as topic during [bitcoin p2p meeting](https://www.erisian.com.au/bitcoin-core-dev/log-2021-04-20.html#l-439) ACKs for top commit: jnewbery: reACK 3f7250b328b8b2f5d63f323702445ac5c989b73d glozow: ACK 3f7250b328b8b2f5d63f323702445ac5c989b73d ajtowns: utACK 3f7250b328b8b2f5d63f323702445ac5c989b73d Tree-SHA512: 29069282af684c1cd37d107c395fdd432dcccb11626f3c2dabfe92fdc4c85e74c7c4056fbdfa88017fec240506639b72ac6c311f8ce7c583112eb15f47e421af
2021-08-02doc: Correct description of CAddrMan::Create()Amiti Uttarwar
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
2021-08-02doc: Update high-level addrman descriptionMartin Zumsande
2021-08-03cli: Add progress bar for -getinfoklementtan
Co-authored-by: jonatack <jon@atack.com>
2021-08-02[GetTransaction] remove unneeded `cs_main` lock acquireSebastian Falbesoner
2021-08-02consensus/params: simplify ValidDeployment check to avoid gcc warningAnthony Towns
2021-08-02fuzz: move init code to the CAddrManDeterministic constructorVasil Dimov
Move the addrman init code from the test case to a newly added `CAddrManDeterministic` constructor. This way it can be reused by other tests.
2021-08-02Merge bitcoin/bitcoin#22570: Ignore banlist.datW. J. van der Laan
fa1eddb1a3d1319ddc3643b6f34fe2014de32764 Fix whitespace in touched files (MarcoFalke) fa4e6afdae7b82df638b60edf37ac36d57a8cb4f Remove unused CSubNet serialize code (MarcoFalke) fa384fdd0b7af73d81fa9619c5fba779452cd2af Ignore banlist.dat (MarcoFalke) Pull request description: The code to read `banlist.dat` should be removed eventually. The major release (22.x) can be used to translate a `banlist.dat` into a `banlist.json`. Thus, it is now possible to remove the reading code. ACKs for top commit: Zero-1729: re-ACK fa1eddb1a3d1319ddc3643b6f34fe2014de32764 laanwj: concept and code review ACK fa1eddb1a3d1319ddc3643b6f34fe2014de32764 vasild: ACK fa1eddb1a3d1319ddc3643b6f34fe2014de32764 jonatack: Light code review utACK fa1eddb1a3d1319ddc3643b6f34fe2014de32764 Tree-SHA512: e136193b7c0ba1d6d2e79c7fb4106ba4af75fa229ed7214675ee64e98e59bb4808779e7a8a09eecce62f7a5d4bc6e16b8a5ad4596129357c8fc5e3b88f214249
2021-08-02Merge bitcoin/bitcoin#21940: refactor: Mark CAddrMan::Select and GetAddr constMarcoFalke
fae108ceb53f61d7338ba205873623ede3c1d3be Fix incorrect whitespace in addrman (MarcoFalke) fa32024d51c098441623e246f304a80f011e29d1 Add missing GUARDED_BY to CAddrMan::insecure_rand (MarcoFalke) fab755b77f88873f01cbd988051de7ad3f0150de fuzz: Actually use const addrman (MarcoFalke) fae0c79351ce34186249d44af0c5c9c7521f4b6c refactor: Mark CAddrMan::GetAddr const (MarcoFalke) fa02934c8c9d290ea4d12683e8680c70967a4d3a refactor: Mark CAddrMan::Select const (MarcoFalke) Pull request description: To clarify that a call to this only changes the random state and nothing else. ACKs for top commit: jnewbery: Code review ACK fae108ceb53f61d7338ba205873623ede3c1d3be theStack: re-ACK fae108ceb53f61d7338ba205873623ede3c1d3be šŸ¦ Tree-SHA512: 3ffb211d4715cc3daeb3bfcdb3fcc6b108ca96df5fa565510436fac0e8da86c93b30c9c4aad0563e27d84f615fcd729481072009a4e2360c8b3d40787ab6506a
2021-07-30Close minor startup race between main and scheduler threadsLarry Ruane
Don't schedule class PeerManagerImpl's background tasks from its constructor, but instead do that from a separate method, StartScheduledTasks(), that can be called later at the end of startup, after other things, such as the active chain, are initialzed.
2021-07-30[style] Small style improvements to DNS parametersAmiti Uttarwar
generated by cgit v1.2.3 (git 2.26.2) at 2025-01-15 02:19:04 +0000