aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2021-09-06Merge bitcoin/bitcoin#22736: log, sync: change lock contention from ↵MarcoFalke
preprocessor directive to log category 7e698732836121912f179b7c743a72dd6fdffa72 sync: remove DEBUG_LOCKCONTENTION preprocessor directives (Jon Atack) 9b08006bc502e67956d6ab518388fad6397cac8d log, sync: improve lock contention logging and add time duration (Jon Atack) 3f4c6b87f1098436693c4990f2082515ec0ece26 log, timer: add timing macro in usec LOG_TIME_MICROS_WITH_CATEGORY (Jon Atack) b7a17444e0746c562ae97b26eba431577947b06a log, sync: add LOCK logging category, apply it to lock contention (Jon Atack) Pull request description: To enable lock contention logging, `DEBUG_LOCKCONTENTION` has to be defined at compilation. Once built, the logging is not limited to a category and is high frequency, verbose and in all-caps. With these factors combined, it seems likely to be rarely used. This patch: - adds a `lock` logging category - adds a timing macro in microseconds, `LOG_TIME_MICROS_WITH_CATEGORY` - updates `BCLog::LogMsg()` to omit irrelevant decimals for microseconds and skip unneeded code and math - improves the lock contention logging, drops the all-caps, and displays the duration in microseconds - removes the conditional compilation directives - allows lock contentions to be logged on startup with `-debug=lock` or at run time with `bitcoin-cli logging '["lock"]'` ``` $ bitcoind -signet -debug=lock 2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 started 2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 completed (4μs) 2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 started 2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 completed (4μs) 2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 started 2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 completed (20μs) 2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 started 2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 completed (3μs) $ bitcoin-cli -signet logging "lock": true, $ bitcoin-cli -signet logging [] '["lock"]' "lock": false, $ bitcoin-cli -signet logging '["lock"]' "lock": true, ``` I've tested this with Clang 13 and GCC 10.2.1, on Debian, with and without `--enable-debug`. ACKs for top commit: hebasto: re-ACK 7e698732836121912f179b7c743a72dd6fdffa72, added a contention duration to the log message since my [previous](https://github.com/bitcoin/bitcoin/pull/22736#pullrequestreview-743764606) review. theStack: re-ACK 7e698732836121912f179b7c743a72dd6fdffa72 🔏 ⏲️ Tree-SHA512: c4b5eb88d3a2c051acaa842b3055ce30efde1f114f61da6e55fcaa27476c1c33a60bc419f7f5ccda532e1bdbe70815222ec2b2b6d9226f29c8e94e598aacfee7
2021-09-06Merge bitcoin/bitcoin#22894: netinfo: clarify client and server versions in ↵MarcoFalke
header e952d7557eaf2610e302e9d70381ef057d07f6bf netinfo: clarify client and server versions in header (Jon Atack) Pull request description: Clarify in -netinfo output that both the client and the server versions are provided. before ``` Bitcoin Core v22.0.0rc3 - 70016/Satoshi:22.99.0/ ``` after ``` Bitcoin Core client v22.0.0rc3 - server 70016/Satoshi:22.99.0/ ``` Closes #22873. ACKs for top commit: benthecarman: utACK e952d7557eaf2610e302e9d70381ef057d07f6bf prayank23: ACK https://github.com/bitcoin/bitcoin/pull/22894/commits/e952d7557eaf2610e302e9d70381ef057d07f6bf Zero-1729: tACK e952d7557eaf2610e302e9d70381ef057d07f6bf Tree-SHA512: 3e817892d398aabacb1401fd5b1816c4d4f563b4f8cf1096bdb8b53f7c4ef82d4caee09f5c7724f1fe292f837434a332acefba735152ed24a238bb6f006df909
2021-09-06Merge bitcoin/bitcoin#22861: test: Update test README and lint scriptfanquake
fa0937de35176fdcf637e1af16be4469725e60cc test: Rename bitcoin-util-test.py to util/test_runner.py (MarcoFalke) fa050bbc0ad479063735b0325daa717ded404c8f test: Update test README and lint script (MarcoFalke) Pull request description: * Remove unused `yq` * Update fuzzing docs ACKs for top commit: Saviour1001: ACK <code>[fa0937d](https://github.com/bitcoin/bitcoin/pull/22861/commits/fa0937de35176fdcf637e1af16be4469725e60cc)</code> practicalswift: cr ACK fa0937de35176fdcf637e1af16be4469725e60cc fanquake: ACK fa0937de35176fdcf637e1af16be4469725e60cc Tree-SHA512: 6b148d838e1fcf219ab92e579948e34ea7ce8b4692a3d28bb2a51aaa34cbc7cdbd79e72ce787b485fdf524e5b3521b033692583602d4e379bd160e0e41d66e28
2021-09-05Merge bitcoin/bitcoin#22591: Util: error if settings json exists, but is ↵MarcoFalke
unreadable 2b071265c37da22f15769945fd159b50a14792a3 error if settings.json exists, but is unreadable (Tyler Chambers) Pull request description: If settings.json exists, but is unreadable, we should error instead of overwriting. Fixes #22571 ACKs for top commit: Zero-1729: tACK 2b071265c37da22f15769945fd159b50a14792a3 ShaMan239: tACK 2b071265c37da22f15769945fd159b50a14792a3 prayank23: tACK https://github.com/bitcoin/bitcoin/pull/22591/commits/2b071265c37da22f15769945fd159b50a14792a3 ryanofsky: Code review ACK 2b071265c37da22f15769945fd159b50a14792a3. Thanks for the fix! Note that PR https://github.com/bitcoin-core/gui/pull/379 will change the appearance of dialogs shown in screenshots above. So it could be interesting to test the two PRs together (but current testing seems more than sufficient) theStack: ACK 2b071265c37da22f15769945fd159b50a14792a3 📁 Tree-SHA512: 6f7f96ce8a13213d0335198a2245d127264495c877105058d1503252435915b332a6e55068ac21088f4c0c017d564689f4956213328d5bdee81d73711efc5511
2021-09-05netinfo: clarify client and server versions in headerJon Atack
2021-09-05Merge bitcoin/bitcoin#22859: Replace uses of boost::trim* with ↵fanquake
locale-independent alternatives (#18130 rebased) 696c76d6604c9c4faddfc4b6684e2788bb577ba6 tests: Add TrimString(...) tests (practicalswift) 4bf18b089e1bb1f3ab513cbdf6674bd1074f4621 Replace use of boost::trim_right with locale-independent TrimString (Ben Woosley) 93551862a18965bcee0c883c54807e8726e2f50f Replace use of boost::trim use with locale-independent TrimString (Ben Woosley) Pull request description: This is [#18130 rebased](https://github.com/bitcoin/bitcoin/pull/18130#issuecomment-900158759). > `TrimString` is an existing alternative. > Note `TrimString` uses `" \f\n\r\t\v"` as the pattern, which is consistent with the default behavior of `std::isspace`. See: https://en.cppreference.com/w/cpp/string/byte/isspace ACKs for top commit: jb55: utACK 696c76d6604c9c4faddfc4b6684e2788bb577ba6 practicalswift: ACK 696c76d6604c9c4faddfc4b6684e2788bb577ba6 jonatack: ACK 696c76d6604c9c4faddfc4b6684e2788bb577ba6 theStack: Code-review ACK 696c76d6604c9c4faddfc4b6684e2788bb577ba6 Tree-SHA512: 6a70e3777602dfa65a60353e5c6874eb951e4a806844cd4bdaa4237cad980a4f61ec205defc05a29f9707776835975838f6cc635259c42adfe37ceb02ba9358d
2021-09-04Merge bitcoin/bitcoin#22278: Add LIFETIMEBOUND to CScript where neededfanquake
fa7e6c56f58678b310898a158053ee9ff8b27fe7 Add LIFETIMEBOUND to InitializeChainstate (MarcoFalke) fa5c896724bb359b4b9a3f89580272bfe5980c1b Add LIFETIMEBOUND to CScript where needed (MarcoFalke) Pull request description: Without this, stack-use-after-scope can only be detected at runtime with ASan or code review, both of which are expensive. Use `LIFETIMEBOUND` to turn this error into a compile warning. See https://releases.llvm.org/12.0.0/tools/clang/docs/AttributeReference.html#lifetimebound Example: ```cpp const CScript a{WITH_LOCK(::cs_main, return CScript{} << OP_0 << OP_1)}; ``` Before: (no warning) After: ``` warning: returning reference to local temporary object [-Wreturn-stack-address] const CScript a{WITH_LOCK(::cs_main, return CScript{} << OP_0 << OP_1)}; ^~~~~~~~~ ./sync.h:276:65: note: expanded from macro 'WITH_LOCK' #define WITH_LOCK(cs, code) [&]() -> decltype(auto) { LOCK(cs); code; }() ^~~~ ACKs for top commit: theuni: utACK fa7e6c56f58678b310898a158053ee9ff8b27fe7. jonatack: Light ACK fa7e6c56f58678b310898a158053ee9ff8b27fe7 debug build with clang 13, reproduced the example compiler warning in the pull description, and briefly looked at `clang::lifetimebound` support in earlier versions of clang; it is in clang 7 (https://releases.llvm.org/7.0.0/tools/clang/docs/AttributeReference.html#lifetimebound-clang-lifetimebound), did not see references to it in earlier docs Tree-SHA512: e915acdc4532445205b7703fab61a5d682231ace78ecfb274cb8523ca2bddefd85828f50ac047cfb1afaff92a331f5f7b5a1472539f999e30f7cf8ac8c3222f3
2021-09-03Merge bitcoin/bitcoin#22860: test: Always clear reject reason in IsStandard ↵MarcoFalke
tx test fa1b08eb1413d547b5e322f20e6907b2f827a162 test: Always clear reject reason in IsStandard tx test (MarcoFalke) Pull request description: For some tests the reject reason wasn't cleared between runs and thus subsequent tests might (theoretically) fail to verify the correct reject reason. ACKs for top commit: benthecarman: ACK fa1b08eb1413d547b5e322f20e6907b2f827a162 theStack: Code-review ACK fa1b08eb1413d547b5e322f20e6907b2f827a162 Tree-SHA512: fcb727a690f92a4cf06127c302ba464f1e8cb997498e4f7fd9e210d193559b07e6efdb9d5c8a0bef3fe643bdfd5fedd431aaace20978dd49e56b8e770cb9f930
2021-09-03Add LIFETIMEBOUND to InitializeChainstateMarcoFalke
2021-09-03Add LIFETIMEBOUND to CScript where neededMarcoFalke
2021-09-03Merge bitcoin/bitcoin#22100: refactor: Clean up new wallet spend, receive ↵Samuel Dobson
files added #21207 b11a195ef450bd138aa03204a5e74fdd3ddced26 refactor: Detach wallet transaction methods (followup for move-only) (Russell Yanofsky) Pull request description: This makes `CWallet` and `CWalletTx` methods in `spend.cpp` and `receive.cpp` files into standalone functions. It's a followup to [#21207 MOVEONLY: CWallet transaction code out of wallet.cpp/.h](https://github.com/bitcoin/bitcoin/pull/21207), which moved code from `wallet.cpp` to new `spend.cpp` and `receive.cpp` files. There are no changes in behavior. This is just making methods into functions and removing circular dependencies created by #21207. There are no comment or documentation changes, either. Removed comments from `transaction.h` are just migrated to `spend.h`, `receive.h`, and `wallet.h`. --- This commit was split off from #21206 so there are a few earlier review comments there ACKs for top commit: achow101: ACK b11a195ef450bd138aa03204a5e74fdd3ddced26 Sjors: utACK b11a195ef450bd138aa03204a5e74fdd3ddced26 meshcollider: light ACK b11a195ef450bd138aa03204a5e74fdd3ddced26 Tree-SHA512: 75ce818d3f03b728b14b12e2d21bd20b7be73978601989cb37ff98254393300d1bb7823281449cd3d9e40756d67d42bd9a46bbdafd2e8baa95aaf2cb1c84549f
2021-09-02Merge bitcoin/bitcoin#18448: rpc: fix/add missing RPCExamples for "Util" RPCsMarcoFalke
ea98d9c2eff86e6537f35ac4381ac169daacde36 rpc: fix/add missing RPCExamples for "Util" RPCs (Sebastian Falbesoner) Pull request description: Similar to https://github.com/bitcoin/bitcoin/pull/18398, this PR gives the RPCExamples in the RPC category "Util" (that currently contains `createmultisig`, `deriveaddresses`, `estimatesmartfee`, `getdescriptorinfo`, `signmessagewithprivkey`, `validateaddress`, `verifymessage`) some love by fixing one broken and adding three missing examples: - fixed `HelpExampleRpc` for `createmultisig` (disturbing escape characters and quotation marks) - added missing `HelpExampleRpc` for - `deriveaddresses` (also put descriptor in a new string constant) - `estimatesmartfee` - `getdescriptorinfo` (also put descriptor in a new string constant) Output for `createmultisig` example on the master branch: ``` $ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, "[\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\",\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\"]"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/ Enter host password for user '__cookie__': {"result":null,"error":{"code":-1,"message":"JSON value is not an array as expected"},"id":"curltest"} ``` Output for `createmultisig` example on the PR branch: ``` $ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, ["03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd","03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626"]]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/ Enter host password for user '__cookie__': {"result":{"address":"3QsFXpFJf2ZY6GLWVoNFFd2xSDwdS713qX","redeemScript":"522103789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd2103dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a6162652ae","descriptor":"sh(multi(2,03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd,03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626))#4djp057k"},"error":null,"id":"curltest"} ``` ACKs for top commit: jonatack: ACK ea98d9c2eff86e6537f35ac4381ac169daacde36 looked at the code, rebased to master, ran the helps, did not try running the added json-rpc examples Tree-SHA512: d6ecb6da66f19517065453357d210102e2cc9f1f8037aeb6a9177ff036d0c21773dddf5e0acdbc71edbbde3026e4d1e7ce7c0935cd3e023c60f34e1b173b3299
2021-09-02Merge bitcoin/bitcoin#22501: netinfo: display addr_{processed, rate_limited, ↵W. J. van der Laan
relay_enabled} and relaytxes data 218862a01848f69d54380c780bb5eae6dfdb1416 Display peers in -netinfo that we don't relay addresses to (Jon Atack) 3834e23b251ed7b4a47bbb981faba65b97ecbba0 Display peers in -netinfo that request we not relay transactions (Jon Atack) 0a9ee3a2c787e97213a0456b0d6253c549b71e09 Simplify a few conditionals in -netinfo (Jon Atack) 5eeea8e2575a36587e70743af3bd7c2d87b8cf36 Add addr_processed and addr_rate_limited stats to -netinfo (Jon Atack) Pull request description: Update CLI -netinfo to display the getpeerinfo `addr_processed`, `addr_rate_limited`, `addr_relay_enabled` and `relaytxes` data with auto-adjusting column widths. ``` $ ./src/bitcoin-cli -netinfo help txn Time since last novel transaction received from the peer and accepted into our mempool, in minutes "*" - the peer requested we not relay transactions to it (relaytxes is false) addrp Total number of addresses processed, excluding those dropped due to rate limiting "." - we do not relay addresses to this peer (addr_relay_enabled is false) addrl Total number of addresses dropped due to rate limiting ``` ![Screenshot from 2021-08-22 14-31-40](https://user-images.githubusercontent.com/2415484/130355514-f6fd4f21-79d6-463b-9791-de01ebef20b1.png) ACKs for top commit: 0xB10C: Code review and tested ACK 218862a01848f69d54380c780bb5eae6dfdb1416 Zero-1729: re-tACK 218862a01848f69d54380c780bb5eae6dfdb1416 vasild: ACK 218862a01848f69d54380c780bb5eae6dfdb1416 jarolrod: tACK 218862a01848f69d54380c780bb5eae6dfdb1416 Tree-SHA512: bb9da4bdd71859b234f6e4c2c46257a57ef0d0e0b363d2b8fded128bcaa28132f64a0a4651c622e1de1e3b7c05c7587a4369e9e79799895884fda9745c63409d
2021-09-02test: Rename bitcoin-util-test.py to util/test_runner.pyMarcoFalke
To normalize the name of all three test runners (fuzz, functional, util).
2021-09-02test: Always clear reject reason in IsStandard tx testMarcoFalke
2021-09-02Merge bitcoin/bitcoin#22846: policy: unit test Segwit dust thresholdsMarcoFalke
97cea1a93a26d535f9bad038b559e50437ea54f7 policy: unit test Segwit dust thresholds (Antoine Poinsot) Pull request description: This is the unit testing part of #22779, hence without the threshold modification. ACKs for top commit: MarcoFalke: cr ACK 97cea1a93a26d535f9bad038b559e50437ea54f7 benthecarman: crACK 97cea1a93a26d535f9bad038b559e50437ea54f7 Tree-SHA512: 96fb194709ae44364455eb920ed3ecff2e11e5327e0a72b9eeec9f9445894302099a0c4ffb1e0c8d4d523c0bfe06c57f1ebb0c03cf3389a73f518e3b174c45aa
2021-09-02tests: Add TrimString(...) testspracticalswift
2021-09-02Replace use of boost::trim_right with locale-independent TrimStringBen Woosley
Note the only use of readStdin is fed to DecodeHexTx, which fails in IsHex on non-hex characters as recorded in p_util_hexdigit.
2021-09-02Replace use of boost::trim use with locale-independent TrimStringBen Woosley
2021-09-02Merge bitcoin/bitcoin#22853: fuzz: Remove addrdb fuzz targetfanquake
fa18553d382a7d8c447cd6698b36e293fb7ecf1f fuzz: Remove addrdb fuzz target (MarcoFalke) Pull request description: The target has several issues: * It is named incorrectly (`addrdb`, but it constructs a `CBanEntry`) * It doesn't do anything meaningful, other than consuming one integer and passing it to a constructor * It consumes CPU time that can be used for the other targets * It is redundant with the banman fuzz target Fix all by removing it. ACKs for top commit: amitiuttarwar: ACK fa18553d382a7d8c447cd6698b36e293fb7ecf1f, thanks for the cleanup Tree-SHA512: 3f8944d3f80913bf466c03062fed070e96073fb72d0938b2bc9a2586960c86879d6f251e16fd81cfeb4e6685ff9eef6bccb25cd3901b218a100c90f25a3c9240
2021-09-02Merge bitcoin/bitcoin#22836: Stricter BIP32 decoding and test vector 5fanquake
56a42f10f452f0ac0e3e333646a8effcbebf6b30 Stricter BIP32 decoding and test vector 5 (Pieter Wuille) Pull request description: This adds detection for various edge cases when decoding BIP32 extended pubkeys/privkeys, and tests them using the proposed https://github.com/bitcoin/bips/pull/921 BIP32 test vector 5. ACKs for top commit: darosior: utACK 56a42f10f452f0ac0e3e333646a8effcbebf6b30 -- Had to implement essentially the same fix in python-bip32. kristapsk: ACK 56a42f10f452f0ac0e3e333646a8effcbebf6b30. Checked that test vectors are the same as in BIP32 and that tests pass. Tree-SHA512: 5cc800cc9dc10e43ae89b659ce4f44026d04ec3cabac4eb5122d2e72ec2ed66cd5ace8c7502259e469a9ecaa5ecca2457e55dfe5fedba59948ecbf6673af67a7
2021-09-02Merge bitcoin/bitcoin#22512: Consolidate XOnlyPubKey lookup hackSamuel Dobson
d9d3ec07cfe45cfa55028cc879dc8a55aecb4d3c Consolidate XOnlyPubKey lookup hack (Andrew Chow) Pull request description: The places where we need to lookup information for a XOnlyPubKey currently implement a hack which makes both serializations of the full pubkey in order to try the CKeyIDs for the lookup functions. Instead of duplicating this everywhere it is needed, we can consolidate the CKeyID generation into a function, and then have wrappers around GetPubKey, GetKey, and GetKeyOrigin which takes the XOnlyPubKey, retrieves all of the CKeyIDs (using the new GetKeyIDs() function in XOnlyPubKey), and tries their respective underlying lookup function. Split from #22364 ACKs for top commit: S3RK: Code Review reACK d9d3ec0 Zero-1729: re-crACK d9d3ec0 theStack: re-ACK d9d3ec07cfe45cfa55028cc879dc8a55aecb4d3c meshcollider: Code review + functional test run ACK d9d3ec07cfe45cfa55028cc879dc8a55aecb4d3c Tree-SHA512: 21a7f6d37fad74483a38006f82b3558337fe9ed30e0b4392e6fff82c22251a42ac996b43f06cdaa9289ee34a768e181d87aa4208b5538e36ae4977954e1fa6a0
2021-09-01sync: remove DEBUG_LOCKCONTENTION preprocessor directivesJon Atack
to allow logging the lock contentions without the need to define DEBUG_LOCKCONTENTION at compile time.
2021-09-01log, sync: improve lock contention logging and add time durationJon Atack
in microseconds. Change the function name in order to print "LockContention" instead of "PrintLockContention" to the log. Add Doxygen documentation. With this change, the lock contention log prints: 2021-09-01T11:29:03Z LockContention: pnode->cs_vSend, net.cpp:1373 started 2021-09-01T11:29:03Z LockContention: pnode->cs_vSend, net.cpp:1373 completed (31μs) 2021-09-01T11:29:03Z LockContention: cs_vNodes, net.cpp:2277 started 2021-09-01T11:29:03Z LockContention: cs_vNodes, net.cpp:2277 completed (6μs) 2021-09-01T11:29:04Z LockContention: cs_vNodes, net.cpp:2242 started 2021-09-01T11:29:04Z LockContention: cs_vNodes, net.cpp:2242 completed (3μs) Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Co-authored-by: practicalswift <practicalswift@users.noreply.github.com>
2021-09-01log, timer: add timing macro in usec LOG_TIME_MICROS_WITH_CATEGORYJon Atack
and update BCLog::LogMsg() to omit irrelevant decimals for microseconds and skip unneeded code and math.
2021-09-01fuzz: Remove addrdb fuzz targetMarcoFalke
2021-09-01refactor: Detach wallet transaction methods (followup for move-only)Russell Yanofsky
Followup to commit "MOVEONLY: CWallet transaction code out of wallet.cpp/.h" that detaches and renames some CWalletTx methods, making into them into standalone functions or CWallet methods instead. There are no changes in behavior and no code changes that aren't purely mechanical. It just gives spend and receive functions more consistent names and removes the circular dependencies added by the earlier MOVEONLY commit. There are also no comment or documentation changes. Removed comments from transaction.h are just migrated to spend.h, receive.h, and wallet.h.
2021-09-01Merge bitcoin/bitcoin#22849: Remove unused SERIALIZE_METHODS for CBanEntryMarcoFalke
fa3bd9de99ee2bdfce2010e9367391a146e41878 Remove CBanEntry::SetNull (MarcoFalke) fab53ff1e5a995f40a110d6f9e1214f263908b46 Remove unused SERIALIZE_METHODS for CBanEntry (MarcoFalke) Pull request description: It would be confusing to keep unused and dead code. ACKs for top commit: ryanofsky: Code review ACK fa3bd9de99ee2bdfce2010e9367391a146e41878. theStack: Code-review ACK fa3bd9de99ee2bdfce2010e9367391a146e41878 Tree-SHA512: 85ab8de2ad1ada08e745806f2992def08bf8ead268caed7700a9fc61e3c7646e4ed7ae50a6d591c5bb9467f8999ea063ce5b5bd4fa0d58d8fc9d89e5a91f35a5
2021-09-01Merge bitcoin/bitcoin#22009: wallet: Decide which coin selection solution to ↵Samuel Dobson
use based on waste metric 86beee05795216738f51fa744539336503c26fd9 Use waste metric for deciding which selection to use (Andrew Chow) b3df0caf7c291a316298e54e73426c765e61c129 tests: Test GetSelectionWaste (Andrew Chow) 4f5ad43b1e05cd7b403f87aae4c4d42e5aea810b Add waste metric calculation function (Andrew Chow) 935b3ddf72aa390087684e03166c707f5b173434 scripted-diff: tests: Use KnapsackSolver directly (Andrew Chow) 6a023a6f904efe38dacd662d919aba74f066b1dc tests: Add KnapsackGroupOutputs helper function (Andrew Chow) d5069fc1aa7d335f3043227f843cbb9d8ba1507b tests: Use SelectCoinsBnB directly instead of AttemptSelection (Andrew Chow) 54de7b47463d98f860167d4e0b7e4ebb3926b59c Allow the long term feerate to be configured, default of 10 sat/vb (Andrew Chow) Pull request description: Branch and Bound introduced a metric that we call waste. This metric is used as part of bounding the search tree, but it can be generalized to all coin selection solutions, including those with change. As such, this PR introduces the waste metric at a higher level so that we can run both of our coin selection algorithms (BnB and KnapsackSolver) and choose the one which has the least waste. In the event that both find a solution with the same change, we choose the one that spends more inputs. Also this PR sets the long term feerate to 10 sat/vb rather than using the 1008 block estimate. This allows the long term feerate to be the feerate that we switch between consolidating and optimizing for fees. This also removes a bug where the long term feerate would incorrectly be set to the fallback fee. While this doesn't matter prior to this PR, it does have an effect following this. The long term feerate can be configured by the user through a new `-consolidatefeerate` option. ACKs for top commit: Xekyo: reACK 86beee0 via git range-diff fe47558...86beee0 meshcollider: re-utACK 86beee05795216738f51fa744539336503c26fd9 Tree-SHA512: 54b154b346538eca68ae2a3b83a033b495c1605c14f842bfc43ded2256b110983ce674c647fe753cf0305b1b178403d8d60d6d4203c7a712bec784be52e90d42
2021-09-01Merge bitcoin/bitcoin#22740: [addrman] Move serialization code to cppfanquake
85b15ddc8ff499fe21d8ab35ece3994f8878b3de [refactor] [addrman] Update constant comments (John Newbery) af9638a0fbb79bec743f4d2275b89e9573cfdc0a [move-only] Extract constants from addrman .h to .cpp (Amiti Uttarwar) 7dc443a62d3c98d8d0849d83060e940356fe32a3 [addrman] Change addrman #define constants to be constexprs (Amiti Uttarwar) a65053f1d44c72c43754a5d5aeb684fc1fca0300 [addrman] Move CAddrMan::Unserialize to cpp file (John Newbery) 1622543cf42feb810d8ea9e7b3238d21f1427c17 [addrman] Move CAddrMan::Serialize to cpp file (John Newbery) Pull request description: Moving the serialization code from the header to the cpp helps clarify interfaces vs internals, as well as speed up the compilation of the whole program with a smaller header file. ACKs for top commit: jnewbery: Code review ACK 85b15ddc8ff499fe21d8ab35ece3994f8878b3de 0xB10C: Code review ACK 85b15ddc8 mzumsande: Code Review ACK 85b15ddc8ff499fe21d8ab35ece3994f8878b3de (+ performed some light testing) Tree-SHA512: a1aac25155601dd0ffd073b37388d9062c3d82c499821bd7ee883286cbc5dc0c7ae87f127c127778dae290006b98166640dc974d1953f3f34c53a67cf7b21613
2021-08-31Remove CBanEntry::SetNullMarcoFalke
2021-08-31Remove unused SERIALIZE_METHODS for CBanEntryMarcoFalke
2021-08-31policy: unit test Segwit dust thresholdsAntoine Poinsot
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2021-08-31Merge bitcoin/bitcoin#22796: RBF move (1/3): extract BIP125 Rule 5 into ↵fanquake
policy/rbf f293c68be0469894c988711559f5528020c0ff71 MOVEONLY: getting mempool conflicts to policy/rbf (glozow) 8d7179633552f58ca0d23305196dcb4249b6dce7 [validation] quit RBF logic earlier and separate loops (glozow) badb9b11a6f7e1e693cecc8cd5aae55a197d70e2 call SignalsOptInRBF instead of checking all inputs (glozow) e0df41d7d584b854c2914d4afe7b21e0af3fbf69 [validation] default conflicting fees and size to 0 (glozow) b001b9f6de7a039a468cf0f9645f3f0a430fa889 MOVEONLY: BIP125 max conflicts limit to policy/rbf.h (glozow) Pull request description: See #22675 for motivation, this is one chunk of it. It extracts some BIP125 logic into policy/rbf: - Defines a constant for specifying the maximum number of mempool entries we'd consider replacing by RBF - Calls the available `SignalsOptInRBF` function instead of manually iterating through inputs - Moves the logic for getting the list of conflicting mempool entries to a helper function - Also does a bit of preparation for future moves - moving declarations around, etc Also see #22677 for addressing the circular dependency. ACKs for top commit: jnewbery: Code review ACK f293c68be0469894c988711559f5528020c0ff71 theStack: Code-review ACK f293c68be0469894c988711559f5528020c0ff71 📔 ariard: ACK f293c68b Tree-SHA512: a60370994569cfc91d4b2ad5e94542d4855a48927ae8b174880216074e4fa50d4523dd4ee36efdd6edf2bf7adb87a8beff9c3aaaf6dd323b286b287233e63790
2021-08-30Stricter BIP32 decoding and test vector 5Pieter Wuille
2021-08-29refactor: remove RecursiveMutex cs_nBlockSequenceIdSebastian Falbesoner
The RecursiveMutex cs_nBlockSequenceId is only used at one place in CChainState::ReceivedBlockTransactions() to atomically read-and-increment the nBlockSequenceId member. At this point, the cs_main lock is set, hence we can use a plain int for the member and mark it as guarded by cs_main.
2021-08-27Use waste metric for deciding which selection to useAndrew Chow
Instead of always choosing BnB if it finds a solution, always do both BnB and KnapsackSolver and choose the one which has the least waste.
2021-08-27tests: Test GetSelectionWasteAndrew Chow
Tests for some waste calculation scenarios add_coin is modified to allow fee and long term fee to be set for an added CInputCoin.
2021-08-27Add waste metric calculation functionAndrew Chow
2021-08-27scripted-diff: tests: Use KnapsackSolver directlyAndrew Chow
When doing the coin selector tests for KnapsackSolver, call it directly instead of using AttemptSelection and hoping/instructing it uses KnapsackSolver. -BEGIN VERIFY SCRIPT- sed -i 's/testWallet\.AttemptSelection( /KnapsackSolver(/g' src/wallet/test/coinselector_tests.cpp sed -i 's/testWallet\.AttemptSelection(/KnapsackSolver(/g' src/wallet/test/coinselector_tests.cpp sed -i 's/, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params)/, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)/g' src/wallet/test/coinselector_tests.cpp sed -i 's/, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)/, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)/g' src/wallet/test/coinselector_tests.cpp sed -i 's/, filter_standard_extra, vCoins, setCoinsRet, nValueRet, coin_selection_params)/, KnapsackGroupOutputs(filter_standard_extra), setCoinsRet, nValueRet)/g' src/wallet/test/coinselector_tests.cpp sed -i 's/BOOST_CHECK( /BOOST_CHECK(/g' src/wallet/test/coinselector_tests.cpp -END VERIFY SCRIPT-
2021-08-27tests: Add KnapsackGroupOutputs helper functionAndrew Chow
In order to change the KnapsackSolver tests to call KnapsackSolver, we need KnapsackGroupOutputs to create the OutputGroups filtered with the filter criteria.
2021-08-27tests: Use SelectCoinsBnB directly instead of AttemptSelectionAndrew Chow
Instead of calling AttemptSelection with the hopes/instruction that it uses BnB, call SelectCoinsBnB directly to test it.
2021-08-27Allow the long term feerate to be configured, default of 10 sat/vbAndrew Chow
The long term feerate is really the highest feerate that the user is comfortable with making consolidatory transactions. This is should thus be something that can be configured by the user via a new startup option -consolidatefeerate. The default value is 10 sat/vbyte, chosen arbitrarily (it seems like a reasonable number).
2021-08-27Merge bitcoin/bitcoin#22782: Remove unused MaybeSetAddrNameMarcoFalke
fa9eade142964d5482fe8d2109fb67a9556ae93d Remove GetAddrName (MarcoFalke) fa786570a5fdf6723b35883054f9f840a3440f92 Remove unused RecursiveMutex cs_addrName (MarcoFalke) fa82f4ea96749115311cffa0919d49d383c4d28b Remove unused MaybeSetAddrName (MarcoFalke) Pull request description: . ACKs for top commit: jnewbery: Code review ACK fa9eade142964d5482fe8d2109fb67a9556ae93d naumenkogs: utACK fa9eade142964d5482fe8d2109fb67a9556ae93d Tree-SHA512: 61501a699add59225dc8127b6dfdda450d768c86f958fdf94e9c28309c3705ecfbee4b064d44228b8c1190c19c39272becc7ede8386ac1406699ea2285881c72
2021-08-27Merge bitcoin/bitcoin#20586: Fix Windows build with --enable-werrorW. J. van der Laan
b367745cfe19f6de3f44b3adc90fa08e36e44bb6 ci: Make Cirrus CI Windows build with --enable-werror (Hennadii Stepanov) c713bb2b243881a771ab288340ffeb623c82d7f6 Fix Windows build with --enable-werror on Ubuntu Focal (Hennadii Stepanov) Pull request description: This PR makes possible to cross-compile Windows build with `--enable-werror --enable-suppress-external-warnings`. Some problems are fixed, others are silenced. Also `--enable-werror` is enabled for Cirrus CI Windows build (the last one on Cirrus CI without `--enable-werror`). ACKs for top commit: practicalswift: cr ACK b367745cfe19f6de3f44b3adc90fa08e36e44bb6: patch looks correct laanwj: Code review ACK b367745cfe19f6de3f44b3adc90fa08e36e44bb6 vasild: ACK b367745cfe19f6de3f44b3adc90fa08e36e44bb6 jarolrod: ACK b367745cfe19f6de3f44b3adc90fa08e36e44bb6 Tree-SHA512: 64f5c99b7dad4c0efce80cd45d7074f275bd8411235dc9e0841287bdab64b812c6f8f9d632c35531d0b8210148531f53aaaac77be7699b29d2d6aaae304dbee0
2021-08-27Merge bitcoin/bitcoin#21862: test: Set regtest.BIP65Height = 111 to speed up ↵fanquake
tests faf7e485e901d6c72db5d969b526fa148060a003 Set regtest.BIP65Height = 111 to speed up tests (MarcoFalke) Pull request description: No need to waste time by forcing creation of more than 1000 blocks to get the benefits of being able to test BIP 65. Also, reducing the height makes it more likely that (third-party) tests are conforming to BIP 65, which is enforced on mainnet for all new blocks. ACKs for top commit: theStack: re-ACK faf7e485e901d6c72db5d969b526fa148060a003 📍 Zero-1729: re-ACK faf7e485e901d6c72db5d969b526fa148060a003 kristapsk: ACK faf7e485e901d6c72db5d969b526fa148060a003 Tree-SHA512: 79a8263e7233838666b9b636b496a8b9eb12398c779f9434677e1d62816732c0a7c7b3e73965be1fb0038d35e05e5a90e665bd74e9610104127dfc4ea38169bf
2021-08-26Merge bitcoin-core/gui#384: Add copy IP/Netmask action for banned peerHennadii Stepanov
ab1461d5d36b70fd4982679ac6143c25e7617dbf qt: Add copy IP/Netmask action for banned peer (Shashwat) Pull request description: This PR adds a Copy IP/Netmask context menu action to the Banned Peers Table. This feature is helpful if a node using GUI might want to alert its peer about a particular malicious user. So it can copy that user’s IP/Netmask and broadcast it to its peers so they can ban it instantly using the setban command in the console. | Master | PR | | ----------- | ----------- | | ![Screenshot_from_2021-07-21_00-01-331](https://user-images.githubusercontent.com/23396902/126377808-bd23bb19-3f47-4f1b-8371-39baa9747bbe.png) | ![Screenshot from 2021-08-20 20-13-28(1)(1)](https://user-images.githubusercontent.com/85434418/130251441-a8d0f816-a2e9-4e63-a22d-94885c5cec98.png) | ACKs for top commit: jarolrod: re-ACK ab1461d hebasto: re-ACK ab1461d5d36b70fd4982679ac6143c25e7617dbf, tested on Linux Mint 20.2 (Qt 5.12.8). Tree-SHA512: a528f089bd4cb5b51fec987550d21c2587459ad80f854b55850bc62c776c21f3fa31052a17e2b0e9e9d0b3468799c8070ed306543730fb7b324f283847151e17
2021-08-26[refactor] [addrman] Update constant commentsJohn Newbery
Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
2021-08-26[move-only] Extract constants from addrman .h to .cppAmiti Uttarwar
Reviewer hint: use `git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space` Co-authored-by: John Newbery <john@johnnewbery.com>
2021-08-26[addrman] Change addrman #define constants to be constexprsAmiti Uttarwar
Co-authored-by: John Newbery <john@johnnewbery.com>