💬 brunoerg commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1487160825)
> @brunoerg are you following up here?
Yes.
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1487160825)
> @brunoerg are you following up here?
Yes.
💬 ismaelsadeeq commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#discussion_r1150828780)
Thanks :100:
(https://github.com/bitcoin/bitcoin/pull/27349#discussion_r1150828780)
Thanks :100:
💬 josibake commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#issuecomment-1487203381)
re-ACK https://github.com/bitcoin/bitcoin/pull/27349/commits/e47ce42f670fc43859c157766b342509ab5916f9
(https://github.com/bitcoin/bitcoin/pull/27349#issuecomment-1487203381)
re-ACK https://github.com/bitcoin/bitcoin/pull/27349/commits/e47ce42f670fc43859c157766b342509ab5916f9
👍 theStack approved a pull request: "test: use address_to_scriptpubkey instead of RPC call"
(https://github.com/bitcoin/bitcoin/pull/27349)
ACK e47ce42f670fc43859c157766b342509ab5916f9 🌱
(https://github.com/bitcoin/bitcoin/pull/27349)
ACK e47ce42f670fc43859c157766b342509ab5916f9 🌱
💬 josibake commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1487242654)
Concept ACK
Regarding the approach, I think we should always apply the filter or allow the argument but have it default to True and only let you change it to false if you are calling `get_utxo` directly (no argument forwarding).
I'm also a little confused by the `mempool_package_limit.py`, because it looks like the test assumes it is already working with mature UTXOs:
https://github.com/bitcoin/bitcoin/blob/master/test/functional/mempool_package_limits.py#L26-L29
Using immature UTXO
...
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1487242654)
Concept ACK
Regarding the approach, I think we should always apply the filter or allow the argument but have it default to True and only let you change it to false if you are calling `get_utxo` directly (no argument forwarding).
I'm also a little confused by the `mempool_package_limit.py`, because it looks like the test assumes it is already working with mature UTXOs:
https://github.com/bitcoin/bitcoin/blob/master/test/functional/mempool_package_limits.py#L26-L29
Using immature UTXO
...
💬 furszy commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#issuecomment-1487314169)
> @furszy, thanks for the thorough review. I will start implementing your suggestions, but do you think you could help me with your comments about encapsulation like [#27224 (comment)](https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1150541159)?
>
> For background, I've been a little detached from the address encapsulation attempt, and personally like m_address_book.find(dest) / m_addressbook[dest] lookups and for loops a little better than calling FindAddress/FindOrInsertAddress/Fo
...
(https://github.com/bitcoin/bitcoin/pull/27224#issuecomment-1487314169)
> @furszy, thanks for the thorough review. I will start implementing your suggestions, but do you think you could help me with your comments about encapsulation like [#27224 (comment)](https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1150541159)?
>
> For background, I've been a little detached from the address encapsulation attempt, and personally like m_address_book.find(dest) / m_addressbook[dest] lookups and for loops a little better than calling FindAddress/FindOrInsertAddress/Fo
...
💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1487331264)
Rebased
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1487331264)
Rebased
💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1487397883)
Added a scripted-diff to completely replace all occurrences of the application-defined `RPC_INVALID_PARAMETER` (-8) with the JSONRPC 2.0 spec-defined `RPC_INVALID_PARAMS` (-32602)
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1487397883)
Added a scripted-diff to completely replace all occurrences of the application-defined `RPC_INVALID_PARAMETER` (-8) with the JSONRPC 2.0 spec-defined `RPC_INVALID_PARAMS` (-32602)
💬 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
...