d2a09c83554cec864a27e704cf2a0d513691a567 doc: mention bech32m/BIP350 in doc/descriptors.md (Pieter Wuille)
Pull request description:
As it says on the tin.
ACKs for top commit:
0xB10C:
ACK d2a09c83554cec864a27e704cf2a0d513691a567
fanquake:
ACK d2a09c83554cec864a27e704cf2a0d513691a567 - can be backported for coherent documentation. Bech32m support was backported into branches back to the 0.19.
Tree-SHA512: a75d8bbf1ab062ec19e14cbe38dd0405a08c58e5dd4e9d94881f2c5efd52cc22dc755eae8d21bbda066feb099d93312b65557900bc2e65a70ba0412614cf0135
387355bb9482a09c1fc9b137bea56745a93b7dfd test, refactor: rpc_rawtransaction PEP8 (Jon Atack)
7d5cec2e498dc059ff1d74a2b60764db45923264 refactor: separate the rpc_rawtransaction tests into functions (Jon Atack)
409779df95f886b08dbf6d44219e2fbeb3405a43 move-only: regroup similar rpc_rawtransaction tests together (Jon Atack)
d861040dd24a321e1ceec1f07c7bb80d59779081 test: remove no longer needed (ASCII art) comments (Jon Atack)
14398b30d6242db14670b3286f988b2badda83fb test: add and harmonize getrawtransaction logging (Jon Atack)
85d8869cf89fedf243748e3e15b3ed39de1b0385 test: run 2nd getrawtransaction section with/without -txindex (Jon Atack)
00977407732969593800d15de39abbb7e0250abc refactor: txid to constant in rpc_rawtransaction to isolate tests (Jon Atack)
8c19d1329f1f28000ca32d826cecf04680c6be69 refactor: dedup/reorg createrawtransaction sequence number tests (Jon Atack)
7f073594c9f5b518dc1fb66dfb0189e8803e3545 Test src/node/transaction::GetTransaction() without -txindex (Jon Atack)
Pull request description:
Following up on https://github.com/bitcoin/bitcoin/pull/22383#pullrequestreview-698583510, this pull adds missing `src/node/transaction::GetTransaction()` test coverage for combinations of `-txindex` and `blockhash` and does some refactoring of the test file.
ACKs for top commit:
mjdietzx:
reACK 387355bb9482a09c1fc9b137bea56745a93b7dfd
josibake:
reACK 387355bb94
MarcoFalke:
Approach ACK 387355bb9482a09c1fc9b137bea56745a93b7dfd 🔆
Tree-SHA512: b47c4ff87d69c61434e5729c954b338bc13744eddaba0879ca9f5f42243ba2cb4640d94c5f74de9f2735a8bf5e66b3d1c3bd3b7c26cd7324da7d3270ce87c6fd
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
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
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
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
fa001602cd5ac61b9258e998ee2b236688c19ef7 ci: Re-enable verify-commits.py check (MarcoFalke)
fa880b10d67542b8eb476a0e1f3ffb67e88d5e53 ci: Unconditionally set the global git author name in cirrys.yml (MarcoFalke)
Pull request description:
Might be useful to detect bugs in the script itself or an accidentally missed signature.
ACKs for top commit:
josibake:
ACK fa001602cd
Zero-1729:
tACK fa001602cd5ac61b9258e998ee2b236688c19ef7
fanquake:
untested ACK fa001602cd5ac61b9258e998ee2b236688c19ef7
Tree-SHA512: 8a13a67d325f2477f4088d1034f0d5e4e04937a01ee3c738435fe66394c02b9f33225529952ad331b0ba19b63ca4b2f26911cb5d264890159840cf3e09085969
0bd882b7405414b5355e69a9fdcd7a533e504b6b refactor: remove RecursiveMutex cs_nBlockSequenceId (Sebastian Falbesoner)
Pull request description:
The RecursiveMutex `cs_nBlockSequenceId` is only used at one place in `CChainState::ReceivedBlockTransactions()` to atomically read-and-increment the nBlockSequenceId member:
83daf47898/src/validation.cpp (L2973-L2976)
~~For this simple use-case, we can make the member `std::atomic` instead to achieve the same result (see https://en.cppreference.com/w/cpp/atomic/atomic/operator_arith).~~
~~This is related to #19303. As suggested in the issue, I first planned to change the `RecursiveMutex` to `Mutex` (still possible if the change doesn't get Concept ACKs), but using a Mutex for this simple operation seems to be overkill. Note that at the time when this mutex was introduced (PR #3370, commit 75f51f2a63e0ebe34ab290c2b7141dd240b98c3b) `std::atomic` were not used in the codebase yet -- according to `git log -S std::atomic` they have first appeared in 2016 (commit 7e908c7b826cedbf29560ce7a668af809ee71524), probably also because the compilers didn't support them properly earlier.~~
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.
ACKs for top commit:
Zero-1729:
ACK 0bd882b
promag:
Code review ACK 0bd882b7405414b5355e69a9fdcd7a533e504b6b.
hebasto:
ACK 0bd882b7405414b5355e69a9fdcd7a533e504b6b
Tree-SHA512: 435271ac8f877074099ddb31436665b500e555f7cab899e5c8414af299b154d1249996be500e8fdeff64e4639bcaf7386e12510b738ec6f20e415e7e35afaea9
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.
e251726affe97da745362c82567c2377ceb07d21 build, qt: Fix typo in QtInputSupport check (Hennadii Stepanov)
Pull request description:
This typo slipped in fecb3723b63d1441f1f786352e9ef330c1ac57a5 (#21565).
I apologize for that as a reviewer of #21565.
ACKs for top commit:
fanquake:
ACK e251726affe97da745362c82567c2377ceb07d21
Tree-SHA512: c14d603ed5a9b7e4b6503bfb93e66b800d5a107f21a8375c6f98465ef937e3e55b9c4fc160407c7e3ee402b495c5820ebb35ec6cb92956c1ee4092415635b915
In order to change the KnapsackSolver tests to call KnapsackSolver, we
need KnapsackGroupOutputs to create the OutputGroups filtered with the
filter criteria.
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).
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
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
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 |
| ----------- | ----------- |
|  |  |
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
b8aa84b1a116599a6dd3b9ddb4e6c178a6688b1b qt, refactor: Replace `if` check with `assert` (Hennadii Stepanov)
fcdc8b0fcb9dc81b76289abc57a4203671f748eb qt, refactor: Drop redundant signalling in WalletView::setWalletModel (Hennadii Stepanov)
37dcf161d3dd1f7862a67bec1e8f2887cbd6de90 qt, refactor: Emit WalletView::encryptionStatusChanged signal directly (Hennadii Stepanov)
7d0d4c04903cafade32be3bf2bf1cad3f33c6c03 qt: Add WalletFrame::currentWalletSet signal (Hennadii Stepanov)
Pull request description:
This PR makes signal-slot paths to reach `setHDStatus` and `setEncryptionStatus` functions shorter and easier to reason about them.
Required to simplify #398 (see https://github.com/bitcoin-core/gui/pull/398#discussion_r686094883).
---
**Note for reviewers.** Please verify that "Encrypt Wallet..." menu item, and the following icons
and updated properly in each and every possible scenario.
ACKs for top commit:
jarolrod:
tACK b8aa84b1a116599a6dd3b9ddb4e6c178a6688b1b
Talkless:
Code review ACK b8aa84b1a116599a6dd3b9ddb4e6c178a6688b1b. Did build on Debian Sid with Qt 5.15.2 but no actual testing performed.
ryanofsky:
Code review ACK b8aa84b1a116599a6dd3b9ddb4e6c178a6688b1b. Only change since last review was rebase
Tree-SHA512: 275737cdba02baff71049df41bc24089e916f96326dd2dea26ec607c7949cb3aae368eeabbe3ad5a0a27651503a1d65536873726de854c5f6af259bcc29727e7
017597767b977bb8fe11be8e2012130df1dffd30 Add I2P network SetReachable/IsReachable unit test assertions (Jon Atack)
b87a9c4d13329a6236124d4b93580c4df8107b32 Improve doc/i2p.md regarding I2P router options/versions (Jon Atack)
bebcf785c080df9273e03b854832ba3dbd4320ec Update i2p.md and tor.md regarding -onlynet config option (Jon Atack)
Pull request description:
This pull addresses https://github.com/bitcoin/bitcoin/issues/22634#issuecomment-894104681 and various user feedback/questions, updates the -onlynet documentation in doc/i2p.md and doc/tor.md per #22651 (src/init.cpp is already fine) and fills in some missing I2P unit test coverage.
Note: this PR depends in part on whether #22651 is merged in order to propose the correct -onlynet documentation (it is currently aligned with the change in #22651), so that PR should be decided or merged first.
ACKs for top commit:
Rspigler:
Re-ACK 017597767b977bb8fe11be8e2012130df1dffd30
prayank23:
reACK 017597767b
vasild:
ACK 017597767b977bb8fe11be8e2012130df1dffd30
Tree-SHA512: ae606437522bfccdfb7508108cddc7dfede2385e30a0561dbd007b784ed2639962c28552eb0e9336412faa323637fe964c26b8d8fc6dcf9fc63734ac00d05736
7720d4f650015272dc7109238230520f71858c6c test: fix failure in feature_nulldummy.py on single-core machines (Sebastian Falbesoner)
646b3885f795c640a2ac979362c509c4a8ee592a test: refactor: use named args for block_submit in feature_nulldummy.py (Sebastian Falbesoner)
Pull request description:
On single-core machines, executing the test `feature_nulldummy.py` results in the following assertion error:
```
...
2021-08-18T15:37:58.805000Z TestFramework (INFO): Test 4: Non-NULLDUMMY base multisig transaction is invalid after activation
2021-08-18T15:37:58.814000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "[...]/test/functional/test_framework/test_framework.py", line 131, in main
self.run_test()
File "[...]/test/functional/feature_nulldummy.py", line 107, in run_test
self.block_submit(self.nodes[0], [test4tx], accept=False)
File "[...]/test/functional/feature_nulldummy.py", line 134, in block_submit
assert_equal(None if accept else 'block-validation-failed', node.submitblock(block.serialize().hex()))
File "[...]/test/functional/test_framework/util.py", line 49, in assert_equal
raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
AssertionError: not(block-validation-failed == non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero))
2021-08-18T15:37:58.866000Z TestFramework (INFO): Stopping nodes
...
```
There are hardly any single-core machines around anymore, but the behaviour can be reproduced on a multi-core machine by patching the function `GetNumCores()` to return 1 on the master branch and running `feature_nulldummy.py`:
```diff
diff --git a/src/util/system.cpp b/src/util/system.cpp
index 30d410381..149b512fc 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -1338,7 +1338,7 @@ bool SetupNetworking()
int GetNumCores()
{
- return std:🧵:hardware_concurrency();
+ return 1;
}
```
As solution, parallel script verification is disabled (`-par=1`) and the exact reject reason is checked, which also increases the precision of the test (the possibility that the block is rejected because of another unintended reason is ruled out). See also related PR #22711 which applies the same approach for the p2p segwit test. The PR also includes a refactoring commit which changes the calls to `self.block_submit()` to use named arguments and removes the default value for parameter `accept` (i.e. explicitely passing `accept=...` is mandatory), with the aim to increase the test readability.
ACKs for top commit:
josibake:
ACK 7720d4f650
Saviour1001:
Tested ACK <code>[7720d4f](7720d4f650)</code>
Tree-SHA512: 8a31ebab3e2ab38e555d7a23139b3324a134a0dedc5b879a2419348ae858323882dbbfcbbf88b68e4f8d7eea8cfe43ee19da1d0d2a36c93ae7878c4980cac31d
