💬 theuni commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3377330552)
Rebased on #33518 temporarily to make sure ci is happy with everything combined.

💬 theuni commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#discussion_r2410969953)
I only did it this way because it's more readable to me that way, and because I figured there may be more logging options for us to use in the future that would make the in-place init more complicated.

Happy to change it if you feel strongly.

💬 theuni commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#discussion_r2410980251)
I'm torn about what to do here, so feedback welcome. Adding another value upstream at any point would cause a crash here. But because we don't know what that future value would be, we can't predict how we should react to it now.

Just commenting because this effectively freezes `mp::Log`, and would require adding new fields to `LogMessage` instead. I'm ok with that, but @ryanofsky might not be.

💬 Sjors commented on pull request "Update libmultiprocess subtree to support reduced logging":
(https://github.com/bitcoin/bitcoin/pull/33518#issuecomment-3377368922)
utACK eda91b07fd9f2a6af3c31659d51f51aacf8989c4

Just checked the subtree update validity. Will do some testing with #33517 and https://github.com/Sjors/sv2-tp

📝 vasild opened a pull request: "node: change a tx-relay on/off flag to enum"
(https://github.com/bitcoin/bitcoin/pull/33567)
Previously the `bool relay` argument to `BroadcastTransaction()` designated:

```
relay=true: add to the mempool and broadcast to all peers
relay=false: add to the mempool
```

Change this to an `enum`, so it is more readable and easier to extend with a 3rd option. Consider these example call sites:

```cpp
Paint(true);
// Or
Paint(/*is_red=*/true);
```

vs

```cpp
Paint(RED);
```

The idea for putting `TxBroadcastMethod` into `node/types.h` by Ryan.

---

This is part o
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3377395591)
`7d837fef28...a9d697c0e1`: minor changes to facilitate chopping off two pieces of this into smaller independent PRs:

https://github.com/bitcoin/bitcoin/pull/33565
https://github.com/bitcoin/bitcoin/pull/33567

📝 Sjors opened a pull request: "doc: how to update a subtree"
(https://github.com/bitcoin/bitcoin/pull/33568)
We have instructions on how to verify a subtree update, but not on how to perform one.

💬 mzumsande commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#issuecomment-3377423901)
> NIT: why keep looping ? is there a path where the node can recover from this ?

Not if we are actually corrupted, but we can't be 100% sure. Maybe we are right, and the next peer would extend the chain that we see as valid.

There is the possibility of an an attack where a miner creates an invalid block with valid PoW (which is costly), and has some sybil nodes feed that chain to others - it wouldn't be good if this could be used to shutdown nodes. Or there could be some kind of chaotic ch
...

💬 Sjors commented on pull request "miner: empty mempool special case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3377434108)
@sipa I ended up updating `test/functional/interface_ipc.py` to reflect the new behavior, but I'm not sure what the original test tried to do.

💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3377440627)
Reviewers may find #33566 interesting.

💬 fanquake commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3377516698)
> Added to the nightly builds in https://github.com/hebasto/bitcoin-core-nightly/pull/74.

Looks like there are failing unit tests (https://github.com/hebasto/bitcoin-core-nightly/actions/runs/18313967330/job/52151177201#step:6:401):
```bash
test\util_string_tests.cpp(43): Entering test case "ConstevalFormatString_NumSpec"
test/util_string_tests.cpp(40): error: in "util_string_tests/ConstevalFormatString_NumSpec": exception "const char*" raised as expected: validation on the raised exceptio
...

💬 hebasto commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3377526162)
> > Added to the nightly builds in https://github.com/hebasto/bitcoin-core-nightly/pull/74.
>
>
>
> Looks like there are failing unit tests (https://github.com/hebasto/bitcoin-core-nightly/actions/runs/18313967330/job/52151177201#step:6:401):
>
> ```bash
>
> test\util_string_tests.cpp(43): Entering test case "ConstevalFormatString_NumSpec"
>
> test/util_string_tests.cpp(40): error: in "util_string_tests/ConstevalFormatString_NumSpec": exception "const char*" raised as expected: validation
...

💬 furszy commented on pull request "doc: how to update a subtree":
(https://github.com/bitcoin/bitcoin/pull/33568#discussion_r2411126967)
Need to add the remote first. E.g.
```
git remote add libmultiprocess <repo_URL>
git subtree pull --prefix=src/ipc/libmultiprocess libmultiprocess master --squash
```

👍 willcl-ark approved a pull request: "[27.x] Fix Qt download URLs"
(https://github.com/bitcoin/bitcoin/pull/33564#pullrequestreview-3310847305)
utACK 9652f4754d93c41f224f29b41110ebd21f55d976

Backports look OK.

💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2411161631)
Any reason we can't use `memory_order_relaxed` on loads and stores of `m_interrupt`?

💬 Sjors commented on pull request "doc: how to update a subtree":
(https://github.com/bitcoin/bitcoin/pull/33568#discussion_r2411169370)
That's explained in the linter doc that's linked right above, but I can duplicate it if needed.

👍 theuni approved a pull request: "Update libmultiprocess subtree to support reduced logging"
(https://github.com/bitcoin/bitcoin/pull/33518#pullrequestreview-3311068924)
utACK eda91b07fd9f2a6af3c31659d51f51aacf8989c4.

The upstream changes are good ones and this is a good time to sync up.

💬 monlovesmango commented on pull request "wallet: don't consider unconfirmed TRUC coins with ancestors":
(https://github.com/bitcoin/bitcoin/pull/33528#issuecomment-3377833421)
Wow so on top of it @glozow :)

A couple things.

When I went back to test the commands I pasted in https://github.com/bitcoin/bitcoin/issues/33368#issuecomment-3319935660 I found that it was still failing silently despite the functional test failing appropriately. I was able to tweak the functional test a bit and get it to also mimic the silent failing behavior in this branch:
https://github.com/monlovesmango/bitcoin/tree/pr33528-functional-test-tweak
I am still trying to figure out the
...

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2411373516)
Are you fuzzing with persistent mode?

👍 hebasto approved a pull request: "[27.x] Fix Qt download URLs"
(https://github.com/bitcoin/bitcoin/pull/33564#pullrequestreview-3311244901)
ACK 9652f4754d93c41f224f29b41110ebd21f55d976.