💬 ajtowns commented on pull request "mempool / miner: regularly flush <=0-fee entries and mine everything in the mempool":
(https://github.com/bitcoin/bitcoin/pull/27018#discussion_r1146970997)
Should say "UNSUPPORTED", not "DEPRECATED" ?

Weak NACK on removing this option -- in [inq#23](https://github.com/bitcoin-inquisition/bitcoin/pull/23#discussion_r1139697940) we've got a use case for accepting txs into the mempool that we don't intend to mine (until they are CPFP'ed).

💬 jonatack commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1146972607)
For example, I've been tweeting a bit about the recent increase in Tor and I2P peers known to my node based on -addrinfo output and mentioning that these were also recently active peer counts (active in the last month).

💬 furszy commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1146945075)
`all_entries` is not used anywhere.

💬 furszy commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1146944495)
This is just a nit but I wouldn't call this `tx5_feerate`, it's the `tx5_tx6_package_feerate`.

At least in my head, when we talk about certain tx feerate, we talk abou the relation between the tx ancestors fee and their size (including the tx itself). Which does not include the descendants (in this case, tx6)

💬 ryanofsky commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1482054469)
Updated d00762c75a1b9ad16e0dadf25886a7baefa84a12 -> 37ffeca9e0363781f8168114faad004d5543260f ([`pr/ignoredconf.7`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.7) -> [`pr/ignoredconf.8`](https://github.com/ryanofsky/bitcoin/commits/pr/ignoredconf.8), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ignoredconf.7..pr/ignoredconf.8)) disabling one of the new tests on win64 since it continues to fail https://cirrus-ci.com/task/5859482064912384

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1482065109)
added a commit that increases fuzz coverage for select by network

@brunoerg
> Also, should we have (at least) one anchor per network instead of only two "generic" ones?

yeah, def think it's worth considering in relation to increasing block-relay-peers. @mzumsande & I have started brainstorming & plan to open an issue once we formulate some initial ideas. in the meanwhile, feel free to reach out directly if you're interested in discussing further :)

💬 theStack commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1147032833)
This fee delta seems excessive (leading to a fee-rate of >1 million sats/vbyte for tx6), I guess just using one of `{high,normal,low}_fee` should also be fine? Then the testing code-path of getting a bumpfee for tx5/tx6 would be hit (see also comment below).

💬 theStack commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1147030012)
This else-branch is currently dead code (as can easily be verified by putting an `assert(false)` inside); due to the huge modified fee-rate of tx6, tx5/tx6 are way above all of the tested fee-rate-targets (>1000sats/vbyte) and always get mini-mined, i.e. no fee-bump happens. See also review comment above.

💬 theStack commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1147037874)
```suggestion
```
nit: Not that it harms in any way, but wtxid seems not to be relevant for these unit tests, so could remove this.

💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1147123753)
I've left that out on purpose, I didn't want to complicate this code any further. So far this case never happened

💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1147124025)
I've implemented the `+ (bytes == 0)` version in 9f947fc3d4b779f017332135323b34e8f216f613

💬 MarcoFalke commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#issuecomment-1482434473)
I bumped from guix 1.3 to 1.4 but that didn't fix anything

💬 vasild commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1482465778)
If you decide to go this way, something like this should work:

```cpp
// in HTTPRequest::HTTPRequest():
if (cannot parse uri) {
const int code{HTTP_BAD_METHOD};
const char* what{"Invalid URI"};
WriteReply(code, what);
throw std::system_error(code, std::generic_category(), what);
}

// in http_request_cb():
std::unique_ptr<HTTPRequest> hreq;
try {
hreq = std::make_unique<HTTPRequest>(...);
} catch (const std::system_error& e) {
LogPrint(BCLog::HTTP, "%d %s
...

💬 vasild commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1147304262)
That is a matter of personal taste (since both are allowed), but I just want to mention here why I, personally, avoid that:
* It is not gdb friendly - you can't set a breakpoint on the `if` body (yes, one can set a breakpoint on the function call, but it becomes tedious if that function is called a lot from other places)
* Creates bigger diff during changes in the future, comapre:

```diff
- if (A) B;
+ if (A) {
+ B;
+ C;
+ }
```

vs

```diff
if (A) {
B;
+ C;

...

👍 TheCharlatan approved a pull request: "guix: import/sync python-lief (0.12.3) package definition from upstream"
(https://github.com/bitcoin/bitcoin/pull/27296)
ACK 24f26e08cc0db4041c51fe8391b1736b47a13af9

I verified that the python-lief bump is indeed just https://github.com/guix-mirror/guix/blob/83bfdb409787cb2737e68b093a319b247b7858e6/gnu/packages/python-xyz.scm#L31235-L31265 .

```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum

2780a866c6a930a413cab8de3840d01c3166712d12d2f17693f30cb162eb4b92 guix-build-24f26e08cc0d/output/aarch64-linux-gnu/SHA256SUMS.part
13ccce6016de1
...

💬 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-1482503279)
> I don't understand how an "exportwatchonly" command is any more efficient or straight forward than `listdescriptors` -> `importdescriptors`

I did not mean it would be more efficient but much more user friendly (as already exists in electrum for example) to have an option that creates a "watch_wallet.DAT" that they can just load instead of manually listdescriptors and import:

`importdescriptors "[{\"desc\": \"wpkh([66bb13d5/84'/0'/0']xpub6CtDSW4S3XVd5uYp9CgsLTZKQcKieJSmjehcvfVJBSy1rPbkKN
...

⚠️ MarcoFalke opened an issue: "test_bitcoin: ./chain.h:261: uint256 CBlockIndex::GetBlockHash() const: Assertion `phashBlock != nullptr' failed."
(https://github.com/bitcoin/bitcoin/issues/27320)
https://cirrus-ci.com/task/6024293113397248?logs=ci#L3199

```
Simulating node restart
2023-03-23T20:11:01.824273Z (mocktime: 2020-08-31T15:36:02Z) [test] [logging/timer.h:58] [Log] [bench] FlushStateToDisk: write block and undo data to disk started
2023-03-23T20:11:01.824318Z (mocktime: 2020-08-31T15:36:02Z) [test] [logging/timer.h:58] [Log] [bench] FlushStateToDisk: write block and undo data to disk completed (0.03ms)
2023-03-23T20:11:01.824330Z (mocktime: 2020-08-31T15:36:02Z) [test] [l
...

💬 MarcoFalke commented on issue "test_bitcoin: ./chain.h:261: uint256 CBlockIndex::GetBlockHash() const: Assertion `phashBlock != nullptr' failed.":
(https://github.com/bitcoin/bitcoin/issues/27320#issuecomment-1482504671)
On master @ 23056436461a8b3af1a504b9638c48e8c8170652

💬 jnewbery commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1147356815)
I agree that it's personal taste and the project style guide allows either.

> It is not gdb friendly - you can't set a breakpoint on the if body (yes, one can set a breakpoint on the function call, but it becomes tedious if that function is called a lot from other places)

Are you familiar with [conditional breakpoints](https://ftp.gnu.org/old-gnu/Manuals/gdb/html_node/gdb_33.html)? You can set the breakpoint to only stop the program if a boolean condition evaluates to true. Here, I think y
...

💬 fanquake commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#issuecomment-1482575732)
> I bumped from guix 1.3 to 1.4 but that didn't fix anything

How did you bump? Was this replacing the 1.3 bin with 1.4, or did you `guix pull`?