aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2020-06-04net: Reformat excessively long if condition into multiple linesMarcoFalke
Can be reviewed with the git option --word-diff-regex=.
2020-06-04Merge #19053: refactor: replace CNode pointers by references within ↵MarcoFalke
net_processing.{h,cpp} 8b3136bd307123a255b9166aa42a497a44bcce70 refactor: replace CNode pointers by references within net_processing.{h,cpp} (Sebastian Falbesoner) Pull request description: This PR is inspired by a [recent code review comment](https://github.com/bitcoin/bitcoin/pull/19010#discussion_r426954791) on a PR that introduced new functions to the net_processing module. The point of the discussion was basically that whenever we pass something not by value (in the concrete example it was about `CNode*` and `CConnman*`) we should either use * a pointer (```CType*```) with null pointer check or * a reference (```CType&```) To keep things simple, this PR for a first approach * only tackles `CNode*` pointers * only within the net_processing module, i.e. no changes that would need adaption in other modules * keeps the names of the variables as they are I'm aware that PRs like this are kind of a PITA to review, but I think the code quality would increase if we get rid of pointers without nullptr check -- bloating up the code by adding all the missing checks would be the worse alternative, in my opinion. Possible follow-up PRs, in case this is received well: * replace CNode pointers by references for net module * replace CConnman pointers by references for net_processing module * ... ACKs for top commit: MarcoFalke: ACK 8b3136bd307123a255b9166aa42a497a44bcce70 🔻 practicalswift: ACK 8b3136bd307123a255b9166aa42a497a44bcce70 Tree-SHA512: 15b6a569ecdcb39341002b9f4e09b38ed4df077e3a3a50dfb1b72d98bdc9f9769c7c504f106456aa7748af8591af7bb836b72d46086df715ab116e4ac3224b3b
2020-06-04Merge #19112: rpc: Remove special case for unknown service flagsWladimir J. van der Laan
fa1433ac1be8481f08c1a0a311a6b87d8a874c6a rpc: Remove special case for unknown service flags (MarcoFalke) Pull request description: The special case to return a bit as an integer is clumsy and undocumented. Probably also irrelevant because there shouldn't currently be a non-misbehaving client that connects to Bitcoin Core and advertises an unknown service flag. Thus, simply remove the code. ACKs for top commit: laanwj: ACK fa1433ac1be8481f08c1a0a311a6b87d8a874c6a Tree-SHA512: 942de6a577a9ee076ce12c92be121617640d53ee8c3424064c45a30a7ff789555d3722a4203670768faf81da2a40adfed3ec5cdeb5da06f04be81ddb53b9db7e
2020-06-04Merge #17994: validation: flush undo files after last block writeWladimir J. van der Laan
ac94141af0c16161afa68de1c3720f254ae4e12c validation: delay flushing undo files in syncing node case (Karl-Johan Alm) Pull request description: Fixes #17890. Replaces #17892. Data files (`{blk|rev}<number>.dat`) pre-allocate space as they are written, and then trims down to the final size once they move on to the next sequence ("finalized flush"). The code currently assumes (incorrectly) that blk and rev files finish at the same time, but because blk files are written as blocks come in, and rev files are written in block height order, rev files end up being written to for awhile after moving on to the next block file, resulting in pre-allocation and waste of up to 1 MB of space per rev file. The exact point at which rev file writing finishes is the highest height block found inside the corresponding block file, which is already available in the CBlockFileInfo vector. This PR moves finalized flushing of undo files to to directly after the undo data for the previous block file has been written. There is a branch with annotation that demonstrates how this is handling flushing here: https://github.com/kallewoof/bitcoin/tree/200124-rev-files-annotated ACKs for top commit: vasild: ACK ac94141af (no changes in the code since ed34e00da). fjahr: Code review re-ACK ac94141af0c16161afa68de1c3720f254ae4e12c jonatack: Code review ACK ac94141af0c16 Tree-SHA512: 1d4e3b3d1d99bd7ebe7a2f632b1231146dd4f9f993c54db3a4090d9c086d95d2e4c327fd936066392b3afc6277b8f3a908d5c5993d4c8e49f72b92a417716dd2
2020-06-04Merge #19131: refactor: Fix unreachable code in init arg checksWladimir J. van der Laan
eea81146571480b2acd12c8cd7f36b04d056c47f build: Enable unreachable-code-loop-increment (Jonathan Schoeller) d15db4b1fc988736b08c092d000ca1d1ff686975 refactor: Fix unreachable code in init arg checks (Jonathan Schoeller) Pull request description: Closes: #19017 In #19015 it's been suggested that we add some new compiler warnings to our build. Some of these, such as `-Wunreachable-code-loop-increment`, generate warnings. We'll likely want to fix these up if we're going to turn these warnings on. ```shell init.cpp:969:5: warning: loop will run at most once (loop increment never executed) [-Wunreachable-code-loop-increment] for (const auto& arg : gArgs.GetUnsuitableSectionOnlyArgs()) { ^~~ 1 warning generated. ``` https://github.com/bitcoin/bitcoin/blob/aa8d76806c74a51ec66e5004394fe9ea8ff0fac4/src/init.cpp#L968-L972 To fix this, collect all errors, and output them in a single error message after the loop completes. This resolves the unreachable code warning, and avoids popup hell that could result from outputting a seperate message for each error or warning one by one. ACKs for top commit: laanwj: Code review ACK eea81146571480b2acd12c8cd7f36b04d056c47f hebasto: re-ACK eea81146571480b2acd12c8cd7f36b04d056c47f, only suggested changes applied since the [previous](https://github.com/bitcoin/bitcoin/pull/19131#pullrequestreview-421772387) review. Tree-SHA512: 2aa3ceb7fab581b6ba2580900668388d8eba1c3001c8ff9c11c1f4a9a10fbc37f30e590249862676858446e3f4950140a252953ba1643ba3bfd772f8eae20583
2020-06-04Merge #19142: validation: Make VerifyDB level 4 interruptiblefanquake
fa3b4f9b8e54ec07aeb2e5e2333da3e784f7be12 validation: Make VerifyDB level 4 interruptible (MarcoFalke) fa1d5800d9c5e33942b76f6667839a818723dee9 validation: Remove unused boost interruption_point (MarcoFalke) Pull request description: level 0,1,2, and 3 are already interruptible, so make level 4 also interruptible ACKs for top commit: laanwj: Code review ACK fa3b4f9b8e54ec07aeb2e5e2333da3e784f7be12 fanquake: ACK fa3b4f9b8e54ec07aeb2e5e2333da3e784f7be12 Tree-SHA512: d302c84a17add1b5993dd78339c88670d27eee45ce208c4d046ae188b50be9843ee5a9584739d5d25453b54ae08fd1cb6eeee8cb1307d84c05cde8a54a7c445b
2020-06-03Merge #19088: validation: use std::chrono throughout some validation functionsMarcoFalke
789e9dd3aa727176797529c35b2848f994630a82 validation: use std::chrono in IsCurrentForFeeEstimation() (fanquake) 47be28c8bc475eafeebd4fc58ea92f0d3df0d8c6 validation: use std::chrono in CChainState::FlushStateToDisk() (fanquake) Pull request description: Probably up for debate as to which type is used for the constants. Personally, swapping these to hours is more readable. ACKs for top commit: MarcoFalke: ACK 789e9dd3aa727176797529c35b2848f994630a82 jonatack: ACK 789e9dd3aa727176797529c35b2848f994630a82 Tree-SHA512: f4a25cbd00a49a54b7783a1f588be83706dd2a475cecb5c2e8b97b2d4b27c0955a7454d7486f2454e96351c44f233b300c4f4b9ca62fc7336277f10da34dd5c3
2020-06-03Merge #18875: fuzz: Stop nodes in process_message* fuzzersMarcoFalke
fab860aed4878b831dae463e1ee68029b66210f5 fuzz: Stop nodes in process_message* fuzzers (MarcoFalke) 6666c828e072a5e99ea0c16394ca3e5b9de07409 fuzz: Give CNode ownership to ConnmanTestMsg in process_message fuzz harness (MarcoFalke) Pull request description: Background is that I saw an integer overflow in net_processing ``` #30629113 REDUCE cov: 25793 ft: 142917 corp: 3421/2417Kb lim: 4096 exec/s: 89 rss: 614Mb L: 1719/4096 MS: 1 EraseBytes- net_processing.cpp:977:25: runtime error: signed integer overflow: 2147483624 + 100 cannot be represented in type 'int' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior net_processing.cpp:977:25 in net_processing.cpp:985:9: runtime error: signed integer overflow: -2147483572 - 100 cannot be represented in type 'int' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior net_processing.cpp:985:9 in ``` Telling from the line numbers, it looks like `nMisbehavior` wrapped around. Fix that by calling `StopNodes` after each exec, which should clear the node state and thus `nMisbehavior`. ACKs for top commit: practicalswift: ACK fab860aed4878b831dae463e1ee68029b66210f5 Tree-SHA512: 891c081d5843565d891aec028b6c27ef3fa39bc40ae78238e81d8f784b4d4b49cb870998574725a5159dd03aeeb2e0b9bc3d3bb51d57d1231ef42e3394b2d639
2020-06-03validation: Make VerifyDB level 4 interruptibleMarcoFalke
2020-06-03validation: Remove unused boost interruption_pointMarcoFalke
ActivateBestChain (ABC) is only called in the "msghand" or one of the RPC threads, neither of which is a boost::thread. However, ABC is also called in ThreadImport (which currently happens to be a boost::thread). In all cases, the interruption_point is redundant with the breakpoint in ABC that triggers when ShutdownRequested() VerifyDB is only called in the main thread ("init") or one of the RPC threads, neither of which is a boost::thread.
2020-06-03Merge #19084: net: improve code documentation for dns seed behaviourfanquake
5cb7ee67a5c0e5da39eb698b64d23722fb2f7b3e net: improve code documentation for dns seed behaviour (Anthony Towns) Pull request description: Some better internal documentation post #16939 ACKs for top commit: hebasto: ACK 5cb7ee67a5c0e5da39eb698b64d23722fb2f7b3e naumenkogs: ACK 5cb7ee6 ariard: ACK 5cb7ee6 fanquake: ACK 5cb7ee67a5c0e5da39eb698b64d23722fb2f7b3e - thanks for following up. Tree-SHA512: 5a12680651cd1e0436aed1cadcbf50047ffeabfdc9f7f2f81fa176c30b10673fc960154c73ec34ed08ace1a000a81cedd44d67587883152654dee46065991b45
2020-06-02Merge #18982: wallet: Minimal fix to restore conflicted transaction ↵MarcoFalke
notifications 7eaf86d3bfc83f2beb3ef449707d5156853126fb trivial: Suggested cleanups to surrounding code (Russell Yanofsky) b604c5c8b5892842f13dee89ae31812a28ab25d1 wallet: Minimal fix to restore conflicted transaction notifications (Russell Yanofsky) Pull request description: This fix is a based on the fix by Antoine Riard (ariard) in https://github.com/bitcoin/bitcoin/pull/18600. Unlike that PR, which implements some new behavior, this just restores previous wallet notification and status behavior for transactions removed from the mempool because they conflict with transactions in a block. The behavior was accidentally changed in two `CWallet::BlockConnected` updates: a31be09bfd77eed497a8e251d31358e16e2f2eb1 and 7e89994133725125dddbfa8d45484e3b9ed51c6e from https://github.com/bitcoin/bitcoin/pull/16624, causing issue https://github.com/bitcoin/bitcoin/issues/18325. The change here could be improved and replaced with a more comprehensive cleanup, so it includes a detailed comment explaining future considerations. Fixes #18325 Co-authored-by: Antoine Riard (ariard) ACKs for top commit: jonatack: Re-ACK 7eaf86d3bfc83f ariard: ACK 7eaf86d, reviewed, built and ran tests. MarcoFalke: ACK 7eaf86d3bfc83f2beb3ef449707d5156853126fb 🍡 Tree-SHA512: 9a1efe975969bb522a9dd73c41064a9348887cb67883cd92c6571fd2df4321b9f4568363891abdaae14a3b9b168ef8142e95c373fc04677e46289b251fb84689
2020-06-02Merge #18792: wallet: Remove boost from PeriodicFlushfanquake
fa1c74fd0342b74d44cc4e41fff3890c1434e8f7 wallet: Remove unused boost::thread_interrupted (MarcoFalke) fa7b885f51ff848d3f913bc6e15d24528300c210 walletdb: Remove unsed boost/thread (MarcoFalke) 5555d978b056ab0e0e59faaf2d2067ec43fffaef wallet: Make PeriodicFlush uninterruptible (MarcoFalke) Pull request description: The `boost::this_thread::interruption_point()` in the code base currently block the replacement of `boost::thread` with `std::thread`. [1] Remove them from the wallet because they are either unused or useless. The feature to interrupt a periodic flush is useless because all wallets have just been flushed https://github.com/bitcoin/bitcoin/blob/9ccaee1d5e2e4b79b0a7c29aadb41b97e4741332/src/init.cpp#L194 and another flush should be a noop. Also, they will be flushed again shortly after https://github.com/bitcoin/bitcoin/blob/9ccaee1d5e2e4b79b0a7c29aadb41b97e4741332/src/init.cpp#L285, so even if repeated flushes weren't a noop, doing 3 instead of 2 shouldn't matter too much at this point. Also, the wallet is flushed every two seconds in the worst case, so if this is an expensive operation, that period should be readjusted. (Or bdb should be removed altogether #18916) [1] Replacement of `boost::thread` with `std::thread` should happen because: * The boost thread dependency is slow to compile * Boost thread is less maintained than the standard lib * Boost thread is mostly redundant to the standard lib * Global interruption points via exceptions are hard to keep track of during review and easy to get wrong during runtime (e.g. accidental `catch (...)`) ACKs for top commit: fanquake: ACK fa1c74fd0342b74d44cc4e41fff3890c1434e8f7 Tree-SHA512: b166619256de2ef4325480fa1367f68bc9371ad785ec503aed61eab41ba61f1a9807aab25451a24efda3db64855c9ba0025645b98bc58557bc3ec56c5b3297d0
2020-06-02Merge #13204: Faster sigcache nonceMarcoFalke
152e8baf08c7379e5cc09f90863e6309bdd4866c Use salted hasher instead of nonce in sigcache (Jeremy Rubin) 5495fa585007b40b2e9285c23be275de71708af8 Add Hash Padding Microbenchmarks (Jeremy Rubin) Pull request description: This PR replaces nonces in two places with pre-salted hashers. The nonce is chosen to be 64 bytes long so that it forces the SHA256 hasher to process the chunk. This leaves the next 64 (or 56 depending if final chunk) open for data. In the case of the script execution cache, this does not make a big performance improvement because the nonce was already properly padded to fit into one buffer, but does make the code a little simpler. In the case of the sig cache, this should reduce the hashing overhead slightly because we are less likely to need an additional processing step. I haven't benchmarked this, but back of the envelope it should reduce the hashing by one buffer for all combinations except compressed public keys with compact signatures. ACKs for top commit: ryanofsky: Code review ACK 152e8baf08c7379e5cc09f90863e6309bdd4866c. No code changes, just rebase since last review and expanded commit message Tree-SHA512: b133e902fd595cfe3b54ad8814b823f4d132cb2c358c89158842ae27daee56ab5f70cde2585078deb46f77a6e7b35b4cc6bba47b65302b7befc2cff254bad93d
2020-06-02wallet: Remove unused boost::thread_interruptedMarcoFalke
FindWalletTx is only called by zapwallet, which is never called in a boost::thread
2020-06-02Merge #19111: Limit scope of all global std::once_flagMarcoFalke
fa9c67559186f5416c1c0b26c0a1d5e72c234ccb Limit scope of all global std::once_flag (MarcoFalke) Pull request description: `once_flag` is a helper (as the name might suggest) to execute a callable only once. Thus, the scope of the flag does never need to extend beyond where the callable is called. Typically this is function scope. Move all the flags to function scope to * simplify code review * avoid mistakes where similarly named flags are accidentally exchanged * avoid polluting the global scope ACKs for top commit: hebasto: ACK fa9c67559186f5416c1c0b26c0a1d5e72c234ccb, tested on Linux Mint 19.3 (x86_64). promag: Code review ACK fa9c67559186f5416c1c0b26c0a1d5e72c234ccb. Tree-SHA512: 095a0c11d93d0ddcb82b3c71676090ecc7e3de3d5e7a2a63ab2583093be279242acac43523bbae2060b4dcfa8f92b54256a0e91fbbae78fa92d2d49e9db62e57
2020-06-02Merge #19104: gui, refactor: Register Qt meta types in application constructorJonas Schnelli
4f49d5222eca11c149713ad34113d5a3d1c577b1 gui, refactor: Register Qt meta types in application constructor (João Barbosa) Pull request description: Removes a warning when running `QT_QPA_PLATFORM=cocoa src/qt/test/test_bitcoin-qt`. ACKs for top commit: jonasschnelli: Re utACK 4f49d5222eca11c149713ad34113d5a3d1c577b1 hebasto: ACK 4f49d5222eca11c149713ad34113d5a3d1c577b1, tested on macOS 10.15.5. Tree-SHA512: e931a022ba83cb0ef04d82544ebd9b18242f8fc2b41443afce4d5c4222f222e8b3517bdb484a1a4f61377c5dceca067d8ccf250da3a727299448e54bec33ed6e
2020-06-02refactor: replace CNode pointers by references within net_processing.{h,cpp}Sebastian Falbesoner
2020-06-02refactor: Fix unreachable code in init arg checksJonathan Schoeller
Building with -Wunreachable-code-loop-increment causes a warning due to always returning on the first iteration of the loop that outputs errors on invalid args. Collect all errors, and output them in a single error message after the loop completes, resolving the warning and avoiding popup hell by outputting a seperate message for each error.
2020-06-01Merge #19072: doc: Expand section on Getting Startedfanquake
facef3d4131f9980a4516282f11731361559509c doc: Explain that anyone can work on good first issues, move text to CONTRIBUTING.md (MarcoFalke) fae2fb2a196ee864e9a13fffc24a0279cd5d17e6 doc: Expand section on Getting Started (MarcoFalke) 100000d1b2c2e38d7a14a31b0af79e0e4316b04c doc: Add headings to CONTRIBUTING.md (MarcoFalke) fab893e0caf510d4836a20194892ef9c71426c51 doc: Fix unrelated typos reported by codespell (MarcoFalke) Pull request description: Some random doc changes: * Add sections to docs, so that they can be linked to * Explain that anyone (even maintainers) are allowed to work on good first issues * Expand section on Getting Started slightly ACKs for top commit: hebasto: ACK facef3d4131f9980a4516282f11731361559509c fanquake: ACK facef3d4131f9980a4516282f11731361559509c Tree-SHA512: 8998e273a76dbf4ca77e79374c14efe4dfcc5c6df6b7d801e1e1e436711dbe6f76b436f9cbc6cacb45a56827babdd6396f3bd376a9426ee7be3bb9b8a3b8e383
2020-05-31Merge #18994: tests: Add fuzzing harnesses for functions in script/MarcoFalke
f898ef65c947776750e49d050633f830546bbdc6 tests: Add fuzzing harness for functions in script/sign.h (practicalswift) c91d2f06150cda258a17e78d9b7065b594d34a85 tests: Add fuzzing harness for functions in script/sigcache.h (practicalswift) d3d8adb79fbe34b15cf29334607f9b76d303aa1a tests: Add fuzzing harness for functions in script/interpreter.h (practicalswift) fa80117cfdeca7e5d3a2a09b385c0e938bf701e9 tests: Add fuzzing harness for functions in script/descriptor.h (practicalswift) 43fb8f0ca331a7f79f0d287817da7f4b894bdbfa tests: Add fuzzing harness for functions in script/bitcoinconsensus.h (practicalswift) 8de72711c685e638fa54d485694fb1b1af024adc tests: Fill fuzzing coverage gaps for functions in script/script.h, script/script_error.h and script/standard.h (practicalswift) c571ecb07145b4ce8c17ca80489f8f1497388c4d tests: Add fuzzing helper functions ConsumeDataStream, ConsumeTxDestination and ConsumeUInt160 (practicalswift) Pull request description: Add fuzzing harnesses for functions in `script/`: * Add fuzzing helper functions `ConsumeDataStream` and `ConsumeUInt160` * Fill fuzzing coverage gaps for functions in `script/script.h`, `script/script_error.h` and `script/standard.h` * Add fuzzing harness for functions in `script/bitcoinconsensus.h` * Add fuzzing harness for functions in `script/descriptor.h` * Add fuzzing harness for functions in `script/interpreter.h` * Add fuzzing harness for functions in `script/sigcache.h` * Add fuzzing harness for functions in `script/sign.h` See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets). Happy fuzzing :) ACKs for top commit: MarcoFalke: ACK f898ef65c947776750e49d050633f830546bbdc6 🔉 Tree-SHA512: f6e77b34dc79f23de5fa9e38ac06e6554b5b946ec3e9a67e2bd982e60aca37ce844f785457ef427a5e3b45e31c305456bca8587cc9f4a0b50b3852e39726eb04
2020-05-31Merge #19044: net processing: Add support for getcfiltersMarcoFalke
9e36067d8cd02830c7e5a88a391dff6ac3adbe0c [test] Add test for cfilters. (Jim Posen) 11106a4722558765a44ae45c7892724a73ce514c [net processing] Message handling for getcfilters. (Jim Posen) e535670726952e43483763dfca6fc6ec2f4b0691 [indexes] Fix default [de]serialization of BlockFilter. (Jim Posen) bb911ae7f5cbe4974ec61266d2334b95067fa49d [refactor] Pass CNode and CConnman by reference (John Newbery) Pull request description: Support `getcfilters` requests when `-peerblockfilters` is set. Does not advertise compact filter support in version messages. ACKs for top commit: Empact: re-Code Review ACK https://github.com/bitcoin/bitcoin/pull/19044/commits/9e36067d8cd02830c7e5a88a391dff6ac3adbe0c MarcoFalke: re-ACK 9e36067d8c , only change is adding commit "[refactor] Pass CNode and CConnman by reference" 🥑 jkczyz: ACK 9e36067d8cd02830c7e5a88a391dff6ac3adbe0c fjahr: Code review ACK 9e36067d8cd02830c7e5a88a391dff6ac3adbe0c Tree-SHA512: b45b42a25905ef0bd9e195029185300c86856c87f78cbe17921f4a25e159ae0f6f003e61714fa43779017eb97cd89d3568419be88e47d19dc8095562939e7887
2020-05-31Merge #16564: test: Always define the raii_event_tests test suiteMarcoFalke
9a19c9ada53e84d1ee8521b48f656faad48b737a Always define the raii_event_tests test suite (Craig Andrews) Pull request description: The test suite must always be defined (even when EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED is not defined) so that the test harness doesn't fail due to not being able to find the raii_event_tests test. This improves upon 95f97f4 actually fixing https://github.com/bitcoin/bitcoin/issues/9493 ACKs for top commit: MarcoFalke: ACK 9a19c9ada53e84d1ee8521b48f656faad48b737a 🎹 Tree-SHA512: 3c42f17a9b5d56c8841f3aa9ac19da91c10aff210026266f31f7eb98a62528740d7c518c121452b68e8f801d6c80ecfb627d137ec6ed533289fa3beb08b4f176
2020-05-30Merge #18807: [doc / test / mempool] unbroadcast follow-upsMarcoFalke
9e1cb1adf1800efe429e348650931f2669b0d2c0 [trivial/doc] Fix comment type (Amiti Uttarwar) 8f30260a67166a6ab7c0f33f7ec1990d3c31761e [doc] Update unbroadcast description in RPC results (Amiti Uttarwar) 750456d6f29c63d57af05bfbdd6035bb9c965de2 [trivial] Remove misleading 'const' (Amiti Uttarwar) fa32e676e5833a5c5fc735ef00c0a80f5fab7a2c [test] Manage node connections better in mempool persist test (Amiti Uttarwar) 1f94bb0c744a103b633c1051e8fbc01e612097dc [doc] Provide rationale for randomization in scheduling. (Amiti Uttarwar) 9c8a55d9cb0ec73f10b196e79b637aa601c0a6b7 [mempool] Don't throw expected error message when upgrading (Amiti Uttarwar) ba5498318233ab81decbc585e9619d8ffe2df1b0 [test] Test that wallet transactions aren't rebroadcast before 12 hours (Amiti Uttarwar) 00d44a534b4e5ae249b8011360c6b0f7dc731581 [test] P2P connection behavior should meet expectations (Amiti Uttarwar) bd093ca15de762fdaf0937a0877d17b0c2bce16e [test] updates to unbroadcast test (Amiti Uttarwar) dab298d9ab5a5a41685f437db9081fa7b395fa73 [docs] add release notes (Amiti Uttarwar) Pull request description: This PR is a follow up to #18038 which introduced the idea of an unbroadcast set & focuses mostly on documentation updates and test fixes. One small functionality update to not throw an expected error in `LoadMempool` when you upgrade software versions. #18895 is another follow up to that addresses other functionality updates. Background context: The unbroadcast set is a mechanism for the mempool to track locally submitted transactions (via wallet or RPC). The node does a best-effort of delivering the transactions to the network via retries every 10-15 minutes until either a `GETDATA` is received or the transaction is removed from the mempool. ACKs for top commit: MarcoFalke: ACK 9e1cb1adf1 👁 gzhao408: ACK [`9e1cb1a`](https://github.com/bitcoin/bitcoin/pull/18807/commits/9e1cb1adf1800efe429e348650931f2669b0d2c0) Tree-SHA512: 0cd51c4ca368b9dce92d50d73ec6e9df278a259e609eef2858f24cb8595ad07acc3db781d9eb0c351715f18fca5a2b4526838981fdb34a522427e9dc868bdaa6
2020-05-30tests: Add fuzzing harness for functions in script/sign.hpracticalswift
2020-05-30tests: Add fuzzing harness for functions in script/sigcache.hpracticalswift
2020-05-30tests: Add fuzzing harness for functions in script/interpreter.hpracticalswift
2020-05-30tests: Add fuzzing harness for functions in script/descriptor.hpracticalswift
2020-05-30tests: Add fuzzing harness for functions in script/bitcoinconsensus.hpracticalswift
2020-05-30tests: Fill fuzzing coverage gaps for functions in script/script.h, ↵practicalswift
script/script_error.h and script/standard.h
2020-05-29rpc: Remove special case for unknown service flagsMarcoFalke
2020-05-29Limit scope of all global std::once_flagMarcoFalke
2020-05-29Merge #18926: test: Pass ArgsManager into getarg_testsMarcoFalke
f871f15c9d760b56a6a3b78b3941d3983d60810c scripted-diff: replace gArgs with argsman (glowang) 357f02bf2942f2944a04a1ceaa687f89d5da7d28 Create a local class inherited from BasicTestingSetup with a localized args manager and put it into the getarg_tests namespace (glowang) Pull request description: Replaced the global argsManager gArgs with a locally defined one in getarg_tests. This is to avoid confusion in arg settings between the test's ArgsManager and the #18804 ACKs for top commit: MarcoFalke: ACK f871f15c9d760b56a6a3b78b3941d3983d60810c ryanofsky: Code review ACK f871f15c9d760b56a6a3b78b3941d3983d60810c. Changes look good and thanks for updating. In future would recommend using clang-format-diff and following [coding style](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c) notes, because it's atypical to indent namespace content, or indent protected keywords or put spaces around ::. Also it's fragile to define test setup class in a namespace, but test setup methods outside of the namespace and inside the test fixture instead. Would be simpler to just define the testing setup completely before using it without a namespace like: https://github.com/bitcoin/bitcoin/blob/8ad5f1c376fe99eeccac2696ac0e18e662323a4d/src/test/rpc_tests.cpp#L23 and it would have been a slightly smaller change too. Tree-SHA512: 016594639396d60667fadec8ea80ef7af634fbb2014c704f02406fe3251c5362757c21f1763d8bdb94ca4a3026ab9dc786a92a9a934efc8cd807655d9deee779
2020-05-29Merge #19106: util: simplify the interface of serviceFlagToStr()Jonas Schnelli
189ae0c38b7d4927c5c73b94664e9542b2b06ed9 util: dedup code in callers of serviceFlagToStr() (Vasil Dimov) fbacad1880341ace31f669530c66d4e322d19235 util: simplify the interface of serviceFlagToStr() (Vasil Dimov) Pull request description: Don't take two redundant arguments in `serviceFlagToStr()`. Introduce `serviceFlagsToStr()` which takes a mask (with more than one bit set) and returns a vector of strings. As a side effect this fixes an issue introduced in https://github.com/bitcoin/bitcoin/pull/18165 due to which the GUI could print something like `UNKNOWN[1033] & UNKNOWN[1033] & UNKNOWN[2^10]` instead of `NETWORK & WITNESS`. ACKs for top commit: MarcoFalke: ACK 189ae0c38b7d4927c5c73b94664e9542b2b06ed9 jonasschnelli: Tested ACK 189ae0c38b7d4927c5c73b94664e9542b2b06ed9 Tree-SHA512: 000c490f16ebbba04458c62ca4ce743abffd344d375d95f5bbd5008742012032787655db2874b168df0270743266261dccf1693761906567502dcbac902bda50
2020-05-29util: dedup code in callers of serviceFlagToStr()Vasil Dimov
Introduce `serviceFlagsToStr()` which hides the internals of the bitmask and simplifies callers of `serviceFlagToStr()`.
2020-05-29util: simplify the interface of serviceFlagToStr()Vasil Dimov
Don't take two redundant arguments in `serviceFlagToStr()`. As a side effect this fixes an issue introduced in https://github.com/bitcoin/bitcoin/pull/18165 due to which the GUI could print something like `UNKNOWN[1033] & UNKNOWN[1033] & UNKNOWN[2^10]` instead of `NETWORK & WITNESS`.
2020-05-29tests: Add fuzzing helper functions ConsumeDataStream, ConsumeTxDestination ↵practicalswift
and ConsumeUInt160
2020-05-30net: improve code documentation for dns seed behaviourAnthony Towns
2020-05-29gui, refactor: Register Qt meta types in application constructorJoão Barbosa
2020-05-29Merge #18452: qt: Fix shutdown when waitfor* cmds are called from RPC consoleJonas Schnelli
da73f1513a637a9f347b64de66564d6cdb2541f8 qt: Fix shutdown when waitfor* cmds are called from RPC console (Hennadii Stepanov) Pull request description: On master (7eed413e72a236b6f1475a198f7063fd24929e23), if the GUI has been started with`-server=1`, `bitcoin-qt` hangs on shutdown during calling any of the `waitfor*` commands in the GUI RPC console. This PR suggests minimal changes to fix this bug. Fix #17495 ACKs for top commit: jonasschnelli: utACK da73f1513a637a9f347b64de66564d6cdb2541f8 Tree-SHA512: 469f5332945a5f2c57d19336cda5df79b123ccc494aea6d58a85eb1293be52708b2b9c5bb6bc2c402a90b7b4e9e8d7ab8fe84cf201cf7ce612c9290c57e43681
2020-05-29Merge #14988: wallet: Fix for confirmed column in csv export for payment to ↵Jonas Schnelli
self transactions 9760293ce632e09f0175368ebf0c8502ac9b10d4 wallet: Fix for exported confirmation field in payment to self transactions (Ben Carman) Pull request description: Closes #3455 ACKs for top commit: jonasschnelli: Tested ACK 9760293ce632e09f0175368ebf0c8502ac9b10d4 Tree-SHA512: 8207768771ad787f716b966c4aa7aeef2da8a602e32e3510e41c7b49ec5ec679a3835d248be5016d4b37764f9914846f7c41c11cf48cddb617cb7ef831318fd7
2020-05-29Merge #16939: p2p: Delay querying DNS seedsfanquake
96954d17948662672cababc940e453dff08e8cbb DNS seeds: don't query DNS while network is inactive (Anthony Towns) fa5894f7f581718ea28bb34b52fcd3b33ff3e644 DNS seeds: wait for 5m instead of 11s if 1000+ peers are known (Anthony Towns) Pull request description: Changes the logic for querying DNS seeds: after this PR, if there's less than 1000 entries in addrman, it will still usually query DNS seeds after 11s (unless the first few peers tried mostly succeed), but if there's more than 1000 entries it won't try DNS seeds until 5 minutes have passed without getting multiple outbound peers. (If there's 0 entries in addrman, it will still immediately query the DNS seeds). Additionally, delays querying DNS seeds while the p2p network is not active. Fixes #15434 ACKs for top commit: fanquake: ACK 96954d17948662672cababc940e453dff08e8cbb - Ran some tests of different scenarios. More documentation is being added in #19084. ariard: Tested ACK 96954d1, on Debian 9.1. Both MANY_PEERS/FEW_PEERS cases work. Sjors: tACK 96954d1 (rebased on master) on macOS 10.15.4. It found it useful to run with `-debug=addrman` and change `DNSSEEDS_DELAY_MANY_PEERS` to something lower to test the behaviour, as well as renaming `peers.dat` to test the peer threshold. naumenkogs: utACK 96954d17948662672cababc940e453dff08e8cbb Tree-SHA512: 73693db3da73bf8e76c3df9e9c82f0a7fb08049187356eac2575c4ffa455f76548dd1c86a11fc6beea8a3baf0ba020e047bebe927883c731383ec72442356005
2020-05-29Merge #18424: qt: Use parent-child relation to manage lifetime of ↵Jonas Schnelli
OptionsModel object 8e08d005989c6b5f7f05e0a1e0ba84f544a76d01 qt: Use parent-child relation to manage lifetime of OptionsModel object (Hennadii Stepanov) Pull request description: Both `BitcoinApplication` and `OptionsModel` classes are derived from the `QObject` class, therefore a parent-child relation could be established to manage the lifetime of an `OptionsModel` object: https://github.com/bitcoin/bitcoin/blob/5236b2e267a58870239673c7ec85e5df0cb8fc8e/src/qt/optionsmodel.cpp#L29-L30 This PR does not change behavior. ACKs for top commit: jonasschnelli: utACK 8e08d005989c6b5f7f05e0a1e0ba84f544a76d01 promag: ACK 8e08d005989c6b5f7f05e0a1e0ba84f544a76d01. Tree-SHA512: 0223dddf5ba28b0bfaefeda1b03b4ff95bf7e7d0c1e7b32368171e561813e22129f2a664f09279fa3b4fa63259b7680d55aa3fe66db9c7ae0039b7f529777ec3
2020-05-29Merge #18165: Consolidate service flag bit-to-name conversion to a shared ↵Jonas Schnelli
serviceFlagToStr function c31bc5bcfddf440e9a1713f7ba2ca2bf9cfa8e2e Consolidate service flag bit-to-name conversion to a shared serviceFlagToStr function (Luke Dashjr) cea91a1e40e12029140ebfba969ce3ef2965029c Bugfix: GUI: Use unsigned long long type to avoid implicit conversion of MSB check (Luke Dashjr) Pull request description: Side effect: this results in the RPC showing unknown service bits as "UNKNOWN[n]" like the GUI. Note that there is no common mask-to-`vector<string>` function because both GUI and RPC would need to iterate through it to convert to their desired target formats. ACKs for top commit: jonasschnelli: utACK ~~cea91a1e40e12029140ebfba969ce3ef2965029c~~ c31bc5bcfddf440e9a1713f7ba2ca2bf9cfa8e2e Tree-SHA512: 32c7ba8ac7ef2d4087f4f317447ae93a328ec9fb9ad81301df2fbaeeb21a3db7a503187a369552b05a9414251b7cf8e15bcde74c1ea2ef36591ea7ffb6721f60
2020-05-29Merge #17993: gui: Balance/TxStatus polling update based on last block hash.Jonas Schnelli
a06e845e826acaeb0db7cf02b2519c177e94dee5 BlockTip struct created and connected to notifyHeaderTip and notifyBlockTip signals. (furszy) 2f867203b0c7a4438ce484be4cfa2b29dbf1abf0 Added best block hash to the NotifyHeaderTip and NotifyBlockTip signals. (furszy) Pull request description: Rationale: The height based polling in the GUI is an issue on chain reorgs. Any new tip signal with the same height as the one that it's cached in the model was not triggering the GUI update (interpreting it as the same same block when it could receive a different one). Ending up with bad information presented in the GUI. This PR essentially changes the last cached height to be a last cached block hash. --- Old historical information of this PR. As the tip height is cached and updated via signaling in clientModel, there is no need to continue locking `cs_main` on every balance poll (`m_node.getNumBlocks()` method call). Extra topic: Would suggest to change the `cachedNumBlocks` field inside `walletModel` to a more understandable name, maybe `nLastBalanceUpdateHeight`. And finally, this will have the equal height reorg issue mentioned [here](https://github.com/bitcoin/bitcoin/pull/17905#issuecomment-577324304), whatever is presented to fix it, this should use the same flow too. **[Edit - 24/01/2020]** Have added #[17905](https://github.com/bitcoin/bitcoin/pull/17905#issuecomment-577324304) comment fix here too. ACKs for top commit: jonasschnelli: utACK a06e845e826acaeb0db7cf02b2519c177e94dee5 - it would be great to have QT unit tests (in this case for a reorg) that either automatically inspect the window content based on accessibility and tests for expected values or at least allow for quick manual re-testing (screenshots, automatically create UI situations). hebasto: re-ACK a06e845e826acaeb0db7cf02b2519c177e94dee5, suggested style changes implemented since the [previous](https://github.com/bitcoin/bitcoin/pull/17993#pullrequestreview-417249705) review. ryanofsky: Code review ACK a06e845e826acaeb0db7cf02b2519c177e94dee5. A lot of changes since the last review: rebase after sync_state introduction #18152 and tryGetBalances revert #18587, reverting getLastBlockTime change, fixing spacing and initializations and renaming some variables Tree-SHA512: 835e587a8296df9899cccd7b3e598a5970942b640e432e6a32de0b4eaea5b40f9271258f089ec033595311707b74a0f7187ecf8ed397c713e1153e2714072975
2020-05-29Merge #17968: qt: Ensure that ModalOverlay is resized properlyJonas Schnelli
4fc1df41d570ab631a8b47e4427a0b84305e37d1 qt: Track QEvent::Resize during animation (Hennadii Stepanov) Pull request description: In certain circumstances the `ModalOverlay` widget is not sized properly: - #17269 - #17967 - https://github.com/bitcoin/bitcoin/pull/17968#pullrequestreview-350753107 On master (f018d0c9cd7f408dac016b6bfc873670de713d27) this bug looks like this: ![DeepinScreenshot_bitcoin-qt_20200120193402](https://user-images.githubusercontent.com/32963518/72748165-298b2a80-3bbf-11ea-810d-2966f08e496a.png) With this PR the wallet frame looks ok: ![DeepinScreenshot_bitcoin-qt_20200120195241](https://user-images.githubusercontent.com/32963518/72748388-c64dc800-3bbf-11ea-8875-1ba1899b3513.png) Fix #17269 Fix #17967 ACKs for top commit: promag: Code review ACK 4fc1df41d570ab631a8b47e4427a0b84305e37d1. jonasschnelli: utACK 4fc1df41d570ab631a8b47e4427a0b84305e37d1 Tree-SHA512: b5d303fbc139c9383cd22edecba05e51b0d6115631aeb7d4474e973e7250a84019c11c0e41b5200e4d9ab10e17908774b45234317535857dc5314c3c28614ad4
2020-05-29Merge #17956: gui: Disable unavailable context menu items in transactions tabJonas Schnelli
2b18fd2242a589988fbb68205dae4afa0b8b3d34 Disable unavailable context menu items in transactions tab (Kristaps Kaupe) Pull request description: Fixes #9192. ACKs for top commit: jonatack: Re-ACK 2b18fd2242a5899 jonasschnelli: codereview utACK 2b18fd2242a589988fbb68205dae4afa0b8b3d34 Tree-SHA512: 4ea19c7b5976f1f0b1baecccb3077cf82f078af7257f92162686bcce2188efe49511a5f557853bc5fe0b10616708957d61c006944babbe60b8105e78751e865f
2020-05-29Merge #17918: qt: Hide non PKHash-Addresses in signing address bookJonas Schnelli
c4ea501e96363e937200bc97b8e2d78162bdb699 qt: Hide non PKHash-Addresses in signing address book (Emil Engler) Pull request description: [Video Demo](https://www.youtube.com/watch?v=T-Rp2pFRmzY) This PR hides all non PKHash addresses in the signing GUI in the Address Book when it is opened through the signing dialog, as non PKHash addresses are useless there. ACKs for top commit: jonasschnelli: Code Review ACK c4ea501e96363e937200bc97b8e2d78162bdb699 Tree-SHA512: e321d45e15534b2d68da5a1297b1c7551cdd784f03203f54c9385c2ce0bb2b7316c09f9e8c3eb41bfa1e7207ecc94c8ed08f012e2d6c117b803996ade26feb2f
2020-05-29Merge #17908: qt: Remove QFont warnings with QT_QPA_PLATFORM=minimalJonas Schnelli
1122817c194ed49abf896e68604e725c3b5c8569 qt: Remove QFont warnings with QPA=minimal (Hennadii Stepanov) Pull request description: This PR removes massive warnings like: ``` QWARN : ... QFont::setPointSizeF: Point size <= 0 (...), must be greater than 0 ``` from `test_bitcoin-qt` output. On master (e258ce792a4849927a6db51786732d71cbbb65fc): ``` $ ./src/qt/test/test_bitcoin-qt | grep QFont | wc -l ~BitcoinApplication : Stopping thread ~BitcoinApplication : Stopped thread 57 ``` With this PR: ``` $ ./src/qt/test/test_bitcoin-qt | grep QFont | wc -l ~BitcoinApplication : Stopping thread ~BitcoinApplication : Stopped thread 0 ``` ACKs for top commit: promag: Code review ACK 1122817c194ed49abf896e68604e725c3b5c8569. jonasschnelli: utACK 1122817c194ed49abf896e68604e725c3b5c8569 Tree-SHA512: 32fa72a5d3db1d4c73a2a324aa9cad807ee46f23fc5319f7d71202987dc73ea7c90082904489b323a432e1afaebd9976b7dd0374236a16153162aa75fe368723
2020-05-29Merge #17597: qt: Fix height of QR-less ReceiveRequestDialogJonas Schnelli
73529f0859c060087dd41e9e176fd4198d0f385f qt: Rename slot to updateDisplayUnit() (Hennadii Stepanov) 68288ef0c15522799d4bf6239d2ae1e8086b5abf qt: Overhaul ReceiveRequestDialog (Hennadii Stepanov) Pull request description: If master (89a1f7a250ef70ff2d65701564f1e24bb9280d90) is compiled without QR support, the "Request payment to..." dialog looks ugly: ![Screenshot from 2019-11-25 19-58-59](https://user-images.githubusercontent.com/32963518/69566647-3d9c1c80-0fc0-11ea-8ff6-183cea9372c5.png) With this PR: ![Screenshot from 2019-12-26 00-42-46](https://user-images.githubusercontent.com/32963518/71451226-221c6100-277a-11ea-94ae-c19a5c6256f7.png) ![Screenshot from 2019-12-26 00-44-34](https://user-images.githubusercontent.com/32963518/71451228-29436f00-277a-11ea-8ac5-1bafe6d73b5c.png) ![Screenshot from 2019-12-26 00-48-51](https://user-images.githubusercontent.com/32963518/71451230-2e082300-277a-11ea-8c4c-726ca7b776e7.png) Other minor changes: - "URI" abbreviation is not translated now - wallet name is shown if available ACKs for top commit: jonasschnelli: Tested ACK 73529f0859c060087dd41e9e176fd4198d0f385f Tree-SHA512: 45f9a41d3c72978d78eb2e8ca98e274b8be5abf5352fd3e1f4f49514ce744994545fb3012e80600ded936ae41c6be4d4825a0c5f7bcb3db671d69e0299f0f65b
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-14 17:53:40 +0000