💬 MarcoFalke commented on pull request "ci: use LLVM/Clang 15 and Valgrind 3.19 in Valgrind jobs":
(https://github.com/bitcoin/bitcoin/pull/27444#discussion_r1164382055)
> using newer tools/compilers

No strong opinion, but using `clang` makes that easier, because you can just bump the tag to `ubuntu:devel` and get clang 16 that way.

💬 instagibbs commented on pull request "Remove BIP35 mempool p2p message":
(https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1505589569)
> Adding a loadmempool RPC seems to be exactly what we want here?

concept ACK on this, if that's the only use-case. We should probably offer a similar service over RPC first, in addition to information gathering that it's not actually being used in meaningful amounts in production. I've never come across usage myself in my production work.

I'm not sure where "20 lines of code" came from as it looks like at least a few multiples of that, but in general, removing a service no one uses to mak
...

💬 fanquake commented on pull request "ci: use LLVM/Clang 15 and Valgrind 3.19 in Valgrind jobs":
(https://github.com/bitcoin/bitcoin/pull/27444#discussion_r1164398634)
Ok. I'll leave this for now, so we can hotswap easier

💬 achow101 commented on pull request "Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet":
(https://github.com/bitcoin/bitcoin/pull/27279#issuecomment-1505619302)
ACK 7ccdd741fe1544c13b2a9b7baa5c5727e84d6e55

💬 instagibbs commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#discussion_r1164413849)
I'm confused, why is the subtraction of incremental relay total fee necessary?

💬 brunoerg commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#discussion_r1164415030)
I think you could use `assert_equal` here and in the other similar cases

```suggestion
assert_equal(len(peer_info), 1)
```

💬 glozow commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#discussion_r1164415485)
New mempool min feerate is the feerate of removed + incremental:
https://github.com/bitcoin/bitcoin/blob/7f4ab67e7be615b2425a85fb65872fc2327f58c3/src/txmempool.cpp#L1063-L1068

🚀 achow101 merged a pull request: "Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet"
(https://github.com/bitcoin/bitcoin/pull/27279)

💬 instagibbs commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#discussion_r1164418072)
Ah, adding a comment to this effect helps. Seems really out of place otherwise.

✅ jonatack closed a pull request: "rpc, test: remove newline escape sequence from wallet warning fields"
(https://github.com/bitcoin/bitcoin/pull/27138)

💬 jonatack commented on pull request "rpc, test: remove newline escape sequence from wallet warning fields":
(https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1505632346)
Superceded by #27279. Thanks everyone!

💬 MarcoFalke commented on pull request "ci: use Debian Bookworm and Valgrind 3.19 in Valgrind jobs":
(https://github.com/bitcoin/bitcoin/pull/27444#discussion_r1164446868)
Ah sorry. You'll need clang/llvm libfuzzer anyway. Discussion can be closed/resolved.

💬 glozow 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_r1164447592)
Resolving - restored support for -blockmintxfee (now it's actually just "deprecated"), default is 0, and added this warning when it is set.

💬 benthecarman commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1164447836)
Looks like it is only networking settings in there and all of them only print error logs, I think we'd want to explicitly throw in this case

👍 MarcoFalke approved a pull request: "ci: use Debian Bookworm and Valgrind 3.19 in Valgrind jobs"
(https://github.com/bitcoin/bitcoin/pull/27444#pullrequestreview-1381802811)
re-lgtm. Didn't test, except that the gcc suppression can be dropped on Bookworm and later.

💬 pinheadmz commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#issuecomment-1505684094)
> 🐙 This pull request conflicts with the target branch and [needs rebase](https://github.com/bitcoin-core/gui/blob/master/CONTRIBUTING.md#rebasing-changes).

Thanks D-bot! rebased.

💬 achow101 commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1164465825)
Since we already have the height of a UTXO, this could instead get the current block height and compute the confirmation count on the fly rather than using the stored confirmations. In fact, we probably shouldn't have the confirmation count for each UTXO at all since it can become outdated very quickly.

💬 benthecarman commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1505698929)
Pushed up some changes to respond to @stickies-v and @MarcoFalke's review

💬 benthecarman commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1505704805)
@ajtowns I don't think I am properly communicating my usecase correctly, but there are lots of concept acks from other developers that run signets as well. People want to use custom signets to have their own test networks, I don't see why disallowing having custom block times shouldn't be allowed. The main concern being people don't copy-paste the param into their conf and have troubles syncing, doesn't seem like a valid one given that they could do the same for `-signetchallenge`. I understand
...

💬 glozow commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1505710683)
> Would it make sense to replace this with a PR that enabled package relay without v3 or package RBF, and included this policy? I think that would be the first ~10 commits from https://github.com/bitcoin/bitcoin/pull/25038? That seems like it would be more meaningful progress...

Right, the changes for package relay only (i.e. no v3, no package RBF) would be this, persist packages across restart, and allow any ancestor package instead of just child-with-parents. Basically this PR + the first 7
...