💬 pinheadmz commented on issue "Export a watch wallet only (with descriptors and without private keys) for an air gap setup":
(https://github.com/bitcoin/bitcoin/issues/24829#issuecomment-1487425971)
I was able to do what you want to do with a few bash commands (see below).
I suppose we could add arguments `-file <path/to/file>` to both `listdescriptors` and `importdescriptors` that would make this even easier...?
```sh
$ bccli -regtest createwallet original
{
"name": "original",
"warning": ""
}
$ bccli -regtest listdescriptors | jq .descriptors -c >> ~/Desktop/watchonly.json
$ bccli -regtest -named createwallet wallet_name=watchonly disable_private_keys=true
{
"name
...
(https://github.com/bitcoin/bitcoin/issues/24829#issuecomment-1487425971)
I was able to do what you want to do with a few bash commands (see below).
I suppose we could add arguments `-file <path/to/file>` to both `listdescriptors` and `importdescriptors` that would make this even easier...?
```sh
$ bccli -regtest createwallet original
{
"name": "original",
"warning": ""
}
$ bccli -regtest listdescriptors | jq .descriptors -c >> ~/Desktop/watchonly.json
$ bccli -regtest -named createwallet wallet_name=watchonly disable_private_keys=true
{
"name
...
💬 willcl-ark commented on pull request "build, qt: Fix handling of `CXX=clang++` when building `qt` package":
(https://github.com/bitcoin/bitcoin/pull/27314#issuecomment-1487431386)
Can confirm this works on clang-16 (now that #27301 is also in) for me:
```
$ make -j16 qt CC=clang CXX=clang++
<snip>
Postprocessing qt...
Caching qt...
$ clang --version
Ubuntu clang version 16.0.0 (++20230315113536+8f3cb98d1fab-1~exp1~20230315113641.56)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
```
(https://github.com/bitcoin/bitcoin/pull/27314#issuecomment-1487431386)
Can confirm this works on clang-16 (now that #27301 is also in) for me:
```
$ make -j16 qt CC=clang CXX=clang++
<snip>
Postprocessing qt...
Caching qt...
$ clang --version
Ubuntu clang version 16.0.0 (++20230315113536+8f3cb98d1fab-1~exp1~20230315113641.56)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
```
💬 pinheadmz commented on issue "Adding interprocess onion access instead local ip":
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1487455451)
This is at least partially a duplicate of https://github.com/bitcoin/bitcoin/issues/5029 with preliminary work done in https://github.com/bitcoin/bitcoin/pull/9979 and https://github.com/bitcoin/bitcoin/pull/9919
However it sounds like your use case is a bit narrower? This isn't about RPC or tor hidden service or P2P connections, just tor proxy?
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1487455451)
This is at least partially a duplicate of https://github.com/bitcoin/bitcoin/issues/5029 with preliminary work done in https://github.com/bitcoin/bitcoin/pull/9979 and https://github.com/bitcoin/bitcoin/pull/9919
However it sounds like your use case is a bit narrower? This isn't about RPC or tor hidden service or P2P connections, just tor proxy?
💬 red0bear commented on issue "Adding interprocess onion access instead local ip":
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1487458874)
Just run a node using tor . there is away to set SocksPort to unix ... but bitcoin seems dont have this way to set it . Just in case i resolved ask to be sure if is possible or not do that.
(https://github.com/bitcoin/bitcoin/issues/27252#issuecomment-1487458874)
Just run a node using tor . there is away to set SocksPort to unix ... but bitcoin seems dont have this way to set it . Just in case i resolved ask to be sure if is possible or not do that.
💬 theStack commented on pull request "test: refactor: dedup mempool_package_limits.py subtests via decorator":
(https://github.com/bitcoin/bitcoin/pull/27350#discussion_r1151103742)
To make this work, we need to convert back from weight to vsize first. Since `fee = (target_weight / WITNESS_SCALE_FACTOR) * 10` looks rather weird, I decided to specify the target size directly in vsize and deduce both fee and weight from that. One more variable, but (hopefully) more readable and less indirect magic number magic involved.
(https://github.com/bitcoin/bitcoin/pull/27350#discussion_r1151103742)
To make this work, we need to convert back from weight to vsize first. Since `fee = (target_weight / WITNESS_SCALE_FACTOR) * 10` looks rather weird, I decided to specify the target size directly in vsize and deduce both fee and weight from that. One more variable, but (hopefully) more readable and less indirect magic number magic involved.
💬 theStack commented on pull request "test: refactor: dedup mempool_package_limits.py subtests via decorator":
(https://github.com/bitcoin/bitcoin/pull/27350#discussion_r1151109250)
Thanks, updated the description! (Makes sense to use "submit" only for sendrawtransaction and submitpackage RPCs; neat idea of using "testmempoolaccept" directly as a verb :)).
(https://github.com/bitcoin/bitcoin/pull/27350#discussion_r1151109250)
Thanks, updated the description! (Makes sense to use "submit" only for sendrawtransaction and submitpackage RPCs; neat idea of using "testmempoolaccept" directly as a verb :)).
💬 brunoerg commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1487543635)
I'm gonna change the approach and touch `mempool_package_limits.py`.
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1487543635)
I'm gonna change the approach and touch `mempool_package_limits.py`.
💬 instagibbs commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#discussion_r1151115104)
I don't think that RPC exists? :)
(https://github.com/bitcoin/bitcoin/pull/27351#discussion_r1151115104)
I don't think that RPC exists? :)
💬 brunoerg commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1151145374)
nit
```suggestion
if (uri_parsed) evhttp_uri_free(uri_parsed);
```
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1151145374)
nit
```suggestion
if (uri_parsed) evhttp_uri_free(uri_parsed);
```
💬 brunoerg commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1487585480)
> On the fuzz ./src/test/fuzz/test http_request.cpp, the issue is that the test itself is passing replySent as true, so the WriteReply() call on the httpserver constructor when the uri_parse is invalid. I'm investigating the rest of the test if we need to change anything there. Also, not sure, in which cases we need to create an instance of the HTTPRequest obj passing replySent as true, I didn't find a case in the entire code, unless as in the current fuzz we are talking about.
Try to set it
...
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1487585480)
> On the fuzz ./src/test/fuzz/test http_request.cpp, the issue is that the test itself is passing replySent as true, so the WriteReply() call on the httpserver constructor when the uri_parse is invalid. I'm investigating the rest of the test if we need to change anything there. Also, not sure, in which cases we need to create an instance of the HTTPRequest obj passing replySent as true, I didn't find a case in the entire code, unless as in the current fuzz we are talking about.
Try to set it
...
📝 dimitaracev opened a pull request: "validation: Move warningcache to ChainstateManager and rename to m_warningcache"
(https://github.com/bitcoin/bitcoin/pull/27357)
Removes `warningcache` and moves it to `ChainstateManager`. Also removes the respective `TODO` completely.
(https://github.com/bitcoin/bitcoin/pull/27357)
Removes `warningcache` and moves it to `ChainstateManager`. Also removes the respective `TODO` completely.
📝 theuni opened a pull request: "contrib: allow multi-sig binary verification v2"
(https://github.com/bitcoin/bitcoin/pull/27358)
Following up on #23020 from @jamesob with @achow101's additional features on top.
Both mentioned that they will be away for the next few weeks, so this is intended to keep review going.
All credit to the @jamesob and @achow101. See #23020 for the original description and [here](https://github.com/bitcoin/bitcoin/pull/23020#issuecomment-1480603300) for the added features.
I squashed the last commit from https://github.com/achow101/bitcoin/tree/pr23020-direct-bins-gpg-parse into the firs
...
(https://github.com/bitcoin/bitcoin/pull/27358)
Following up on #23020 from @jamesob with @achow101's additional features on top.
Both mentioned that they will be away for the next few weeks, so this is intended to keep review going.
All credit to the @jamesob and @achow101. See #23020 for the original description and [here](https://github.com/bitcoin/bitcoin/pull/23020#issuecomment-1480603300) for the added features.
I squashed the last commit from https://github.com/achow101/bitcoin/tree/pr23020-direct-bins-gpg-parse into the firs
...
💬 ajtowns commented on pull request "validation: Move warningcache to ChainstateManager and rename to m_warningcache":
(https://github.com/bitcoin/bitcoin/pull/27357#discussion_r1151158663)
It might be better if this were `private`. I guess something that would be possible if you moved the `for (int bit = 0; bit < VERSIONBITS_NUM_BITS; bit++) { ... }` loop into a method like:
```c++
std::vector<std::tuple<ThresholdState, int>> CheckWarnings(const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
```
then looped through the resulting vector to pull out `state` and `bit`.
(https://github.com/bitcoin/bitcoin/pull/27357#discussion_r1151158663)
It might be better if this were `private`. I guess something that would be possible if you moved the `for (int bit = 0; bit < VERSIONBITS_NUM_BITS; bit++) { ... }` loop into a method like:
```c++
std::vector<std::tuple<ThresholdState, int>> CheckWarnings(const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
```
then looped through the resulting vector to pull out `state` and `bit`.
💬 ajtowns commented on pull request "validation: Move warningcache to ChainstateManager and rename to m_warningcache":
(https://github.com/bitcoin/bitcoin/pull/27357#discussion_r1151155670)
This loop can be dropped entirely, no? The destructor's already running, so `m_warningcache` will be deleted momentarily too.
(https://github.com/bitcoin/bitcoin/pull/27357#discussion_r1151155670)
This loop can be dropped entirely, no? The destructor's already running, so `m_warningcache` will be deleted momentarily too.
💬 hebasto commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1151165179)
2e187ffafc9cb0490eedd079ce1cfffafecd9dba: trailing whitespace
```suggestion
- you can run the packaged `verifybinaries.py ... --import-keys` script to
```
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1151165179)
2e187ffafc9cb0490eedd079ce1cfffafecd9dba: trailing whitespace
```suggestion
- you can run the packaged `verifybinaries.py ... --import-keys` script to
```
💬 brunoerg commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1487611348)
Force-pushed changing the approach:
1. there is only a flag in `get_utxo` (`avoid_immature_coinbase=True`), removed that in other functions.
2. changed `mempool_package_limits` from `self.generate(self.nodes[0], COINBASE_MATURITY)` to `self.generate(self.wallet, COINBASE_MATURITY)`.
Seems like the fix in `mempool_package_limits` was simpler than I thought.
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1487611348)
Force-pushed changing the approach:
1. there is only a flag in `get_utxo` (`avoid_immature_coinbase=True`), removed that in other functions.
2. changed `mempool_package_limits` from `self.generate(self.nodes[0], COINBASE_MATURITY)` to `self.generate(self.wallet, COINBASE_MATURITY)`.
Seems like the fix in `mempool_package_limits` was simpler than I thought.
💬 apoelstra commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#discussion_r1151174283)
Lol. Updated the doccomment.
(https://github.com/bitcoin/bitcoin/pull/27351#discussion_r1151174283)
Lol. Updated the doccomment.
💬 dimitaracev commented on pull request "validation: Move warningcache to ChainstateManager and rename to m_warningcache":
(https://github.com/bitcoin/bitcoin/pull/27357#discussion_r1151194578)
Makes sense, didn't want to change much since its my first PR. Removed it with 8bba010
(https://github.com/bitcoin/bitcoin/pull/27357#discussion_r1151194578)
Makes sense, didn't want to change much since its my first PR. Removed it with 8bba010
💬 Tracachang commented on issue "Export a watch wallet only (with descriptors and without private keys) for an air gap setup":
(https://github.com/bitcoin/bitcoin/issues/24829#issuecomment-1487653224)
> I was able to do what you want to do with a few bash commands (see below).
>
> I suppose we could add arguments `-file <path/to/file>` to both `listdescriptors` and `importdescriptors` that would make this even easier...?
Yes, that would be great.
(https://github.com/bitcoin/bitcoin/issues/24829#issuecomment-1487653224)
> I was able to do what you want to do with a few bash commands (see below).
>
> I suppose we could add arguments `-file <path/to/file>` to both `listdescriptors` and `importdescriptors` that would make this even easier...?
Yes, that would be great.
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1487679446)
> Try to set it to `false` since we are calling `ReadBody()` to read and process the result.
I think we need to re-think this fuzz test (`http_request.cpp`), as due to the new approach we have these issues with it:
- `replySent` is set to `true` from test and it will fail in WriteReply() as assert(!replySent && req);.
- even we set the above to `false`, later during `GetPeer()` as the connection is null (we can see in the fuzz test checking later `assert(service.ToStringAddrPort() == "[::]:
...
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1487679446)
> Try to set it to `false` since we are calling `ReadBody()` to read and process the result.
I think we need to re-think this fuzz test (`http_request.cpp`), as due to the new approach we have these issues with it:
- `replySent` is set to `true` from test and it will fail in WriteReply() as assert(!replySent && req);.
- even we set the above to `false`, later during `GetPeer()` as the connection is null (we can see in the fuzz test checking later `assert(service.ToStringAddrPort() == "[::]:
...