Age | Commit message (Collapse) | Author |
|
8b2f471a1bff753cc4df29805ef38c3623f64f6e qa: Fix double-negative arg test (Hennadii Stepanov)
Pull request description:
Commit 67518f7cc61bf59ddfa0fd7c8dbbdec3653b9556 tests do not catch that a pointer is returned instead of a value.
This PR makes test to not accept trailing characters after 0.
From [IRC](http://www.erisian.com.au/bitcoin-core-dev/log-2020-01-07.html#l-358):
> \<hebasto\> ryanofsky: hmm, why test/functional/feature_config_args.py passed on 67518f7cc61bf59ddfa0fd7c8dbbdec3653b9556 ?
> \<hebasto\> I see now: test is broken.
> \<ryanofsky\> test should be unaffected by that change, do you see a break somewhere?
> \<hebasto\> yes: "-connect=0x7fff50369968" != "-connect=0"
> ...
> \<ryanofsky\> Oh I see how that would happen, it should not be a problem in the current PR.
> \<hebasto\> going to submit a pr to fix test
> \<ryanofsky\> in the commit you mentioned, value is a pointer to a string, and it was printing the pointer address instead of the string on: LogPrintf("Warning: parsed potentially confusing double-negative -%s=%s\n", key, value);
> \<hebasto\> correct
> \<ryanofsky\> oh I see, test could be fixed to more robust and not accept trailing characters after 0
ACKs for top commit:
ryanofsky:
Code review ACK 8b2f471a1bff753cc4df29805ef38c3623f64f6e. I don't know how you found this but it's a nice catch! This change should make the test more reliable.
Tree-SHA512: 454b3d4415771d353a2da766f6ae6e0bfae7bdf485aaa7bfdd323595282356eeaf3f40e556b39f753bc35f578cbe9684368887eef2d63c5d7f0d7d9fa971697a
|
|
f9abf4ab6d3d3e4d4b7e90723020b5422a141a6f Add logging for CValidationInterface events (Jeffrey Czyz)
6edebacb2191373e76d79a4972d6192300976096 Refactor FormatStateMessage for clarity (Jeffrey Czyz)
72f3227c83810936e7a334304e5fd7c6dab8e91b Format CValidationState properly in all cases (Jeffrey Czyz)
428ac70095253225f64462ee15c595644747f376 Add VALIDATION to BCLog::LogFlags (Jeffrey Czyz)
Pull request description:
Add logging of `CValidationInterface` callbacks using a new `VALIDATIONINTERFACE` log flag (see #12994). A separate flag is desirable as the logging can be noisy and thus may need to be disabled without affecting other logging.
This could help debug issues where there may be race conditions at play, such as #12978.
ACKs for top commit:
jnewbery:
ACK f9abf4ab6d3d3e4d4b7e90723020b5422a141a6f
hebasto:
ACK f9abf4ab6d3d3e4d4b7e90723020b5422a141a6f
ariard:
ACK f9abf4a, only changes since 0cadb12 are replacing log indication `VALIDATIONINTERFACE` by `VALIDATION` and avoiding a forward declaration with a new include
ryanofsky:
Code review ACK f9abf4ab6d3d3e4d4b7e90723020b5422a141a6f. Just suggested changes since last review (thanks!)
Tree-SHA512: 3e0f6e2c8951cf46fbad3ff440971d95d526df2a52a2e4d6452a82785c63d53accfdabae66b0b30e2fe0b00737f8d5cb717edbad1460b63acb11a72c8f5d4236
|
|
|
|
|
|
- https://github.com/bitcoin/bitcoin/pull/17578#discussion_r363975411
- https://github.com/bitcoin/bitcoin/pull/17578#discussion_r363969721
- https://github.com/bitcoin/bitcoin/pull/17578#discussion_r362703553
|
|
3e730bf90aaf53c41ff3a778f6aac15d163d1c0c zmq: Fix due to invalid argument and multiple notifiers (João Barbosa)
Pull request description:
ZMQ initialization is interrupted if any notifier fails, and in that case all notifiers are destroyed. The notifier shutdown assumes that the initialization had occurred. This is not valid when there are multiple notifiers and any except the last fails to initialize.
Can be tested by running test/functional/interface_zmq.py from this branch with bitcoind from master.
Closes #17185.
ACKs for top commit:
laanwj:
Code review ACK 3e730bf90aaf53c41ff3a778f6aac15d163d1c0c, thanks for adding a test
Tree-SHA512: 5da710e97dcbaa94896d019e75162d470f6d381ee07c60e5b3e9db93d11e8f7ca9bf2c509efa4486199e88c96c3e720cc96b4e35b62725d4c7db8e8e9bf6e09d
|
|
fa37e0a68bea65979f9f8f2e5258fe608acf2bdf test: Show debug log on unit test failure (MarcoFalke)
Pull request description:
Often, it is hard to debug unit test failures without the debug log. Especially when the failure happens remotely (e.g. on a ci system).
Fix that by printing the log on failure.
ACKs for top commit:
jamesob:
ACK fa37e0a68bea65979f9f8f2e5258fe608acf2bdf ([`jamesob/ackr/16975.1.MarcoFalke.test_show_debug_log_on_u`](https://github.com/jamesob/bitcoin/tree/ackr/16975.1.MarcoFalke.test_show_debug_log_on_u))
Tree-SHA512: 2ca4150c4ae3d4ad47e03b5e5e70da2baffec928ddef1fdf53a3ebc061f14aee249205387cb1b12ef6d4eb55711ef0080c0b41d9d18000b5da124ca80299793b
|
|
|
|
8925df86c4df16b1070343fef8e4d238f3cc3bd1 doc: update release notes (Jon Atack)
8bb405bbadf11391ccba7b334b4cfe66dc85b390 test: getaddressinfo labels purpose deprecation test (Jon Atack)
60aba1f2f11529add115d963d05599130288ae28 rpc: simplify getaddressinfo labels, deprecate previous behavior (Jon Atack)
7851f14ccf2bcd1e9b2ad48e5e08881be06d9d21 rpc: incorporate review feedback from PR 17283 (Jon Atack)
Pull request description:
This PR builds on #17283 (now merged) and is followed by #17585.
It modifies the value returned by rpc getaddressinfo `labels` to an array of label name strings and deprecates the previous behavior of returning an array of JSON hash structures containing label `name` and address `purpose` key/value pairs.
before
```
"labels": [
{
"name": "DOUBLE SPEND",
"purpose": "receive"
}
```
after
```
"labels": [
"DOUBLE SPEND"
]
```
The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=labelspurpose`.
For context, see:
- https://github.com/bitcoin/bitcoin/pull/17283#issuecomment-554458001
- http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-13.html#l-425 (lines 425-427)
- http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622
Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=labelspurpose` flag while verifying the rpc getaddressinfo help text and `labels` output.
Next steps: deprecate the rpc getaddressinfo `label` field (EDIT: done in #17585) and add support for multiple labels per address. This PR will unblock those.
ACKs for top commit:
jnewbery:
reACK 8925df8
promag:
Code review ACK 8925df86c4df16b1070343fef8e4d238f3cc3bd1.
meshcollider:
Code review ACK 8925df86c4df16b1070343fef8e4d238f3cc3bd1
Tree-SHA512: c2b717209996da32b6484de7bb8800e7048410f9ce6afdb3e02a6866bd4a8f2c730f905fca27b10b877b91cf407f546e69e8c4feb9cd934325a6c71c166bd438
|
|
091a876664af4427db670ea8244d713b1b840048 Test watchonly wallet bumpfee with PSBT return (Gregory Sanders)
e9b4f9419cc778b5164708991a55014abef6c5f9 bumpfee: Return PSBT when wallet has privkeys disabled (Gregory Sanders)
75a5e478b631d3d0821d003300c4eae3c8433973 Change bumpfee to use watch-only funds for legacy watchonly wallets (Gregory Sanders)
Pull request description:
The main use-case here is for using with watch-only wallets with PSBT-signing cold wallets of all kinds.
ACKs for top commit:
achow101:
ACK 091a876664af4427db670ea8244d713b1b840048
Sjors:
Tested ACK 091a876664af4427db670ea8244d713b1b840048
meshcollider:
utACK 091a876664af4427db670ea8244d713b1b840048
Tree-SHA512: f7cf663e1af0b029e5c99eac88c5fdc3bc9e9a3841da8a608e8a9957e9bcf6a78864b8c2706fcaf78a480ffe11badd80c4fad29f97c0bb929e0470fafda5c22e
|
|
09502452bbbe21bb974f1de8cf53196373921ab9 IsUsedDestination should count any known single-key address (Gregory Sanders)
Pull request description:
This plugs the privacy leak detailed at https://github.com/bitcoin/bitcoin/issues/17605, at least for the single-key case.
ACKs for top commit:
meshcollider:
Code Review ACK 09502452bbbe21bb974f1de8cf53196373921ab9
Tree-SHA512: e1d68281675f05072b3087171cba1df9416a69c9ccf70c72e8555e55eadda2d0fd339e5a894e3a3438ff94b9e3827fb19b8b701faade70c08756b19ff157ee0c
|
|
|
|
-BEGIN VERIFY SCRIPT-
s() { contrib/devtools/copyright_header.py insert "$1"; }
s build_msvc/bitcoin_config.h
s build_msvc/msvc-autogen.py
s build_msvc/testconsensus/testconsensus.cpp
s contrib/devtools/circular-dependencies.py
s contrib/devtools/gen-manpages.sh
s contrib/filter-lcov.py
s contrib/gitian-build.py
s contrib/install_db4.sh
s src/crypto/sha256_avx2.cpp
s src/crypto/sha256_sse41.cpp
s src/fs.cpp
s src/qt/test/addressbooktests.cpp
s src/qt/test/addressbooktests.h
s src/qt/test/util.cpp
s src/qt/test/util.h
s src/qt/test/wallettests.cpp
s src/qt/test/wallettests.h
s src/test/blockchain_tests.cpp
s test/functional/combine_logs.py
s test/lint/lint-locale-dependence.sh
sed -i '1G' test/lint/lint-shebang.sh
s test/lint/lint-shebang.sh
-END VERIFY SCRIPT-
|
|
|
|
1f0adb3dac461b3f24f2a98b736d972e9713d67e tests: Add std::to_string to list of locale dependent functions (practicalswift)
Pull request description:
Add `std::to_string` to list of locale dependent functions:
> `std::to_string` relies on the current locale for formatting purposes […]
Context https://github.com/bitcoin/bitcoin/pull/17808#issuecomment-570329665
ACKs for top commit:
hebasto:
ACK 1f0adb3dac461b3f24f2a98b736d972e9713d67e, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 4cd6f567f5931dd166cdb9b065a939fb0bc02c93de18a9501655d98caf18b7c4d81f1881ea900dcdf2ec103d3ab1bdc9c68d3257b76dd2468a59e74d278b0d8d
|
|
This could help debug issues where there may be race conditions at play,
such as #12978.
Fixes #12994.
|
|
|
|
- change the value returned in the RPC getaddressinfo `labels` field to an array
of label name strings
- deprecate the previous behavior of returning a JSON hash structure containing
label `name` and address `purpose` key/value pairs
- update the relevant tests
|
|
|
|
|
|
3bd8db80d8d335ab63ece4f110b0fadd562e80b7 [validation] fix comments in CheckInputScripts() (John Newbery)
6f6465cefcd599c89c00f7b51f42a4b87a5ffb0b scripted-diff: [validation] Rename CheckInputs to CheckInputScripts (John Newbery)
Pull request description:
CheckInputs() used to check no double spends, scripts & sigs and amounts. Since
832e074, the double spend and amount checks
have been moved to CheckTxInputs(), and CheckInputs() now just validates
input scripts. Rename the function to CheckInputScripts().
Also fix incorrect comments.
ACKs for top commit:
MarcoFalke:
re-ACK 3bd8db80d8d335ab63ece4f110b0fadd562e80b7, did the rebase myself, checked the scripted diff 👡
promag:
ACK 3bd8db80d8d335ab63ece4f110b0fadd562e80b7 :trollface:
Tree-SHA512: 7b3f8597d210492798fb784ee8ea47ea6377519111190161c7cc34a967509013f4337304f52e9bedc97b7710de7b0ff8880e08cd7f867754567f82e7b02c794c
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
|
|
Previously these were allowed but ignored.
|
|
|
|
Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>
|
|
in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus
|
|
in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus
|
|
7d263571bee8c36fbe3c854b69c6f31cf1ee3b9b rpc: require second argument only for scantxoutset start action (Andrew Chow)
Pull request description:
It was reported on [IRC](http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-11.html#l-377) that `scantxoutset`'s API was broken in 0.19.0:
```
<belcher> i think scantxoutset may have been broken in bitcoin core 0.19 ? regardless of what parameters i run it with (e.g. "scantxoutset abort", "scantxoutset status") it just returns the help doc, according to the release notes the only change was https://github.com/bitcoin/bitcoin/pull/16285/files but i dont see anything that wouldve broken it, it works fine in 0.18
<belcher> im on regtest, in case its important
<harding> I can confirm `scantxoutset abort` returns the help doc on latest master. Waiting for 0.18.1 to start now to attempt to reproduce there.
<harding> It looks like it's expecting a second parameter (even though that doesn't make sense with "abort").
<jonatack> Same for me as well
<harding> Can also confirm that `scantxoutset abort` returns the expected result on 0.18.1.
```
As noted in the conversation, previously, the second argument of `scanobjects` is only required for the `start` action. `Stop` and `abort` actions did not and could work without them.
It appears that this was broken by #16240 which enforced the size of the arguments to match the listed required arguments.
To fix this issue, this PR makes the `scanobjects` argument an optional argument. Then only in the `start` action do we check whether the `scanobjects` argument is there and throw an informative error about that. Also a test is added for this case.
ACKs for top commit:
laanwj:
ACK 7d263571bee8c36fbe3c854b69c6f31cf1ee3b9b
promag:
ACK 7d263571bee8c36fbe3c854b69c6f31cf1ee3b9b.
Tree-SHA512: 828bdfe47f4fffa5d00a2cf88db6cea4a2714d9c49276841ca5cbdd1603b87bb6862147b86edcf36d7b40314ddb80b1a07fd399faf288572c55cc788c5cf9526
|
|
2081442c421cc4376e5d7839f68fbe7630e89103 test: Add test for rpc_whitelist (Emil Engler)
7414d3820c833566b4f48c6c120a18bf53978c55 Add RPC Whitelist Feature from #12248 (Jeremy Rubin)
Pull request description:
Summary
====
This patch adds the RPC whitelisting feature requested in #12248. RPC Whitelists help enforce application policies for services being built on top of Bitcoin Core (e.g., your Lightning Node maybe shouldn't be adding new peers). The aim of this PR is not to make it advisable to connect your Bitcoin node to arbitrary services, but to reduce risk and prevent unintended access.
Using RPC Whitelists
====
The way it works is you specify (in your bitcoin.conf) configurations such as
```
rpcauth=user1:4cc74397d6e9972e5ee7671fd241$11849357f26a5be7809c68a032bc2b16ab5dcf6348ef3ed1cf30dae47b8bcc71
rpcauth=user2:181b4a25317bff60f3749adee7d6bca0$d9c331474f1322975fa170a2ffbcb176ba11644211746b27c1d317f265dd4ada
rpcauth=user3:a6c8a511b53b1edcf69c36984985e$13cfba0e626db19061c9d61fa58e712d0319c11db97ad845fa84517f454f6675
rpcwhitelist=user1:getnetworkinfo
rpcwhitelist=user2:getnetworkinfo,getwalletinfo, getbestblockhash
rpcwhitelistdefault=0
```
Now user1 can only call getnetworkinfo, user2 can only call getnetworkinfo or getwalletinfo, while user3 can still call all RPCs.
If any rpcwhitelist is set, act as if all users are subject to whitelists unless rpcwhitelistdefault is set to 0. If rpcwhitelistdefault is set to 1 and no rpcwhitelist is set, act as if all users are subject to whitelists.
Review Request
=====
In addition to normal review, would love specific review from someone working on LN (e.g., @ roasbeef) and someone working on an infrastructure team at an exchange (e.g., @ jimpo) to check that this works well with their system.
Notes
=====
The rpc list is spelling sensitive -- whitespace is stripped though. Spelling errors fail towards the RPC call being blocked, which is safer.
It was unclear to me if HTTPReq_JSONRPC is the best function to patch this functionality into, or if it would be better to place it in exec or somewhere else.
It was also unclear to me if it would be preferred to cache the whitelists on startup or parse them on every RPC as is done with multiUserAuthorized. I opted for the cached approach as I thought it was a bit cleaner.
Future Work
=====
In a future PR, I would like to add an inheritance scheme. This seemed more controversial so I didn't want to include that here. Inheritance semantics are tricky, but it would also make these whitelists easier to read.
It also might be good to add a `getrpcwhitelist` command to facilitate permission discovery.
Tests
=====
Thanks to @ emilengler for adding tests for this feature. The tests cover all cases except for where `rpcwhitelistdefault=1` is used, given difficulties around testing with the current test framework.
ACKs for top commit:
laanwj:
ACK 2081442c421cc4376e5d7839f68fbe7630e89103
Tree-SHA512: 0dc1ac6a6f2f4b0be9c9054d495dd17752fe7b3589aeab2c6ac4e1f91cf4e7e355deedcb5d76d707cbb5a949c2f989c871b74d6bf129351f429569a701adbcbf
|
|
|
|
json.dumps()
b6f9e3576a1ea18572e4803aeb3f39330f0cb759 test: re-enable CLI test support by using EncodeDecimal in json.dumps() (fanquake)
Pull request description:
As mentioned in https://github.com/bitcoin/bitcoin/pull/17675#issuecomment-563188648.
ACKs for top commit:
practicalswift:
ACK b6f9e3576a1ea18572e4803aeb3f39330f0cb759 assuming Travis is happy too -- diff looks correct :)
MarcoFalke:
> ACK b6f9e35 assuming Travis is happy too -- diff looks correct :)
Tree-SHA512: 79fa535cc1756c8ee610a3d6a316a1c4f036797d6990a5620e44985393a2e52f78450f8e0021d0a148c08705fd1ba765508464a365f9030ae0d2cacbd7a93e19
|
|
The second argument of scanobjects is only required for the start action.
Stop and abort actions do not need this.
|
|
numbers, JSON and HD keypaths (bip32)
a1308b7e12e6af7482954e439f594b771eb62b73 tests: Add fuzzing harnesses for various JSON/univalue parsing functions (practicalswift)
e3d2bcf5cf7a53e5ca671cfed1fe7b6cf0c191ba tests: Add fuzzing harnesses for various number parsing functions (practicalswift)
fb8c12093aa37f5536a1a4ba341ee8bab4dabe60 tests: Add ParseScript(...) (core_io) fuzzing harness (practicalswift)
074cb6451b16158589d743488930963bcf4b024c tests: Add ParseHDKeypath(...) (bip32) fuzzing harness (practicalswift)
0dc5907d0f0490036c50cb7aee19e31075bbf402 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 harnesses for `DecodeRawPSBT(...)`, `ParseHDKeypath(...)`, `ParseScript(...)`, various number parsing functions and various JSON/univalue parsing functions.
**Testing this PR**
As usual the best way to test proposed fuzzing harnesses is to use `test_fuzzing_harnesses.sh` (#17000) to quickly verify that the relevant code regions are triggered, that the fuzzing throughput seems reasonable, etc.
`test_fuzzing_harnesses.sh 'psbt|hd_keypath|numbers|parse_script|univalue' 10` runs all fuzzers matching the regexp and gives them ten seconds of runtime each.
```
$ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
$ make
$ contrib/devtools/test_fuzzing_harnesses.sh 'psbt|hd_keypath|numbers|parse_script|univalue' 10
Testing fuzzer parse_hd_keypath during 10 second(s)
A subset of reached functions:
NEW_FUNC[0/2]: 0x55bc23a76940 in ParsePrechecks(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/util/strencodings.cpp:267
NEW_FUNC[1/2]: 0x55bc23a77300 in ParseUInt32(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned int*) src/util/strencodings.cpp:309
stat::number_of_executed_units: 34237
stat::average_exec_per_sec: 3112
stat::new_units_added: 113
stat::slowest_unit_time_sec: 0
stat::peak_rss_mb: 282
Number of unique code paths taken during fuzzing round: 30
Testing fuzzer parse_numbers during 10 second(s)
A subset of reached functions:
stat::number_of_executed_units: 31309
stat::average_exec_per_sec: 2846
stat::new_units_added: 688
stat::slowest_unit_time_sec: 0
stat::peak_rss_mb: 234
Number of unique code paths taken during fuzzing round: 149
Testing fuzzer parse_script during 10 second(s)
A subset of reached functions:
NEW_FUNC[1/11]: 0x5636ff61ba00 in IsDigit(char) src/./util/strencodings.h:70
NEW_FUNC[0/14]: 0x5636fe6c6280 in CScript::operator<<(opcodetype) src/./script/script.h:448
NEW_FUNC[1/14]: 0x5636fe6e0290 in prevector<28u, unsigned char, unsigned int, int>::insert(prevector<28u, unsigned char, unsigned int, int>::iterator, unsigned char const&) src/./prevector.h:342
NEW_FUNC[2/14]: 0x5636fe6e1040 in prevector<28u, unsigned char, unsigned int, int>::size() const src/./prevector.h:277
NEW_FUNC[3/14]: 0x5636fe6e1250 in prevector<28u, unsigned char, unsigned int, int>::capacity() const src/./prevector.h:295
NEW_FUNC[4/14]: 0x5636fe6e1cb0 in prevector<28u, unsigned char, unsigned int, int>::item_ptr(int) src/./prevector.h:196
NEW_FUNC[0/10]: 0x5636fe6c5650 in CScript::operator<<(std::vector<unsigned char, std::allocator<unsigned char> > const&) src/./script/script.h:462
NEW_FUNC[2/10]: 0x5636fe6e0a20 in void prevector<28u, unsigned char, unsigned int, int>::insert<__gnu_cxx::__normal_iterator<unsigned char const*, std::vector<unsigned char, std::allocator<unsigned char> > > >(prevector<28u, unsigned char, unsigned int, int>::iterator, __gnu_cxx::__normal_iterator<unsigned char const*, std::vector<unsigned char, std::allocator<[32/1902]
char> > >, __gnu_cxx::__normal_iterator<unsigned char const*, std::vector<unsigned char, std::allocator<unsigned char> > >) src/./prevector.h:368
NEW_FUNC[5/10]: 0x5636fe6e2350 in void prevector<28u, unsigned char, unsigned int, int>::fill<__gnu_cxx::__normal_iterator<unsigned char const*, std::vector<unsigned char, std::allocator<unsigned char> > > >(unsigned char*, __gnu_cxx::__normal_iterator<unsigned char const*, std::vector<unsigned char, std::allocator<unsigned char> > >, __gnu_cxx::__normal_iterator<unsign
ed char const*, std::vector<unsigned char, std::allocator<unsigned char> > >) src/./prevector.h:204
NEW_FUNC[0/1]: 0x5636ff8e48b0 in IsHex(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/util/strencodings.cpp:61
NEW_FUNC[0/2]: 0x5636fe6e1410 in prevector<28u, unsigned char, unsigned int, int>::change_capacity(unsigned int) src/./prevector.h:165
NEW_FUNC[1/2]: 0x5636fe6e1f00 in prevector<28u, unsigned char, unsigned int, int>::indirect_ptr(int) src/./prevector.h:161
NEW_FUNC[0/1]: 0x5636fe6e0580 in void prevector<28u, unsigned char, unsigned int, int>::insert<unsigned char*>(prevector<28u, unsigned char, unsigned int, int>::iterator, unsigned char*, unsigned char*) src/./prevector.h:368
NEW_FUNC[0/3]: 0x5636fe85f0d0 in CScript::push_int64(long) src/./script/script.h:394
NEW_FUNC[1/3]: 0x5636fe85f520 in prevector<28u, unsigned char, unsigned int, int>::push_back(unsigned char const&) src/./prevector.h:422
NEW_FUNC[2/3]: 0x5636ff8ed730 in atoi64(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/util/strencodings.cpp:417
stat::number_of_executed_units: 8153
stat::average_exec_per_sec: 741
stat::new_units_added: 296
stat::slowest_unit_time_sec: 0
stat::peak_rss_mb: 237
Number of unique code paths taken during fuzzing round: 98
Testing fuzzer parse_univalue during 10 second(s)
A subset of reached functions:
NEW_FUNC[0/19]: 0x560db8655950 in tinyformat::detail::formatImpl(std::ostream&, char const*, tinyformat::detail::FormatArg const*, int) src/./tinyformat.h:791
NEW_FUNC[4/19]: 0x560db86582b0 in tinyformat::detail::printFormatStringLiteral(std::ostream&, char const*) src/./tinyformat.h:564
NEW_FUNC[5/19]: 0x560db8658690 in tinyformat::detail::streamStateFromFormat(std::ostream&, bool&, int&, char const*, tinyformat::detail::FormatArg const*, int&, int) src/./tinyformat.h:601
NEW_FUNC[6/19]: 0x560db865f090 in tinyformat::detail::FormatArg::format(std::ostream&, char const*, char const*, int) const src/./tinyformat.h:513
NEW_FUNC[12/19]: 0x560db8661ba0 in void tinyformat::detail::FormatArg::formatImpl<int>(std::ostream&, char const*, char const*, int, void const*) src/./tinyformat.h:530
NEW_FUNC[13/19]: 0x560db8661d90 in void tinyformat::formatValue<int>(std::ostream&, char const*, char const*, int, int const&) src/./tinyformat.h:317
NEW_FUNC[14/19]: 0x560db875c8b0 in void tinyformat::detail::FormatArg::formatImpl<unsigned int>(std::ostream&, char const*, char const*, int, void const*) src/./tinyformat.h:530
NEW_FUNC[15/19]: 0x560db875caa0 in void tinyformat::formatValue<unsigned int>(std::ostream&, char const*, char const*, int, unsigned int const&) src/./tinyformat.h:317
NEW_FUNC[16/19]: 0x560db9473ef0 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<int, unsigned int>(char const*, int const&, unsigned int const&) src/./tinyformat.h:976
NEW_FUNC[17/19]: 0x560db94749a0 in void tinyformat::format<int, unsigned int>(std::ostream&, char const*, int const&, unsigned int const&) src/./tinyformat.h:968
NEW_FUNC[18/19]: 0x560db9474cf0 in tinyformat::detail::FormatListN<2>::FormatListN<int, unsigned int>(int const&, unsigned int const&) src/./tinyformat.h:885
stat::number_of_executed_units: 14089
stat::average_exec_per_sec: 1280
stat::new_units_added: 135
stat::slowest_unit_time_sec: 0
stat::peak_rss_mb: 356
Number of unique code paths taken during fuzzing round: 62
Testing fuzzer psbt_input_deserialize during 10 second(s)
A subset of reached functions:
NEW_FUNC[0/46]: 0x557847ce3530 in prevector<28u, unsigned char, unsigned int, int>::~prevector() src/./prevector.h:456
NEW_FUNC[3/46]: 0x557847cfdcf0 in prevector<28u, unsigned char, unsigned int, int>::size() const src/./prevector.h:277
NEW_FUNC[4/46]: 0x557847cfe0c0 in prevector<28u, unsigned char, unsigned int, int>::change_capacity(unsigned int) src/./prevector.h:165
NEW_FUNC[13/46]: 0x557847d3c890 in unsigned long ReadCompactSize<CDataStream>(CDataStream&) src/./serialize.h:290
NEW_FUNC[14/46]: 0x557847d47b60 in prevector<28u, unsigned char, unsigned int, int>::resize(unsigned int) src/./prevector.h:311
NEW_FUNC[16/46]: 0x557847d48800 in CTxOut::CTxOut() src/./primitives/transaction.h:140
NEW_FUNC[17/46]: 0x557847d4b050 in CTxOut::SetNull() src/./primitives/transaction.h:155
NEW_FUNC[18/46]: 0x557847d4b140 in CScript::clear() src/./script/script.h:563
NEW_FUNC[19/46]: 0x557847d4ead0 in void Unserialize_impl<CDataStream, unsigned char, std::allocator<unsigned char> >(CDataStream&, std::vector<unsigned char, std::allocator<unsigned char> >&, unsigned char const&) src/./serialize.h:746
NEW_FUNC[0/58]: 0x557847cfdf00 in prevector<28u, unsigned char, unsigned int, int>::capacity() const src/./prevector.h:295
NEW_FUNC[1/58]: 0x557847cfe960 in prevector<28u, unsigned char, unsigned int, int>::item_ptr(int) src/./prevector.h:196
NEW_FUNC[2/58]: 0x557847cfebb0 in prevector<28u, unsigned char, unsigned int, int>::indirect_ptr(int) src/./prevector.h:161
NEW_FUNC[3/58]: 0x557847d03990 in uint256::uint256() src/./uint256.h:123
NEW_FUNC[0/3]: 0x557847d47430 in void CScript::SerializationOp<CDataStream, CSerActionUnserialize>(CDataStream&, CSerActionUnserialize) src/./script/script.h:418
NEW_FUNC[1/3]: 0x557847d47730 in void Unserialize_impl<CDataStream, 28u, unsigned char>(CDataStream&, prevector<28u, unsigned char, unsigned int, int>&, unsigned char const&) src/./serialize.h:666
NEW_FUNC[2/3]: 0x557847d60dd0 in CDataStream& CDataStream::operator>><CScript&>(CScript&) src/./streams.h:460
NEW_FUNC[1/78]: 0x557847cffae0 in prevector<28u, unsigned char, unsigned int, int>::item_ptr(int) const src/./prevector.h:197
NEW_FUNC[2/78]: 0x557847cffd30 in prevector<28u, unsigned char, unsigned int, int>::indirect_ptr(int) const src/./prevector.h:162
NEW_FUNC[0/1]: 0x557847d65f90 in OverrideStream<CDataStream>& OverrideStream<CDataStream>::operator>><unsigned char&>(unsigned char&) src/./streams.h:46
NEW_FUNC[0/3]: 0x557847d470e0 in void SerReadWriteMany<CDataStream, CScript&>(CDataStream&, CSerActionUnserialize, CScript&) src/./serialize.h:989
NEW_FUNC[1/3]: 0x557847d4ac50 in void CTxOut::SerializationOp<CDataStream, CSerActionUnserialize>(CDataStream&, CSerActionUnserialize) src/./primitives/transaction.h:149
NEW_FUNC[2/3]: 0x557847d5f860 in void UnserializeFromVector<CDataStream, CTxOut>(CDataStream&, CTxOut&) src/./script/sign.h:90
NEW_FUNC[0/1]: 0x557847d60840 in void UnserializeFromVector<CDataStream, int>(CDataStream&, int&) src/./script/sign.h:90
NEW_FUNC[0/1]: 0x557847d41010 in CMutableTransaction::HasWitness() const src/./primitives/transaction.h:398
stat::number_of_executed_units: 13615
stat::average_exec_per_sec: 1237
stat::new_units_added: 357
stat::slowest_unit_time_sec: 0
stat::peak_rss_mb: 446
Number of unique code paths taken during fuzzing round: 152
Testing fuzzer psbt_output_deserialize during 10 second(s)
A subset of reached functions:
NEW_FUNC[0/27]: 0x55c9347e5940 in prevector<28u, unsigned char, unsigned int, int>::~prevector() src/./prevector.h:456
NEW_FUNC[5/27]: 0x55c93483eca0 in unsigned long ReadCompactSize<CDataStream>(CDataStream&) src/./serialize.h:290
NEW_FUNC[6/27]: 0x55c934850ee0 in void Unserialize_impl<CDataStream, unsigned char, std::allocator<unsigned char> >(CDataStream&, std::vector<unsigned char, std::allocator<unsigned char> >&, unsigned char const&) src/./serialize.h:746
NEW_FUNC[14/27]: 0x55c934858500 in PSBTOutput::PSBTOutput() src/./psbt.h:281
NEW_FUNC[15/27]: 0x55c934858870 in CDataStream& CDataStream::operator>><PSBTOutput&>(PSBTOutput&) src/./streams.h:460
NEW_FUNC[0/1]: 0x55c934800100 in prevector<28u, unsigned char, unsigned int, int>::size() const src/./prevector.h:277
NEW_FUNC[0/4]: 0x55c934849840 in void CScript::SerializationOp<CDataStream, CSerActionUnserialize>(CDataStream&, CSerActionUnserialize) src/./script/script.h:418
NEW_FUNC[1/4]: 0x55c934849b40 in void Unserialize_impl<CDataStream, 28u, unsigned char>(CDataStream&, prevector<28u, unsigned char, unsigned int, int>&, unsigned char const&) src/./serialize.h:666
NEW_FUNC[2/4]: 0x55c934849f70 in prevector<28u, unsigned char, unsigned int, int>::resize(unsigned int) src/./prevector.h:311
NEW_FUNC[3/4]: 0x55c93485dc60 in CDataStream& CDataStream::operator>><CScript&>(CScript&) src/./streams.h:460
NEW_FUNC[0/3]: 0x55c934800310 in prevector<28u, unsigned char, unsigned int, int>::capacity() const src/./prevector.h:295
NEW_FUNC[1/3]: 0x55c934800d70 in prevector<28u, unsigned char, unsigned int, int>::item_ptr(int) src/./prevector.h:196
NEW_FUNC[2/3]: 0x55c934849d40 in prevector<28u, unsigned char, unsigned int, int>::resize_uninitialized(unsigned int) src/./prevector.h:381
NEW_FUNC[0/1]: 0x55c93485ddd0 in void DeserializeHDKeypaths<CDataStream>(CDataStream&, std::vector<unsigned char, std::allocator<unsigned char> > const&, std::map<CPubKey, KeyOriginInfo, std::less<CPubKey>, std::allocator<std::pair<CPubKey const, KeyOriginInfo> > >&) src/./script/sign.h:103
stat::number_of_executed_units: 19130
stat::average_exec_per_sec: 1739
stat::new_units_added: 195
stat::slowest_unit_time_sec: 0
stat::peak_rss_mb: 411
Number of unique code paths taken during fuzzing round: 64
Tested fuzz harnesses seem to work as expected.
```
Top commit has no ACKs.
Tree-SHA512: baf1630a6e438d02d33c77b9e602c99546b9e8d83705e67c2749e0600039c37707cdf419cee19282f069e8d787c536ed4960f9c47e93bd0f0251495b83780ada
|
|
In decodepsbt if an invalid amount is seen, don't calculate the fee
but still show the invalid value in the decode.
In analyze psbt, if an invalid amount is seen, set the next step to
be the creator as the creator needs to remake the transaction so that
it is valid.
|
|
Valgrind
5db506ba5943868cc2c845f717508739b7f05714 tests: Add option --valgrind to run nodes under valgrind in the functional tests (practicalswift)
Pull request description:
What is better than fixing bugs? Fixing entire bug classes of course! :)
Add option `--valgrind` to run the functional tests under Valgrind.
Regular functional testing under Valgrind would have caught many of the uninitialized reads we've seen historically.
Let's kill this bug class once and for all: let's never use an uninitialized value ever again. Or at least not one that would be triggered by running the functional tests! :)
My hope is that this addition will make it super-easy to run the functional tests under Valgrind and thus increase the probability of people making use of it :)
Hopefully `test/functional/test_runner.py --valgrind` will become a natural part of the pre-release QA process.
**Usage:**
```
$ test/functional/test_runner.py --help
…
--valgrind run nodes under the valgrind memory error detector:
expect at least a ~10x slowdown, valgrind 3.14 or
later required
```
**Live demo:**
First, let's re-introduce a memory bug by reverting the recent P2P uninitialized read bug fix from PR #17624 ("net: Fix an uninitialized read in ProcessMessage(…, "tx", …) when receiving a transaction we already have").
```
$ git diff
diff --git a/src/consensus/validation.h b/src/consensus/validation.h
index 3401eb64c..940adea33 100644
--- a/src/consensus/validation.h
+++ b/src/consensus/validation.h
@@ -114,7 +114,7 @@ inline ValidationState::~ValidationState() {};
class TxValidationState : public ValidationState {
private:
- TxValidationResult m_result = TxValidationResult::TX_RESULT_UNSET;
+ TxValidationResult m_result;
public:
bool Invalid(TxValidationResult result,
const std::string &reject_reason="",
```
Second, let's test as normal without Valgrind:
```
$ test/functional/p2p_segwit.py -l INFO
2019-11-28T09:30:42.810000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test__fc8q3qo
…
2019-11-28T09:31:57.187000Z TestFramework (INFO): Subtest: test_non_standard_witness_blinding (Segwit active = True)
…
2019-11-28T09:32:08.265000Z TestFramework (INFO): Tests successful
```
Third, let's test with `--valgrind` and see if the test fail (as we expect) when the unitialized value is used:
```
$ test/functional/p2p_segwit.py -l INFO --valgrind
2019-11-28T09:32:33.018000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_gtjecx2l
…
2019-11-28T09:40:36.702000Z TestFramework (INFO): Subtest: test_non_standard_witness_blinding (Segwit active = True)
2019-11-28T09:40:37.813000Z TestFramework (ERROR): Assertion failed
ConnectionRefusedError: [Errno 111] Connection refused
```
ACKs for top commit:
MarcoFalke:
ACK 5db506ba5943868cc2c845f717508739b7f05714
jonatack:
ACK 5db506ba5943868cc2c845f717508739b7f05714
Tree-SHA512: 2eaecacf4da166febad88b2a8ee6d7ac2bcd38d4c1892ca39516b6343e8f8c8814edf5eaf14c90f11a069a0389d24f0713076112ac284de987e72fc5f6cc3795
|
|
773d4572a4864ab7b6380858d07d9579ff6dd9a2 Mark PSBTs spending unspendable outputs as invalid in analysis (Andrew Chow)
638e40cb6080800c7b0a7f4028f63326acbe4700 Have a PSBTAnalysis state that indicates invalid PSBT (Andrew Chow)
Pull request description:
When analyzing an unspendable PSBT, report that it is unspendable and exit analysis early.
ACKs for top commit:
Sjors:
ACK 773d457
instagibbs:
After some thought ACK https://github.com/bitcoin/bitcoin/commit/773d4572a4864ab7b6380858d07d9579ff6dd9a2
Tree-SHA512: 99b0cb2fa1ea37593fc65a20effe881639d69ddeeecf5197bc87bc7f2220cbeb40f1d429d517e4d27f2e9fb563a00cd845d2b4b1ce05246a75a6cb56fb9b0ba5
|
|
|
|
in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus
|
|
This can be done now that prevector is properly aligned.
|
|
only integrals
597d10ceb9fd2a118c7e551cd6263379691d9295 tests: Add fuzzing harness for various functions consuming only integrals (practicalswift)
575383b3e1361e60ba88738a34d92b1662f915a7 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
|
|
As mentioned in
https://github.com/bitcoin/bitcoin/pull/17675#issuecomment-563188648
|
|
functions
d5766f223f627bf2eb731ce8552dfafa2b824378 tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus (practicalswift)
e75ecb91c730115290e1201371492c2cd334e9b4 tests: Add fuzzing harness for various CTxOut related functions (practicalswift)
ce935292c041162e160d95fc6afeda3dceded2cf tests: Add fuzzing harness for various CTxIn related functions (practicalswift)
Pull request description:
Add fuzzing harness for various `CTx{In,Out}` related functions.
**Testing this PR**
Run:
```
$ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/tx_in
…
$ src/test/fuzz/tx_out
…
# 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 '^tx_'
```
`test_fuzzing_harnesses.sh` can be found in PR #17000.
Top commit has no ACKs.
Tree-SHA512: f1374307a2581ebc3968d012ea2438061bbb84ece068e584fae9750669a6cd003723dde14db88e77c9579281ecd4eaa2a7ff0614f253d8c075e6dd16dd2e68d5
|
|
|
|
|
|
in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus
|
|
in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus
|
|
897849d8c225045f0dd3a2fe99b5d69bdf84b4e2 tests: Add deserialization fuzzing harnesses (practicalswift)
16f0a186dcee563bb1000e1ffc51da87e7623bc6 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 deserialization fuzzing harnesses.
**Testing this PR**
Run:
```
$ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
$ make
$ contrib/devtools/test_fuzzing_harnesses.sh 'addr_info|block_file_info|block_filter|block_header|ext_key|ext_pub_key|fee_rate|flat_file|key_origin|merkle_block|mutable_transaction|out_point|partial_merkle_tree|partially_signed_transaction|prefilled_transaction|psbt_input|psbt_output|pub_key|script_deserialize|sub_net|tx_in' 10
```
`test_fuzzing_harnesses.sh` can be found in PR #17000.
ACKs for top commit:
laanwj:
thanks, ACK 897849d8c225045f0dd3a2fe99b5d69bdf84b4e2
Tree-SHA512: 5a270a3002cc23b725f7b35476a43777b2b00b4d089cc006372e2fcc7afa430afaa3c1430f778ae08fc53dd85a13e7bd2fab0449c319f676423226e189a417f6
|
|
dddd09eb33d14fabda0aa40fa008b23b2bd6e589 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 dddd09eb33d14fabda0aa40fa008b23b2bd6e589
Tree-SHA512: ab7061f946b5e5388f825dddceadb125f5197b24af3a7fcf1e700235d106a323419a56bfb4d84a2e27442e0de63e540c623b704343d83a98deaab3c02fcbdcbe
|
|
01c87015597021bf1c0856f7f6be175bdde844b2 util: remove unwanted fields from bitcoin-cli -getinfo (malevolent)
Pull request description:
Removed the following fields from -getinfo: protocolversion, walletversion and keypoololdest. This change closes #17314 .
ACKs for top commit:
laanwj:
ACK 01c87015597021bf1c0856f7f6be175bdde844b2
achow101:
ACK 01c87015597021bf1c0856f7f6be175bdde844b2
practicalswift:
ACK 01c87015597021bf1c0856f7f6be175bdde844b2 -- diff looks correct
Tree-SHA512: c98f2e8a3fee04d46766f70cb88f4e49e892a4424eff8940a7d48e9e808597b702427225788f984f5c3641591fd8d86acee56774afde1d57a4259c31d971ea08
|