💬 ajtowns commented on pull request "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup":
(https://github.com/bitcoin/bitcoin/pull/33324#discussion_r2338593448)
I was surprised when it hit 100% of the undo files then restarted on the block files and was much slower -- might be better to do the slow files first, or ideally to do block and undo files intermixed so you just have a single 0% to 100% run.

💬 ajtowns commented on pull request "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup":
(https://github.com/bitcoin/bitcoin/pull/33324#discussion_r2338660825)
This doesn't seem like it would work correctly with pruning, when blk0000.dat has been deleted?

💬 l0rinc commented on pull request "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup":
(https://github.com/bitcoin/bitcoin/pull/33324#discussion_r2338680285)
We could do that with the recently introduced buffered readers - but that's considerably slower.
Is it a problem to read all of it in memory when we don't even have `dbcache` yet? The total memory usage is just 160 MB during migration, we should be fine until 1 GB at least, right?

💬 ajtowns commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3278199407)
> After `reindex`, my node has ~841k headers, a lot lower than the current tip of ~914k.
> The default `assumevalid` block (height 912,683) is in that missing range,

I think the behaviour here is probably fine -- it's a weak way of preventing you from triggering assumevalid behaviour on an invalid chain with substantially lower total work than the main chain. It might be nice to have a warning that your headers chain doesn't meet minchainwork though. It would probably be nicer to be able to
...

💬 ajtowns commented on pull request "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup":
(https://github.com/bitcoin/bitcoin/pull/33324#discussion_r2338802041)
I don't think buffered readers is the right thing (that's for when you want to process small amounts of data while still reading it from the file in large chunks), and trying the above didn't seem particularly slow to me.

I guess it could be simplified a bit to:

```c++
buf.resize(2 * MAX_BLOCK_SERIALIZED_SIZE);
while (true) {
size_t size = old_blocks.detail_fread(buf);
if (size == 0) break;
new_blocks.write_buffer(std::span(buf, 0, size));
}
```

🤔 w0xlt reviewed a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3209275659)
Approach ACK

Using `std::optional<bool>` introduces a third state (unset), which guarantees exactly one initial log message—regardless of whether the first real value is true or false.

For reviewers, here’s a simple way to verify the behavior change:

```
mkdir temp_data_dir
./build/bin/bitcoind -regtest -datadir=temp_data_dir -daemon
./build/bin/bitcoin-cli -regtest -datadir=temp_data_dir createwallet w
./build/bin/bitcoin-cli -regtest -datadir=temp_data_dir getnewaddress
./build/b
...

📝 jalateras opened a pull request: "Fix #25980: Validate transactions in combinerawtransaction"
(https://github.com/bitcoin/bitcoin/pull/33361)
## Summary

This PR fixes issue #25980 where `combinerawtransaction` would silently accept unrelated transactions and only use the first one, ignoring the rest.

## Problem

When passed two or more unrelated transactions (transactions with different inputs/outputs), `combinerawtransaction` would:
- Take the first transaction as the base
- Silently ignore the unrelated parts of subsequent transactions
- Only merge signatures for matching inputs

This behavior was confusing and could lead to unexp
...

💬 kannapoix commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2339166383)
It looks like the bitcoin binary might not have been built.
As far as I know, BUILD_BITCOIN_BIN=ON is the default, but maybe you built with a non-default setting.
If you build with bitcoin binary explicitly enabled, it should be generated at the path:

```
cmake -B build -DBUILD_BITCOIN_BIN=ON
```

💬 kannapoix commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#issuecomment-3278940886)
reACK 5424b4f1ce74c82b7ae01034bbc1592088048128

Tested and reviewed the new functional test.

📝 vasild opened a pull request: "Run feature_bind_port_(discover|externalip).py in CI"
(https://github.com/bitcoin/bitcoin/pull/33362)
This PR includes https://github.com/bitcoin/bitcoin/pull/32757 (first two commits here).

Then, on top of that:

`feature_bind_port_discover.py` and `feature_bind_port_externalip.py` require a routable address on the machine to run. Since that was not predictably available on CI, those tests required a manual setting up of IP addresses (e.g. using `ifconfig`) and then running the tests with a command line option telling them that the addresses are set up. The tests were not run in CI and got
...

💬 vasild commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3279026280)
> I would really like to know if its possible to run this test in CI ...

> `IFF_LOOPBACK` addresses are being filtered out [here](https://github.com/bitcoin/bitcoin/blob/e17fb86382eafb7ccfddb56bca981f6a12201c85/src/common/netif.cpp#L340). So I don't think the problem from https://github.com/bitcoin/bitcoin/issues/31336 is fixed by this as the OP says.

Both resolved in https://github.com/bitcoin/bitcoin/pull/33362

🤔 Eunovo reviewed a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3209656734)
ACK https://github.com/bitcoin/bitcoin/pull/33336/commits/39f41135c764c6c4705ce2cd36781ca8d43f0114:

Previously, users could only verify that script checks were enabled by confirming the absence of a "Disabling signature validations at block #1" message in the logs, which is a confusing negative confirmation pattern.
This PR adds explicit positive feedback by ensuring an "Enabling signature validations at block #1" message appears when script checks are enabled, making the system state cleare
...

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3279099513)
> After `reindex`, my node has ~841k headers, a lot lower than the current tip of ~914k. The default `assumevalid` block (height 912,683) is in that missing range, so the header lookup fails since the block isn't found in the block index (even though the log claims "Assuming ancestors of block ... have valid signatures"). Even when I set a custom `assumevalid` to a block I actually have, my `chainwork` is still below the bumped `nMinimumChainWork` from [755152ac](https://github.com/bitcoin/bitco
...

👍 hodlinator approved a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3209753464)
re-ACK 39f41135c764c6c4705ce2cd36781ca8d43f0114

Only made test less racy (https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2337774261) since the first ACK.

---

> > After `reindex`, my node has ~841k headers, a lot lower than the current tip of ~914k.
> > The default `assumevalid` block (height 912,683) is in that missing range, [would say: is outside that range]
>
> I think the behaviour here is probably fine -- it's a weak way of preventing you from triggering assumevalid
...

💬 vasild commented on pull request "Run feature_bind_port_(discover|externalip).py in CI":
(https://github.com/bitcoin/bitcoin/pull/33362#issuecomment-3279213012)
`69177404ba...f047fce8ab`: fix typo

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r2339623672)
`INTERNET_TRAFFIC_EXPECTED` is a means to turn this into a non-fatal error. For folks that want to run the CI shell scripts outside of CI, in environments where other programs on the same machine create internet traffic. It came from here: https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2591928909

⚠️ vasild opened an issue: "Early detect non-loopback traffic from unit tests"
(https://github.com/bitcoin/bitcoin/issues/33363)
Tests should not attempt to create network connections over the internet because that has unpredictable timing and outcome. It could also reveal the location (IP address) where somebody is running Bitcoin Core tests, if the target address is very specific, e.g. not `github.com:443`, but `1.2.3.4:8333` for example. See [#31339](https://github.com/bitcoin/bitcoin/issues/31339).

https://github.com/bitcoin/bitcoin/pull/31349 is an attempt to detect such cases in CI. In the [discussions](https://git
...

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3279520674)
> Another idea could be to have unit test setup use CreateSock to throw errors ...

I think that is worth exploring. Opened https://github.com/bitcoin/bitcoin/issues/33363 to track it, so that it does not get forgotten.

💬 willcl-ark commented on pull request "ci: disable cirrus cache in 32bit arm job":
(https://github.com/bitcoin/bitcoin/pull/33302#discussion_r2339795470)
Done in ff18b6bbaf3.

LMK what you think of the new approach. I agree it's clearer.

💬 willcl-ark commented on pull request "ci: disable cirrus cache in 32bit arm job":
(https://github.com/bitcoin/bitcoin/pull/33302#discussion_r2339799367)
Run on my fork at: https://github.com/willcl-ark/bitcoin/actions/runs/17626151823