💬 kevkevinpal commented on pull request "ci: check if file or directory already exists for ${HOME}/.bitcoin instead of failing":
(https://github.com/bitcoin/bitcoin/pull/33486#issuecomment-3348307035)
> > For me it is just weird to see the script failing here since creating a file or dir shouldn't fail
>
> It actually should fail. The goal of the check is to verify no mainnet datadir access is happening from any of the tests. Maybe the comment could be clarified to mention this?
>
> (Disabling this check would be better by just removing it)

gotcha that makes more sense to me now, I can open up PR modifying the comment to reflect that.

📝 kevkevinpal opened a pull request: "doc: add clarifying comment that creating the .bitcoin file is to avoid any mainnet datadir access"
(https://github.com/bitcoin/bitcoin/pull/33503)
referenced from https://github.com/bitcoin/bitcoin/pull/33486#issuecomment-3347684298

This adds a clarifying comment to tell the reader that the creation of the `${HOME}/.bitcoin` file is to ensure that there is no mainnet datadir access happening from any of the tests

💬 kevkevinpal commented on pull request "ci: check if file or directory already exists for ${HOME}/.bitcoin instead of failing":
(https://github.com/bitcoin/bitcoin/pull/33486#issuecomment-3348325661)
I opened this PR https://github.com/bitcoin/bitcoin/pull/33503

💬 kevkevinpal commented on pull request "tests: Add witness commitment if we have a witness transaction in `FullBlockTest.update_block()`":
(https://github.com/bitcoin/bitcoin/pull/31823#issuecomment-3348355115)
reACK [43c2613](https://github.com/bitcoin/bitcoin/pull/31823/commits/43c2613305697255d294d9dd02f1cd19246913c9)

💬 mzumsande commented on pull request "wallet: reduce unconditional logging during load":
(https://github.com/bitcoin/bitcoin/pull/33299#issuecomment-3348361581)
Changed to approach by @furszy which removes the logging from the sqlite ctor similar as suggested by stickies-v, and should also pass the linter.

I'm only trying to remove the repeated logging here, no idea if the `VerifyWallets()` / `g_sqlite_count` interactions that the discussion here revealed are worth fixing - something for another PR or Issue perhaps.

👋 mzumsande's pull request is ready for review: "wallet: reduce unconditional logging during load"
(https://github.com/bitcoin/bitcoin/pull/33299)

💬 ryanofsky commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3348387503)
That worked! Am able to reproduce with `gdb -ex run --args build/bin/bitcoin -m node -ipcbind=unix -regtest` and `cargo run --example logger "$HOME/.bitcoin/regtest/node.sock"` and `killall bitcoin-node`.

Then I see a hang with same stack trace that you posted.

If I run `bitcoin-cli -regtest stop` instead of `killall` there is no problem and shutdown works.

Patch in #33387 does not seem sufficient to fix the problem and does not seem to stop the hang or change the stack trace. Apparently it d
...

💬 fanquake commented on pull request "ci: add libcpp hardening flags to macOS fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33462#discussion_r2388883509)
Rebased on #33489.

🤔 furszy reviewed a pull request: "wallet: reduce unconditional logging during load"
(https://github.com/bitcoin/bitcoin/pull/33299#pullrequestreview-3281260225)
utACK fc861332b351c9390400054ff74193ce26eb0713

💬 fanquake commented on pull request "ci: use latest versions of lint deps":
(https://github.com/bitcoin/bitcoin/pull/33487#discussion_r2388925121)
Yea. Using 0.13.2 now.

✅ fanquake closed an issue: "build: `test_bitcoin` link failure with `-flto=thin` & address sanitizer"
(https://github.com/bitcoin/bitcoin/issues/33147)

💬 fanquake commented on issue "build: `test_bitcoin` link failure with `-flto=thin` & address sanitizer":
(https://github.com/bitcoin/bitcoin/issues/33147#issuecomment-3348652112)
I think either, given we've got this working in oss-fuzz again.

👍 ryanofsky approved a pull request: "ci: detect outbound internet traffic generated while running tests"
(https://github.com/bitcoin/bitcoin/pull/31349#pullrequestreview-3281414481)
Code review ACK f4fdf81d3a1811561ed35d6ce2424978be284d51
, just dropping INTERNET_TRAFFIC_EXPECTED variable since last review

📝 instagibbs opened a pull request: "Mempool: Do not enforce TRUC checks on reorg"
(https://github.com/bitcoin/bitcoin/pull/33504)
This was the intended behavior but our tests didn't cover the scenario where in-block transactions themselves violate TRUC topological constraints.

The behavior in master will potentially lead to many erroneous evictions during a reorg, where evicted packages may be very high feerate and make sense to mine all together in the next block.

This issue exists since the merge of https://github.com/bitcoin/bitcoin/pull/28948/files#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b
...

💬 instagibbs commented on pull request "Mempool: Do not enforce TRUC checks on reorg":
(https://github.com/bitcoin/bitcoin/pull/33504#issuecomment-3348753470)
cc @glozow @sdaftuar

💬 fanquake commented on pull request "Mempool: Do not enforce TRUC checks on reorg":
(https://github.com/bitcoin/bitcoin/pull/33504#issuecomment-3348792877)
https://github.com/bitcoin/bitcoin/actions/runs/18108538923/job/51529011305?pr=33504#step:8:1778:
```bash
Error processing input "/Users/runner/work/bitcoin/bitcoin/ci/scratch/qa-assets/fuzz_corpora/tx_pool/93f59991917f0dc980deabf80d553cf4573ebdd5"

test/util/txmempool.cpp:191 CheckMempoolTRUCInvariants: Assertion `entry.GetCountWithDescendants() <= TRUC_DESCENDANT_LIMIT' failed.
Error processing input "/Users/runner/work/bitcoin/bitcoin/ci/scratch/qa-assets/fuzz_corpora/tx_pool/93f59991917
...

🤔 mzumsande reviewed a pull request: "net: support overriding the proxy selection in ConnectNode()"
(https://github.com/bitcoin/bitcoin/pull/33454#pullrequestreview-3281486955)
Concept ACK

💬 mzumsande commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#discussion_r2389093456)
Is this an oversight? Before, we'd use the network of `target_addr` instead of `addrConnect`, which makes more sense to me.

💬 mzumsande commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#discussion_r2389142994)
should be `proxy_arg` to be consistent with the definition (although maybe `proxy_override` would be better?)

💬 instagibbs commented on pull request "Mempool: Do not enforce TRUC checks on reorg":
(https://github.com/bitcoin/bitcoin/pull/33504#issuecomment-3348879737)
@fanquake thanks, TRUC topology is no longer actually being stopped when limits are bypassed(when a reorg happens), so invariants checks for TRUC may fail now. I now removed the limits check flags because they're not actually being used to model reorgs. If we want to fuzz reorgs, we should do that instead.