Nov-19 #1
Merged
Nov-19 #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Because the call to MaybePunishNode() in PeerLogicValidation::BlockChecked() only previously happened if the REJECT code was > 0 and < REJECT_INTERNAL, then there are cases were MaybePunishNode() can get called where it wasn't previously: - when AcceptBlockHeader() fails with CACHED_INVALID. - when AcceptBlockHeader() fails with BLOCK_MISSING_PREV. Note that BlockChecked() cannot fail with an 'internal' reject code. The only internal reject code was REJECT_HIGHFEE, which was only set in ATMP. This change restores the behaviour pre-commit 5d08c9c which did punish nodes that sent us CACHED_INVALID and BLOCK_MISSING_PREV blocks.
Remove the BIP61 REJECT code from error messages and logs when a transaction is rejected. BIP61 support was removed from Bitcoin Core in fa25f43. The REJECT codes will be removed from the codebase entirely in the following commit.
We no longer send BIP 61 REJECT messages, so there's no need to set a REJECT code in the CValidationState object.
…stand it By blanket passing --disable-dependency-tracking to all depends packages we end up with some warnings like: configure: WARNING: unrecognized options: --disable-dependency-tracking So instead, only pass it to packages that understand it. Related to #16354.
The wallet should not be able to directly access global configuration from the node. Remove access of "-limitancestorcount" and "-limitdescendantcount".
Prior to this PR, the wallet would not allow the `-rescan` option at startup if pruning was enabled. This is unnecessarily restrictive. It should be possible to rescan if pruning is enabled, as long as no blocks have actually been pruned yet. Remove the pruning check from WalletInit::ParameterInteraction(). If any blocks have been pruned, that will be caught in CreateWalletFromFile().
The test case tests a chain reorganization, however the two chains were generated in the same manner and thus produced the same blocks.
Review hint: git show -w Co-authored-by: MarcoFalke <falke.marco@gmail.com> Co-authored-by: keneanung <keneanung@googlemail.com> Co-authored-by: Vadim Peretokin <vperetokin@gmail.com>
Belts and suspenders: make sure outgoing log messages don't contain
potentially suspicious characters, such as terminal control codes.
This escapes control characters except newline ('\n') in C syntax.
It escapes instead of removes them to still allow for troubleshooting
issues where they accidentally end up in strings.
This removes the code introduced in [#4399](#4399) that attempts to add additional entroy to the OpenSSL PRNG using Windows messages. Note that this is specific to bitcoin-qt running on Windows. ``` RAND_event() collects the entropy from Windows events such as mouse movements and other user interaction. It should be called with the iMsg, wParam and lParam arguments of all messages sent to the window procedure. It will estimate the entropy contained in the event message (if any), and add it to the PRNG. The program can then process the messages as usual. ``` Besides BIP70, this is the last place we are directly using OpenSSL in the GUI code. All other OpenSSL usage is in random.cpp. Note that we are still also doing Windows specific entropy gathering in multiple other places. Such as [RandAddSeedPerfmon](https://github.com/bitcoin/bitcoin/blob/master/src/random.cpp#L268) and [RAND_screen()](https://github.com/bitcoin/bitcoin/blob/master/src/random.cpp#L600). Also note that if RAND_event returns 0 (PRNG has NOT been seeded with enough data), we're just logging a message and continuing on, which seems less than ideal.
66b2984 change wallet pointers to references in feebumper (Adam Jonas) 9be6666 typo and unneccessary parentheses (Adam Jonas) Pull request description: Picking up some of the suggestions in the comments of #16727 including: #16727 (comment) #16727 (comment) #16727 (comment) ACKs for top commit: promag: Code review ACK 66b2984. MarcoFalke: ACK 66b2984 (looked at the diff on GitHub) fjahr: ACK 66b2984 reviewed code Tree-SHA512: d118f7689970fe39d9f5318dc818f13283cce9194370b3ce4758f298172e4681ae119ddc809f5c0b7602677137ac0d38147b915422ff616531a76a570b766fa2
b96ed03 [wallet] Remove pruning check for -rescan option (John Newbery) eea462d [wallet] Remove package limit config access from wallet (John Newbery) Pull request description: Removes wallet access to `-limitancestorcount`, `-limitdescendantcount` and `-prune`: - `-limitancestorcount` and `-limitdescendantcount` are now accessed with a method `getPackageLimits` in the `Chain` interface. - `-prune` is not required. It was only used in wallet component initiation to prevent running `-rescan` when pruning was enabled. This check is not required. Partially addresses #17137. ACKs for top commit: MarcoFalke: Tested ACK b96ed03 ryanofsky: Code review ACK b96ed03 promag: Code review ACK b96ed03. ariard: ACK b96ed03, check there isn't left anymore wallet access to node arguments. Tree-SHA512: 90c8e3e083acbd37724f1bccf63dab642cf9ae95cc5e684872a67443ae048b4fdbf57b52ea47c5a1da6489fd277278fe2d9bbe95e17f3d4965a1a0fbdeb815bf
8019b6b gui: Make RPCConsole::TabTypes an enum class (João Barbosa) Pull request description: This change makes the compiler emit a warning/error if a missing enum value is not handled. See also #17134. ACKs for top commit: MarcoFalke: unsigned ACK 8019b6b hebasto: re-ACK 8019b6b fanquake: ACK 8019b6b Tree-SHA512: 329161097f4d079f48d5fb33bf3a07e314fbb2ac325cafb08bafa9e76229ecff0f9010fe3c1c15ccd02d4539b5c93839c846b42bfeaffa897a917cea599bf811
…etween address types
3b3b931 nsis: Write to correct filename in first place (Carl Dong) Pull request description: Per MarcoFalke's suggestion here #17029 (comment) ACKs for top commit: MarcoFalke: unsigned ACK 3b3b931, makes sense to name it that way because it will raise the "unsinged" error in Windows Tree-SHA512: da72aae438505e162d0b3cd27d873b7ad8176178bb459a738e61b6e2ad0fa739d905b3109fab641bb1a3950fe59ad526c5568d12cf48a305166cdb7db6686543
…dding ECCVerifyHandle dependency 9cae3d5 tests: Add fuzzer initialization (hold ECCVerifyHandle) (practicalswift) Pull request description: The fuzzers `eval_script` and `script_flags` require holding `ECCVerifyHandle`. This is a follow-up to #17235 which accidentally broke those two fuzzers. Sorry about the temporary breakage my fuzzing friends: it took a while to fuzz before reaching these code paths. That's why this wasn't immediately caught. Sorry. Top commit has no ACKs. Tree-SHA512: 67ebb155ba90894c07eac630e33f2f985c97bdf96dc751f312633414abeccdca20315d7d8f2ec4ee3ac810b666a1e44afb4ea8bc28165151cd51b623f816cac2
git-subtree-check fails if the directory is given with a trailing slash, eg: ``` > test/lint/git-subtree-check.sh src/univalue/ ERROR: src/univalue/ is not a subtree ``` Shell autocompletes will add the trailing slash when autofilling the path name, which will therefore cause the script to fail. Just ignore any trailing slash.
Co-authored-by: Wladimir J. van der Laan <laanwj@protonmail.com>
This helps avoid accidentally truncating the debug.log while manually debugging.
53fe0b7 Fix missing strFailReason in CreateTransaction (Russell Yanofsky) 4b28a05 Fix misplaced AssertLockHeld (Russell Yanofsky) 2632b1f doc: Clarify WalletStorage / Wallet relation (Russell Yanofsky) 628d11b Add back mistakenly removed AssertLockHeld (Russell Yanofsky) 52cf68f Refactor: Add GetLegacyScriptPubKeyMan helper (Russell Yanofsky) Pull request description: This PR implements suggested code cleanups from #17260 review comments ACKs for top commit: Sjors: ACK 53fe0b7 achow101: ACK 53fe0b7 MarcoFalke: ACK 53fe0b7 Tree-SHA512: a577b96cb21a9aa7185d7d900e4db0665c302adcd12097957b9d8e838a8548c7de8f901bcb83e7c46d231b841221345c9264f5e29ed069f3d9236896430f959b
60582d6 [linter] Strip trailing / in path for git-subtree-check (John Newbery) Pull request description: git-subtree-check fails if the directory is given with a trailing slash, eg: ``` > test/lint/git-subtree-check.sh src/univalue/ ERROR: src/univalue/ is not a subtree ``` Shell autocompletes will add the trailing slash when autofilling the path name, which will therefore cause the script to fail. Just ignore any trailing slash. ACKs for top commit: laanwj: ACK 60582d6 dongcarl: ACK 60582d6 fanquake: ACK 60582d6 - tested before and after. Tree-SHA512: 5a91979b60e1d4b1310fd02a0ccc5465dbff57d9c94bba81e4758442a627cfa32217ab8f973990a17b5d961ecae61fb56b56ccf10f87e61dd03e88a1e0b8f99d
c5377ff [qa] Add shrinkdebugfile=0 to regtest bitcoin.conf (Suhas Daftuar) Pull request description: This helps avoid accidentally truncating the debug.log while manually debugging. ACKs for top commit: MarcoFalke: ACK c5377ff dongcarl: ACK c5377ff Tree-SHA512: a05d6a7c494ee2c300fa1a9dbaa48b7fed63a44b1fa823198cbf401cff1c4aa8e44eb431fa635b27a1987d0eb1a5f8e2712514dcea7c5dd05240a445c8bd505d
8a0ca5e log: Fix log message for -par=1 (Hennadii Stepanov) Pull request description: Fix #17139 ACKs for top commit: jnewbery: Tested ACK 8a0ca5e laanwj: ACK 8a0ca5e Tree-SHA512: 09f5416c00cd3e4f85cd9040267dc825d5c5623f8903e9d2373013686dce7f6b3ba573648787adc1d58e6f1d5012e26c78700d585273d4b508cb25312b3fa06b
test/functional/rpc_fundrawtransaction.py is fairly long to run and has no logging, so it can appear to be stalled. This commit adds info logging at each test to provide feedback on the test run.
a8b8286 Fix incorrect help-debug for -checkpoints (Antoine Riard) Pull request description: ACKs for top commit: jnewbery: ACK a8b8286 for improving the `-prune` help text. MarcoFalke: ACK a8b8286 Tree-SHA512: 973fa97436be09a9939386dc00023420a7296a9e268356bf26aa06468f9f0d2c822205a4f1ce8f44a0562aa64ad90a43dec5697af656ef28ba6829e4e4360e94
Doc changes only to test/functional/rpc_fundrawtransaction.py: - remove ascii art or convert to a docstring when sufficiently different from the logging - touch up other comments while here
fa0b3da Squashed 'src/univalue/' changes from 7890db9..5a58a46 (MarcoFalke) Pull request description: Only change is a performance improvement. See bitcoin-core/univalue-subtree#21 (comment) and bitcoin-core/univalue-subtree#15 (comment) ACKs for top commit: jnewbery: ACK fa439e8 laanwj: ACK fa439e8 Tree-SHA512: 35ea8f76ea4806182949c8eb5a8b652d1aaeec03ee023838e7cb29abcb81c61d59b38f15708564a78574722df57d13f61ef17d0f670a4056819705ef892321e0
8734c85 Replace the LogPrint function with a macro (Jeffrey Czyz) Pull request description: Calling `LogPrint` with a category that is not enabled results in evaluating the remaining function arguments, which may be arbitrarily complex (and possibly expensive) expressions. Defining `LogPrint` as a macro prevents this unnecessary expression evaluation. This is a partial revert of #14209. The decision to revert is discussed in #16688, which adds verbose logging for validation event notification. ACKs for top commit: jnewbery: ACK 8734c85 Tree-SHA512: 19e995eaef0ff008a9f8c1fd6f3882f1fbf6794dd7e2dcf5c68056be787eee198d2956037d4ffba2b01e7658b47eba276cd7132feede78832373b3304203961e
ff22751 test: rm ascii art in rpc_fundrawtransaction (Jon Atack) 94fcc08 test: add rpc_fundrawtransaction logging (Jon Atack) Pull request description: `test/functional/rpc_fundrawtransaction.py` is fairly slow to run and has no logging, so it can appear to be stalled. This commit adds info logging at each test to provide feedback on the test run. ACKs for top commit: instagibbs: utACK ff22751 jnewbery: tACK ff22751 Tree-SHA512: f4fabad8ef51c29981351bb4e66fb0c0e0517418a4a15892ef804df11d16b2d2ae1a1abc958d2b121819850278de90a2003b0edb8d7098d00360b89fa76e9062
58d0393 build: update retry to current version (randymcmillann) Pull request description: This commit eliminates spelling and white space errors that are flagged in the linting process ACKs for top commit: practicalswift: ACK 58d0393 Tree-SHA512: c241ed0775026c890dd29d1f7231c5540e9c9285867a99844605753a3007d08f0bd4f7a59f078e4c65b741301ff7fa8a871e2e3c64b9a9fe47b3ea74c4228498
…ansform keys between address types a6f6f77 QA: Add wallet_implicitsegwit to test the ability to transform keys between address types (Luke Dashjr) Pull request description: This makes sure the wallet recognises payments to keys via address types they weren't created with. While we don't *want* this behaviour, it might make sense to explicitly test that it works until we remove it. ACKs for top commit: adamjonas: utACK a6f6f77 Tree-SHA512: b208405729277e9ce06eb772b45e8d1683c4dc5703754448b8f19a590b37522abd7bb46d4dbd41513b3d46d7f9e8769ce4f15fa4114be600f31a1ebbc1157840
Also adds missing Boost packages. Installing only the currently listed packages was not sufficient to complete a build.
…B::Read() 59853c3 test: Do not instantiate CAddrDB for static call (Hennadii Stepanov) Pull request description: Ref: https://github.com/bitcoin/bitcoin/blob/6a7c40bee403cadedeecd4b1c6528575522094eb/src/addrdb.h#L84-L94 ACKs for top commit: MarcoFalke: unsigned ACK 59853c3 naumenkogs: ACK 59853c3 Tree-SHA512: 06b2e8e4f2b4a4f20454a4828a786878fc2bd441b0ee53f1128a7a0b6ad54dd3ca7598cee000ec60e5e4ef02570a09c01974d2d8260bd11fa9baf16b3ff9ba08
162d003 doc: compiling with Visual Studio is now supported on Windows (fanquake) b1f1fb5 doc: update MSVC instructions to remove Qt configuration (fanquake) Pull request description: Follow up from #17165. Flips `-openssl-linked` to `-no-openssl`. Also adds some missing packages to the vcpkg install instructions. ACKs for top commit: sipsorcery: tACK 162d003. Tree-SHA512: 40577a3759a30170a14fd27e3eeac5a78a7852ae88daacecf584ad46c685708c167153d39357aa77a4f65bfd5a349f7589f20aa16fdf3d2895b4076b381e2c9c
5710dad test: fix script_p2sh_tests OP_PUSHBACK2/4 missing (kodslav) Pull request description: Cleans up #15140 which fixes commit 6b25f29 where opcodes were lost in translation. ACKs for top commit: laanwj: code review ACK 5710dad Tree-SHA512: 3f7fbcaf0dd199626d9ec9fdf3c5b5c5c2a91c4cfe81fae5b1d5662a48e52cf4bd27c94f8f42ebdfe7a076c5d600ada5661a6902b03eb5dc3dc953f4524345ac
b0c774b Add new mempool benchmarks for a complex pool (Jeremy Rubin) Pull request description: This PR is related to #17268. It adds a mempool stress test which makes a really big complicated tx graph, and then, similar to mempool_eviction test, trims the size. The test setup is to make 100 original transactions with Rand(10)+2 outputs each. Then, 800 times: we create a new transaction with Rand(10) + 1 parents that are randomly sampled from all existing transactions (with unspent outputs). From each such parent, we then select Rand(remaining outputs) +1 50% of the time, or 1 outputs 50% of the time. Then, we trim the size to 3/4. Then we trim it to just a single transaction. This creates, hopefully, a big bundle of transactions with lots of complex structure, that should really put a strain on the mempool graph algorithms. This ends up testing both the descendant and ancestor tracking. I don't love that the test is "unstable". That is, in order to compare this test to another, you really can't modify any of the internal state because it will have a different order of invocations of the deterministic randomness. However, it certainly suffices for comparing branches. Top commit has no ACKs. Tree-SHA512: cabe96b849b9885878e20eec558915e921d49e6ed1e4b011b22ca191b4c99aa28930a8b963784c9adf78cc8b034a655513f7a0da865e280a1214ae15ebb1d574
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updating from base