backport: trivial backports Feb 26 2022 #4709
Merged
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
PastaPastaPasta
force-pushed
the
develop-trivial-2022-02-26
branch
4 times, most recently
from
886fc28
to
6d468a1
Compare
|
This pull request has conflicts, please rebase. |
PastaPastaPasta
force-pushed
the
develop-trivial-2022-02-26
branch
from
6d468a1
to
af76d42
Compare
UdjinM6
requested changes
Mar 6, 2022
2dbfb37 Fix Char as Bool in interfaces (Jeremy Rubin) Pull request description: In a few places in src/wallet/wallet.h, we use a char when semantically we want a bool. This is kind of an issue because it means we can unserialize the same transaction with different fFromMe flags (as differing chars) and evaluate the following section in wallet/wallet.cpp ```c++ if (wtxIn.fFromMe && wtxIn.fFromMe != wtx.fFromMe) { wtx.fFromMe = wtxIn.fFromMe; fUpdated = true; } ``` incorrectly (triggering an fUpdated where both fFromMe values represent true, via different chars). I don't think this is a vulnerability, but it's just a little messy and unsemantic, and could lead to issues with stored wtxIns not being findable in a map by their hash. The serialize/unserialize code for bool internally uses a char, so it should be safe to make this substitution. NOTE: Technically, this is a behavior change -- I haven't checked too closely that nowhere is depending on storing information in this char. Theoretically, this could break something because after this change a tx unserialized with such a char would preserve it's value, but now it is converted to a ~true~ canonical bool. ACKs for top commit: achow101: Code review ACK 2dbfb37 meshcollider: Code review ACK 2dbfb37 Tree-SHA512: 8c0dc9cf672aa2276c694facbf50febe7456eaa8bf2bd2504f81a61052264b8b30cdb5326e1936893adc3d33504667aee3c7e207a194c71d87b3e7b5fe199c9d
…by re-adding 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 bitcoin#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
f44abe4 refactor: Remove addrdb.h dependency from node.h (Hennadii Stepanov) Pull request description: `node.h` includes `addrdb.h` just for the sake of `banmap_t` type. This PR makes dependencies simpler and explicit. ~Also needless `typedef` has been removed from `enum BanReason`.~ ACKs for top commit: laanwj: ACK f44abe4 practicalswift: ACK f44abe4 Tree-SHA512: 33a1be20e5c629daf4a61ebbf93ea6494b9256887cebd4974de4782f6d324404b6cc84909533d9502b2cc19902083f1f9307d4fb7231e67db5b412b842d13072
286f197 Add util_ArgParsing test (Russell Yanofsky) Pull request description: ArgsManager test coverage for parsing of integer and boolean values is currently very poor and doesn't give us a way of knowing whether changes to ArgsManager may unintentionally break backwards compatibility, so this adds a new test to catch regressions. ACKs for top commit: promag: ACK 286f197, more surprising results 😱 laanwj: ACK 286f197 Tree-SHA512: 9e1db3ef87e55abbc280af60c088f35765a1f9e2ec20507ad0c1992027b875490016868dcb8cc287e6df279dd0e00f10550901af3de3d36287867249e0bd8207
…ndonconflict dddd09e test: Wait until mempool is loaded in wallet_abandonconflict (MarcoFalke) Pull request description: This might or might not fix intermittent issues such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/28724018#L4091 I believe the mempool was not loaded fully after the restart, in which case it was not dumped either on the next restart. Thus, the previous mempool was attempted to be loaded a second time, which succeeded and contained the txs. ACKs for top commit: laanwj: ACK dddd09e Tree-SHA512: ab7061f946b5e5388f825dddceadb125f5197b24af3a7fcf1e700235d106a323419a56bfb4d84a2e27442e0de63e540c623b704343d83a98deaab3c02fcbdcbe
… consuming only integrals 597d10c tests: Add fuzzing harness for various functions consuming only integrals (practicalswift) 575383b tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus (practicalswift) Pull request description: Add fuzzing harness for various functions consuming only integrals. **Testing this PR** Run: ``` $ CC=clang CXX=clang++ ./configure --enable-fuzz \ --with-sanitizers=address,fuzzer,undefined $ make $ src/test/fuzz/integer ``` Top commit has no ACKs. Tree-SHA512: f0ccbd63671636f8e661385b682e16ad287fef8f92e7f91327ee2093afc36fcd424e1646fe90279388e28a760bcc795766eb80cf6375e0f873efff37fc7e2393
…nd other CBlock related functions 893aa20 tests: Add fuzzing harness for CheckBlock(...) and other CBlock related functions (practicalswift) ec8dcb0 tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus (practicalswift) Pull request description: Add fuzzing harness for `CheckBlock(...)` and other `CBlock` related functions. **Testing this PR** Run: ``` $ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined $ make $ src/test/fuzz/block … # And to to quickly verify that the relevant code regions are triggered, that the # fuzzing throughput seems reasonable, etc. $ contrib/devtools/test_fuzzing_harnesses.sh '^block$' ``` `test_fuzzing_harnesses.sh` can be found in PR bitcoin#17000. Top commit has no ACKs. Tree-SHA512: 275abd46d8ac970b28d8176f59124988b1e07c070173e001acd55995b830333417f301c309199fc589da08a6ac4c03aa74650d5e1638f6e3023dfbd3c9f6921d
…rs.md c7ec9a1 Add missing supported rpcs to doc/descriptors.md (Andrew Toth) Pull request description: Improve descriptor docs by adding missing rpcs. ACKs for top commit: fanquake: ACK c7ec9a1 - I think this has been bikeshed enough. jonatack: ACK c7ec9a1 Tree-SHA512: 783219928ed7edc904b507bb30e2eefd8ca9f11225e1460fedecd755f9511055adcc52cc49f66ba840e121883e40753061db76a243ee6e0091daf1fc396ae59a
…ing to existing fuzzing harness 7e9c711 compressor: Make the domain of CompressAmount(...) explicit (practicalswift) 4a7fd7a tests: Add amount compression/decompression fuzzing to existing fuzzing harness: test compression round-trip (practicalswift) Pull request description: Small fuzzing improvement: Add amount compression/decompression fuzzing to existing fuzzing harness: test compression round-trip (`DecompressAmount(CompressAmount(…))`). Make the domain of `CompressAmount(…)` explicit. Amount compression primer: ``` Compact serialization for amounts Special serializer/deserializer for amount values. It is optimized for values which have few non-zero digits in decimal representation. Most amounts currently in the txout set take only 1 or 2 bytes to represent. ``` **How to test this PR** ``` $ make distclean $ ./autogen.sh $ CC=clang CXX=clang++ ./configure --enable-fuzz \ --with-sanitizers=address,fuzzer,undefined $ make $ src/test/fuzz/integer … ``` Top commit has no ACKs. Tree-SHA512: 0f7c05b97012ccd5cd05a96c209e6b4d7d2fa73138bac9615cf531baa3f614f9003e29a198015bcc083af9f5bdc752bb52615b82c5df3c519b1a064bd4fc6664
…ng garbage value on failure a652ba6 rpc/wallet: initialize nFeeRequired to avoid using garbage value on failure (Karl-Johan Alm) Pull request description: Initialize the `nFeeRequired` variable to avoid using an uninitialized value for errors happening before it is set to 0. Note: this originally fixed `nFeeRet` in `wallet.cpp`. ACKs for top commit: promag: ACK a652ba6. Sjors: utACK a652ba6 practicalswift: ACK a652ba6 -- patch looks correct meshcollider: utACK a652ba6 Tree-SHA512: 0d12f1ffd0851ed5ce6d109d2c87f55e8b1d57da297e684feeabb57229200c4078f029c55ca5aa5712bd18e26dda3ce538443dfe68a7a6d504428068f81fded0
…h-Makefile 1f97b69 build: remove double LIBBITCOIN_SERVER from bench-Makefile (Harris) Pull request description: This PR removes the redundant **LIBBITCOIN_SERVER** linking from bench's Makefile. This PR is similar to bitcoin#17910 Originally, this PR was part of bitcoin#18377, which later got replaced by a better one bitcoin#18397 written by **hebasto**. ACKs for top commit: Empact: Code Review ACK bitcoin@1f97b69 theStack: ACK bitcoin@1f97b69 hebasto: ACK 1f97b69 Tree-SHA512: e43035262361d4458a7dcfc920445540f19301387814cde1be0539c936fc20da0dcbe49e5ea25385e6d36d9639515b7a4171228223da568d93427e9c32810945
PastaPastaPasta
force-pushed
the
develop-trivial-2022-02-26
branch
from
af76d42
to
1b057e0
Compare
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.
(this will likely fail ci, have only built locally)