Age | Commit message (Collapse) | Author |
|
CheckInputsFromMempooAndCache
0a433fc876d82df1005f175c1254fff62f0f36f8 [validation] Remove unused cacheSigStore from CheckInputsFromMempoolAndCache (John Newbery)
Pull request description:
CheckInputsFromMempoolAndCache() is only called in one place, and
cacheSigStore is set to true in that call site. Remove the argument
entirely.
Also improve commenting.
ACKs for top commit:
MarcoFalke:
unsigned ACK 0a433fc876d82df1005f175c1254fff62f0f36f8 Comment looks good
jamesob:
ACK https://github.com/bitcoin/bitcoin/pull/17242/commits/0a433fc876d82df1005f175c1254fff62f0f36f8
laanwj:
ACK 0a433fc876d82df1005f175c1254fff62f0f36f8
fanquake:
ACK 0a433fc876d82df1005f175c1254fff62f0f36f8. Checked that `CheckInputsFromMempoolAndCache` is only called once, in `MemPoolAccept::ConsensusScriptChecks`, and that `cacheSigStore` is true.
Tree-SHA512: e4b4d2550e35df55c8f8fa4c539174cc2d3728112ddb937cb2ff759d8630a01566b5ec42a70a82e33994e6586f5a457a75a59f64b15d27c65331c723cbb097af
|
|
b05ec410f2d9f209796a5df31860e23efd729dfe Add unit testing for the CompressScript functions (marcaiaf)
Pull request description:
Salvaging #15104 which adds unit tests for CompressScript function in `compressor.cpp`
Tested following cases for the CScript:
- CKeyID
- CScriptID
- Uncompressed CPubKey (of size: 65)
- Compressed CPubKey (of size: 32)
ACKs for top commit:
theStack:
ACK https://github.com/bitcoin/bitcoin/commit/b05ec410f2d9f209796a5df31860e23efd729dfe
Tree-SHA512: 7e23ace39383122802dfe5f7d38190d772f5db4045a67b7a9bd4c06797a17e0cdc41d6fac92d448057eb7df50172155dc824587c16c68c79fd1a4de37b772001
|
|
skipped and remove it
fa928134075220254a15107c1d9702f4e66271f8 consensus: Explain why fCheckDuplicateInputs can not be skipped and remove it (MarcoFalke)
Pull request description:
As a follow up to CVE-2018-17144, this removes the unused `fCheckDuplicateInputs` parameter and explains why the test can not be disabled. Apart from protecting against a dumb accident in the future, this should document the logic in the code. There is a technical write-up that explains how the underlying coins database behaves if this test is skipped: https://bitcoincore.org/en/2018/09/20/notice/#technical-details. However, it does not explicitly mention why the test can not be skipped. I hope my code comment does that.
ACKs for top commit:
jnewbery:
ACK fa928134075220254a15107c1d9702f4e66271f8
amitiuttarwar:
utACK fa928134075220254a15107c1d9702f4e66271f8
Empact:
Code review ACK https://github.com/bitcoin/bitcoin/commit/fa928134075220254a15107c1d9702f4e66271f8
promag:
ACK fa928134075220254a15107c1d9702f4e66271f8.
Tree-SHA512: fc1ef670f1a467c543b84f704b9bd8cc7a59a9f707be048bd9b4e85fe70830702aa560a880efa2c840bb43818ab44dfdc611104df04db2ddc14ff92f46bfb28e
|
|
168b781fe7f3f13b24c52a151f36de4cdd0a340a Continue relaying transactions after they expire from mapRelay (Anthony Towns)
Pull request description:
This change allows peers to request transactions even after they've expired from mapRelay and even if they're not doing mempool requests. This is intended to allow for CPFP of old transactions -- if parent tx P wasn't relayed due to low fees, then a higher fee rate child C is relayed, peers will currently request the parent P, but we prior to this patch, we will not relay it due to it not being in mapRelay.
ACKs for top commit:
MarcoFalke:
re-ACK 168b781fe7f3f13b24c52a151f36de4cdd0a340a (only change is comment fixup)
sdaftuar:
re-ACK 168b781fe7f3f13b24c52a151f36de4cdd0a340a
sipa:
ACK 168b781fe7f3f13b24c52a151f36de4cdd0a340a
Tree-SHA512: b206666dd1450cd0a161ae55fd1a7eda2c3d226842ba27d91fe463b551fd924b65b92551b14d6786692e15cf9a9a989666550dfc980b48ab0f8d4ca305bc7762
|
|
5b44a75493a1a098404d5e21dc384e74eae1892e refactor: Remove unused CExt{Pub,}Key (de)serialization methods (Sebastian Falbesoner)
Pull request description:
As pointed out in issue #17130, the serialization/deserialization methods for the classes `CExtKey` and
`CExtPubKey` are only used in the BIP32 unit tests and hence can be removed (see comments https://github.com/bitcoin/bitcoin/issues/17130#issuecomment-543750290, https://github.com/bitcoin/bitcoin/issues/17130#issuecomment-543794408 and https://github.com/bitcoin/bitcoin/issues/17130#issuecomment-543814727).
ACKs for top commit:
practicalswift:
ACK 5b44a75493a1a098404d5e21dc384e74eae1892e -- -60 LOC diff looks correct :)
promag:
ACK 5b44a75493a1a098404d5e21dc384e74eae1892e.
MarcoFalke:
unsigned ACK 5b44a75493a1a098404d5e21dc384e74eae1892e
fjahr:
ACK 5b44a75
jonatack:
Light ACK 5b44a75493a1a098404d5e21dc384e74eae1892e. Built, ran tests and bitcoind. `git blame` shows most of the last changes are from commit 90604f16af63ec066d6561337f476ccd8acec326 in 2015 to add bip32 pubkey serialization.
Tree-SHA512: 6887573b76b9e54e117a076557407b6f7908719b2202fb9eea498522baf9f30198b3f78b87a62efcd17ad1ab0886196f099239992ce7cbbaee79979ffe9e5f2c
|
|
|
|
CheckInputsFromMempoolAndCache() is only called in one place, and
cacheSigStore is set to true in that call site. Remove the argument
entirely.
Also improve commenting.
|
|
ECCVerifyHandle only when needed.
c2f964a6745be085f2891c909d6c998687de9080 tests: Remove Cygwin WinMain workaround (practicalswift)
db4bd32cc31789fc017f5db0b86a69ee43e41575 tests: Skip unnecessary fuzzer initialisation. Hold ECCVerifyHandle only when needed. (practicalswift)
Pull request description:
Skip unnecessary fuzzer initialisation. Hold `ECCVerifyHandle` only when needed.
As suggested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/17018#discussion_r336645391.
Top commit has no ACKs.
Tree-SHA512: 598da44859d736e3fdc143b93e07f444d8ad19dfdab0cfe7c6ccff8644e862664d869337dfe6b49416ed09a0024e4a5f2220ca6246de568f9e9227d721baa28e
|
|
0fc81a1e8724547a226e5fae5c32fc32d8dfb733 gui: Fix payAmount tooltip in SendCoinsEntry (João Barbosa)
Pull request description:
Before the tooltip shows in wrong places:
![Screenshot 2019-10-23 at 11 33 49](https://user-images.githubusercontent.com/3534524/67384904-f6b6a380-f589-11e9-832c-ec1643014b96.png)
![Screenshot 2019-10-23 at 11 33 23](https://user-images.githubusercontent.com/3534524/67384905-f74f3a00-f589-11e9-9944-a52fee097e02.png)
Now only shows in the amount field:
![Screenshot 2019-10-23 at 11 35 30](https://user-images.githubusercontent.com/3534524/67384919-ff0ede80-f589-11e9-8ce4-c122e11fe885.png)
ACKs for top commit:
laanwj:
ACK 0fc81a1e8724547a226e5fae5c32fc32d8dfb733
Tree-SHA512: 0857e568c21d380a68c81e9be3212b1745d7d3199a1d5fdef9afc8feed0272f215726fa98bbf8a3fb332389c5454f2316bc1581f1a2ccd76cef46a0e3ac6f99f
|
|
9075d13153ce06cd59a45644831ecc43126e1e82 [docs] Add release notes for removal of REJECT reasons (John Newbery)
04a2f326ec0f06fb4fce1c4f93500752f05dede8 [validation] Fix REJECT message comments (John Newbery)
e9d5a59e34ff2d538d8f5315efd9908bf24d0fdc [validation] Remove REJECT code from CValidationState (John Newbery)
0053e16714323c1694c834fdca74f064a1a33529 [logging] Don't log REJECT code when transaction is rejected (John Newbery)
a1a07cfe99fc8cee30ba5976dc36b47b1f6532ab [validation] Fix peer punishment for bad blocks (John Newbery)
Pull request description:
We no longer send BIP 61 REJECT messages, so there's no need to set
a REJECT code in the CValidationState object.
Note that there is a minor bug fix in p2p behaviour here. 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 reverts a minor bug introduced in 5d08c9c579ba8cc7b684105c6a08263992b08d52.
ACKs for top commit:
ariard:
ACK 9075d13, changes since last reviewed are splitting them in separate commits to ease understanding and fix nits
fjahr:
ACK 9075d13153ce06cd59a45644831ecc43126e1e82, confirmed diff to last review was fixing nits in docs/comments.
ryanofsky:
Code review ACK 9075d13153ce06cd59a45644831ecc43126e1e82. Only changes since last review are splitting the main commit and updating comments
Tree-SHA512: 58e8a1a4d4e6f156da5d29fb6ad6a62fc9c594bbfc6432b3252e962d0e9e10149bf3035185dc5320c46c09f3e49662bc2973ec759679c0f3412232087cb8a3a7
|
|
9e95931865186d7a9a6dc54b64bd96507e9fea4b [wallet] Remove `state` argument from CWallet::CommitTransaction (John Newbery)
d1734f9a3b138ab046f38ee44a09bc3847bf938a [wallet] Remove return value from CommitTransaction() (John Newbery)
b6f486a02b463ffeaf82ec11fc6f74f439c037ae [wallet] Add doxygen comment to CWallet::CommitTransaction() (John Newbery)
8bba91b22d22a8dfea7c947b542b1022bfc1c0ea [wallet] Fix whitespace in CWallet::CommitTransaction() (John Newbery)
Pull request description:
`CommitTransaction()` returns a bool to indicate success, but since commit
b3a7410 (#9302) it only returns true, even if the transaction was not
successfully broadcast. This commit changes CommitTransaction() to return
void.
All dead code in `if (!CommitTransaction())` branches has been removed.
Two additional commits fix up the idiosyncratic whitespace in `CommitTransaction` and add a doxygen comment for the function.
ACKs for top commit:
laanwj:
ACK 9e95931865186d7a9a6dc54b64bd96507e9fea4b
Tree-SHA512: a55a2c20369a45222fc0e02d0891495655a926e71c4f52cb72624768dd7b9c1dca716ea67d38420afb90f40c6e0fd448caa60c18fd693bb10ecb110b641820e6
|
|
|
|
when needed.
|
|
4896bacc00549c14f3284f5a2b61fb848ac31be0 Add testcase to simulate bitcoin schema in leveldb (MapleLaker)
Pull request description:
Resurrecting #14125 with updates based on comments of closed PR
ACKs for top commit:
laanwj:
ACK 4896bacc00549c14f3284f5a2b61fb848ac31be0
dongcarl:
ACK 4896bacc00549c14f3284f5a2b61fb848ac31be0
Tree-SHA512: 3290ea7e1e998901d5ee8921d1d76cec399cae30ac1911a45b86826afed47cee1acf92bd6438f1fa11ed785a3b17abdcb1c169bc0419945eda9fe4c089d0b6eb
|
|
|
|
b5ffa9f3dbff7dd008d4d00a88590d411ef991f2 tests: Add Parse(...) (descriptor) fuzzing harness (practicalswift)
fdef8bbf2f824a87f70b755155e9e1a8cd19fdcb tests: Allow for using non-default fuzzing initialization (practicalswift)
Pull request description:
Add `Parse(...)` (descriptor) fuzzing harness.
To test this PR:
We can run `test_fuzzing_harnesses.sh` (#17000) during ten seconds to quickly verify that the newly added fuzz harness seem to hit relevant code regions, that the fuzzing throughput seems reasonable, etc.
`test_fuzzing_harnesses.sh descriptor 10` runs all fuzzers matching the regexp `descriptor` giving 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 descriptor 10
Testing fuzzer descriptor_parse during 10 second(s)
A subset of reached functions:
NEW_FUNC[0/17]: 0x55ec8a240c90 in tinyformat::detail::formatImpl(std::ostream&, char const*, tinyformat::detail::FormatArg const*, int) src/./tinyformat.h:791
NEW_FUNC[4/17]: 0x55ec8a2435f0 in tinyformat::detail::printFormatStringLiteral(std::ostream&, char const*) src/./tinyformat.h:564
NEW_FUNC[5/17]: 0x55ec8a2439d0 in tinyformat::detail::streamStateFromFormat(std::ostream&, bool&, int&, char const*, tinyformat::detail::FormatArg const*, int&, int) src/./tinyformat.h:601
NEW_FUNC[6/17]: 0x55ec8a24a3d0 in tinyformat::detail::FormatArg::format(std::ostream&, char const*, char const*, int) const src/./tinyformat.h:513
NEW_FUNC[12/17]: 0x55ec8a29cd70 in void tinyformat::detail::FormatArg::formatImpl<long>(std::ostream&, char const*, char const*, int, void const*) src/./tinyformat.h:530
NEW_FUNC[13/17]: 0x55ec8a29cf50 in void tinyformat::formatValue<long>(std::ostream&, char const*, char const*, int, long const&) src/./tinyformat.h:317
NEW_FUNC[14/17]: 0x55ec8a2ea450 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<long>(char const*, long const&) src/./tinyformat.h:976
NEW_FUNC[15/17]: 0x55ec8a346ac0 in void tinyformat::format<long>(std::ostream&, char const*, long const&) src/./tinyformat.h:968
NEW_FUNC[16/17]: 0x55ec8a346d80 in tinyformat::detail::FormatListN<1>::FormatListN<long>(long const&) src/./tinyformat.h:885
NEW_FUNC[0/16]: 0x55ec8a210c90 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >(char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/./tinyformat.h:976
NEW_FUNC[2/16]: 0x55ec8a25c3e0 in void tinyformat::format<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >(std::ostream&, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/./tinyformat.h:968
NEW_FUNC[3/16]: 0x55ec8a25c6a0 in tinyformat::detail::FormatListN<1>::FormatListN<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/./tinyformat.h:885
NEW_FUNC[4/16]: 0x55ec8a25c980 in void tinyformat::detail::FormatArg::formatImpl<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >(std::ostream&, char const*, char const*, int, void const*) src/./tinyformat.h:530
NEW_FUNC[6/16]: 0x55ec8b29cc60 in (anonymous namespace)::ParseScript(Span<char const>&, (anonymous namespace)::ParseScriptContext, FlatSigningProvider&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) src/script/descriptor.cpp:810
NEW_FUNC[8/16]: 0x55ec8b2a4710 in (anonymous namespace)::Expr(Span<char const>&) src/script/descriptor.cpp:657
NEW_FUNC[9/16]: 0x55ec8b2a4d40 in (anonymous namespace)::Func(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, Span<char const>&) src/script/descriptor.cpp:647
NEW_FUNC[15/16]: 0x55ec8b2d7dd0 in Span<char const>::subspan(long) const src/./span.h:33
NEW_FUNC[0/1]: 0x55ec8b2d7830 in Span<char const>::operator[](long) const src/./span.h:31
NEW_FUNC[0/10]: 0x55ec8a2ea090 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<char const*>(char const*, char const* const&) src/./tinyformat.h:976
NEW_FUNC[1/10]: 0x55ec8a345d40 in void tinyformat::format<char const*>(std::ostream&, char const*, char const* const&) src/./tinyformat.h:968
NEW_FUNC[2/10]: 0x55ec8a346000 in tinyformat::detail::FormatListN<1>::FormatListN<char const*>(char const* const&) src/./tinyformat.h:885
NEW_FUNC[3/10]: 0x55ec8a3462e0 in void tinyformat::detail::FormatArg::formatImpl<char const*>(std::ostream&, char const*, char const*, int, void const*) src/./tinyformat.h:530
NEW_FUNC[4/10]: 0x55ec8a3464b0 in void tinyformat::formatValue<char const*>(std::ostream&, char const*, char const*, int, char const* const&) src/./tinyformat.h:317
NEW_FUNC[8/10]: 0x55ec8b438ef0 in ParsePrechecks(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/util/strencodings.cpp:267
NEW_FUNC[9/10]: 0x55ec8b4398b0 in ParseUInt32(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned int*) src/util/strencodings.cpp:309
NEW_FUNC[0/3]: 0x55ec8a2e9430 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > tinyformat::format<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >(char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/./tinyformat.h:976
NEW_FUNC[1/3]: 0x55ec8a33a6f0 in void tinyformat::format<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >(std::ostream&, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/./tinyformat.h:968
NEW_FUNC[2/3]: 0x55ec8a33aa40 in tinyformat::detail::FormatListN<2>::FormatListN<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/./tinyformat.h:885
NEW_FUNC[1/2]: 0x55ec8b4331b0 in IsHex(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/util/strencodings.cpp:61
NEW_FUNC[13/24]: 0x55ec8b126eb0 in Params() src/chainparams.cpp:384
NEW_FUNC[14/24]: 0x55ec8b19a500 in DecodeDestination(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) src/key_io.cpp:217
NEW_FUNC[15/24]: 0x55ec8b19a610 in (anonymous namespace)::DecodeDestination(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, CChainParams const&) src/key_io.cpp:74
NEW_FUNC[18/24]: 0x55ec8b357160 in IsValidDestination(boost::variant<CNoDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessUnknown> const&) src/script/standard.cpp:325
NEW_FUNC[19/24]: 0x55ec8b36fe40 in DecodeBase58(char const*, std::vector<unsigned char, std::allocator<unsigned char> >&) src/base58.cpp:36
stat::number_of_executed_units: 54900
stat::average_exec_per_sec: 4990
stat::new_units_added: 421
stat::slowest_unit_time_sec: 0
stat::peak_rss_mb: 412
Number of unique code paths taken during fuzzing round: 93
Tested fuzz harnesses seem to work as expected.
```
Top commit has no ACKs.
Tree-SHA512: f18d0a6798c55d2c85ef9e604af2c1d626da2b81c01ea3f77c5cecd4ce35b197030778b3cfebab4869dab84a022325dba94fd83290026bfbc59814938e1daa02
|
|
a0fc0764763329df2c4b11a4e22715ccfc0cf7d3 refactor: test/bench: dedup Build{Crediting,Spending}Transaction() (Sebastian Falbesoner)
Pull request description:
prototypes used in `src/test/script_tests.cpp`:
- `CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey, int nValue = 0);`
- `CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CScriptWitness& scriptWitness, const CTransaction& txCredit);`
prototypes used in `bench/verify_script.cpp`:
- `CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey);`
- `CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit);`
The more generic versions from the script tests are moved into `setup_common.cpp` and the calls are adapted accordingly in the verify_script benchmark (passing the nValue of 1 explicitely for `BuildCreditingTransaction()`, passing empty scriptWitness explicitely and converting txCredit parameter to CTransaction in `BuildSpendingTransaction()`).
Top commit has no ACKs.
Tree-SHA512: 8444f8a18f15070eeec1e5dfd255b55a851dfc2e6647c12b1995a6f7abd7196e830db2181d0e860bcd4cf4c815967584a3756dd450346bca70649dd1d4493e04
|
|
|
|
|
|
|
|
cd68594dcdadc195bd2ea9394fa04edfdbdf1149 Only check the hash of transactions loaded from disk (Andrew Chow)
Pull request description:
It feels unnecessary to do a full `CheckTransaction` for every transaction saved in the wallet. It should not be possible for an invalid transaction to get into the wallet in the first place, and if there is any disk corruption, the hash check will catch it.
ACKs for top commit:
MarcoFalke:
ACK cd68594dcdadc195bd2ea9394fa04edfdbdf1149
laanwj:
ACK cd68594dcdadc195bd2ea9394fa04edfdbdf1149
promag:
ACK cd68594dcdadc195bd2ea9394fa04edfdbdf1149, AFAICT the check is not needed, hash comparison gives data integrity.
Tree-SHA512: 5b2e719f76097cfbf125392db6cc6c764355c81f0b7a5b60aee4b06af1afcca80cfd38a3cf5307fd9e2c1afc405f8321929a4552943099a8161e6762965451fb
|
|
088a730fe6f893d43b55ec52e6b5080fd445dd9e static tooltip (JeremyCrookshank)
Pull request description:
I noticed that on Bitcoin sends the tooltip wasn't very clear for new users and I hope my PR is more concise. If it needs changing more will happily change too :+1:
![IMG_20191017_192739](https://user-images.githubusercontent.com/46864828/67036925-75d45380-f114-11e9-88bf-bab58161f80a.jpg)
ACKs for top commit:
laanwj:
ACK 088a730fe6f893d43b55ec52e6b5080fd445dd9e
Tree-SHA512: 2b1103ac934d8f68d22333af3c0f5d4228b665b1e507378d4ae5b83cc2b6d6aeb46a3d68298cca93feb839db5caa560322c8df5261dc2f7db5abeed9f0dd9c69
|
|
|
|
prototypes used in src/test/script_tests.cpp:
- CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey, int nValue = 0);
- CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CScriptWitness& scriptWitness, const CTransaction& txCredit);
prototypes used in bench/verify_script.cpp:
- CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey);
- CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit);
The more generic versions from the script tests are moved into a new file pair
transaction_utils.cpp/h and the calls are adapted accordingly in the
verify_script benchmark (passing the nValue of 1 explicitely for
BuildCreditingTransaction(), passing empty scriptWitness explicitely and
converting txCredit parameter to CTransaction in BuildSpendingTransaction()).
|
|
|
|
The serialization/deserialization methods for the classes CExtKey and
CExtPubKey were only used in the BIP32 unit tests, where the relevant parts are
removed as well.
|
|
|
|
facec1c643105d0ae74b5d32cf33d593f9e82a36 wallet: Avoid showing GUI popups on RPC errors (MarcoFalke)
Pull request description:
RPC errors and warnings are shown as popups in the GUI instead of being returned to the RPC caller. For example,
```
$ ./src/bitcoin-cli loadwallet $(pwd)/./test/functional/data/wallets/high_minversion/
error code: -4
error message:
Wallet loading failed.
```
gives me a GUI popup and no reason why loading the wallet failed.
After this pull request:
```
$ ./src/bitcoin-cli loadwallet $(pwd)/./test/functional/data/wallets/high_minversion/
error code: -4
error message:
Wallet loading failed: Error loading /home/marco/workspace/btc_bitcoin_core/./test/functional/data/wallets/high_minversion/wallet.dat: Wallet requires newer version of Bitcoin Core
ACKs for top commit:
laanwj:
Code review ACK facec1c643105d0ae74b5d32cf33d593f9e82a36
Tree-SHA512: c8274bbb02cfcf71676eeec1e773e51fb3538cf93f82e7cb8536f4716d44ed819cdc162dfc039ac7386a4db381a734cdb27fd32567043a1180c02519fbcba194
|
|
57e2edea0bfea664e3f12dad2508139eb7f461bc Send amount shows minimum amount placeholder (JeremyCrookshank)
Pull request description:
Noticed that there wasn't a default value for the send amount. However if you put a value in or click the up and down arrows you're unable to get it blank again, so it makes sense that it has a default value. I hope this also makes it more clear that users can send less than 1 BTC if it shows the 8 decimal places
PR:
![Capture](https://user-images.githubusercontent.com/46864828/67132088-549c6180-f1ff-11e9-9ba5-67fdcd6db894.PNG)
ACKs for top commit:
promag:
ACK 57e2edea0bfea664e3f12dad2508139eb7f461bc.
GChuf:
ACK 57e2edea0bfea664e3f12dad2508139eb7f461bc
laanwj:
ACK 57e2edea0bfea664e3f12dad2508139eb7f461bc, this is a surprisingly compact solution too
Tree-SHA512: 354590d2a88231b8649f7ae985c8a7864d74ca0e1f8603cb1730ba46747084de90ee6285ce4d39ee04b054fb9cd2d78ebc71146f3af694c37a8a3aff7f051800
|
|
e892f9648ae5f72b2020bdaa1e28901e8378e9fc random: remove call to RAND_screen() (Windows only) (fanquake)
Pull request description:
Follow up to https://github.com/bitcoin/bitcoin/pull/17151 where there were multiple calls to also remove our call to RAND_screen().
ACKs for top commit:
MarcoFalke:
unsigned ACK e892f9648ae5f72b2020bdaa1e28901e8378e9fc
laanwj:
ACK e892f9648ae5f72b2020bdaa1e28901e8378e9fc
Tree-SHA512: 1b846016d91e8113f90466b61fcaf0574edb6b4726eba1947549e2ac28907e1318d893f7b303e756f19730c8507c79b10e08d54b97153224b585ff1e0ac1953e
|
|
|
|
|
|
b5f0be38790ecc87a1e537edeca5923f608bc68c util: Filter out macOS process serial number (Hennadii Stepanov)
Pull request description:
Fix #17179
ACKs for top commit:
laanwj:
ACK b5f0be38790ecc87a1e537edeca5923f608bc68c
MarcoFalke:
unsigned ACK b5f0be38790ecc87a1e537edeca5923f608bc68c
promag:
ACK b5f0be38790ecc87a1e537edeca5923f608bc68c.
fanquake:
ACK b5f0be38790ecc87a1e537edeca5923f608bc68c - Tested that this fixes #17179.
Tree-SHA512: 84ce859e53ebc7ad2d0a45e954243ef6efee640f1e0212322f68a317e4361a216ecb4b5a3a410ab31613adc285c8d3840fbf41fa9da9019be3d734db6b9427cd
|
|
Follow up to https://github.com/bitcoin/bitcoin/pull/17151 where
there were multiple calls to also remove our call to RAND_screen().
|
|
cc3b5289ef648dca30469ee4afa615a1ed5b4e04 gui: remove OpenSSL PRNG seeding (Windows, Qt only) (fanquake)
Pull request description:
This removes the code introduced in [#4399](https://github.com/bitcoin/bitcoin/pull/4399) that attempts to add additional entroy to the OpenSSL PRNG using `RAND_event()`. 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](https://github.com/bitcoin/bitcoin/blob/master/src/random.cpp).
Note that we are still also still doing other Windows specific gathering using [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) on top of the other generation we do.
Also note that if RAND_event returns `0` here (PRNG has **NOT** been seeded with enough data), we're just logging a single message and continuing, which also seems less than ideal.
ACKs for top commit:
laanwj:
ACK cc3b5289ef648dca30469ee4afa615a1ed5b4e04
MarcoFalke:
unsigned ACK cc3b5289ef648dca30469ee4afa615a1ed5b4e04
theuni:
ACK cc3b5289ef648dca30469ee4afa615a1ed5b4e04.
Tree-SHA512: 0bb18779cf37f6670e3e5ac6a6a38e5f95199491b2684f9e56391c76f030fe1621d6df064239c2a398f228129fdf3f2220fc8cd15b2b92ecf2ea6d98a79b2175
|
|
7d8d3e6a2ad827fa916e3909a18dedb9f7fdce43 Add tests for util/vector.h's Cat and Vector (Pieter Wuille)
e65e61c812df90a56e3ce4a8e76c4b746766f387 Add some general std::vector utility functions (Pieter Wuille)
Pull request description:
This is another general improvement extracted from #16800 .
Two functions are added are:
* Vector(arg1,arg2,arg3,...) constructs a vector with the specified arguments as elements. The vector's type is derived from the arguments. If some of the arguments are rvalue references, they will be moved into place rather than copied (which can't be achieved using list initialization).
* Cat(vector1,vector2) returns a concatenation of the two vectors, efficiently moving elements when relevant.
Vector generalizes (and replaces) the `Singleton` function in src/descriptor.cpp, and `Cat` replaces the function in bech32.cpp
ACKs for top commit:
laanwj:
ACK 7d8d3e6a2ad827fa916e3909a18dedb9f7fdce43
MarcoFalke:
ACK 7d8d3e6a2ad827fa916e3909a18dedb9f7fdce43 (enjoyed reading the tests, but did not compile)
Tree-SHA512: 92325f14e90d7e7d9d920421979aec22bb0d730e0291362b4326cccc76f9c2d865bec33a797c5c0201773468c3773cb50ce52c8eee4c1ec1a4d10db5cf2b9d2a
|
|
The `state` return argument has not been set since commit 611291c198.
Remove it (and the one place that it's used in a calling function).
|
|
CommitTransaction returns a bool to indicate success, but since commit
b3a74100b8 it only returns true, even if the transaction was not
successfully broadcast. This commit changes CommitTransaction() to return
void.
All dead code in `if (!CommitTransaction())` branches has been removed.
|
|
|
|
7005d6ab8f4393545ec27f528d8c03fc4516ddab gui: Add placeholder text to the sign message field (Danny-Scott)
Pull request description:
When using the sign message functionality I noticed the "message" field had no label or placeholder text to highlight what it's for.
I've added the placeholder text to match the tool tip to help it be more user friendly.
ACKs for top commit:
hebasto:
Re-ACK 7005d6ab8f4393545ec27f528d8c03fc4516ddab
fanquake:
ACK 7005d6ab8f4393545ec27f528d8c03fc4516ddab
Tree-SHA512: 17fe51c134f6373d8d5f9ca98b15bd936da4e61aa5258ceb5d318575d49b43cbfde6f4c3f720eb5928206902e6ba52811ba08737a03c95224e45dabc947d9d11
|
|
|
|
b3b6b6f62fcabea9818e8049dba714d0d0ef8ab6 gui: don't disable the sync overlay when wallet is disabled (Ben Carman)
Pull request description:
Continuation of #13848.
When running with `-disablewallet` the sync modal is now available by clicking on the progress bar or `syncing` icon.
[Current Image of what the window looks like](https://imgur.com/6LsoT2l)
Fixes #13828.
ACKs for top commit:
jonasschnelli:
Tested ACK b3b6b6f62fcabea9818e8049dba714d0d0ef8ab6
Tree-SHA512: 325bc22a0b692bfb8fcc9d84e02dfc506146028b97b3609e23c2c45288c79b8aead1ad2e9b8d692f5f6771b4d2aee63fbe71bfaeaf17d260865da32ab3631e07
|
|
|
|
fa0467326ff004a0a20c6af9c8f6adcc74a0c8af chain: Set all CBlockIndex members to null, remove SetNull helper (MarcoFalke)
Pull request description:
The first commit removes the `SetNull` helper and inlines the member initialization (C++11). See https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#c-data-structures for rationale.
<strike>The second commit adds the `cs_main` lock annotation to `RaiseValidity`. See also #17161.</strike>
ACKs for top commit:
promag:
Code review ACK fa0467326ff004a0a20c6af9c8f6adcc74a0c8af.
practicalswift:
ACK fa0467326ff004a0a20c6af9c8f6adcc74a0c8af -- diff still looks correct :)
laanwj:
ACK fa0467326ff004a0a20c6af9c8f6adcc74a0c8af, this makes it easy to see that all fields are initialized.
Tree-SHA512: 1b2b9fb0951c03c75b9cce322b89d4ecc9a364ae78b94d91b0b4669437824394dfada820ab6f74dfac3193f602899abfdc244ae2d9351ad293f555488f03470e
|
|
85016e52f6adc01735beefe5a8aefcc4f0c4aa25 [rpc] Fix broken bitcoin-cli examples (Andrew Toth)
Pull request description:
This fixes the `bitcoin-cli` examples for `combinerawtransaction`, `combinepsbt` and `testmempoolaccept`. They currently return `Error parsing JSON`.
ACKs for top commit:
laanwj:
ACK 85016e52f6adc01735beefe5a8aefcc4f0c4aa25
Tree-SHA512: b561f68f7a188dc91dab1ceb98da3ac3e232143ab2b906c90f95c6b74b584599d0f3b51f067cdd3b1153931f95b3dc385e453b1a0dde86f9cb549b94560f219d
|
|
5013171eaf111d43ff824a212aebe40901221d6d doc: correct function name in ReportHardwareRand() (fanquake)
Pull request description:
The function is `InitHardwareRand` not `HWRandInit`.
https://github.com/bitcoin/bitcoin/blob/46d6930f8c7ba7cbcd7d86dd5d0117642fcbc819/src/random.cpp#L99
ACKs for top commit:
laanwj:
ACK 5013171eaf111d43ff824a212aebe40901221d6d
theStack:
ACK https://github.com/bitcoin/bitcoin/commit/5013171eaf111d43ff824a212aebe40901221d6d
Tree-SHA512: c25e1bb56e923961fc8a9178d751222b60f5ca36be84abf8fd1ac971f3a9b79b587ed9d8a4a175981b66f3fd5ad7edd6697d343e4dc4852351a1510718745455
|
|
f59bbb61af5055844c16939a4f8e58c4f73843c1 test: Fix bug in blockfilter_index_tests. (Jim Posen)
Pull request description:
The test case tests a chain reorganization, however the two chains were generated in the same manner and thus produced the same blocks.
This issue was [pointed out](https://github.com/bitcoin/bitcoin/pull/14121#discussion_r334282663) by MarcoFalke.
ACKs for top commit:
MarcoFalke:
Thanks! ACK f59bbb61af5055844c16939a4f8e58c4f73843c1 (looked at the diff on GitHub, didn't compile, nor run tests)
Tree-SHA512: a2f063ae9312051ffc2a3fcc1116a6a8ac09beeef261bc40aa3ff7270ff4de22a790eb19fec6b15ba1eb46e78f1f317bfd91472d8581b95bb9441a56b102554e
|
|
|
|
084e17cebd424b8e8ced674bc810eef4e6ee5d3b Remove unused includes (practicalswift)
Pull request description:
As requested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/16273#issuecomment-521332089:
This PR removes unused includes.
Please note that in contrast to #16273 I'm limiting the scope to the trivial cases of pure removals (i.e. no includes added) to make reviewing easier.
I'm seeking "Concept ACK":s for this obviously non-urgent minor cleanup.
Rationale:
* Avoids unnecessary re-compiles in case of header changes.
* Makes reasoning about code dependencies easier.
* Reduces compile-time memory usage.
* Reduces compilation time.
* Warm fuzzy feeling of being lean :-)
ACKs for top commit:
ryanofsky:
Code review ACK 084e17cebd424b8e8ced674bc810eef4e6ee5d3b. PR only removes include lines and it still compiles. In the worst case someone might have to explicitly add an include later for something now included implicitly. But maybe some effort was taken to avoid this, and it wouldn't be a tragedy anyway.
Tree-SHA512: 89de56edc6ceea4696e9579bccff10c80080821685b9fb4e8c5ef593b6e43cf662f358788701bb09f84867693f66b2e4db035b92b522a0a775f50b7ecffd6a6d
|
|
|