backport: Merge bitcoin#21712,21192, 21695,21244,21595 #5904
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vijaydasmp
changed the title
vijaydasmp
force-pushed
the
bp22_30
branch
4 times, most recently
from
204231f to
e72ed22
Compare
vijaydasmp
changed the title
vijaydasmp
changed the title
vijaydasmp
force-pushed
the
bp22_30
branch
4 times, most recently
from
e140b7d to
844b208
Compare
vijaydasmp
changed the title
vijaydasmp
force-pushed
the
bp22_30
branch
2 times, most recently
from
1b77864 to
eb35fae
Compare
vijaydasmp
changed the title
vijaydasmp
changed the title
vijaydasmp
changed the title
UdjinM6
reviewed
Mar 16, 2024
src/qt/test/rpcnestedtests.cpp
Outdated
There was a problem hiding this comment.
21557: missing changes in rpcNestedTest_rpc probably due to missing backports
Author
There was a problem hiding this comment.
one of the dependent backport (bitcoin#19717) is in #5936, , will merge after this
src/net_processing.cpp
Outdated
There was a problem hiding this comment.
20724: missing changes after if (msg_type == NetMsgType::VERACK) probably due to missing backports
vijaydasmp
changed the title
vijaydasmp
commented
Mar 19, 2024
src/qt/test/rpcnestedtests.cpp
Outdated
Author
There was a problem hiding this comment.
21557: added missing changes in rpcNestedTest_rpc, after merge of dash#5936,
vijaydasmp
commented
Mar 19, 2024
src/net_processing.cpp
Outdated
Author
There was a problem hiding this comment.
20724: added this missing change after including backport bitcoin#20146
|
This pull request has conflicts, please rebase. |
vijaydasmp
changed the title
vijaydasmp
changed the title
vijaydasmp
changed the title
vijaydasmp
changed the title
vijaydasmp
force-pushed
the
bp22_30
branch
2 times, most recently
from
b94dee2 to
8ee9122
Compare
UdjinM6
requested changes
Apr 10, 2024
vijaydasmp
force-pushed
the
bp22_30
branch
3 times, most recently
from
7b9c329 to
e837bdc
Compare
Author
|
Hello @knst @PastaPastaPasta , requesting review |
knst
approved these changes
Apr 16, 2024
src/bitcoin-cli.cpp
Outdated
Collaborator
There was a problem hiding this comment.
nit: Consider to do bitcoin#21753 also which adds docs for this option.
Can be done by next PR, not a blocker to get merged.
PastaPastaPasta
approved these changes
Apr 16, 2024
Member
PastaPastaPasta
left a comment
PastaPastaPasta
left a comment
There was a problem hiding this comment.
utACK e837bdc0f12fbc71f8f0627a93b09e2170be9808
44dab42 qa: Test default include_mempool value of gettxout (João Barbosa) Pull request description: With the following diff the functional test would pass. Fix by testing the default value. ```diff --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1142,7 +1142,7 @@ static RPCHelpMan gettxout() uint256 hash(ParseHashV(request.params[0], "txid")); int n = request.params[1].get_int(); COutPoint out(hash, n); - bool fMempool = true; + bool fMempool = false; if (!request.params[2].isNull()) fMempool = request.params[2].get_bool(); ``` ACKs for top commit: MarcoFalke: cr ACK 44dab42 Tree-SHA512: 14db21b29d6b2c01d1d1278e18a0cf35d6ae566e33e45515d1fe2983dda94ad1ff6065c217601d283f9515cae39b57e981b62ac71ec2002de5359bd8a9e3efa9
…info 882ce25 cli: Treat high detail levels as the maximum in -netinfo (Wladimir J. van der Laan) Pull request description: I somehow often type `-netinfo 5` which gets treated as `-netinfo 0`, after this change it's `-netinfo 4` which seems more convenient behavior. ACKs for top commit: jonatack: ACK 882ce25 theStack: Tested ACK 882ce25 Tree-SHA512: 5ed13213d00940c81f70c5fe5092f5fcc78c0184e1cc83b6c58a7bf24cf0f634816ce28f468aac588a4d202a6a7c1b411c0690099f1a8bf1999e662de4afcccd
…m the repo 5f2be6e Remove no longer used contrib/bitcoin-qt.pro from the repo (Hennadii Stepanov) Pull request description: From [IRC](http://www.erisian.com.au/bitcoin-core-dev/log-2021-04-15.html#l-209): > \<hebasto> wumpus: I cannot see any way how the `contrib/bitcoin-qt.pro` is used in the translation process, neither in the main repo nor in https://github.com/bitcoin-core/bitcoin-maintainer-tools. Besides it looks outdated and unmaintained. May I ask you to confirm/deny my assumption? > \<wumpus> hebasto: it is not used for anything, it exists to be able to edit the qt forms in qt designer nothing more > \<wumpus> i'm not sure if it is even *necessary* for that, but it is why it is there > \<hebasto> wumpus: thanks, qt designer does not need *.pro file at all > \<hebasto> maybe qt creator does > \<wumpus> feel free to create a PR to remove it, best way to find out if someone wants to keep it, you are right it hasn't been updated in a long time > \<hebasto> ok > \<wumpus> fwiw, the only question i get about it ever is why it exists > \<hebasto> it was in use with `qmake` years ago (what I found digging into the repo history) > \<wumpus> yes, that was the original reason, but when we switched to automake it was kept around for use w/ qt's GUI tools > \<hebasto> I've noticed it in https://github.com/bitcoin/bitcoin/blame/master/doc/translation_process.md#L25 > \<wumpus> what it says there is definitely not true anymore ACKs for top commit: laanwj: ACK 5f2be6e jarolrod: ACK 5f2be6e Tree-SHA512: 7c105612f28185097fee9e4108b162b4c8b07cc527f4438bdf5bcab08c65421ea301de8584d58770cd113fa871f6781daa8145bd6463278523449e28bfc49d06
bb8d1c6 Change ClearDataDirPathCache() to ArgsManager.ClearPathCache(). (Kiminuo) b4190ef Change GetBlocksDir() to ArgsManager.GetBlocksDirPath(). (Kiminuo) 83292e2 scripted-diff: Modify unit tests to use the ArgsManager in the BasicTestingSetup class instead of implicitly relying on gArgs. (Kiminuo) 55c68e6 scripted-diff: Replace m_args with m_local_args in getarg_tests.cpp (Kiminuo) 511ce3a BasicTestingSetup: Add ArgsManager. (Kiminuo) 1cb52ba Modify "util_datadir" unit test to not use gArgs. (Kiminuo) 1add318 Move GetDataDir(fNetSpecific) implementation to ArgsManager. (Kiminuo) 70cdf67 Move StripRedundantLastElementsOfPath before ArgsManager class. (Kiminuo) Pull request description: This PR attempts to contribute to "Remove gArgs" (bitcoin#21005). Main changes: * `GetDataDir()` function is moved to `ArgsManager.GetDataDirPath()`. * `GetBlocksDir()` function is moved to `ArgsManager.GetBlocksDirPath()`. ACKs for top commit: ryanofsky: Code review ACK bb8d1c6. Just minor const/naming changes and splitting/scripting commits since last review MarcoFalke: review ACK bb8d1c6 📓 hebasto: re-ACK bb8d1c6, addressed comments, and two commits made scripted-diffs since my [previous](bitcoin#21244 (review)) review. Tree-SHA512: ba9408c22129d6572beaa103dca0324131766f06d562bb7d6b9e214a0a4d40b0216ce861384562bde24b744003b3fbe6fac239061c8fd798abd3981ebc1b9019
06c4320 cli: use C++17 std::array class template argument deduction (CTAD) (Jon Atack) edf3167 addrinfo: raise helpfully on server error or incompatible server version (Jon Atack) bb85cbc doc: add cli -addrinfo release note (Jon Atack) 5056a37 cli: add -addrinfo command (Jon Atack) db4d2c2 cli: create AddrinfoRequestHandler class (Jon Atack) Pull request description: While looking at issue bitcoin#21351, it turned out that the problem was a lack of tor v3 addresses known to the node. It became clear (e.g. bitcoin#21351 (comment)) that a CLI command returning the number of addresses the node knows per network (with a tor v2 / v3 breakdown) would be very helpful. This patch adds that. `-addrinfo` is useful to see if your node knows enough addresses in a network to use options like `-onlynet=<network>`, or to upgrade to the upcoming tor release that no longer supports tor v2, for which you'll need to be sure your node knows enough tor v3 peers. ``` $ bitcoin-cli --help | grep -A1 addrinfo -addrinfo Get the number of addresses known to the node, per network and total. $ bitcoin-cli -addrinfo { "addresses_known": { "ipv4": 14406, "ipv6": 2511, "torv2": 5563, "torv3": 2842, "i2p": 8, "total": 25330 } } $ bitcoin-cli -addrinfo 1 error: -addrinfo takes no arguments ``` This can be manually tested, for example, with commands like this: ``` $ bitcoin-cli getnodeaddresses 0 | jq '.[] | (select(.address | contains(".onion")) | select(.address | length <= 22)) | .address' | wc -l 5563 $ bitcoin-cli getnodeaddresses 0 | jq '.[] | (select(.address | contains(".onion")) | select(.address | length > 22)) | .address' | wc -l 2842 $ bitcoin-cli getnodeaddresses 0 | jq '.[] | .address' | wc -l 25330 ``` ACKs for top commit: laanwj: Tested ACK 06c4320 Tree-SHA512: b668b47718a4ce052aff218789f3da629bca730592c18fcce9a51034d95a0a65f8e6da33dd47443cdd8f60c056c02696db175b0fe09a688e4385a76c1d8b7aeb
PastaPastaPasta
force-pushed
the
bp22_30
branch
from
e837bdc to
262fe0a
Compare
5 tasks
PastaPastaPasta
added a commit
that referenced
this pull request
May 28, 2024
, bitcoin#22547, bitcoin#22544, bitcoin#22959, bitcoin#23324, partial bitcoin#20764 (cli backports: part 2) bde72a4 merge bitcoin#23324: print peer counts for all reachable networks in -netinfo (Kittywhiskers Van Gogh) 4b24544 merge bitcoin#22959: Display all proxies in -getinfo (Kittywhiskers Van Gogh) 30b0fcf merge bitcoin#22544: drop torv2; torv3 becomes onion per GetNetworkName() (Kittywhiskers Van Gogh) b6ca36e merge bitcoin#22547: Add progress bar for -getinfo (Kittywhiskers Van Gogh) 1f89bfd merge bitcoin#21832: Implement human readable -getinfo (Kittywhiskers Van Gogh) 2200b78 merge bitcoin#20877: user help and argument parsing improvements (Kittywhiskers Van Gogh) bd934c7 partial bitcoin#20764: cli -netinfo peer connections dashboard updates (Kittywhiskers Van Gogh) b2d8656 merge bitcoin#21261: update inbound eviction protection for multiple networks, add I2P peers (Kittywhiskers Van Gogh) 0b16b50 cli: fix loop counter comparison in `ProcessReply` (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependency for #6035 * Dependency for #6031 * In [dash#5904](#5904) ([bitcoin#21595](bitcoin#21595)), one of the loops in `ProcessReply` is supposed to iterate `rows.size()` times (which at the time was hardcoded to `3`), the backport erroneously set the value to `m_networks.size()` (which also evaluated to `3`) as part of increasing `m_networks.size()` usage. As this pull request includes [bitcoin#23324](bitcoin#23324), which changes it over to `rows.size()`, the above has been corrected in a separate commit for documentation purposes. * `-addrinfo` output  * `-getinfo` output (diamonds are due to rendering limitations of my terminal and are not indicative of the symbols used)  * `-netinfo` output  ## Breaking Changes * CLI `-addrinfo` now returns a single field for the number of `onion` addresses known to the node instead of separate `torv2` and `torv3` fields, as support for TorV2 addresses was removed from Dash Core in 18.0. * `-getinfo` has been updated to return data in a user-friendly format that also reduces vertical space. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone ACKs for top commit: PastaPastaPasta: utACK bde72a4 Tree-SHA512: 921cb45b7e243a321a32c835eb23d5ba8df610ff234a548a9051436a2c21845ce70097fb9a9bb812b77b04373f9f0a9f90264168d97b08da1890be06bfd9f99c
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
backport: Merge bitcoin#21712,21192, 21695,21244,21595