| Age | Commit message (Collapse) | Author |
|---|
|
-BEGIN VERIFY SCRIPT-
find_regex='((assert|CHECK_NONFATAL)\(std::addressof|TODO: REVIEW-ONLY)' \
&& git grep -l -E "$find_regex" -- . \
| xargs sed -i -E "/${find_regex}/d"
-END VERIFY SCRIPT-
|
|
CreateTransactionInternal
96c2c9520e80ee4fed92f0e1ab859d59fcbdb110 scripted-diff: Rename SelectCoinsMinConf to AttemptSelection (Andrew Chow)
b583f73354c617ede9145f9738f13cedf1c13e08 Move vin filling to before final fee setting (Andrew Chow)
d39cac0547c960df0a890e89f43b458147b4b07a Set m_subtract_fee_outputs during recipients vector loop (Andrew Chow)
364e0698a543a19e81ae407cc523970e6ed924e8 Move variable initializations to where they are used (Andrew Chow)
32ab430651594ed3d10a6ed75f19de5197f0e9b0 Move recipients vector checks to beginning of CreateTransaction (Andrew Chow)
cd1d6d3324a841087f6d5da723394e8d7df07ec7 Rename nSubtractFeeFromAmount in CreateTransaction (Andrew Chow)
dac21c793f8fbb4d5debc55ac97c406c7c93ff48 Rename nValue and nValueToSelect (Andrew Chow)
d2aee3bbc765a1f02e4ceadb2fa5928ac524f1a7 Remove extraneous scope in CreateTransactionInternal (Andrew Chow)
b2995963b5d0b9bca503b0cc69c747f4cedec1e4 Move cs_wallet lock in CreateTransactionInternal to top of function (Andrew Chow)
Pull request description:
#17331 did some refactors and cleanup of `CreateTransactionInternal` to make it easier to understand, however it is still a bit convoluted even though it doesn't have to be. This PR does additional cleanup and refactoring to `CreateTransactionInternal` so that it is easier to understand. Some unnecessary code was removed, some variables moved around to where they matter, and several indents removed.
ACKs for top commit:
glozow:
reACK 96c2c95
ryanofsky:
Code review ACK 96c2c9520e80ee4fed92f0e1ab859d59fcbdb110 also acked previously (was reverted).
meshcollider:
re-utACK 96c2c9520e80ee4fed92f0e1ab859d59fcbdb110
Tree-SHA512: 3dba67ed436968a07bfd82d435d566ad74e116c6e50ac9baed7144a46ad5c0f630b1ba59d91e8e8972ac2af559d7c0576f0560f09684d2ab20fad6689902866f
|
|
44d05d0a69c14ed295b0a7f6c8ec4379d44155e4 test: remove sanitizer suppression for nanobench (Martin Ankerl)
e3c866e3ca85f841671a828712e6207e24d0d996 test: update nanobench from release 4.0.0 to 4.3.4 (Martin Ankerl)
Pull request description:
This updates the third-party library nanobench with the latest release. It contains mostly minor bugfixes, a new pyperf output format, ability to suppress warnings with environment variable `NANOBENCH_SUPPRESS_WARNINGS`. Full changelog:
v4.0.2
* Changed `doNotOptimizeAway` to what google benchmark is doing. The old code did not work on some machines.
* fix: display correct "total" value
* minor Documentation updates
v4.1.0
* Updated link to new pyperf home
* Adds ability to configure console output time unit
* Add support for environment variable `NANOBENCH_SUPPRESS_WARNINGS`
* Nanobench is now usable with CMake's FetchContent (see documentation: https://nanobench.ankerl.com/tutorial.html#cmake-integration)
v4.2.0
* Ability to store and later compare results added, through `pyperf`.
* See https://nanobench.ankerl.com/tutorial.html#pyperf-python-pyperf-module-output
* Added lots of build targets to travis, similar to bitcoin's build.
* Some minor API & documentation improvements
v4.3.0
* `ankerl::nanobench::Rng` can now return the state with `std::vector<uint64_t> Rng::state()`, and this can also be used to initialize the Rng.
v4.3.1
* Minor cmake improvements when integrationg as a third-party library: add alias `nanobench::nanobench`, default to C++17
v4.3.2
* Fixed a MSVC 2015 build problem
* updates license to 2021.
* build should now work with very old linux headers
* Also disable UBSAN (bitcoin needed to add a suppression)
v4.3.3
* Do not use locale-dependent `std::to_string`
v4.3.4
* Add missing sanitizer suppression to `rotl`
ACKs for top commit:
MarcoFalke:
review ACK 44d05d0a69c14ed295b0a7f6c8ec4379d44155e4
Tree-SHA512: 3291c85057720cfc84a44bfaa305a7d0df4dc35779169d20de73d32e40d4cdbf3f005bf343f79710eca517441de2459e8118c195c5f5136f99d1f50ebd5dfd08
|
|
auxiliary modules
7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a index: refactor-only: Reuse CChain ref (Carl Dong)
db33cde80fff749c6adff9e91fca5f27f4bb6278 index: Add chainstate member to BaseIndex (Carl Dong)
f4a47a1febfa35ab077f2a841fe31a8cd9618250 bench: Use existing chainman in AssembleBlock (Carl Dong)
91226eb91769aad5a63bc671595e1353a2b2247a bench: Use existing NodeContext in DuplicateInputs (Carl Dong)
e6b4aa6eb53dc555ecab2922af35e7a2572faf4f miner: Pass in chainman to RegenerateCommitments (Carl Dong)
9ecade14252ad1972f668d2d2e4ef44fdfcb944a rest: Add GetChainman function and use it (Carl Dong)
fc1c282845f6b8436d1ea4c68eb3511034c29bea rpc/blockchain: Use existing blockman in gettxoutsetinfo (Carl Dong)
Pull request description:
Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)
The first 2 commits are fixups addressing review for the last bundle: #21391
NEW note:
1. I have opened #21766 which keeps track of potential improvements where the flaws already existed before the de-globalization work, please post on that issue about these improvements, thanks!
Note to reviewers:
1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
3. Remove `old_function`
ACKs for top commit:
jarolrod:
ACK 7a799c9
ariard:
Code Review ACK 7a799c9
fjahr:
re-ACK 7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a
MarcoFalke:
review ACK 7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a 🌠
ryanofsky:
Code review ACK 7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a. Basically no change since last review except fixed rebase conflicts and a new comment about REST Ensure()
jamesob:
conditional ACK 7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a ([`jamesob/ackr/21767.1.dongcarl.bundle_6_n_prune_g_chai`](https://github.com/jamesob/bitcoin/tree/ackr/21767.1.dongcarl.bundle_6_n_prune_g_chai))
Tree-SHA512: 531c00ddcb318817457db2812d9a9d930bc664e58e6f7f1c746350732b031dd624270bfa6b9f49d8056aeb6321d973f0e38e4ff914acd6768edd8602c017d10e
|
|
This updates the third-party library nanobench with the latest release. It contains mostly minor bugfixes, a new pyperf output format, ability to suppress warnings with environment variable `NANOBENCH_SUPPRESS_WARNINGS`. Full changelog:
v4.0.2
* Changed `doNotOptimizeAway` to what google benchmark is doing. The old code did not work on some machines.
* fix: display correct "total" value
* minor Documentation updates
v4.1.0
* Updated link to new pyperf home
* Adds ability to configure console output time unit
* Add support for environment variable `NANOBENCH_SUPPRESS_WARNINGS`
* Nanobench is now usable with CMake's FetchContent (see documentation: https://nanobench.ankerl.com/tutorial.html#cmake-integration)
v4.2.0
* Ability to store and later compare results added, through `pyperf`.
* See https://nanobench.ankerl.com/tutorial.html#pyperf-python-pyperf-module-output
* Added lots of build targets to travis, similar to bitcoin's build.
* Some minor API & documentation improvements
v4.3.0
* `ankerl::nanobench::Rng` can now return the state with `std::vector<uint64_t> Rng::state()`, and this can also be used to initialize the Rng.
v4.3.1
* Minor cmake improvements when integrationg as a third-party library: add alias `nanobench::nanobench`, default to C++17
v4.3.2
* Fixed a MSVC 2015 build problem
* updates license to 2021.
* build should now work with very old linux headers
* Also disable UBSAN (bitcoin needed to add a suppression)
v4.3.3
* Do not use locale-dependent `std::to_string`
v4.3.4
* Add missing sanitizer suppression to `rotl`
|
|
SelectCoinsMinConf is a bit of a misnomer now since it really just does
all of the coin selection given some parameters. So rename this to
something less annoying to say and makes a bit more sense.
-BEGIN VERIFY SCRIPT-
sed -i 's/SelectCoinsMinConf/AttemptSelection/g' $(git grep -l SelectCoinsMinConf ./src)
-END VERIFY SCRIPT-
|
|
|
|
|
|
51a3ac242c92e69b59df26f8f9e287b31e5c3b0f Have OutputGroup determine the value to use (Andrew Chow)
6d6d2784759878ef0c4ac128d12aac68add1edca Change SelectCoins_test to actually test SelectCoins (Andrew Chow)
9d3bd74ab4430532d6e53eef8cf77ad999044b14 Remove CreateTransaction while loop and some related variables (Andrew Chow)
6f0d5189af4c881fe8b97a0c28ce1ffa33480715 Remove use_bnb and bnb_used (Andrew Chow)
de26eb0e1fa2b6f03c58ba104d00f7a8ffead39c Do both BnB and Knapsack coin selection in SelectCoinsMinConf (Andrew Chow)
01dc8ebda50a382d45d3d169b2c3f3965869dcae Have KnapsackSolver actually use effective values (Andrew Chow)
bf26e018de33216d6f0ed0d6ff822b93536f7cc1 Roll static tx fees into nValueToSelect instead of having it be separate (Andrew Chow)
cc3f14b27c06b7a0da1472f5c7100c3f0b76fd98 Move output reductions for fee to after coin selection (Andrew Chow)
d97d25d95006725e705635530b27643363d6b2a4 Make cost_of_change part of CoinSelectionParams (Andrew Chow)
af5867c89688b06173b295b7c32a42845ea455da Move some calculations to common code in SelectCoinsMinConf (Andrew Chow)
1bf4a62cb61bd4b91d9cd4e379fea2b914786342 scripted-diff: rename some variables (Andrew Chow)
Pull request description:
Changes `KnapsackSolver` to use effective values instead of just the nominal txout value. Since fees are taken into account during the selection itself, we finally get rid of the `CreateTransaction` loop as well as a few other things that only were only necessary because of that loop.
This should not change coin selection behavior at all (except maybe remove weird edge cases that were caused by the loop). In order to keep behavior the same, `KnapsackSolver` will select outputs with a negative effective value (as it did before).
ACKs for top commit:
ryanofsky:
Code review ACK 51a3ac242c92e69b59df26f8f9e287b31e5c3b0f. Looks good to go!
instagibbs:
review ACK https://github.com/bitcoin/bitcoin/pull/17331/commits/51a3ac242c92e69b59df26f8f9e287b31e5c3b0f
meshcollider:
re-light-utACK 51a3ac242c92e69b59df26f8f9e287b31e5c3b0f
Tree-SHA512: 372c27e00edcd5dbf85177421ba88f20bfdaf1791b6e3dc022c44876ecc379403e2375ed69e71c512c49e6af87641001ff385c4b25ab93684b3a08a53bf3824e
|
|
getnodeaddresses by network
ce6bca88e8c685c69686e0b8dc095ffc3e2ac34d doc: release note for getnodeaddresses by network (Jon Atack)
3f89c0e9902338ad8a507a938dceeeb3191eece6 test: improve getnodeaddresses coverage, test by network (Jon Atack)
6c98c099918bd20e2d3aa123643d6e3594e080e4 rpc: enable filtering getnodeaddresses by network (Jon Atack)
80ba294854e5025bcada58f1403858e6ea1d4380 p2p: allow CConnman::GetAddresses() by network, add doxygen (Jon Atack)
a49f3ddbbabfb971a537f0a6c7affb24e20ff192 p2p: allow CAddrMan::GetAddr() by network, add doxygen (Jon Atack)
c38981e748f438d972ba12ba998c8a8a597e01c1 p2p: pull time call out of loop in CAddrMan::GetAddr_() (João Barbosa)
d35ddca91ebbcf8d8b790c3b9f8cf218fafb7a53 p2p: enable CAddrMan::GetAddr_() by network, add doxygen (Jon Atack)
Pull request description:
This patch allows passing a network argument to CAddrMan::GetAddr(), CConnman::GetAddresses(), and rpc getnodeaddresses to return only addresses of that network.
It also contains a performance optimisation by promag.
ACKs for top commit:
laanwj:
Code review and lightly tested ACK ce6bca88e8c685c69686e0b8dc095ffc3e2ac34d
vasild:
ACK ce6bca88e8c685c69686e0b8dc095ffc3e2ac34d
Tree-SHA512: 40e700d97091248429c73cbc0639a1f03ab7288e636a7b9026ad253e9708253c6b2ec98e7d9fb2d56136c0f762313dd648915ac98d723ee330d713813a43f99d
|
|
These booleans are no longer needed
|
|
The fees for transaction overhead and recipient outputs are now included
in nTargetValue instead of being a separate parameter. For the coin
selection algorithms, it doesn't matter that these are separate as in
either case, the algorithm needs to select enough to cover these fees.
Note that setting nValueToSelect is changed as it now includes
not_input_fees. Without the change to how nValueToSelect is increased
for KnapsackSolver, this would result in overpaying fees. The change to
increase by the difference between nFeeRet and not_input_fees allows
this to have the same behavior as previously.
Additionally, because we assume that KnapsackSolver will always find a
solution that requires change (we assume that BnB always finds a
non-change solution), we also include the fee for the change output in
KnapsackSolver's target. As part of this, we also use the changeless
nFeeRet when iterating for KnapsackSolver. This is because we include
the change fee when doing KnapsackSolver, so nFeeRet on further
iterations won't include the change fee.
|
|
|
|
|
|
|
|
725d7ae0494d4a45f5a840bbbd19c008a7363965 Use PrecomputedTransactionData in signet check (Pieter Wuille)
497718b467330b2c6bb0d44786020c55f1aa75f9 Treat amount<0 also as missing data for P2WPKH/P2WSH (Pieter Wuille)
3820090bd619ac85ab35eff376c03136fe4a9f04 Make all SignatureChecker explicit about missing data (Pieter Wuille)
b77b0cc507bdc716e5236b1d9880e648147e0af9 Add MissingDataBehavior and make TransactionSignatureChecker handle it (Pieter Wuille)
Pull request description:
Currently we have 2 levels of potentially-missing data in the transaction signature hashes:
* P2WPKH/P2WSH hashes need the spent amount
* P2TR hashes need all spent outputs (amount + scriptPubKey)
Missing amounts are treated as -1 (thus leading to unexpected signature failures), while missing outputs in P2TR validation cause assertion failure. This is hard to extend for signing support, and also quite ugly in general.
In this PR, an explicit configuration option to {Mutable,}TransactionSignatureChecker is added (MissingDataBehavior enum class) to either select ASSERT_FAIL or FAIL. Validation code passes ASSERT_FAIL (as at validation time all data should always be passed, and anything else is a serious bug in the code), while signing code uses FAIL.
The existence of the ASSERT_FAIL option is really just an abundance of caution. Always using FAIL should be just fine, but if there were for some reason a code path in consensus code was introduced that misses certain data, I think we prefer as assertion failure over silently introducing a consensus change.
Potentially useful follow-ups (not for this PR, in my preference):
* Having an explicit script validation error code for missing data.
* Having a MissingDataBehavior::SUCCEED option as well, for use in script/sign.cpp DataFromTransaction (if a signature is present in a witness, and we don't have enough data to fully validate it, we should probably treat it as valid and not touch it).
ACKs for top commit:
sanket1729:
reACK 725d7ae0494d4a45f5a840bbbd19c008a7363965
Sjors:
ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965
achow101:
re-ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965
benthecarman:
ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965
fjahr:
Code review ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965
Tree-SHA512: d67dc51bae9ca7ef6eb9acccefd682529f397830f77d74cd305500a081ef55aede0e9fa380648c3a8dd4857aa7eeb1ab54fe808979d79db0784ac94ceb31b657
|
|
Remove the implicit MissingDataBehavior::ASSERT_FAIL in the
*TransationSignatureChecker constructors, and instead specify
it explicit in all call sites:
* Test code uses ASSERT_FAIL
* Validation uses ASSERT_FAIL (through CachingTransactionSignatureChecker)
(including signet)
* libconsensus uses FAIL, matching the existing behavior of the
non-amount API (and the extended required data for taproot validation
is not available yet)
* Signing code uses FAIL
|
|
03346022d611871f2cc185440b19d928b9264d9d naming nits (Fabian Jahr)
2e7c80fb5be82ad4a3f737cab65b31f70a772a23 Add signet support to gen_key_io_test_vectors.py (Pieter Wuille)
fe5e495c31de47b0ec732b943db11fe345d874af Use Bech32m encoding for v1+ segwit addresses (Pieter Wuille)
25b1c6e13ddf1626210d5e3d37298d1f3a78a94f Add Bech32m test vectors (Pieter Wuille)
da2bb6976dadeec682d163c258c9afecc87d6428 Implement Bech32m encoding/decoding (Pieter Wuille)
Pull request description:
This implements [BIP 350](https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki):
* For segwit v1+ addresses, a new checksum algorithm called Bech32m is used.
* Segwit v0 address keep using Bech32 as specified in [BIP 173](https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki).
ACKs for top commit:
Sjors:
utACK 0334602
jnewbery:
utACK 03346022d6
achow101:
ACK 0334602
fjahr:
re-ACK 0334602
benthecarman:
ACK 03346022d611871f2cc185440b19d928b9264d9d
Tree-SHA512: 4424cfd44869d813d6152fb3ed867b204036736bc2344a039b93700b6f36a43e9110478f138eb81c97c77ab27ecb776dada5ba632cb5a3a9d244924d2540a557
|
|
ebc4ab721b0371c0ef217c0f5bd7d42613e951e6 refactor: post Optional<> removal cleanups (fanquake)
57e980d13ca488031bde6ef197cf34d493d36796 scripted-diff: remove Optional & nullopt (fanquake)
Pull request description:
Same rationale & motivation as #21404, which turned out to be quite low in the number of potential conflicts. Lets see what the bot has to say here.
ACKs for top commit:
practicalswift:
cr ACK ebc4ab721b0371c0ef217c0f5bd7d42613e951e6: patch looks correct
jnewbery:
utACK ebc4ab721b0371c0ef217c0f5bd7d42613e951e6
laanwj:
Code review ACK ebc4ab721b0371c0ef217c0f5bd7d42613e951e6
Tree-SHA512: 550fbeef09b9d35ddefaa805d1755c18c8fd499c4b0f77ebfece8c20296a7abd1cf6c699e2261f92fe3552deeb7555ec2a2287ffe3ab9e98bb9f8612a4d43be3
|
|
|
|
selection
f9cd2bfbccb7a2b8ff07cec5f6d2adbeca5f07c3 Rename CoinSelectionParams::effective_fee to m_effective_feerate (Andrew Chow)
bdd0c2934b7f389ffcfae3b602ee3ecee8581acd wallet: Move discard feerate fetching to CreateTransaction (Andrew Chow)
448d04b931f86941903e855f831249ff5ec77485 wallet: Move long term feerate setting to CreateTransaction (Andrew Chow)
e2f429e6bbf7098f278c0247b954ecd3ba53cf37 wallet: Replace nFeeRateNeeded with effective_fee (Andrew Chow)
1a6a0b0dfb90f9ebd4b86d7934c6aa5594974f5f wallet: Use existing feerate instead of getting a new one (Andrew Chow)
Pull request description:
During coin selection, there are various places where we need to have a feerate. We need the feerate for the transaction itself, the discard fee rate, and long term feerate. Fetching these each time we need them can lead to a race condition where two feerates that should be the same are actually different. One particular instance where this can happen is during the loop in `CreateTransactionInternal`. After inputs are chosen, the expected transaction fee is calculated using a newly fetched feerate. If `pick_new_inputs == false`, the loop will go again with the assumption that the fee for the transaction remains the same. However because the feerate is fetched again, it is possible that it actually isn't and this causes coin selection to fail.
Instead of fetching the feerate each time it is needed, we fetch them all at once at the top of `CreateTransactionInternal`, store them in `CoinSelectionParams`, and use them where needed.
While some of these fee rates probably don't need this caching, I've done it for consistency and the guarantee that they remain the same.
Fixes #19229
ACKs for top commit:
glozow:
reACK https://github.com/bitcoin/bitcoin/commit/f9cd2bfbccb7a2b8ff07cec5f6d2adbeca5f07c3
fjahr:
Code review re-ACK f9cd2bfbccb7a2b8ff07cec5f6d2adbeca5f07c3
Xekyo:
tACK https://github.com/bitcoin/bitcoin/pull/21083/commits/f9cd2bfbccb7a2b8ff07cec5f6d2adbeca5f07c3
meshcollider:
Code review + test run ACK f9cd2bfbccb7a2b8ff07cec5f6d2adbeca5f07c3
Tree-SHA512: be83ff64ba473c3cdd3469c812e214659b6e2a9584c22ed2b1595618fce0d4b35d0901e61068cd1069fc1a8fb911db01dd7312d05c3b8cbafbe2504ab7a3e863
|
|
It's a feerate, not a fee. Also follow the style guide for member names.
|
|
Instead of fetching the discard feerate for each SelectCoinsMinConf
iteration, fetch and cache it once during CreateTransaction so that it
is shared for each SelectCoinsMinConf through
coin_selection_params.m_discard_feerate.
Does not change behavior.
|
|
Instead of setting the long term feerate for each SelectCoinsMinConf
iteration, set it once during CreateTransaction and let it be shared
with each SelectCoinsMinConf through
coin_selection_params.m_long_term_feerate.
Does not change behavior.
|
|
|
|
-BEGIN VERIFY SCRIPT-
git rm src/optional.h
sed -i -e 's/Optional</std::optional</g' $(git grep -l 'Optional<' src)
sed -i -e 's/{nullopt}/{std::nullopt}/g' $(git grep -l 'nullopt' src)
sed -i -e 's/ nullopt;/ std::nullopt;/g' $(git grep -l 'nullopt' src)
sed -i -e 's/ nullopt)/ std::nullopt)/g' $(git grep -l 'nullopt' src)
sed -i -e 's/(nullopt)/(std::nullopt)/g' $(git grep -l 'nullopt' src)
sed -i -e 's/ nullopt,/ std::nullopt,/g' $(git grep -l 'nullopt' src)
sed -i -e 's/? nullopt :/? std::nullopt :/g' $(git grep -l 'nullopt' src)
sed -i -e 's/: nullopt}/: std::nullopt}/g' $(git grep -l 'nullopt' src)
sed -i -e '/optional.h \\/d' src/Makefile.am
sed -i -e '/#include <optional.h>/d' src/test/fuzz/autofile.cpp src/test/fuzz/buffered_file.cpp src/test/fuzz/node_eviction.cpp
sed -i -e 's/#include <optional.h>/#include <optional>/g' $(git grep -l '#include <optional.h>' src)
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
git rm src/util/memory.h
sed -i -e 's/MakeUnique/std::make_unique/g' $(git grep -l MakeUnique src)
sed -i -e '/#include <util\/memory.h>/d' $(git grep -l '#include <util/memory.h>' src)
sed -i -e '/util\/memory.h \\/d' src/Makefile.am
-END VERIFY SCRIPT-
|
|
Can be reviewed with --color-moved=dimmed-zebra
|
|
e3e0a2432c587ee06e469c37ffae133b7ac55c77 Add benchmark to write JSON into a string (Martin Ankerl)
Pull request description:
The benchmark `BlockToJsonVerbose` only tests generating (and destroying)
the JSON data structure, but serializing into a string is also a
performance critical aspect of the RPC calls.
Extracts test setup into a `struct TestBlockAndIndex`, and uses it in
both `BlockToJsonVerbose` and `BlockToJsonVerboseWrite`.
Also, use `ankerl::nanobench::doNotOptimizeAway` to make sure the compiler
can't optimize the result of the calls away.
Here are benchmark results on my Intel i7-8700:
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 71,807,017.00 | 13.93 | 0.4% | 555,782,961.00 | 220,788,645.00 | 2.517 | 102,279,341.00 | 0.4% | 0.80 | `BlockToJsonVerbose`
| 27,916,835.00 | 35.82 | 0.1% | 235,084,034.00 | 89,033,525.00 | 2.640 | 42,911,139.00 | 0.3% | 0.32 | `BlockToJsonVerboseWrite`
ACKs for top commit:
laanwj:
Code review ACK e3e0a2432c587ee06e469c37ffae133b7ac55c77
Tree-SHA512: bc4d6d1588d47d4bd7af8e7908e44b8561bc391a2d73eccd7c0aa37fc40e8a9ce1fa1f3c29b416eef24a73c6bce3036839c0bbfe1b8dbd6d1bba3718b7ca5383
|
|
-BEGIN VERIFY SCRIPT-
find_regex='\bAcceptToMemoryPool\(' \
&& git grep -l -E "$find_regex" -- src \
| grep -v '^src/validation\.\(cpp\|h\)$' \
| xargs sed -i -E 's@'"$find_regex"'@\0::ChainstateActive(), @g'
-END VERIFY SCRIPT-
|
|
e829c9afbf75e930db6c3fe77a269b0af5e7a3ad refactor: replace sizeof(a)/sizeof(a[0]) by std::size (C++17) (Sebastian Falbesoner)
365539c84691d470b44d35df374d8c049f8c1192 refactor: init vectors via std::{begin,end} to avoid pointer arithmetic (Sebastian Falbesoner)
63d4ee1968144cc3d115f92baef95785abf813ac refactor: iterate arrays via C++11 range-based for loops if idx is not needed (Sebastian Falbesoner)
Pull request description:
This refactoring PR picks up the idea of #19626 and replaces all occurences of `sizeof(x)/sizeof(x[0])` (or `sizeof(x)/sizeof(*x)`, respectively) with the now-available C++17 [`std::size`](https://en.cppreference.com/w/cpp/iterator/size) (as [suggested by sipa](https://github.com/bitcoin/bitcoin/pull/19626#issuecomment-666487228)), making the macro `ARRAYLEN` obsolete.
As preparation for this, two other changes are done to eliminate `sizeof(x)/sizeof(x[0])` usage:
* all places where arrays are iterated via an index are changed to use C++11 range-based for loops If the index' only purpose is to access the array element (as [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/19626#discussion_r463404541)).
* `std::vector` initializations are done via `std::begin` and `std::end` rather than using pointer arithmetic to calculate the end (also [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/20429#discussion_r567418821)).
ACKs for top commit:
practicalswift:
cr ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad: patch looks correct
fanquake:
ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad
MarcoFalke:
review ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad 🌩
Tree-SHA512: b01d32c04b9e04d562b7717cae00a651ec9a718645047a90761be6959e0cc2adbd67494e058fe894641076711bb09c3b47a047d0275c736f0b2218e1ce0d193d
|
|
The benchmark BlockToJsonVerbose only tests generating (and destroying)
the JSON data structure, but serializing into a string is also a
performance critical aspect of the RPC calls.
Also, use ankerl::nanobench::doNotOptimizeAway to make sure the compiler
can't optimize the result of the calls away.
|
|
53e716ea119658c28935fee24eb50090907c500e [refactor] improve style for touched code (gzhao408)
174cb5330af4b09f3a66974d3bae783ea43b190e [refactor] const ATMPArgs and non-const Workspace (gzhao408)
f82baf0762f60c2ca5ffc339b095f9271d7c2f33 [refactor] return MempoolAcceptResult (gzhao408)
9db10a55061e09021ff8ea1d6637d99f7959035f [refactor] clean up logic in testmempoolaccept (gzhao408)
Pull request description:
This is the first 4 commits of #20833, and does refactoring only. It should be relatively simple to review, and offers a few nice things:
- It makes accessing values that don't make sense (e.g. fee) when the tx is invalid an error.
- Returning `MempoolAcceptResult` from ATMP makes the interface cleaner. The caller can get a const instead of passing in a mutable "out" param.
- We don't have to be iterating through a bunch of lists for package validation, we can just return a `std::vector<MempoolAcceptResult>`.
- We don't have to refactor all ATMP call sites again if/when we want to return more stuff from it.
ACKs for top commit:
MarcoFalke:
ACK 53e716ea119658c28935fee24eb50090907c500e 💿
jnewbery:
Code review ACK 53e716ea119658c28935fee24eb50090907c500e
ariard:
Code Review ACK 53e716e, I did tweak a bit the touched paths to see if we had good test coverage. Didn't find holes.
Tree-SHA512: fa6ec324a08ad9e6e55948615cda324cba176255708bf0a0a0f37cedb7a75311aa334ac6f223be7d8df3c7379502b1081102b9589f9a9afa1713ad3d9ab3c24f
|
|
This creates a cleaner interface with ATMP, allows us to make results const,
and makes accessing values that don't make sense (e.g. fee when tx is
invalid) an error.
|
|
e99db77a6e73996d33d7108f8336938dd57037a7 Drop boost/preprocessor dependencies (Hennadii Stepanov)
12f5028d4957bce3ba176e80527894b497c04a3b refactor: Move STRINGIZE macro to macros.h (Hennadii Stepanov)
Pull request description:
Use own macros instead of boost's ones.
ACKs for top commit:
laanwj:
Code review ACK e99db77a6e73996d33d7108f8336938dd57037a7
practicalswift:
cr ACK e99db77a6e73996d33d7108f8336938dd57037a7
Tree-SHA512: 7ec15c2780a661e293c990f64c41b5b451d894cc191aa7872fbcaf96da91915a351209b1f1003ab12a7a16cb464e50ac58a028db02beeedfa5f6931752c2d9e2
|
|
|
|
fa292724598c273867bc6dbf311f1440fe2541ba Remove redundant MakeUCharSpan wrappers (MarcoFalke)
faf4aa2f47c0de4f3a0c5f5fe5b3ec32f611eefd Remove CDataStream::Init in favor of C++11 member initialization (MarcoFalke)
fada14b948cac147198e3b685b5dd8cb72dc2911 Treat CDataStream bytes as uint8_t (MarcoFalke)
fa8bdb048e65cae2d26bea3f991717a856e2fb39 refactor: Drop CDataStream constructors in favor of one taking a Span of bytes (MarcoFalke)
faa96f841fe45bc49ebb6e07ac82a129fa9c40bf Remove unused CDataStream methods (MarcoFalke)
Pull request description:
Using `uint8_t` for raw bytes has a style benefit:
* The signedness is clear from reading the code, as it does not depend on the architecture
Other clean-ups in this pull include:
* Remove unused methods
* Constructor is simplified with `Span`
* Remove `Init()` member in favor of C++11 member initialization
ACKs for top commit:
laanwj:
code review ACK fa292724598c273867bc6dbf311f1440fe2541ba
theStack:
ACK fa292724598c273867bc6dbf311f1440fe2541ba 🍾
Tree-SHA512: 931ee28bd99843d7e894b48e90e1187ffb0278677c267044b3c0c255069d9bbd9298ab2e539b1002a30b543d240450eaec718ef4ee95a7fd4be0a295e926343f
|
|
eligibility on grouping
5d4597666d589e39354e0d8dd5b2afbe1a5d7d8e Rewrite OutputGroups to be clearer and to use scriptPubKeys (Andrew Chow)
f6b305273910db0e46798d361413a7e878cb45f7 Explicitly filter out partial groups when we don't want them (Andrew Chow)
416d74fb1687ae1d47a58c153d09d9afe0b6dc60 Move OutputGroup positive only filtering into Insert (Andrew Chow)
d895e98b594b873f3d34c8ba63e9b55125d51b5a Move EligibleForSpending into GroupOutputs (Andrew Chow)
99b399aba5d27476b61b4865cc39553d03965d57 Move fee setting of OutputGroup to Insert (Andrew Chow)
6148a8acda5e594bb9b3b2d989056f9e03ddbdbd Move GroupOutputs into SelectCoinsMinConf (Andrew Chow)
2acad036575ec998f8bbe4f10f6206b1c8ad3d23 Remove OutputGroup non-default constructors (Andrew Chow)
Pull request description:
Even after #17458, we still deal with setting fees of an `OutputGroup` and filtering the `OutputGroup` outside of the struct. We currently make all of the `OutputGroup`s in `SelectCoins` and then copy and modify them within each `SelectCoinsMinConf` scenario. This PR changes this to constructing the `OutputGroup`s within the `SelectCoinsMinConf` so that the scenario can be taken into account during the group construction. Furthermore, setting of fees and filtering for effective value is moved into `OutputGroup::Insert` itself so that we don't add undesirable outputs to an `OutputGroup` rather than deleting them afterwards.
To facilitate fee calculation and effective value filtering during `OutputGroup::Insert`, `OutputGroup` now takes the feerates in its constructor and computes the fees and effective value for each output during `Insert`.
While removing `OutputGroup`s in accordance with the `CoinEligibilityFilter` still requires creating the `OutputGroup`s first, we can do that within the function that makes them - `GroupOutput`s.
ACKs for top commit:
Xekyo:
Code review ACK: https://github.com/bitcoin/bitcoin/pull/20040/commits/5d4597666d589e39354e0d8dd5b2afbe1a5d7d8e
fjahr:
Code review ACK 5d4597666d589e39354e0d8dd5b2afbe1a5d7d8e
meshcollider:
Light utACK 5d4597666d589e39354e0d8dd5b2afbe1a5d7d8e
Tree-SHA512: 35965b6d49a87f4ebb366ec4f00aafaaf78e9282481ae2c9682b515a3a9f2cbcd3cd6e202fee29489d48fe7f3a7cede4270796f5e72bbaff76da647138fb3059
|
|
|
|
bb6fcc75d1ec94b733d1477c816351c50be5faf9 refactor: Drop boost::thread stuff in CCheckQueue (Hennadii Stepanov)
6784ac471bb32b6bb8e2de60986f123eb4990706 bench: Use CCheckQueue local thread pool (Hennadii Stepanov)
dba30695fc42f45828db008e7e5b81cb2b5d8551 test: Use CCheckQueue local thread pool (Hennadii Stepanov)
01511776acb0c7ec216dc9c8112531067763f1cb Add local thread pool to CCheckQueue (Hennadii Stepanov)
0ef938685b5c079a6f5a98daf0e3865d718d817b refactor: Use member initializers in CCheckQueue (Hennadii Stepanov)
Pull request description:
This PR:
- gets rid of `boost::thread_group` in the `CCheckQueue` class
- allows thread safety annotation usage in the `CCheckQueue` class
- is alternative to #14464 (https://github.com/bitcoin/bitcoin/pull/18710#issuecomment-616618525, https://github.com/bitcoin/bitcoin/pull/18710#issuecomment-617291612)
Also, with this PR (I hope) it could be easier to resurrect a bunch of brilliant ideas from #9938.
Related: #17307
ACKs for top commit:
laanwj:
Code review ACK bb6fcc75d1ec94b733d1477c816351c50be5faf9
LarryRuane:
ACK bb6fcc75d1ec94b733d1477c816351c50be5faf9
jonatack:
Code review ACK bb6fcc75d1ec94b733d1477c816351c50be5faf9 and verified rebase to master builds cleanly with unit/functional tests green
Tree-SHA512: fddeb720d5a391b48bb4c6fa58ed34ccc3f57862fdb8e641745c021841c8340e35c5126338271446cbd98f40bd5484f27926aa6c3e76fa478ba1efafe72e73c1
|
|
Currently it was not possible to run just the BlockToJsonVerboes benchmarsk because it did not set up everything it needed, running `bench_bitcoin -filter=BlockToJsonVerbose` caused this assert to fail:
```
bench_bitcoin: chainparams.cpp:506: const CChainParams& Params(): Assertion `globalChainParams' failed.
```
Initializing TestingSetup fixes this.
|
|
9815332d5158d69a94abeaf465a2c07bd8e43359 test: Change MuHash Python implementation to match cpp version again (Fabian Jahr)
01297fb3ca57e4b8cbc5a89fc7c6367de33b0bc6 fuzz: Add MuHash consistency fuzz test (Fabian Jahr)
b111410914041b72961536c3e4037eba103a8085 test: Add MuHash3072 fuzz test (Fabian Jahr)
c1225273857f9fa2e2276396e3f8b3ea48306df3 bench: Add Muhash benchmarks (Fabian Jahr)
7b1242229d1fcc9277238a3aefb3431061c82bfa test: Add MuHash3072 unit tests (Fabian Jahr)
adc708c98dbf03b1735edc91f813a36580781a95 crypto: Add MuHash3072 implementation (Fabian Jahr)
0b4d290bf5b0a4d156c523431bf89aaa9ffe92e5 crypto: Add Num3072 implementation (Fabian Jahr)
589f958662a2dcaacdb9a66f1088c74828a39577 build: Check for 128 bit integer support (Fabian Jahr)
Pull request description:
This is the first split of #18000 which implements the Muhash algorithm and uses it to calculate the UTXO set hash in `gettxoutsetinfo`.
ACKs for top commit:
laanwj:
Code review ACK 9815332d5158d69a94abeaf465a2c07bd8e43359
Tree-SHA512: 4bc090738f0e3d80b74bdd8122e24a8ce80121120fd37c7e4335a73e7ba4fcd7643f2a2d559e2eebf54b8e3a3bd5f12cfb27ba61ded135fda210a07a233eae45
|
|
Co-authored-by: Peter Yordanov <ppyordanov@yahoo.com>
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
|
|
Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
|
|
|
|
5021810650afc3073c2af6953ff046ad4d27a1fc Make CanFlushToDisk a const member function (practicalswift)
281cf995547f7683a9e9186bc6384a9fb6035d10 Do not run functions with necessary side-effects in assert() (practicalswift)
Pull request description:
Do not run functions with necessary side-effects in `assert()`.
ACKs for top commit:
laanwj:
Code review ACK 5021810650afc3073c2af6953ff046ad4d27a1fc
sipa:
utACK 5021810650afc3073c2af6953ff046ad4d27a1fc
theStack:
Code Review ACK 5021810650afc3073c2af6953ff046ad4d27a1fc 🟢
Tree-SHA512: 38b7faccc2f16a499f9b7b1b962b49eb58580b2a2bbf63ea49dcc418a5ecc8f21a0972fa953f66db9509c7239af67cfa2f9266423fd220963d091034d7332b96
|
|
Suggested https://github.com/bitcoin/bitcoin/pull/19425#discussion_r456236407
|
|
|
