aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2021-05-12scripted-diff: add NetPermissionFlags scopes where not already presentJon Atack
-BEGIN VERIFY SCRIPT- s() { git grep -l "$1" -- 'src' ':!src/net_permissions.h' | xargs sed -i -E "s/([^:])$1/\1NetPermissionFlags::$1/"; } s 'PF_NONE' s 'PF_BLOOMFILTER' s 'PF_RELAY' s 'PF_FORCERELAY' s 'PF_DOWNLOAD' s 'PF_NOBAN' s 'PF_MEMPOOL' s 'PF_ADDR' s 'PF_ISIMPLICIT' s 'PF_ALL' -END VERIFY SCRIPT- Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2021-05-12Merge bitcoin/bitcoin#19064: refactor: Cleanup thread ctor callsMarcoFalke
792be53d3e9e366b9f6aeee7a1eeb912fa28062e refactor: Replace std::bind with lambdas (Hennadii Stepanov) a508f718f3e087c96a306399582a85df2e1d53ae refactor: Use appropriate thread constructor (Hennadii Stepanov) 30e44482152488a78f2c495798a75e6f553dc0c8 refactor: Make TraceThread a non-template free function (Hennadii Stepanov) Pull request description: This PR does not change behavior. Its goal is to improve readability and maintainability of the code. ACKs for top commit: jnewbery: utACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e jonatack: tACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e MarcoFalke: cr ACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e Tree-SHA512: a03142f04f370f6bc02bd3ddfa870819b51740fcd028772241d68c84087f95a2d78207cbd5edb3f7c636fcf2d76192d9c59873f8f0af451d3b05c0cf9cf234df
2021-05-11fuzz: Avoid timeout in EncodeBase58MarcoFalke
2021-05-11Merge bitcoin/bitcoin#21892: fuzz: Avoid excessively large min fee rate in ↵MarcoFalke
tx_pool 99993f066405863c66ccaec0a8427129f4515768 fuzz: Avoid excessively large min fee rate in tx_pool (MarcoFalke) Pull request description: Any fee rate above 1 BTC / kvB is clearly nonsense, so no need to fuzz this. Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34078 ACKs for top commit: practicalswift: cr ACK 99993f066405863c66ccaec0a8427129f4515768: patch looks correct despite no `fa` prefix in commit hash Tree-SHA512: bd3651d354b13d889ad1708d2b385ad0479de036de74a237346eefad5dbfb1df76ec02b55ec00487ec598657ef6102f992302b14c4e47f913a9962f81f4157e6
2021-05-11Merge bitcoin/bitcoin#21909: fuzz: Limit max insertions in timedata fuzz testMarcoFalke
fa95555a491dc01952703f476836e607ac34eab4 fuzz: Limit max insertions in timedata fuzz test (MarcoFalke) Pull request description: It is debatable whether a size of the median filter other than `200` (the only size used in production) should be fuzzed. For now add a minimal patch to cap the max insertions. Otherwise the complexity is N^2 log(N), where N is the size of the fuzz input. Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34167 ACKs for top commit: practicalswift: cr ACK fa95555a491dc01952703f476836e607ac34eab4: patch looks correct Tree-SHA512: be7737e9f4c906053e355641de84dde31fed37ed6be4c5e92e602ca7675dffdaf06b7063b9235ef541b05d3d5fd689c99479317473bb15cb5271b8baabffd0f2
2021-05-11Merge bitcoin/bitcoin#21644: p2p, bugfix: use NetPermissions::HasFlag() in ↵W. J. van der Laan
CConnman::Bind() 36fb036d25e2a3016b36873456e5a9e6251ffef8 p2p: allow NetPermissions::ClearFlag() only with PF_ISIMPLICIT (Jon Atack) 4e0d5788ba5771c81bc0ff2e6523cf9accddae46 test: add net permissions noban/download unit test coverage (Jon Atack) dde69f20a01acca64ac21cb13993c6e4f8709f23 p2p, bugfix: use NetPermissions::HasFlag() in CConnman::Bind() (Jon Atack) Pull request description: This is a bugfix follow-up to #16248 and #19191 that was noticed in #21506. Both v0.21 and master are affected. Since #19191, noban is a multi-flag that implies download, so the conditional in `CConnman::Bind()` using a bitwise AND on noban will return the same result for both the noban status and the download status. This means that download peers are incorrectly not being added to local addresses because they are mistakenly seen as noban peers. The second commit adds unit test coverage to illustrate and test the noban/download relationship and the `NetPermissions` operations involving them. The final commit adds documentation and disallows calling `NetPermissions::ClearFlag()` with any second param other than `NetPermissionFlags` "implicit" -- per current usage in the codebase -- because `ClearFlag()` should not be called with any second param that is a subflag of a multiflag, e.g. "relay" or "download," as that would leave the result in an invalid state corresponding to none of the existing NetPermissionFlags. Thanks to Vasil Dimov for noticing this. ACKs for top commit: theStack: re-ACK 36fb036d25e2a3016b36873456e5a9e6251ffef8 ☕ vasild: ACK 36fb036d25e2a3016b36873456e5a9e6251ffef8 hebasto: ACK 36fb036d25e2a3016b36873456e5a9e6251ffef8, I have reviewed the code and it looks OK, I agree it can be merged. kallewoof: Code review ACK 36fb036d25e2a3016b36873456e5a9e6251ffef8 Tree-SHA512: 5fbc7ddbf31d06b35bf238f4d77ef311e6b6ef2e1bb9893f32f889c1a0f65774a3710dcb21d94317fe6166df9334a9f2d42630809e7fe8cbd797dd6f6fc49491
2021-05-11Merge bitcoin/bitcoin#21752: doc: Clarify that feerates are per virtual sizeMarcoFalke
fae196147bae11202c0d54543dc12ba5d92ab0cc doc: Clarify that feerates are per virtual size (MarcoFalke) fa83e95ac6f318caa38016a08fa4e402c3b05833 scripted-diff: Clarify that feerates are per virtual size (MarcoFalke) Pull request description: By implementing segwit, it is already clear that all feerates in Bitcoin Core are denoted in (amount/virtual size). Though, there is inconsistency, as some places use kvB, some use kB. Thus, replace all with "kvB". See also commit 6da3afbaee5809ebf6d88efaa3958c505c2d71c7, which did the replacement for wallet RPCs. ACKs for top commit: ryanofsky: Code review ACK fae196147bae11202c0d54543dc12ba5d92ab0cc. Checked instances where units were being added in the second commit and they all looked right. Tree-SHA512: ab70d13cde7d55c1ac931bddc2b45aa218fc75ef46cb6ea9e5a30b1d4dbf27889c2b6357299a6c5427912443a46ec3592a4809dae335e03162bd2120a0f7f8ad
2021-05-11Merge bitcoin-core/gui#271: Don't clear console prompt when font resizingW. J. van der Laan
7962e0dde8bbd0fa3dd702e2224774f1edaadcb6 qt: Do not clear console prompt when font resizing (Hennadii Stepanov) d2cc3390054616c73f72a59f864700f6de14067b qt, refactor: Drop redundant history cleaning in RPC console (Hennadii Stepanov) 4f0ae472e22990ad9e734faea4adacef8df449bb qt: Untie irrelevant signal-slot parameters (Hennadii Stepanov) Pull request description: On master, a console resize event will clear the prompt. To fix this, we store the content of the prompt and re-set it upon a resize. This preserves the prompt text throughout resizes. The text will still clear when you click the clear button, as it should. **Master** | Before Resize | After Resize | | ----------------- | ------------ | | ![master-beforeresize](https://user-images.githubusercontent.com/23396902/113553721-2a428d80-95c6-11eb-971b-bb77151bc6d5.png) | ![master-afterresize](https://user-images.githubusercontent.com/23396902/113553769-3d555d80-95c6-11eb-9cdb-9ad1fd7208a9.png) | **PR** | Before Resize | After Resize | | ----------------- | ------------ | | ![pr-beforeresize](https://user-images.githubusercontent.com/23396902/113553885-6f66bf80-95c6-11eb-8317-0975f1ebd444.png) | ![pr-afterresize](https://user-images.githubusercontent.com/23396902/113553906-75f53700-95c6-11eb-9a32-b64d8aba98e5.png) | Closes #269 ACKs for top commit: laanwj: Code review ACK 7962e0dde8bbd0fa3dd702e2224774f1edaadcb6 hebasto: ACK 7962e0dde8bbd0fa3dd702e2224774f1edaadcb6 Talkless: tACK 7962e0dde8bbd0fa3dd702e2224774f1edaadcb6, tested on Debian Sid with Qt 5.15.2 Tree-SHA512: a6f19d3f80e2e47725cff5d6e15862b6cb793a65dfcaded15f23bba051088cd3317f068f93290c9b09d0a90f5fcac1c5a4610cc417cc5961ba6d005fe5049ab0
2021-05-11fuzz: Limit max insertions in timedata fuzz testMarcoFalke
2021-05-11Merge bitcoin/bitcoin#21895: refactor: Add TSA annotations to the WorkQueue ↵MarcoFalke
class members 34b04eec4448bd37a8dbf560e4d99c7e7ca7e9c0 refactor: Add TSA annotations to the WorkQueue class members (Hennadii Stepanov) Pull request description: Noted while reviewing #19033, and hoping this will not conflict with it :) ACKs for top commit: promag: Code review ACK 34b04eec4448bd37a8dbf560e4d99c7e7ca7e9c0. Tree-SHA512: 4c15729acd95223263c19bc0dd64b9e7960872b48edee6eee97a5d0c2b99b8838185ac3a2ccd5bee992cb3a12498633427fe9919be5a12da9949fcf69a6275a0
2021-05-11Merge bitcoin/bitcoin#21902: refactor: Remove useless extern keywordfanquake
fa4bbd306e1ca369d02eb864983fbb4d64b50ca9 refactor: Remove useless extern keyword (MarcoFalke) Pull request description: It is redundant, confusing and useless. https://en.cppreference.com/w/cpp/language/storage_duration#external_linkage ACKs for top commit: practicalswift: cr ACK fa4bbd306e1ca369d02eb864983fbb4d64b50ca9: patch looks correct Talkless: utACK fa4bbd306e1ca369d02eb864983fbb4d64b50ca9, built successfully on Debian Sid, looks OK. jonatack: Light code review ACK fa4bbd306e1ca369d02eb864983fbb4d64b50ca9 hebasto: ACK fa4bbd306e1ca369d02eb864983fbb4d64b50ca9, I've verified that all of the remained `extern` keywords specify either (a) a variable with external linkage, or (b) a symbol with "C" language linkage. promag: Code review ACK fa4bbd306e1ca369d02eb864983fbb4d64b50ca9. Tree-SHA512: 1d77d661132defa52ccb2046f7a287deb3669b68835e40ab75a0d9d08fe6efeaf3bea7c0e76c754fd18bfe45972c253a39462014080d014cc5d810498784e3e4
2021-05-11Merge bitcoin-core/gui#293: Enable wordWrap for ServicesHennadii Stepanov
a0f797867433b0d3e8d7851ddf05743fe70d320a qt: enable wordWrap for peers-tab detail services (randymcmillan) Pull request description: Enable wordWrap for peers-tab detailView Services ACKs for top commit: Talkless: tACK a0f797867433b0d3e8d7851ddf05743fe70d320a on same environment as previously. hebasto: ACK a0f797867433b0d3e8d7851ddf05743fe70d320a, tested on Linux Mint 20.1 (Qt 5.12.8): kristapsk: re-ACK a0f797867433b0d3e8d7851ddf05743fe70d320a. Tested under Gentoo Linux with Xfce4 (Qt 5.15.2). Tree-SHA512: 872e511d2ecfa72fea0fd3284a958b45ee8aee138469ce7f9cd853cd9098b9583917909934b0a5c96f9b81ea1567bcea6a037558829bb79f2a3f413a83df06e6
2021-05-11Merge bitcoin-core/gui#280: Remove user input from URI error messageHennadii Stepanov
3bad0b3fada9ab7c5b03d31dc33d72654c1ba2be Remove user input from URI error message (unknown) Pull request description: Removes the user input from error message to avoid it being used in attacks. Its not really a vulnerability in Bitcoin Core because involves social engineering, dependency on user environment etc. But this PR improves security and by avoiding abuse of URI error in future. Example of an attack: 1. User opens a link in firefox: ``` bitcoin:tb1qag2e6yhl52hr53vdxzaxvnjtueupvuftan4yfu%0A%0AWARNING%3A%20DO%20NOT%20CLOSE%20THIS%20WINDOW%20OR%20TURN%20OFF%20YOUR%20PC!%20IF%20YOU%20ABORT%20THIS%20PROCESS%2C%20YOU%20COULD%20DESTROY%20ALL%20OF%20YOU%20DATA!%20PLEASE%20ENSURE%20THAT%20YOUR%20POWER%20CABLE%20IS%20PLUGGED%20IN!%0A%0AYou%20became%20victim%20of%20the%20XYZ%20RANSOMWARE!%0A%0AThe%20hard%20disks%20of%20your%20computer%20have%20been%20encrypted%20with%20a%20military%20grade%20encryption%20algorithm.%20There%20is%20no%20way%20to%20restore%20your%20data%20without%20a%20special%20key.%20You%20can%20purchase%20this%20key%20on%20the%20darknet%20page%20shown%20in%20step%202.%0ATo%20purchase%20your%20key%20and%20restore%20your%20data%2C%20please%20follow%20these%20three%20easy%20steps%3A%0A%0A1.%20Download%20the%20Tor%20browser%20at%20%E2%80%9Chttps%3A%2F%2Fwww.torproject.org%2F%E2%80%9C.%0A2.%20Visit%20one%20of%20the%20following%20pages%20with%20the%20Tor%20Browser%3A%0Ahttp%3A%2F%2Frandomchars.onion%2Fabc123%0A3.%20Send%20BTC%20by%20following%20the%20instructions%20on%20the%20page ``` 2. User selects Bitcoin Core to open the link: ![image](https://user-images.githubusercontent.com/13405205/114619801-8ee9a080-9cc8-11eb-9fad-23a2b831e8df.png) 3. User is asked to send BTC with some message convincing enough which can be different depending on the victim: ![image](https://user-images.githubusercontent.com/13405205/114620061-d3753c00-9cc8-11eb-8314-e3362ebb90ac.png) **After this PR** (_No user input mentioned in the error_): ![image](https://user-images.githubusercontent.com/13405205/114624342-2b627180-9cce-11eb-93a8-0b2438d71571.png) ACKs for top commit: hebasto: ACK 3bad0b3fada9ab7c5b03d31dc33d72654c1ba2be, tested on Linux Mint 20.1 (Qt 5.12.8). jarolrod: tACK 3bad0b3fada9ab7c5b03d31dc33d72654c1ba2be Tree-SHA512: aac2fdfcaa7a9cd6582750c1960682554795640f5aacb78bdae121724e1151da3cbb62b8f8b1e0bc37347afe78b3e9a446277cab8e009d2a1050c0e971f001b3
2021-05-10Merge bitcoin-core/gui#194: Save/restore RPCConsole geometry only for windowHennadii Stepanov
01d9586ae85e49efaa00f11c1f26c24c9b82b278 qt: Save/restore RPCConsole geometry only for window (Hennadii Stepanov) Pull request description: After using the GUI with `-disablewallet` the "Node window" inherits the geometry of the main window, that could be unexpected for users. This PR provides independent geometry settings for `RPCConsole` in both modes: - window sizes and `QSplitter` sizes when `-disablewallet=0` - only `QSplitter` sizes when `-disablewallet=1` ACKs for top commit: Talkless: tACK 01d9586ae85e49efaa00f11c1f26c24c9b82b278, tested on Debian Sid with Qt 5.15.2. I've managed to reproduce issue using https://github.com/bitcoin-core/gui/pull/194#issuecomment-782822663 instructions, and I see that this PR does detach main window and information window sizes. Built with `--enable-wallet` and `--disable-wallet`. jarolrod: ACK 01d9586ae85e49efaa00f11c1f26c24c9b82b278, tested on macOS 11.2 Qt 5.15.2 promag: Code review ACK 01d9586ae85e49efaa00f11c1f26c24c9b82b278. Tree-SHA512: 9934cf04d4d5070dfc4671ea950e225cda9988858227e5481dad1baafa14af477bdbf4f91307ca687fde0cad6e4e605a3a99377e70d67eb115a19955ce2516f5
2021-05-10Merge bitcoin-core/gui#257: refactor: Use template function qOverload in ↵Hennadii Stepanov
signal-slot connections cdbc2bd1f1c171848c1fef7f217afe140e1afb06 qt: Use template function qOverload in signal-slot connections (Hennadii Stepanov) Pull request description: A nice template function [`qOverload`](https://doc.qt.io/qt-5/qtglobal.html#qOverload) is available for us now (https://github.com/bitcoin/bitcoin/pull/20413, https://github.com/bitcoin/bitcoin/pull/21286). Its usage makes code much more readable. This PR does not change behavior. ACKs for top commit: Talkless: utACK cdbc2bd1f1c171848c1fef7f217afe140e1afb06. promag: Code review ACK cdbc2bd1f1c171848c1fef7f217afe140e1afb06. Tree-SHA512: 72002aa646b1a79bab62d498825b3f245dc7ebdc189280f8bd3b4076e1bb50be8802c02bc872ff6f70c1ea81faec66d3bec36471119dd98c9e70d87b990396ae
2021-05-10Merge bitcoin/bitcoin#21359: rpc: include_unsafe option for fundrawtransactionW. J. van der Laan
11d6459b6e101f05f36e13799c400bef82d2fc21 rpc: include_unsafe option for fundrawtransaction (t-bast) Pull request description: Allow RPC users to opt-in to unsafe inputs when funding a raw transaction. Applications that need to manage a complex RBF flow (such as lightning nodes using anchor outputs) are very limited if they can only use safe inputs. I also added this option to `send` and `walletcreatefundedpsbt` who internally delegate to `fundrawtransaction`. Fixes #21299 ACKs for top commit: laanwj: Code review ACK 11d6459b6e101f05f36e13799c400bef82d2fc21 Tree-SHA512: 5e542a4febcfd6f41cf784678ff02ec9282eae2082c274983f72c5ea87b7ebbe1bd5fdc6a020d7a9d5996157754eb4966b8aeb6c1ceebf0b1519f735579b8bac
2021-05-10Merge bitcoin-core/gui#296: Do not use QObject::tr plural syntax for numbers ↵Hennadii Stepanov
with a unit symbol 3adde72bc99215062c8dabd38f8c34ad093452b5 qt: Do not use QObject::tr plural syntax for numbers with a unit symbol (Hennadii Stepanov) Pull request description: Working on translation, I found this is useless and unnecessarily burdensome for translators. I guess, this statement is correct internationally wide :) ACKs for top commit: jarolrod: ACK 3adde72bc99215062c8dabd38f8c34ad093452b5 promag: Code review ACK 3adde72bc99215062c8dabd38f8c34ad093452b5. Agree with OP, looks reasonable to me. Tree-SHA512: bde65c122ca0feb7771d932cce63fd1aef1e7a9dda0188d19c577d57b279172204ac1bfcb6106a78b2c4d55d628e6dc0967051e064ec40d3c5aeafd4a48f0589
2021-05-10Merge bitcoin/bitcoin#21891: fuzz: Remove strprintf test cases that are ↵W. J. van der Laan
known to fail facfc0f65dd0a7d54c0f6d56bff793e57b12ee12 fuzz: Remove strprintf test cases that are known to fail (MarcoFalke) Pull request description: They are still waiting to be fixed (see https://github.com/c42f/tinyformat/issues/70 ), so no need for us to carry them around in our source code. They can be added back once upstream is fixed. Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34082 ACKs for top commit: laanwj: Code review ACK facfc0f65dd0a7d54c0f6d56bff793e57b12ee12 Tree-SHA512: d9d3d35555b6d58740a041ae45797ca85149f60990e2ed632c5dadf363e1d2362d2447681d7ceaa1fbffcd6e7bc8da5bc15d3923b68829a86c25b364a599afc8
2021-05-10Merge bitcoin/bitcoin#21745: refactor: Add missing includes in ↵W. J. van der Laan
pubkey.cpp/pubkey.h 71c824ed6cf70b39ca09e8b3962f452f69523af0 cleaned up and added missing "include" statements for pubkey.cpp and pubkey.h (William Bright) Pull request description: #### Problem: Many symbols in the files were undefined and causing issues when I was working on building independent sections of the codebase. The hidden imports from the "secp256k1" library was a particular pain point. The other standard and missing includes are following best practices and will help with refactoring, build process and others. #### Changes: Clean up and declared imports/include for `pubkey.cpp` and `pubkey.h` ACKs for top commit: jnewbery: utACK 71c824ed6c laanwj: Code review ACK 71c824ed6cf70b39ca09e8b3962f452f69523af0 Tree-SHA512: bce605cfde24d8e3be82a596cabab7a8577fec0aef7c5e6f7a56603357046d8e8dea11ac8e3dbe79600550291be7784e35c7a55ebf40b46525b8949e4bedae96
2021-05-10Merge bitcoin/bitcoin#21836: scripted-diff: Replace three dots with ellipsis ↵W. J. van der Laan
in the UI strings d66f283ac07edce432b964f7f814631f5a5bc33b scripted-diff: Replace three dots with ellipsis in the UI strings (Hennadii Stepanov) Pull request description: This PR is split from #21463. The change was suggested on [Transifex.com](https://www.transifex.com/bitcoin/bitcoin/), and it does not touch `LogPrint` and `LogPrintf` calls. The only comment on #21463 [was](https://github.com/bitcoin/bitcoin/pull/21463/commits/9030e4b5a6de54e041c59e98d91adecbebf3611a#r597220100): > Mind that these messages also end up in the log. In principle the log is already UTF-8 (as are all strings and text in bitcoind). But, just noting, that it might make browsing the log a less pleasant experience on systems with misconfigured locale like some BSDs by default. ACKs for top commit: laanwj: ACK d66f283ac07edce432b964f7f814631f5a5bc33b Tree-SHA512: 5ab1cb3160f3f996f1ad7d7486662da3eb7f06a857f4a1874963ce10caed5b86b0ad6151b1b9ebeb2b8aa5f0c85efad3b768ea9cafe5db86f78f88912b756d1e
2021-05-10Merge bitcoin/bitcoin#21708: build: Drop pointless sed commandsW. J. van der Laan
f52fafc9351e4948d0656b1736788bb5896dcc3f build: Drop pointless sed commands (Hennadii Stepanov) Pull request description: Since moving to Autotools build system (35b8af92265ed74de63c3818e5290c27b3f35df2, #2943, 2013-09), tag strings created by Qt specialized compilers ([uic](https://doc.qt.io/qt-5/uic.html), [moc](https://doc.qt.io/qt-5/moc.html), [rcc](https://doc.qt.io/qt-5/rcc.html)) were being removed. A bit later (70c71c50ce552c0358679653c04d7cc72a40222c, #4241, 2014-06) this rule was dropped for the uic, and since then all of the generated `ui_*.h` files contain the following string: ``` ** Created by: Qt User Interface Compiler version 5.12.8 ``` Such strings do not contain any timestamps, and cannot cause any non-determinism. The removing of them seems pointless. Diffs for some files: ```diff --- master/intro.moc +++ pr/intro.moc @@ -1,6 +1,7 @@ /**************************************************************************** ** Meta object code from reading C++ file 'intro.cpp' ** +** Created by: The Qt Meta Object Compiler version 67 (Qt 5.12.8) ** ** WARNING! All changes made in this file will be lost! *****************************************************************************/ ``` ```diff --- master/moc_addressbookpage.cpp +++ pr/moc_addressbookpage.cpp @@ -1,6 +1,7 @@ /**************************************************************************** ** Meta object code from reading C++ file 'addressbookpage.h' ** +** Created by: The Qt Meta Object Compiler version 67 (Qt 5.12.8) ** ** WARNING! All changes made in this file will be lost! *****************************************************************************/ ``` ```diff --- master/qrc_bitcoin.cpp +++ pr/qrc_bitcoin.cpp @@ -1,6 +1,7 @@ /**************************************************************************** ** Resource object code ** +** Created by: The Resource Compiler for Qt version 5.12.8 ** ** WARNING! All changes made in this file will be lost! *****************************************************************************/ ``` ACKs for top commit: laanwj: ACK f52fafc9351e4948d0656b1736788bb5896dcc3f Tree-SHA512: 31f5c19b37645b4914f17d8c234b7ae8781a0499c4b250ffef07d70b7552954fb682f58a75d76162f98ab5e1667288b3a041df2705573fb00523e87b9c1fd47f
2021-05-10refactor: Remove useless extern keywordMarcoFalke
2021-05-10Merge bitcoin/bitcoin#21786: wallet: ensure sat/vB feerates are in range ↵MarcoFalke
(mantissa of 3) 847288df07b45ca535c849e518b22818ab492896 test: fee rate values that cannot be represented as sat/vB (Jon Atack) 06a90fa0381c790f7bde2ab9bf47d2b22acef4a5 rpc: for sat/vB fee rates, limit ParseFixedPoint decimals to 3 (Jon Atack) 0742c7840f03505597fd2de87db97f12597ef667 rpc: enable passing decimals to AmountFromValue, add doxygen (Jon Atack) 8ce3ef57a3e9ad13c0aaa4648e8584241d53592d test: ParseFixedPoint with 3 decimals for sat/vB fee rates (Jon Atack) b5033275979a2a495b02b25f70cadbdcc8b6eb6a test: type error and out of range fee rates where missing (Jon Atack) c5fd4344f7fcc257062a610c8ff26ffcc9b53953 test: explicit fee rates with invalid amounts (Jon Atack) ea6f76b66ecc52360719053489e0ec9f9a673eab test: improve zero-value explicit fee rate coverage (Jon Atack) Pull request description: - Improve/close gaps in existing test coverage before making the change - Enable passing `decimals` to `ParseFixedPoint()` when calling `AmountFromValue()` - Limit explicit fee rates in sat/vB passed in by users to 3 decimals, and raise otherwise - Add regression test coverage Closes #20534. ACKs for top commit: MarcoFalke: review ACK 847288df07b45ca535c849e518b22818ab492896 🔷 Tree-SHA512: c539d07ae9b21c0d6c8ea460beb9c8dad5559445518aace560abc3c05c588907bae189b6fd7602b3b397de4a42356136c3ec6f960d3dcf2d5d16377aef4ab5a2
2021-05-10Merge bitcoin/bitcoin#21897: rpc: adjust incorrect RPCHelpMan typesMarcoFalke
7031721f2cc3eef30c46ff50c52328e9ba8090e0 rpc/listaddressgroupings: redefine inner-most array as ARR_FIXED (Karl-Johan Alm) 8500f7bf54d3e27fd2fa7fda15ad833f5688c331 rpc/createrawtransaction: redefine addresses as OBJ_USER_KEYS (Karl-Johan Alm) d9e2183c50f50465b9f173171fee240949bf8bd2 rpc: include OBJ_USER_KEY in RPCArg constructor checks (Karl-Johan Alm) Pull request description: This PR adjusts the two issues I encountered while developing a tool that converts RPCHelpMan objects into bindings for other language(s). The first is in createrawtransaction, where the address part, e.g. bc1qabc in > createrawtransaction '[]' '[{"bc1qabc": 1.0}]' is declared as a `Type::OBJ`, when in reality it should be a `Type::OBJ_USER_KEYS`, defined as such: https://github.com/bitcoin/bitcoin/blob/5925f1e652768a9502831b9ccf78d16cf3c37d29/src/rpc/util.h#L126 (coincidentally, this is the first and only (afaict) usage of this `RPCArg::Type`). The second is in the `listaddressgroupings` RPC, which returns an array of arrays of arrays, where the innermost one is a tuple-thingie with an optional 3rd item; this is an `ARR_FIXED`, not an `ARR`. ACKs for top commit: MarcoFalke: ACK 7031721f2cc3eef30c46ff50c52328e9ba8090e0 🐀 Tree-SHA512: 769377416c6226d1738a956fb685498e009f9e7eb2d45bc679b81c5364b9520fdbcb49392c937ab45598aa0d33589e8e6a59ccc101cf8d8e7dfdafd58d4eefd0
2021-05-10Merge bitcoin/bitcoin#21581: streams: Accept URef obj for VectorReader ↵MarcoFalke
unserialize fa2204f6adef493079d1ca5148b0fdc2b55816e6 streams: Accept URef obj for VectorReader unserialize (MarcoFalke) Pull request description: Missed in commit 172f5fa738d419efda99542e2ad2a0f4db5be580. An URef may collapse into an LRef or RRef depending on context. There is no reason to forbid RRef in `VectorReader::operator>>`, so add it for consistency. ACKs for top commit: ryanofsky: Code review ACK fa2204f6adef493079d1ca5148b0fdc2b55816e6, just expanded test since last review Tree-SHA512: 09ff4e8a918e15b08cebd8c125d37e78bfb3a635c38546fc8454a97a882b2c81c55ef552243617e78744799d31127e6fbf78c4e319c030480b370aab6f38b645
2021-05-09qt: enable wordWrap for peers-tab detail servicesrandymcmillan
2021-05-09rpc/listaddressgroupings: redefine inner-most array as ARR_FIXEDKarl-Johan Alm
ARR_FIXED is for cases like this, where the elements are in an array for convenience, rather than due to being dynamically sized lists.
2021-05-09rpc/createrawtransaction: redefine addresses as OBJ_USER_KEYSKarl-Johan Alm
The OBJ type is for actual objects with defined keys; OBJ_USER_KEYS is for objects with user-defined keys (such as the bitcoin address(es) in the createrawtransaction output object.
2021-05-09rpc: include OBJ_USER_KEY in RPCArg constructor checksKarl-Johan Alm
2021-05-09rpc: for sat/vB fee rates, limit ParseFixedPoint decimals to 3Jon Atack
2021-05-09rpc: enable passing decimals to AmountFromValue, add doxygenJon Atack
2021-05-09test: ParseFixedPoint with 3 decimals for sat/vB fee ratesJon Atack
2021-05-09refactor: Add TSA annotations to the WorkQueue class membersHennadii Stepanov
2021-05-09fuzz: Avoid excessively large min fee rate in tx_poolMarcoFalke
2021-05-09fuzz: Remove strprintf test cases that are known to failMarcoFalke
2021-05-09fuzz: Limit ParseISO8601DateTime fuzzing to 32-bitMarcoFalke
2021-05-09Merge bitcoin/bitcoin#21872: net: Sanitize message type for loggingMarcoFalke
09205b33aa74e385caa2803aa6febc18ad1efa32 net: Clarify message header validation errors (W. J. van der Laan) 955eee76803c098978cf0bbc7f1f6d3c230544e2 net: Sanitize message type for logging (W. J. van der Laan) Pull request description: - Use `SanitizeString` when logging message errors to make sure that the message type is sanitized. I have checked all logging in `net.cpp`. - For the `MESSAGESTART` error don't inspect and log header details at all: receiving invalid start bytes makes it likely that the packet isn't even formatted as valid P2P message. Logging the four unexpected start bytes (as hex) should be enough. - Update `p2p_invalid_messages.py` test to check this. - Improve error messages in a second commit. Issue reported by gmaxwell. ACKs for top commit: MarcoFalke: re-ACK 09205b33aa74e385caa2803aa6febc18ad1efa32 only change is log message fixup 🔂 practicalswift: re-ACK 09205b33aa74e385caa2803aa6febc18ad1efa32 Tree-SHA512: 8fe5326af135cfcf39ea953d9074a8c966b9b85a810b06a2c45b8a745cf115de4f321e72fc769709d6bbecfc5953aab83176db6735b04c0bc6796f59272cadce
2021-05-08fuzz: Remove unused --enable-danger-fuzz-link-all optionMarcoFalke
2021-05-07net: Clarify message header validation errorsW. J. van der Laan
Make the errors less shouty and more descriptive.
2021-05-07fuzz: Add WRITE_ALL_FUZZ_TARGETS_AND_ABORTMarcoFalke
2021-05-07Merge bitcoin/bitcoin#21802: refactor: Avoid UB in util/asmap (advance a ↵MarcoFalke
dereferenceable iterator outside its valid range) fa098713201a6999ec4c12d0a8bde0adcf47b095 refactor: Avoid sign-compare compiler warning in util/asmap (MarcoFalke) Pull request description: Can be reproduced on current master with `D_GLIBCXX_DEBUG`: ``` /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/debug/safe_iterator.h:883: In function: __gnu_debug::_Safe_iterator<type-parameter-0-0, type-parameter-0-1, std::random_access_iterator_tag>::_Self __gnu_debug::operator+(const __gnu_debug::_Safe_iterator<type-parameter-0-0, type-parameter-0-1, std::random_access_iterator_tag>::_Self &, __gnu_debug::_Safe_iterator<type-parameter-0-0, type-parameter-0-1, std::random_access_iterator_tag>::difference_type) Error: attempt to advance a dereferenceable iterator 369 steps, which falls outside its valid range. Objects involved in the operation: iterator @ 0x0x7ffd3d613138 { type = std::__cxx1998::_Bit_const_iterator (constant iterator); state = dereferenceable; references sequence with type 'std::__debug::vector<bool, std::allocator<bool> >' @ 0x0x7ffd3d663590 } ==65050== ERROR: libFuzzer: deadly signal #0 0x559ab9787690 in __sanitizer_print_stack_trace (/bitcoin/src/test/fuzz/fuzz+0x5a1690) #1 0x559ab9733998 in fuzzer::PrintStackTrace() (/bitcoin/src/test/fuzz/fuzz+0x54d998) #2 0x559ab9718ae3 in fuzzer::Fuzzer::CrashCallback() (/bitcoin/src/test/fuzz/fuzz+0x532ae3) #3 0x7f70a0e723bf (/lib/x86_64-linux-gnu/libpthread.so.0+0x153bf) #4 0x7f70a0b3418a in raise (/lib/x86_64-linux-gnu/libc.so.6+0x4618a) #5 0x7f70a0b13858 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x25858) #6 0x7f70a0f21148 (/lib/x86_64-linux-gnu/libstdc++.so.6+0xa1148) #7 0x559ab9f60a96 in __gnu_debug::operator+(__gnu_debug::_Safe_iterator<std::__cxx1998::_Bit_const_iterator, std::__debug::vector<bool, std::allocator<bool> >, std::random_access_iterator_tag> const&, long) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/debug/safe_iterator.h:881:2 #8 0x559ab9f61062 in SanityCheckASMap(std::__debug::vector<bool, std::allocator<bool> > const&, int) util/asmap.cpp:159:21 #9 0x559ab9e4fdfa in SanityCheckASMap(std::__debug::vector<bool, std::allocator<bool> > const&) netaddress.cpp:1242:12 #10 0x559ab9793fcb in addrman_fuzz_target(Span<unsigned char const>) test/fuzz/addrman.cpp:43:14 #11 0x559ab978a03c in std::_Function_handler<void (Span<unsigned char const>), void (*)(Span<unsigned char const>)>::_M_invoke(std::_Any_data const&, Span<unsigned char const>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:300:2 #12 0x559aba2692c7 in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14 #13 0x559aba269132 in LLVMFuzzerTestOneInput test/fuzz/fuzz.cpp:63:5 #14 0x559ab971a1a1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/bitcoin/src/test/fuzz/fuzz+0x5341a1) #15 0x559ab97198e5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) (/bitcoin/src/test/fuzz/fuzz+0x5338e5) #16 0x559ab971bb87 in fuzzer::Fuzzer::MutateAndTestOne() (/bitcoin/src/test/fuzz/fuzz+0x535b87) #17 0x559ab971c885 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/bitcoin/src/test/fuzz/fuzz+0x536885) #18 0x559ab970b23e in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/bitcoin/src/test/fuzz/fuzz+0x52523e) #19 0x559ab9734082 in main (/bitcoin/src/test/fuzz/fuzz+0x54e082) #20 0x7f70a0b150b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2) #21 0x559ab96dffdd in _start (/bitcoin/src/test/fuzz/fuzz+0x4f9fdd) ACKs for top commit: sipa: utACK fa098713201a6999ec4c12d0a8bde0adcf47b095 vasild: ACK fa098713201a6999ec4c12d0a8bde0adcf47b095 Tree-SHA512: 802fda33bda40fe2521f1e3be075ceddc5fd9ba185bd494286e50019931dfd688da7a6513601138b1dc7bb8e80ae47c8572902406eb59f68990619ddb2656748
2021-05-07Merge bitcoin/bitcoin#21845: net processing: Don't require locking cs_main ↵fanquake
before calling RelayTransactions() 39e19713cd6594f93db835e8ef7eef5824a9ba02 [net processing] Add internal _RelayTransactions() (John Newbery) Pull request description: As part of the general effort to reduce cs_main usage in net_processing, this removes the need to be holding `cs_main` when calling `RelayTransactions()` from outside net_processing. Internally, we lock `cs_main` and call an internal `_RelayTransactions()` function that _does_ require `cs_main`. ACKs for top commit: MarcoFalke: re-unsigned-code-review ACK 39e19713cd6594f93db835e8ef7eef5824a9ba02 promag: Code review ACK 39e19713cd6594f93db835e8ef7eef5824a9ba02, just included sync.h since last review. ajtowns: ACK 39e19713cd6594f93db835e8ef7eef5824a9ba02 Tree-SHA512: dc08441233adfb8eaac501cf497cb4bad029eb723bd3fa8a3d8b7e49cc984c98859b95780ad15f5701d62ac745a8223beb0df405e3d49d95a8c86c8be17c9543
2021-05-06net: Sanitize message type for loggingW. J. van der Laan
- Use `SanitizeString` when logging message errors to make sure that the message type is sanitized. - For the `MESSAGESTART` error don't inspect and log header details at all: receiving invalid start bytes makes it likely that the packet isn't even formatted as valid P2P message. Logging the four unexpected start bytes should be enough. - Update `p2p_invalid_messages.py` test to check this. Issue reported by gmaxwell.
2021-05-06Merge bitcoin/bitcoin#21798: fuzz: Create a block template in tx_pool targetsMarcoFalke
fa03d0acd6bd8bb6d3d5227512f042ff537ad993 fuzz: Create a block template in tx_pool targets (MarcoFalke) fa61ce5cf5c1d73d352173806571bcd7799ed2ee fuzz: Limit mocktime to MTP in tx_pool targets (MarcoFalke) fab646b8ea293bb2b03707c6ef6790982625e492 fuzz: Use correct variant of ConsumeRandomLengthString instead of hardcoding a maximum size (MarcoFalke) fae2c8bc54e6c0fe69a82bd1b232c52edd1acd34 fuzz: Allow to pass min/max to ConsumeTime (MarcoFalke) Pull request description: Relatively simple check to ensure a block can always be created from the mempool ACKs for top commit: practicalswift: Tested ACK fa03d0acd6bd8bb6d3d5227512f042ff537ad993 Tree-SHA512: e613376ccc88591cbe594db14ea21ebc9b2b191f6325b3aa4ee0cd379695352ad3b480e286134ef6ee30f043d486cf9792a1bc7e44445c41045ac8c3b931c7ff
2021-05-05streams: Accept URef obj for VectorReader unserializeMarcoFalke
2021-05-05fuzz: Create a block template in tx_pool targetsMarcoFalke
2021-05-05Merge bitcoin/bitcoin#21681: validation: fix ActivateSnapshot to use ↵MarcoFalke
hardcoded nChainTx 91d93aac4e3fe6fff5ef492ed152c4d8fa6f2672 validation: remove nchaintx from assumeutxo metadata (James O'Beirne) 931684b24a89aba884cb18c13fa67ccca339ee8c validation: fix ActivateSnapshot to use hardcoded nChainTx (James O'Beirne) Pull request description: This fixes an oversight from the move of nChainTx from the user-supplied snapshot metadata into the hardcoded assumeutxo chainparams. Since the nChainTx is now unused in the metadata, it should be removed in a future commit. See: https://github.com/bitcoin/bitcoin/pull/19806#discussion_r612165410 ACKs for top commit: Sjors: utACK 91d93aac4e3fe6fff5ef492ed152c4d8fa6f2672 ryanofsky: Code review ACK 91d93aac4e3fe6fff5ef492ed152c4d8fa6f2672. No change to previous commit, just new commit removing now unused utxo snapshot field and updating tests. Tree-SHA512: 445bdd738faf007451f40bbcf360dd1fb4675e17a4c96546e6818c12e33dd336dadd95cf8d4b5f8df1d6ccfbc4bf5496864bb5528e416cea894857b6b732140c
2021-05-05Merge bitcoin/bitcoin#21817: refactor: Replace &foo[0] with foo.data()MarcoFalke
fac30eec42c486ec1bfd696293040a7aa0f04625 refactor: Replace &foo[0] with foo.data() (MarcoFalke) faece47c4706783e0460ed977390a44630b2d44c refactor: Avoid &foo[0] on C-Style arrays (MarcoFalke) face9611093377e8502d91f2ff56f9319a56357c refactor: Use only one temporary buffer in CreateObfuscateKey (MarcoFalke) fa05dddc42770809fdae4d9c35155f8117960019 refactor: Use CPubKey vector constructor where possible (MarcoFalke) fabb6dfe6e734eadd91448122f2ce8c1612c39a6 script: Replace address-of idiom with vector data() method (Guido Vranken) Pull request description: The main theme of this refactor is to replace `&foo[0]` with `foo.data()`. The first commit is taken from #21781 with the rationale: * In CSignatureCache::ComputeEntryECDSA, change the way a vector pointer is resolved to prevent invoking undefined behavior if the vector is empty. The other commits aim to remove all `&foo[0]`, where `foo` is any kind of byte representation. The rationale: * Sometimes alternative code without any raw data pointers is easier to read (refer to the respective commit message for details) * If the raw data pointer is needed, `foo.data()` should be preferred, as pointed out in the developer notes. This addresses the instances that have been missed in commit 592404f03f2b734351d734f0c9ca1fdce997321b, and https://github.com/bitcoin/bitcoin/pull/9804 ACKs for top commit: laanwj: Code review ACK fac30eec42c486ec1bfd696293040a7aa0f04625 practicalswift: cr ACK fac30eec42c486ec1bfd696293040a7aa0f04625: patch looks correct promag: Code review ACK fac30eec42c486ec1bfd696293040a7aa0f04625. Tree-SHA512: e7e73146edbc78911a8e8c728b0a1c6b0ed9a88a008e650aa5dbffe72425bd42c76df70199a9cf7e02637448d7593e0eac52fd0f91f59240283e1390ee21bfa5
2021-05-05Merge bitcoin/bitcoin#21710: doc: update helps for addnode rpc and ↵W. J. van der Laan
-addnode/-maxconnections config options b4fcbcfb49461b96bc72fb64d6152de7c5ce00de doc: update -maxconnections config option help (Jon Atack) 79685a8992ad302833b506cc6d03aab1cc127de0 doc: update -addnode config option help (Jon Atack) 2896c6c4cc6d382d8369c037e274c08dd8e32c69 doc: update addnode rpc help (Jon Atack) Pull request description: Since #9319 proposed by Gregory Maxwell and released in v0.14, peers manually added through the `-addnode` config option or using the `addnode` RPC have their own separate limit of 8 connections that does not compete with other inbound or outbound connection usage and is not subject to the limitation imposed by the `-maxconnections` option. This PR updates the `-addnode` and `-maxconnections` config options and the `addnode` RPC help docs with this information. `-addnode` config option help ``` $ bitcoind -h | grep -A5 addnode= -addnode=<ip> Add a node to connect to and attempt to keep the connection open (see the addnode RPC help for more info). This option can be specified multiple times to add multiple nodes; connections are limited to 8 at a time and are counted separately from the -maxconnections limit. $ bitcoind -h | grep -A3 maxconnections= -maxconnections=<n> Maintain at most <n> connections to peers (default: 125). This limit does not apply to connections manually added via -addnode or the addnode RPC, which have a separate limit of 8. ``` `addnode` rpc help ``` $ bitcoin-cli help addnode addnode "node" "command" Attempts to add or remove a node from the addnode list. Or try a connection to a node once. Nodes added using addnode (or -connect) are protected from DoS disconnection and are not required to be full nodes/support SegWit as other outbound peers are (though such peers will not be synced from). Addnode connections are limited to 8 at a time and are counted separately from the -maxconnections limit. ``` ACKs for top commit: prayank23: ACK https://github.com/bitcoin/bitcoin/commit/b4fcbcfb49461b96bc72fb64d6152de7c5ce00de jarolrod: ACK b4fcbcfb49461b96bc72fb64d6152de7c5ce00de Tree-SHA512: b6d69baa6cbf6d53f91bac5b39b549d49db6c95f92ea1bdd3588a6432794a25ac2c8b3c89e2c72bb9097e61f2717c8b5ecc404745d5992b88e523db03200898f
2021-05-05Merge bitcoin/bitcoin#21727: refactor: Move more stuff to blockstorageW. J. van der Laan
fa09a9eac8d8ab65ce4064c35a9f21349a644982 style: Add { } to multi-line if (MarcoFalke) fadafab83379ff10d86ada179c6f9641d19464fe move-only: Move functions to blockstorage (MarcoFalke) fa7e64d58615fffea91cd64dc4a2790221ceff0a move-only: Move constants to blockstorage (MarcoFalke) fa247a327fc7c7cea6bc8f93637b8babd3015ffa refactor: Move block storage globals to blockstorage (MarcoFalke) fa81c30c6f1adac79517c958090db174eb6aeda2 refactor: Move pruning/reindex/importing globals to blockstorage (MarcoFalke) Pull request description: See #21575 ACKs for top commit: Sjors: ACK fa09a9eac8d8ab65ce4064c35a9f21349a644982 kiminuo: ACK fa09a9e laanwj: Code review ACK fa09a9eac8d8ab65ce4064c35a9f21349a644982 promag: Code review ACK fa09a9eac8d8ab65ce4064c35a9f21349a644982. Since last review Tree-SHA512: 2eb6962ff44da6b77f3058fc02ec66ab742e25ae8dcc8ec62b062896571910d43ca7c4bb16fb3ccb5e5245195b8dec6384b6c8d442fa97ca28d93bdff347d677