Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backport: bitcoin#23002, 24281, 24360, 24347, 24376, 24370, 24305 #6435

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

vijaydasmp
Copy link

Backports

@vijaydasmp vijaydasmp changed the title backport : bitcoin#24349, 23002, 24281, 24360, 24347, 24376, 24343, 24137, 24370, 24305 backport: bitcoin#24349, 23002, 24281, 24360, 24347, 24376, 24343, 24137, 24370, 24305 Nov 27, 2024
@vijaydasmp vijaydasmp force-pushed the Dec_2024_01 branch 4 times, most recently from 7158b15 to f468c77 Compare November 30, 2024 15:09
MarcoFalke and others added 5 commits December 3, 2024 11:08
9c1052a wallet: Default new wallets to descriptor wallets (Andrew Chow)
f19ad40 rpc, wallet: Descriptor wallets are no longer experimental (Andrew Chow)

Pull request description:

  Changes the default wallet type from legacy to descriptors. Descriptor wallets will now by the default type. Additionally, descriptor wallets will no longer be marked as experimental.

  This follows the timeline proposed in bitcoin#20160

ACKs for top commit:
  lsilva01:
    Tested ACK bitcoin@9c1052a on Ubuntu 20.04
  prayank23:
    tACK bitcoin@9c1052a
  meshcollider:
    Code review ACK 9c1052a

Tree-SHA512: 834e6fec88e0c18673af7ebe135bd5333694d1be502164eb93a90e3e76c27974165aa4e59426945100c88e4eca07356e16886ef5b05cf789683ecb23fc71a12a
8e9699c Update doc to match new default wallet type (Bitcoin Hodler)

Pull request description:

  bitcoin#23002 changed the default wallet type to descriptors, so this doc was out of date.

ACKs for top commit:
  achow101:
    ACK 8e9699c

Tree-SHA512: 2f69b23c153163bf2a091dbf728b713d28f795cc81e031bf201160882d2456494e94955ff6385634615fdcfece11542749ad1c982e2994e64ed69011380a2353
…m users and devs

a4da16f Improve -netinfo help based on feedback from users and devs (Jon Atack)

Pull request description:

  Clarify which networks are displayed by the peer counts table (*reachable* networks; follow-up to bitcoin#23324) in response to questions received over the past months, and a few other improvements.

ACKs for top commit:
  laanwj:
    Code review ACK a4da16f
  w0xlt:
    ACK a4da16f
  kristapsk:
    utACK a4da16f

Tree-SHA512: e6522c08421aa7f10d50723156d0a8fc5ec82cad2f0bd931bbec603077fcd4921c6505ef743d57386fba81c95dcfc77df75abf3378319886368e4ae33f9a6d73
…hain

fa8dad0 rpc: Fix implicit-integer-sign-change in verifychain (MarcoFalke)

Pull request description:

  It doesn't really make sense to treat `DEFAULT_CHECKLEVEL` as unsigned as long as `VerifyDB` accepts a signed integer.

  Making it signed also avoids a cast round trip from signed->unsigned->signed in the RPC.

ACKs for top commit:
  luke-jr:
    utACK fa8dad0
  theStack:
    Code-review ACK fa8dad0

Tree-SHA512: 75499dbe4ace2962792e5fbec7defb10c25fdbbfde951d5e542a91daa880cc50395da0287173e2c84a28e18267c74af7b44b9f38ce364bcb0216c402f65b7641
…comment)

62cc138 Rename wallet-tool to bitcoin-wallet in code comment (Kristaps Kaupe)
0db3ad3 Mention -signet in bitcoin-wallet help output (Kristaps Kaupe)

Pull request description:

  * Mention `-signet` in sentence where there is already `-testnet/-signet` in help output.
  * Rename `wallet-tool` to `bitcoin-wallet` in single remaining place in code comments (was already done in bitcoin#17648 at other places).

ACKs for top commit:
  RandyMcMillan:
    tACK 62cc138

Tree-SHA512: c5df7811b8200f61943908dcf3b2b788fe991bf00bef28f069ab8784924556ffd5d86fc0ba2ad0b3c3f9be2ba73a34bc67059d7c057bba646c1801ffa3cb2070
MarcoFalke added 2 commits December 3, 2024 21:16
…getnodeaddresses and -addrinfo

ce69084 cli: describe quality/recency filtering in -addrinfo (Jon Atack)
7c97561 rpc: describe quality/recency filtering in getnodeaddresses (Jon Atack)

Pull request description:

  Addresses bitcoin#24278.

  ```
  $ bitcoin-cli help getnodeaddresses
  getnodeaddresses ( count "network" )

  Return known addresses, after filtering for quality and recency.
  These can potentially be used to find new peers in the network.
  The total number of addresses known to the node may be higher.
  ```
  ```
  $ bitcoin-cli -help | grep -A3 addrinfo
    -addrinfo
         Get the number of addresses known to the node, per network and total,
         after filtering for quality and recency. The total number of
         addresses known to the node may be higher.
  ```

ACKs for top commit:
  mzumsande:
    Thanks, Code Review ACK ce69084
  prayank23:
    reACK bitcoin@ce69084

Tree-SHA512: 82d23b15e64a99411eb8e70d7267a1b4f23182fabe072e824277569d9677e392b466be63f00e3d157d7db94bbe032d53f12ad4ab30b55b7b8a629c37d80d1d8c
e50a9be Remove outdated comment on CFeeRate (Murch)

Pull request description:

  This comment described how the constructor of CFeeRate was previously indirectly used to parse fee rate arguments from RPCs. The command line input was actually in sat/vB but due to the use of AmountFromValue() it got converted to BTC/vB which then got rectified in the constructor by creating a CFeeRate from that given value and COIN as the transaction size. Since this usage pattern was removed from the codebase some months ago, the comment is now obsolete.

ACKs for top commit:
  michaelfolkson:
    ACK e50a9be
  jonatack:
    ACK e50a9be

Tree-SHA512: f17bf0baeeca85a5c7883edadd407da845f6e3af1c949e93116bd67c02e601682a5f7f1ab2497172472e3acf1c4e3c234b01161a77e7d7f028e3551da34777f0
@vijaydasmp vijaydasmp marked this pull request as ready for review December 3, 2024 18:09
@vijaydasmp vijaydasmp changed the title backport: bitcoin#24349, 23002, 24281, 24360, 24347, 24376, 24343, 24137, 24370, 24305 backport: bitcoin#23002, 24281, 24360, 24347, 24376, 24370, 24305 Dec 4, 2024
@vijaydasmp
Copy link
Author

Hello @PastaPastaPasta @UdjinM6 @knst requesting review

@@ -31,6 +31,7 @@ static void SetupWalletToolArgs(ArgsManager& argsman)
argsman.AddArg("-dumpfile=<file name>", "When used with 'dump', writes out the records to this file. When used with 'createfromdump', loads the records into a new wallet.", ArgsManager::ALLOW_STRING, OptionsCategory::OPTIONS);
argsman.AddArg("-debug=<category>", "Output debugging information (default: 0).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-descriptors", "Create descriptors wallet. Only for 'create'", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS);
argsman.AddArg("-legacy", "Create legacy wallet. Only for 'create'", ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

23002: @knst please evaluate if we are ready for descriptor by default. I thought we had more we needed to fix / implement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants