💬 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.

💬 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 :)).

💬 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`.

💬 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? :)

💬 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);
```

💬 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
...

📝 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.

📝 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
...

💬 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`.

💬 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.

💬 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
```

💬 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.

💬 apoelstra commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(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

💬 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.

💬 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() == "[::]:
...

⚠️ Honzmonz opened an issue: ". Åååååååaß2Œ"
(https://github.com/bitcoin/bitcoin/issues/27359)
.

_Originally posted by @Honzmonz in https://github.com/bitcoin/bitcoin/issues/26568_

💬 Honzmonz commented on issue ". Åååååååaß2Œ":
(https://github.com/bitcoin/bitcoin/issues/27359#issuecomment-1487840458)
[_**Bank.Poland.of;”oryginalne_pisanie i projekt polskiego systemu bankowego? Oficjalny bank bitcoina;”Bank Polski”;”kanclerz banków”;”¥•”**_]()asset_growth of wealth”,”nie moja wina”;”follow docs”;”design_approved”;”methods_proven_new_modelinbanking”;”bank of Poland”bankofYœ)

✅ pinheadmz closed an issue: ". Åååååååaß2Œ"
(https://github.com/bitcoin/bitcoin/issues/27359)

:lock: fanquake locked an issue: ". Åååååååaß2Œ"
(https://github.com/bitcoin/bitcoin/issues/27359)

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1151002690)
Just a note to other reviews who also think this looks weird: `it` is a `const` reference being passed to `visited()` so you'd think `it` (or what it points to) can't be modified, so seems like `visited()` must have no effect. Yet we're ignoring its return value. The answer is that `visited()` does modify `it->m_epoch_marker` but that member is `mutable`. To me, it seems like this shouldn't be marked mutable because it does change the state of `*it` in a way that affects behavior, not just cachi
...