💬 kristapsk commented on pull request "Fix -netinfo backward compat with getpeerinfo pre-v26":
(https://github.com/bitcoin/bitcoin/pull/29212#discussion_r1446693131)
I like that extra space as is, IMHO it visually looks better.

(https://github.com/bitcoin/bitcoin/pull/29212#discussion_r1446693131)
I like that extra space as is, IMHO it visually looks better.

⚠️ mzumsande opened an issue: "clang-format returns error"
(https://github.com/bitcoin/bitcoin/issues/29214)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
clang-format is curently broken for me (possibly related to #29056, fyi @maflcko )
I'm using Ubuntu clang-format version 14.0.0-1ubuntu1.1
### Expected behaviour
no error
### Steps to reproduce
Add a newline anywhere in e.g. `addrman.cpp`, commit, then `git diff -U0 HEAD~1.. | ./contrib/devtools/clang-format-diff.py -p1 -i -v` as described in `contrib/Readme.md`
### Relevant log o
...
(https://github.com/bitcoin/bitcoin/issues/29214)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
clang-format is curently broken for me (possibly related to #29056, fyi @maflcko )
I'm using Ubuntu clang-format version 14.0.0-1ubuntu1.1
### Expected behaviour
no error
### Steps to reproduce
Add a newline anywhere in e.g. `addrman.cpp`, commit, then `git diff -U0 HEAD~1.. | ./contrib/devtools/clang-format-diff.py -p1 -i -v` as described in `contrib/Readme.md`
### Relevant log o
...
💬 maflcko commented on issue "clang-format returns error":
(https://github.com/bitcoin/bitcoin/issues/29214#issuecomment-1883957090)
Not sure why that happens, maybe the setting was a boolean before? You could try upgrading your OS or clang-format: https://packages.ubuntu.com/jammy/clang-format-15
Also, it should be fine to remove this line in your config.
Finally, it can also be removed from the config in this repo, if needed.
(https://github.com/bitcoin/bitcoin/issues/29214#issuecomment-1883957090)
Not sure why that happens, maybe the setting was a boolean before? You could try upgrading your OS or clang-format: https://packages.ubuntu.com/jammy/clang-format-15
Also, it should be fine to remove this line in your config.
Finally, it can also be removed from the config in this repo, if needed.
💬 mzumsande commented on issue "clang-format returns error":
(https://github.com/bitcoin/bitcoin/issues/29214#issuecomment-1883966638)
> Not sure why that happens, maybe the setting was a boolean before?
Yes, that's the case, [14.0.0 doc](https://releases.llvm.org/14.0.0/tools/clang/docs/ClangFormatStyleOptions.html)
The other added option, `RequiresExpressionIndentation`, also leads to an error since it was only added in clang-format 16 (https://clang.llvm.org/docs/ClangFormatStyleOptions.html) so I would need to upgrade to that.
Happy to upgrade if this not a problem for anyone else, I usually use gcc anyway.
(https://github.com/bitcoin/bitcoin/issues/29214#issuecomment-1883966638)
> Not sure why that happens, maybe the setting was a boolean before?
Yes, that's the case, [14.0.0 doc](https://releases.llvm.org/14.0.0/tools/clang/docs/ClangFormatStyleOptions.html)
The other added option, `RequiresExpressionIndentation`, also leads to an error since it was only added in clang-format 16 (https://clang.llvm.org/docs/ClangFormatStyleOptions.html) so I would need to upgrade to that.
Happy to upgrade if this not a problem for anyone else, I usually use gcc anyway.
💬 ryanofsky commented on pull request "wallet, rpc: add BIP44 `account` in `createwallet`":
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1883982591)
re: https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1869859432
> I think this might be a better candidate for a `createwalletdescriptor` (#29130) equivalent for external signers rather than continuing to jam more arguments into `createwallet`.
This makes sense, and I added this idea to my list of potential createwalletdescriptor extensions: https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435607572
But in general, as long as the createwallet `blank` option defaults
...
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1883982591)
re: https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1869859432
> I think this might be a better candidate for a `createwalletdescriptor` (#29130) equivalent for external signers rather than continuing to jam more arguments into `createwallet`.
This makes sense, and I added this idea to my list of potential createwalletdescriptor extensions: https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435607572
But in general, as long as the createwallet `blank` option defaults
...
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446759709)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446759709)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446759761)
Fixed
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446759761)
Fixed
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446759842)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446759842)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760427)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760427)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760540)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760540)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760584)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760584)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760618)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760618)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760656)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760656)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760708)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760708)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760770)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760770)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760887)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760887)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446761034)
Updated the test to check this.
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446761034)
Updated the test to check this.
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446761065)
Done
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446761065)
Done
💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446761386)
I think that would require a much more invasive refactor as it requires accessing much lower level data. Perhaps for a followup.
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446761386)
I think that would require a much more invasive refactor as it requires accessing much lower level data. Perhaps for a followup.
📝 theStack opened a pull request: "test: assumeutxo: spend coin from snapshot chainstate after loading"
(https://github.com/bitcoin/bitcoin/pull/29215)
This PR extends the AssumeUTXO functional test by submitting a spending transaction for an UTXO that is only available in a the snapshot chainstate (after loading via `loadtxoutset`), i.e. it hasn't been seen in a block before. With that we can verify that snapshot coins are visible to the mempool.
Note that we unfortunately can't use MiniWallet here, as the only available UTXO to spend from the snapshot chainstate is at height 200, where a P2PKH created from the test framework's deterministi
...
(https://github.com/bitcoin/bitcoin/pull/29215)
This PR extends the AssumeUTXO functional test by submitting a spending transaction for an UTXO that is only available in a the snapshot chainstate (after loading via `loadtxoutset`), i.e. it hasn't been seen in a block before. With that we can verify that snapshot coins are visible to the mempool.
Note that we unfortunately can't use MiniWallet here, as the only available UTXO to spend from the snapshot chainstate is at height 200, where a P2PKH created from the test framework's deterministi
...