💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2365160404)
Did a rebase for the changes since last master, including CMake and comparing Span<> using `std::equal_range`. No other changes.

> https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1658764974 avoid unreachable nets (not given to -onlynet=)
> https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1658949236 could announce an addr:port where we do not listen (no -bind)
> https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1684368824 could announce the wrong port because it us
...

💬 brunoerg commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1769550928)
```suggestion
// This is because our peer may have added the conflicting transaction
```

💬 pinheadmz commented on issue "rpc auth fails 'Error parsing command line arguments: Invalid parameter -rpcpasssword=password":
(https://github.com/bitcoin/bitcoin/issues/30939#issuecomment-2365175283)
try removing `-rpcpassword` ?

💬 pinheadmz commented on issue "rpc auth fails 'Error parsing command line arguments: Invalid parameter -rpcpasssword=password":
(https://github.com/bitcoin/bitcoin/issues/30939#issuecomment-2365175753)
also what is `start` for?

💬 hebasto commented on pull request "depends: Fix build with `MULTIPROCESS=1` in Guix environment":
(https://github.com/bitcoin/bitcoin/pull/30940#issuecomment-2365201817)
My Guix build:
```
aarch64
753aee81b01ff7a560a6c2c9ad64e2f3d3a535d0f5484364ea6b0f7ac7d8d595 guix-build-953aa067c565/output/aarch64-linux-gnu/SHA256SUMS.part
2eec13b0a0a28d16b9ec2b09908d6785666b49cd2076fd7b677259be0f30d624 guix-build-953aa067c565/output/aarch64-linux-gnu/bitcoin-953aa067c565-aarch64-linux-gnu-debug.tar.gz
732591be3acdc241d8d7a649bad755561dafe97839e00105682d962844acbe54 guix-build-953aa067c565/output/aarch64-linux-gnu/bitcoin-953aa067c565-aarch64-linux-gnu.tar.gz
279a5e97
...

📝 tdb3 opened a pull request: "test: simplify timewarp boundary test"
(https://github.com/bitcoin/bitcoin/pull/30941)
Follow up from PR #30698 (comments https://github.com/bitcoin/bitcoin/pull/30698#discussion_r1729632270).

`mining_basic` checks that a block with wall time is rejected when the previous block is `MAX_FUTURE_BLOCK_TIME` in the future, then checks that a block just beyond the `MAX_TIMEWARP` is also rejected.

This PR removes the first check, since they are essentially checking the same thing twice.
Also incorporates the suggestion in https://github.com/bitcoin/bitcoin/pull/30698#discussion_r
...

💬 Sjors commented on pull request "depends: Fix build with `MULTIPROCESS=1` in Guix environment":
(https://github.com/bitcoin/bitcoin/pull/30940#issuecomment-2365220991)
@hebasto can you rebase your `240921-guix-mp.DEMO` branch so it builds directly on this PR? That makes it easier to compare guix builds.

I'll make a regular guix build for this PR first, without `MULTIPROCESS=1`.

💬 l0rinc commented on pull request "doc: Add `nproc` support for Mac through `coreutils`":
(https://github.com/bitcoin/bitcoin/pull/30936#issuecomment-2365231090)
Updated, as I think this can be useful.
Once the docs transition to Ninja, we can of course remove these manual steps.

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1769603571)
This was moved to the `AddFlags` already - even though it seems to me the code already makes all of that clear:

> Adding a flag also requires a self reference to the pair that contains this entry in the CCoinsCache map

`AddFlags(uint8_t flags, CoinsCachePair& self`

> and a reference to the sentinel of the flagged pair linked list

, `CoinsCachePair& sentinel`

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1769603574)
This was meant to be a temporary transition, since we're removing the rest of the flags in your PR - but I'm fine with Assume as well - changed.

📝 fjahr opened a pull request: "test: Remove dead code from interface_zmq test"
(https://github.com/bitcoin/bitcoin/pull/30942)
The loop removed here appears to be effectively dead code: In case `get_raw_seq` is behind `zmq_mem_seq` the loop runs and tries to get a more recent (higher) number for `get_raw_seq`. However, the exact number of `get_raw_seq` is asserted in the line above: `assert_equal(get_raw_seq, 6)`. If the loop would actually achieve its purpose this assert would need to be racy. This does not seem to be the case and 6 appears to be the final number. `zmq_mem_seq` however does take some time to catch up (
...

💬 hebasto commented on pull request "depends: Fix build with `MULTIPROCESS=1` in Guix environment":
(https://github.com/bitcoin/bitcoin/pull/30940#issuecomment-2365310194)
@Sjors
> can you rebase your `240921-guix-mp.DEMO` branch so it builds directly on this PR? That makes it easier to compare guix builds.

Done. The PR description has been updated accordingly.

💬 kegdeg commented on issue "rpc auth fails 'Error parsing command line arguments: Invalid parameter -rpcpasssword=password":
(https://github.com/bitcoin/bitcoin/issues/30939#issuecomment-2365310228)
changed it to ./bitcoin-cli -rpcuser=user -stdinrpcpass -netinfo, 'error: Authorization failed: Incorrect rpcuser or rpcpassword

Also trying:
curl --user user:password --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "getblockchaininfo", "params": [] }' -H 'content-type: text/plain;' http://127.0.0.1:8332/

the server shows
2024-09-21T20:21:44Z ThreadRPCServer incorrect password attempt from 127.0.0.1:44938

💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1769650699)
> hmm, so you are thinking about a condition that passes at the beginning, does not passes somewhere during the test sleep time and ends up passing at the end. Maybe the test condition should be changed if such scenario exist?

Yes, exactly, and in order to know that it should be changed the person writing the test will see that `ensure_for` fails. This is the protection against introducing flakiness that I mentioned.

> which would flood the active logs

I don't know if that's a problem b
...

💬 fjahr commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2365313958)
I have introduced a delay parameter that allows to adjust the delay between checking the predicate. The default is set at 0.2 instead of previously 0.05.

I have also added a second commit adding the delay parameter to `wait_until` and using it in places where a `time.sleep` is currently used and a high frequency of checking does not seem appropriate.

There are still a few sleeps spread across the tests, I think some of them require some further thought on how to remove them and some of the
...

⚠️ agzht opened an issue: "bitcoin core发送后，链上没有"
(https://github.com/bitcoin/bitcoin/issues/30943)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

用bitcoin core 發送比特幣後,比特幣余额扣了，
但blockchain 找不到這交易

### Expected behaviour

退回bitcoin

### Steps to reproduce

运行bitcoin core

### Relevant log output

_No response_

### How did you obtain Bitcoin Core

Other

### What version of Bitcoin Core are you using?

25

### Operating system and version

window

### Machine specifications

thinkpad

✅ pinheadmz closed an issue: "bitcoin core发送后，链上没有"
(https://github.com/bitcoin/bitcoin/issues/30943)

⚠️ Peroqu opened an issue: "Minar"
(https://github.com/bitcoin/bitcoin/issues/30944)

💬 fjahr commented on pull request "sync: improve CCoinsViewCache ReallocateCache - 2nd try":
(https://github.com/bitcoin/bitcoin/pull/30370#issuecomment-2365335400)
> this seems to completely contradict what ReallocateCache was introduced in the first place

Can you explain what you mean by this? I don't understand how making it optional contradicts it.

💬 l0rinc commented on pull request "sync: improve CCoinsViewCache ReallocateCache - 2nd try":
(https://github.com/bitcoin/bitcoin/pull/30370#issuecomment-2365336998)
> Can you explain what you mean by this

I expanded on it in https://github.com/bitcoin/bitcoin/pull/30370#discussion_r1753749012.

My understanding is that `ReallocateCache` was introduced to save memory when shrinking the map after evictions, and this PR prohibits shrinking and reallocates the same capacity we were just trying to get rid of. If I'm mistaken here, I'd appreciate and explanation for why we can't solve it in a simpler way - e.g. by getting rid of `ReallocateCache` instead or
...