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

💬 mzumsande commented on pull request "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting":
(https://github.com/bitcoin/bitcoin/pull/27411#issuecomment-1505713147)
- Updated the PR to separate privacy checks from reachability checks.
- Passed a CNode to `GetLocal()` and replaced pointers with references in `GetLocal()` and `GetReachabilityFrom()`
- Added a unit test

📝 1440000bytes opened a pull request: "doc: remove incorrect line from example"
(https://github.com/bitcoin/bitcoin/pull/27454)
Inputs are always present when using `walletcreatefundedpsbt`

💬 earuak commented on pull request "test: added coverage to rpc_scantxoutset.py":
(https://github.com/bitcoin/bitcoin/pull/27453#issuecomment-1505715474)
The test is now run before starting an RPC procedure to ensure a valid RPC command is entered.

This change appears to be good programming practice, as it helps ensure that the system works correctly and that users receive immediate feedback if they enter an invalid command. RPC failing if "start", "status" or "abort" doesn't stop the first command also seems to be a proper way to handle the problem.

Overall, including rigorous unit testing is a best practice in software programming, especi
...

💬 stickies-v commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#discussion_r1164480479)
You're right, we do want to throw. `AppInitParameterInteraction()` does seem to raise `InitError` based on bad parameter interaction. I'm not sure why we have both `AppInitParameterInteraction()` and `InitParameterInteraction()` and which needs to be used when, it just seems like since this effectively is parameter interaction we'd like to raise that somewhere in here. Hopefully someone else can chime in?

🤔 brunoerg reviewed a pull request: "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py"
(https://github.com/bitcoin/bitcoin/pull/27452#pullrequestreview-1381854522)
Concept ACK

💬 earuak commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#issuecomment-1505723431)
It is important to include test cases to ensure that the code is working correctly and that adding new features like logic to compute the addrv2 serialization of the onion v3 address can help improve the functionality of Bitcoin. Also, it's always good to keep working on future coverage for other modules and features.

👍 instagibbs approved a pull request: "mempool: disallow txns under min relay fee, even in packages"
(https://github.com/bitcoin/bitcoin/pull/26933#pullrequestreview-1381762702)
ACK 6fb01d26c57f6af7205721e528bbafee8fa41027

💬 instagibbs commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#discussion_r1164440396)
Could we use `CTxMemPool::info` on parent and child to assert that the (fee + nFeeDelta) summed == 0 for the test so I don't have to think so hard to know this is right