fa2d176016683eac82dabfbfc276b8a7b07b7499 Move txoutproof RPCs to txoutproof.cpp (MarcoFalke)
Pull request description:
The txoutproof RPCs don't really fit into `rawtransaction.cpp`, as they deal with txids, not with raw transactions. As they are placed in the `blockchain` RPC category, they could be moved there. However, `blockchain.cpp` already takes about 20 seconds to compile (and `rawtransaction.cpp` even longer), so move them to a separate file.
Can be reviewed with `--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`.
ACKs for top commit:
achow101:
ACK fa2d176016683eac82dabfbfc276b8a7b07b7499
theStack:
Concept and code-review ACK fa2d176016683eac82dabfbfc276b8a7b07b7499
Tree-SHA512: 6250e5f87b6237f604d69643f9a809b238702d73f041792c537aeadeafdb60ab8e0dca1d83347d0d6c85900ce179df14365ae303ca3930ed33a528a862f85aa3
These operators are used only by the tests in std::mismatch. As
std::mismatch can take a binary predicate, we can use a lambda that
achieves the same instead.
Instead of having a pointer to the CWalletTx in COutput, we can just
store the COutPoint and the CTxOut as those are the only things we need
from the CWalletTx. Other things CWalletTx used to provide were time and
fIsFromMe but these are also being stored by COutput.
fa7deaa0464576a229b5a6ab13ad033c16d0dada wallet: Pass FastRandomContext& to coin selection (MarcoFalke)
77773b061cb13229a8afb46f6f3ab89fc70eabe3 wallet: Pass FastRandomContext& to DiscourageFeeSniping (MarcoFalke)
Pull request description:
Passing around a single randomness context shouldn't come with any downsides, but documents better where randomness is used and allows the unit test to be deterministic, if they wish to be so.
ACKs for top commit:
achow101:
ACK fa7deaa0464576a229b5a6ab13ad033c16d0dada
promag:
Code review ACK fa7deaa0464576a229b5a6ab13ad033c16d0dada.
glozow:
light code review ACK fa7deaa0464576a229b5a6ab13ad033c16d0dada
Tree-SHA512: c16287708cc82ce58311710595d0127af42fb156c93fbcaa5bde634ce323d325f4d8c99a74af24423ab22b5ad58163dd771e8b1a0e7d6bff39c9fb2a1cb21bc7
7f90dc26c8938f348938929b6d8bf1ea6f149209 options: flip listenonion to false if not listening (Vasil Dimov)
Pull request description:
If the user has unchecked "Allow incoming connections" in
`Settings->Options...->Network` then `fListen=false` is saved in
`~/.config/Bitcoin/Bitcoin-Qt.conf`. This flips `-listen` to `false`
during startup, but leaves `-listenonion` to `true`.
This flipping of `-listen` is done in `OptionsModel::Init()` after
`InitParameterInteraction()` has been executed which would have flipped
`-listenonion`, should it have seen `-listen` being `false`
(this is a difference between `bitcoind` and `bitcoin-qt`).
Fixes: https://github.com/bitcoin-core/gui/issues/567
ACKs for top commit:
mzumsande:
Tested ACK 7f90dc26c8938f348938929b6d8bf1ea6f149209
hebasto:
ACK 7f90dc26c8938f348938929b6d8bf1ea6f149209
jonatack:
utACK 7f90dc26c8938f348938929b6d8bf1ea6f149209
ryanofsky:
Code review ACK 7f90dc26c8938f348938929b6d8bf1ea6f149209.
Tree-SHA512: ff5095096858eae696293dc58d1cd5bd1bb60ef7c5d07d87308a0cf71c67da88cc00b301b550704625f136c4ba3a29905a934a766535a6422fe85d9662299d32
If the user has unchecked "Allow incoming connections" in
`Settings->Options...->Network` then `fListen=false` is saved in
`~/.config/Bitcoin/Bitcoin-Qt.conf`. This flips `-listen` to `false`
during startup, but leaves `-listenonion` to `true`.
This flipping of `-listen` is done in `OptionsModel::Init()` after
`InitParameterInteraction()` has been executed which would have flipped
`-listenonion`, should it have seen `-listen` being `false`
(this is a difference between `bitcoind` and `bitcoin-qt`).
Fixes: https://github.com/bitcoin-core/gui/issues/567
38a1b0b1967272cbb984f37bcc86b6640930319f doc: remove unneeded documentation on basic package management on FreeBSD (jessebarton)
Pull request description:
In reference to #24618
ACKs for top commit:
fanquake:
ACK 38a1b0b1967272cbb984f37bcc86b6640930319f - Thanks. In future, please re-use existing PRs, so that discussion and changes are kept together.
Tree-SHA512: ece5b85bca7f11e11d47c0674a6b96a72c3bb65dd02ab25553db511a001a9fc682c0ff8276e39d979fdd1f57a64137f586cfa548aab5c08cd9341455217b9181
9b526727000509dc6ef90f2ce6a9049edebf959c For descriptor pubkey parse errors, include context information (Ben Woosley)
Pull request description:
This adds readily-available context information to the error string, for further disambiguation.
This is a revival of #16123 which was largely addressed in #16542.
Note 'Multi:' is used rather than 'multi():' as it also encompasses 'sortedmulti():'
ACKs for top commit:
achow101:
ACK 9b526727000509dc6ef90f2ce6a9049edebf959c
theStack:
ACK 9b526727000509dc6ef90f2ce6a9049edebf959c
Tree-SHA512: 96533ea8c3ac7010f9b62e75b4bd20b65aff843030eb91c7a88312975acecaaf17909b7d1841f45edc86dbf7fa402d208adb85f0673bd79b857dbebacb8c9395
acd98adaf1d83b71eda235c49d41a92f30c16313 qt: Avoid potential -Wdeprecated-enum-enum-conversion warning (Hennadii Stepanov)
d8641f04e4350755e5264e55730392730ab7c9ee qt: Use human-readable strings in preference to hard-coded integers (Hennadii Stepanov)
Pull request description:
This PR is related to bitcoin/bitcoin#24169. It adjusts code in order to avoid `-Wdeprecated-enum-enum-conversion` warnings instead of disabling them.
Could be tested with gcc 11.2.
ACKs for top commit:
MarcoFalke:
Approach ACK acd98adaf1d83b71eda235c49d41a92f30c16313
fanquake:
untested ACK acd98adaf1d83b71eda235c49d41a92f30c16313 - thanks.
promag:
Code review ACK acd98adaf1d83b71eda235c49d41a92f30c16313.
Tree-SHA512: e8043d997d85f8dba0f37ca02f1c60eb756a1732cf76a75908b01eb2cf7a4c6d4aaf6007271a929c213de37a0c1d96bc25280f0ee9eca488f370904461222ede
Adapted in part from the CJDNS description in #23077 by Vasil Dimov
and from CJDNS documentation and feedback by Caleb James DeLisle.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
b2774fc0bed53dfaf98206d353d42c474c5bbb1a torcontrol: Query Tor for correct -onion configuration (Luke Dashjr)
Pull request description:
Currently, we just assume any running Tor instance provides localhost port 9050 for SOCKS, and configure `-onion` accordingly when we get a Tor control connection.
This actually queries the Tor node for its SOCKS listeners, and uses the configured port instead.
For backward compatibility, it falls back to localhost:9050 if it can't get any better port info. I'm not sure if that's the correct action to take when the Tor daemon explicitly says there are no ports listening...
ACKs for top commit:
laanwj:
Tested ACK (FreeBSD) b2774fc0bed53dfaf98206d353d42c474c5bbb1a
vasild:
ACK b2774fc0bed53dfaf98206d353d42c474c5bbb1a
jonatack:
ACK b2774fc0bed53dfaf98206d353d42c474c5bbb1a review, rebased to master, debug build, ran unit tests, tested happy path only
Tree-SHA512: 2fa93a3cf0cb675801d1b51322ce953ea9b2317f78154a53b603244d74252f434cc1eaa5ae48cb3fe6bdc4ce984a6d976ff95bb046f7933b9740332942378c02
fa9086d085f664a96561eeb5dd29fc1a4e4f926a test: Limit scope of id global which is shared between subtests (MarcoFalke)
Pull request description:
Globals aren't too nice when testing, as leak state between subtests run in the same process. For example, when checking peer ids in the tests, they might pass/fail depending on other tests run in the same process.
Fix this by making `id` not a global.
ACKs for top commit:
promag:
Code review ACK fa9086d085f664a96561eeb5dd29fc1a4e4f926a.
Tree-SHA512: 0a53dde428570086f4557b23112e6460d6413bedf6ef487bd56e88f83cd5f4526f44effa8076cdeaf4761ecc062c346948e0bff434808bbf9b558eabd81328e3
facd5d92e185cde608289462b400b19bba2b901c doc: Fix getblockchaininfo/getdeploymentinfo RPC docs (MarcoFalke)
Pull request description:
Also, fix whitespace to be `4` spaces. Can be reviewed with `--ignore-all-space --word-diff-regex=.`.
Found by https://github.com/bitcoin/bitcoin/pull/23083
ACKs for top commit:
luke-jr:
crACK facd5d92e185cde608289462b400b19bba2b901c
Tree-SHA512: 113228a6b140009cecd9068fb634d352148670589f647350e41c02a35e0ca306b4a2d3f2588cd9ef14a2ab7d1f23d0d2f83b5ebb00b60f17a1d16a8d71386fd2
fa8593f89892ddb09b64a7740087b725b3d7b5eb test: Fix generate calls and comments in feature_segwit (MarcoFalke)
Pull request description:
There are currently a few incorrect comments: Block `432` is mined "twice" (The second one is actually 433).
There isn't any need to mine this many blocks anyway, so remove a few calls.
ACKs for top commit:
theStack:
Tested ACK fa8593f89892ddb09b64a7740087b725b3d7b5eb
Tree-SHA512: b034077b85e6c978a80aa4de493797b4ae451d686cfb3e4fe40f37a38f41f7cb886f8e00a1c245a284be3502164b17414097fcb0bef66d155a1c1db5cfbe9e8f
fa48ea3067698954dd6630748964429686d8eaba Use MiniWallet in feature_coinstatsindex (MarcoFalke)
fab61437f68d2fa9c791f09ab6d53e3c7be535e0 test: Refactor MiniWallet get_utxo helper (MarcoFalke)
Pull request description:
Allows the test to be run even without a wallet compiled
ACKs for top commit:
josibake:
ACK fa48ea3067
ayush933:
tACK fa48ea3 . The test runs successfully with the wallet disabled.
willcl-ark:
tACK fa48ea3067 both with and without wallet compiled in.
Tree-SHA512: e04e04ea0f236c062d6be68909ece2770130ce1d5343823893073d95aebc6eedb1ad1dc5bc41e5b0cb0bf2cd9018bb1d668f0e7f5f1101ed4e0b007ed6b00f69
The regex includes [/ ] which is supposed to match either a forward slash or a
space, but m4 treats the brackets as special characters and effectively strips
them out, leading to -isystem /usr/include paths except for in the typical
scenario where it is the final parameter in the flag string.
9d2005285c77f6c4148bccfa0b8b9135abfa021c doc: Revise comments and whitespace to clarify (Ben Woosley)
def43a4d888b4a21c082404d1b25707c481d7625 refactor: Rename i to curr_try in SelectCoinsBnB (Ben Woosley)
1dd092367789749527777ac2b256e639f5706584 refactor: Track BnB selection by index (Ben Woosley)
Pull request description:
This is prompted by #13167 and presented as a friendly alternative to it.
IMO you can improve code readability and performance by about 20% by tracking the selected utxos by index, rather than by position. This reduces the storage access complexity from roughly O(utxo_size) to O(selection_size).
On my machine (median of 5 trials):
```
BnBExhaustion, 5, 650, 2.2564, 0.000672999, 0.000711565, 0.000693112 - master
BnBExhaustion, 5, 650, 1.76232, 0.000528563, 0.000568806, 0.000539147 - this PR
```
ACKs for top commit:
achow101:
reACK 9d2005285c77f6c4148bccfa0b8b9135abfa021c
glozow:
code review ACK 9d2005285c77f6c4148bccfa0b8b9135abfa021c
Xekyo:
reACK 9d2005285c77f6c4148bccfa0b8b9135abfa021c
Tree-SHA512: 453ea11ad58c48928dc76956e3e98916f6924e95510eb02fe89a899ff102fe9cc08a04d557f381ad0218a210275e5383101d971c1ffad38b06b1c57d81144315
fa61dd44f99323d10b0122c13224bc5cdb5e3d2a p2p: Serialize cmpctblock at most once in NewPoWValidBlock (MarcoFalke)
Pull request description:
Instead of serializing for each peer, serialize at most once and copy the raw data for each peer.
ACKs for top commit:
shaavan:
reACK fa61dd44f99323d10b0122c13224bc5cdb5e3d2a
theStack:
Code-review ACK fa61dd44f99323d10b0122c13224bc5cdb5e3d2a
Tree-SHA512: ed029aeaea67fdac8ddb865069f8166bc0dd8480418c405628e3e1a43b61161584a09a1814668bcd220602e8732e188be2bfed9242aa81bdbd92c64c702ed138
702759588d62869321b3359f0716247d8c4a1471 add ci/scratch dir to gitignore (josibake)
Pull request description:
Not sure if I'm missing some context as to why this isn't already ignored?
ACKs for top commit:
hebasto:
re-ACK 702759588d62869321b3359f0716247d8c4a1471, tested on Ubuntu 22.04.
Tree-SHA512: 1f13041cb27cd3687619105ac1bb3af4c31d000fcd98e5f84160c34649de532fcd8b98cb8a5bed0ba68e25b3bb344f669ea3567b9c9d86cf73386ddf276f292e
4d2b503d6cbb593cf75cf053fd480de64b3f4fc0 gui: improve "Addresses Rate-Limited" translator comments and tooltip in peers tab (Jon Atack)
81ef1f7ef182050efaed7ab40b1b159a8ada3739 gui: improve "Addresses Processed" translator comments and tooltip in peers tab (Jon Atack)
77f24aac52288532f2a818e49482b0d4761dff27 gui: improve "Address Relay" translator comments and tooltip in peers tab (Jon Atack)
Pull request description:
Per translator feedback in this thread: https://github.com/bitcoin-core/gui/pull/526#discussion_r809237830
*"The lack of string context in Transifex is a real problem for this project, as proper context (dev notes and/or screenshots) are essential to achieve quality translations."*
This pull adds developer notes for transifex translators via `extracomment` tags, and it improves the existing ones and their tooltips with more context, clarity and completeness for the following peer tab fields as a follow-up to bitcoin-core/gui#526:
- address relay
- addresses processed
- addressed rate-limited
It looks like only six lines of diff, but they are loooong lines.
If this is the right direction, the same can be done for other fields in follow-ups.
ACKs for top commit:
jarolrod:
re-ACK [4d2b503](4d2b503d6c)
hebasto:
ACK 4d2b503d6cbb593cf75cf053fd480de64b3f4fc0, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: a185f46a66375a5fd6854640745b7d1d00740cf7be58db03256f44d71acc351e1770de137cb3bc9c1f0ea3cabd7cfa1cb1ccb87ec0df222680924ca3dab6c8bf
307215b6c5e4a4689fabb37f246a410767c8a3f4 doc: clarify that BDB is only required for the legacy wallet (fanquake)
Pull request description:
ACKs for top commit:
gruve-p:
ACK 307215b6c5
darosior:
ACK 307215b6c5e4a4689fabb37f246a410767c8a3f4
Tree-SHA512: d77d013831e3e76a596603fbea80958c1cf4d3e65591debd66cd4f5ff77300dae7e81df8e7d79f3f4d2e561bb3e8090434b704586e2568ca8e89ba8196de173c
d6b24e14d2a828ba7328f0cae1d0d7c5f4f6b3bd test: Fix intermittent failure in feature_segwit (DrahtBot)
Pull request description:
There are intermittent failures on current master (https://github.com/bitcoin/bitcoin/pull/24421#issuecomment-1067451667) as the witness txs occasionally make it into other node's mempools when they shouldn't.
Fix this by removing the test as suggested earlier in https://github.com/bitcoin/bitcoin/pull/24421#discussion_r822933721.
Fixes https://github.com/bitcoin/bitcoin/issues/24590
ACKs for top commit:
aureleoules:
tACK d6b24e14d2a828ba7328f0cae1d0d7c5f4f6b3bd.
jnewbery:
Code review ACK d6b24e14d2a828ba7328f0cae1d0d7c5f4f6b3bd
Tree-SHA512: ef7be52bdbde97b6921b44824e7fbc2dd92b77b8f076794867259f2b0bff08e0def06f4b2576f63a8ccb737a15cd8441fb7db20fa1f0d4fa9c7c76d5b83388ef
