aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
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-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-05Merge bitcoin/bitcoin#22619: test: refactor: use consistent bytes <-> ↡MarcoFalke
hex-string conversion in functional test framework 5a1bef60a03b57de708a1a751bd90b8245fd8b83 test: refactor: remove binascii from test_framework (Zero-1729) Pull request description: This PR continues the work started in PR #22593, regarding using the `bytes` built-in module. In this PR specifically, instances of `binascii`'s methods `hexlify`, `unhexlify`, and `a2b_hex` have been replaced with the build-in `bytes` module's `hex` and `fromhex` methods where appropriate to make bytes <-> hex-string conversions consistent across the functional test files and test_framework. Additionally, certain changes made are based on the following assumption: ``` bytes.hex(data) == binascii.hexlify(data).decode() bytes.hex(data).encode() == binascii.hexlify(data) ``` Ran the functional tests to ensure behaviour is still consistent and changes didn't break existing tests. closes #22605 ACKs for top commit: theStack: Code-review ACK 5a1bef60a03b57de708a1a751bd90b8245fd8b83 πŸ”’ Tree-SHA512: 8f28076cf0580a0d02a156f3e1e94c9badd3d41c3fbdfb2b87cd8a761dde2c94faa5f4c448d6747b1ccc9111c3ef1a1d7b42a11c806b241fa0410b7529e2445f
2021-08-05Merge bitcoin/bitcoin#22630: test: Add missing includefanquake
6666ec9e059ea66065a11f5e25c0a17d7d0834fe test: Add missing include (MarcoFalke) Pull request description: Fix the silent merge conflict introduced in commit 7d60f7ec6bbf293c6f23f442a0fd985111a15ff7 ACKs for top commit: fanquake: ACK 6666ec9e059ea66065a11f5e25c0a17d7d0834fe - fixes: Tree-SHA512: c712715db4a41cf6ed991fb35482d8fd9016b5a27e1309654018219ebbfd8d4f98a389bdf5d7b792e7a4690928aeef943c716465b0c5e00f70320ad2e134ebb5
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-04test: refactor: remove binascii from test_frameworkZero-1729
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#22603: doc: Add release notes for 22570 (ignore ↡MarcoFalke
banlist.dat) fa2c868886a52352631d32c398d98a2740639e86 doc: Add release notes for 22570 (ignore banlist.dat) (MarcoFalke) Pull request description: Follow-up to #22570 ACKs for top commit: tryphe: ACK fa2c868886a52352631d32c398d98a2740639e86 vasild: ACK fa2c868886a52352631d32c398d98a2740639e86 Zero-1729: ACK fa2c868886a52352631d32c398d98a2740639e86 Tree-SHA512: 58eef340b6211bcdecf3826ac145afd476c397f110daa6783521c0c1e1be1ffbd2d24ad20c77921abbe5cdb8e644d3cd64e14e2819746cf0e5123fb7cc445d63
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#22532: test : improve mempool_accept_wtxid.pyMarcoFalke
91b05974fc1ea38062f12d36152201af81bda1a2 Improve mempool_accept_wtxid.py (naiza) Pull request description: Follow-up to #22253 adding changes suggested in [#22253 (review)](https://github.com/bitcoin/bitcoin/pull/22253#discussion_r666933370) ACKs for top commit: glozow: utACK 91b05974fc1ea38062f12d36152201af81bda1a2 Tree-SHA512: 383064138a5b2160d769c9df370470fd585c91682083013a6fa15e14448a4b481bc09b3a0ed6e75554db2c378df6b2263c65f209f973c9e9d577e15814a4be1d
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-03Improve mempool_accept_wtxid.pynaiza
Improve mempool_accept_wtxid.py Improve mempool_accept_wtxid.py Improve mempool_accept_wtxid.py Improve mempool_accept_wtxid.py
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-02[GetTransaction] remove unneeded `cs_main` lock acquireSebastian Falbesoner
2021-08-02doc: Add release notes for 22570 (ignore banlist.dat)MarcoFalke
2021-08-02Merge bitcoin/bitcoin#22378: test: remove confusing `MAX_BLOCK_BASE_SIZE`MarcoFalke
607076d01bf23c69ac21950c17b01fb4e1130774 test: remove confusing `MAX_BLOCK_BASE_SIZE` (Sebastian Falbesoner) 4af97c74edcda56cd15523bf3a335adea2bad14a test: introduce `get_weight()` helper for CBlock (Sebastian Falbesoner) a084ebe1330bcec15715e08b0f65319142927ad1 test: introduce `get_weight()` helper for CTransaction (Sebastian Falbesoner) Pull request description: This is a very late follow-up PR to #10618, which removed the constant `MAX_BLOCK_BASE_SIZE` from the core implementation about four years ago (see also #10608 in why it was considered confusing and superfluous). Since there is also no point in still keeping it in the functional test framework, the PR switches to weight-based accounting on the relevant test code parts and use `MAX_BLOCK_WEIGHT` instead for the block limit checks. To prepare that, the first two commits introduce `get_weight()` helpers for the classes CTransaction and CBlock, respectively. ACKs for top commit: MarcoFalke: review ACK 607076d01bf23c69ac21950c17b01fb4e1130774 🚴 Tree-SHA512: d59aa0b6b3dfd0a849b8063e66de275d252f705f99e25cd3bf6daec028b47d946777ee5b42a060f5283cb18e917ac073119c2c0e11bbc21211f69ef0a6ed335a
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#22001: doc: Generate doxygen documentation for test ↡W. J. van der Laan
sources 5d37cc44f9200861a278d074f8caa99e8db6be02 Generate doxygen documentation for test sources (Patrick Kamin) Pull request description: Fixes #19248 While searching for the documentation of the test utilities I realized they were excluded from doxygen. I agree with the statement in #19248. It's also helpful for new contributors to gain a broader understanding of the class dependencies visually (see BasicTestSetup) ACKs for top commit: laanwj: ACK 5d37cc44f9200861a278d074f8caa99e8db6be02 Tree-SHA512: 32f0abab2970c65621af5cee7f620c2653bd9688366e125543262bd078841e64a5a1e24cf0241e9f6ec538b8759e06108d5ff056449aa1c98d5f287deef18c86
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-08-02Merge bitcoin/bitcoin#22593: test: refactor: remove `hex_str_to_bytes` helperMarcoFalke
ca6c154ef116e8d2e0484cdb1af13b34a0c86c17 test: refactor: remove `hex_str_to_bytes` helper (Sebastian Falbesoner) Pull request description: Use the built-in class method `bytes.fromhex()` instead, which is available since Python 3.0 (https://docs.python.org/3.0/library/stdtypes.html?highlight=fromhex#bytes.fromhex). This would be nice to solve with a scripted-diff, but it's tricky to also tackle the imports (which need to be removed in the same commit, otherwise the linter would complain). ACKs for top commit: theStack: For the previous ACK (thanks Zero-1729!), it can be trivially re-reviewed via `git range-diff ee5462683...ca6c154ef` practicalswift: cr ACK ca6c154ef116e8d2e0484cdb1af13b34a0c86c17 tryphe: Concept ACK, cr ACK ca6c154ef116e8d2e0484cdb1af13b34a0c86c17 Zero-1729: re-ACK ca6c154ef116e8d2e0484cdb1af13b34a0c86c17, clean rebase. Tree-SHA512: 1cda70e7bad523ed937239f6e999c2b43754533a507b18b3ff2503fd0b0ab604efbef23e155726818b970621569d5bcfd42927f1e329184fc1bcbc92651e47ca
2021-08-02Merge bitcoin/bitcoin#22589: net, doc: update I2P hardcoded seeds and docs ↡W. J. van der Laan
for 22.0 d2dffd5be4c8f6a1942dd971d09707c3620a1689 doc: add info to i2p.md about IBD time and multiple networks (Jon Atack) 2962640c49cf38f76345e45e63045a8f0eed5c61 contrib, p2p: update I2P hardcoded seeds (Jon Atack) Pull request description: - Update the I2P seeds with the latest ones I can connect to (feedback welcome). - Update the I2P doc with frequently asked and reported info: IBD speed and running with other networks. ACKs for top commit: tryphe: ACK d2dffd5be4c8f6a1942dd971d09707c3620a1689 duncandean: ACK d2dffd5 willcl-ark: ACK d2dffd5be4c8f6a1942dd971d09707c3620a1689 laanwj: ACK d2dffd5be4c8f6a1942dd971d09707c3620a1689 vasild: ACK d2dffd5be4c8f6a1942dd971d09707c3620a1689 Tree-SHA512: 1edda0eb77b4e224b44961849b36c5a13e1eb14bdc50f909ec44d7fe926c259cada62404906594a1cf68ffd96254647ac49f24248214b42403ecebae45135e63
2021-08-01test: refactor: remove `hex_str_to_bytes` helperSebastian Falbesoner
Use the built-in class method bytes.fromhex() instead, which is available since Python 3.0.
2021-08-01Merge bitcoin/bitcoin#22429: test: refactor: fix segwit terminology ↡MarcoFalke
(s/witness_program/witness_script/) 8a2b58db9ee6a14d36b5d8e430b35f18e7c7b0c5 test: fix segwit terminology (s/witness_program/witness_script/) (Sebastian Falbesoner) Pull request description: This PR fixes wrong uses of the term "witness program", which according to [BIP141](https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#Witness_program) is defined as follows: > A scriptPubKey (or redeemScript as defined in BIP16/P2SH) that consists of a 1-byte push opcode (for 0 to 16) followed by a data push between 2 and 40 bytes gets a new special meaning. The value of the first push is called the "version byte". **The following byte vector pushed is called the "witness program".** In most cases where "witness program" is used in tests (concerns comments, variable names and in one instance even a function name) what we really want to denote is the "witness script". Thanks to [MarcoFalke for pointing this out in a review comment](https://github.com/bitcoin/bitcoin/pull/22363#discussion_r666794261)! Some historical background: At the time when the P2P segwit tests were first introduced (commit 330b0f31ee5719d94f9e52dfc83c5d82168241f9, PR #8149), the term "witness program" was not used consistently in BIP141: https://bitcoin.stackexchange.com/questions/46451/what-is-the-precise-definition-of-witness-program This was fixed in PR https://github.com/bitcoin/bips/pull/416 later. So in some way, this PR can be seen as a very late follow-up to the BIP141 fix that also reflects these changes in the tests. ACKs for top commit: josibake: tACK https://github.com/bitcoin/bitcoin/pull/22429/commits/8a2b58db9ee6a14d36b5d8e430b35f18e7c7b0c5 Tree-SHA512: f36bb9e53d1b54b86bfa87ec12f33e3ebca64b5f59d97e9662fe35ba12c25e1c9a4f93a5425d0eaa3879dce9e50368d345555b927bfab76945511f873396892b
2021-07-31Merge bitcoin/bitcoin#22573: fuzz: document faster throughput configurationMarcoFalke
8a4f0fcd3fc1a35c1482975114555b0fed75a1c0 Document faster throughput configuration (Alex Groce) Pull request description: This is a small change to the fuzzing doc that I think might help more people improve the corpus coverage, which I think is low partly just due to lack of long, low-overhead, runs, in addition to the need to apply a more diverse set of fuzzers and coverage notions. ACKs for top commit: practicalswift: ACK 8a4f0fcd3fc1a35c1482975114555b0fed75a1c0 tryphe: ACK 8a4f0fcd3fc1a35c1482975114555b0fed75a1c0 Tree-SHA512: 0f1802f5c551d6ade7393cd2ac439ffd485786b17c4fd0f1a321f69f8ed0db1167ae04b5cae7bf904e89aba03e89b6d974bff564bfc6a78a571893719f323434
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