💬 maflcko commented on pull request "log: Enforce trailing newline":
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1782454398)
Sure, done both

💬 maflcko commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#issuecomment-2385303342)
(rebased)

💬 maflcko commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-2385328826)
Are you still working on this? 28.x will be released without this, so it would be good to explain why it should be merged for 29.x (or later), possibly in a release note. I haven't looked if there is any unaddressed feedback waiting for your reply, but at a minimum you'll have to rebase this for a fresh CI run and to pick up the other AU stuff.

💬 maflcko commented on issue "intermittent issue in wallet_upgradewallet.py AssertionError: [node 2] Node returned unexpected exit code (1) vs (0) when stopping":
(https://github.com/bitcoin/bitcoin/issues/30798#issuecomment-2385338965)
https://cirrus-ci.com/task/4769312399949824?logs=ci#L3305

💬 maflcko commented on issue "intermittent issue in wallet_upgradewallet.py AssertionError: [node 2] Node returned unexpected exit code (1) vs (0) when stopping":
(https://github.com/bitcoin/bitcoin/issues/30798#issuecomment-2385343968)
https://cirrus-ci.com/task/6220062225334272?logs=ci#L3044

💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1782493367)
@luke-jr that doesn't work, because we'll refuse to load it. The attacker would have to give them a malicious Bitcoin Core binary, but such a binary can just sweep the wallet.

💬 laanwj commented on issue "Add IPv6 pinhole support using UPnP / NAT-PMP":
(https://github.com/bitcoin/bitcoin/issues/17012#issuecomment-2385422315)
> An observation also is that the usage of this option in a Datacenter/VPS environment may not take any effect as these firewalls/routers normally will not have UPnP/PCP running.

Yes, you're right. As mentioned in #30043 the goal of enabling it by default would be to have more connectable nodes outside datacenters. In datacenters, this will do nothing, their default gateways ignore PCP/NATPMP packets.

That said there is no hurry. Better to let this sit for.a release so that remaining issue
...

💬 stickies-v commented on pull request "log: Enforce trailing newline":
(https://github.com/bitcoin/bitcoin/pull/30929#issuecomment-2385423082)
re-ACK fa2b7d8d6b3f8d53199921e1e542072441b26fab

No changes except for rebase and addressing [review comments](https://github.com/bitcoin/bitcoin/pull/30929#pullrequestreview-2338066360) (test/lint/doc updates and commit re-ordering).

💬 fanquake commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2385471084)
From the tidy job (https://api.cirrus-ci.com/v1/task/4831368570470400/logs/ci.log):
```bash
[21:19:24.426] Built target bitcoin_crypto_avx2
[21:19:26.500] Error: netif.cpp.o depends on netaddress.cpp.o symbol 'CNetAddr::CNetAddr(in_addr const&)', can suppess with:
[21:19:26.500] SUPPRESS["netif.cpp.o netaddress.cpp.o _ZN8CNetAddrC1ERK7in_addr"]=1
[21:19:26.502] Error: pcp.cpp.o depends on netaddress.cpp.o symbol 'CNetAddr::CNetAddr(in_addr const&)', can suppess with:
[21:19:26.502]
...

💬 fanquake commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#issuecomment-2385475097)
Looks like this doesn't fail when it detects a problem? i.e the tidy job is passing (most recent run: https://api.cirrus-ci.com/v1/task/5757501226876928/logs/ci.log), but is currently outputting:
```bash
[21:19:24.426] Built target bitcoin_crypto_avx2
[21:19:26.500] Error: netif.cpp.o depends on netaddress.cpp.o symbol 'CNetAddr::CNetAddr(in_addr const&)', can suppess with:
[21:19:26.500] SUPPRESS["netif.cpp.o netaddress.cpp.o _ZN8CNetAddrC1ERK7in_addr"]=1
[21:19:26.502] Error: pcp.cpp.
...

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2385476424)
MSAN says https://cirrus-ci.com/task/6248232848719872:

```
SUMMARY: MemorySanitizer: use-of-uninitialized-value /usr/src/mp/proxy.cpp:146:25 in mp::Connection::addAsyncCleanup(std::__1::function<void ()>)
[10:00:27.923] Exiting
```

💬 fanquake commented on pull request "refactor: add clang-tidy `modernize-use-starts-ends-with` check":
(https://github.com/bitcoin/bitcoin/pull/30868#issuecomment-2385506318)
Seems like newer Clang 19 has improved this check, and throws out more (historical) instances to change. i.e:
```bash
bitcoin/src/torcontrol.cpp:359:27: error: use starts_with instead of compare() == 0 [modernize-use-starts-ends-with,-warnings-as-errors]
359 | if (0 == line.compare(0, 20, "net/listeners/socks=")) {
| ~~~~ ^~~~~~~~~~~~~~ ~
| starts_with( )
bitcoin/src/torco
...

⚠️ toxictoskey opened an issue: "Trading Tool"
(https://github.com/bitcoin/bitcoin/issues/31006)
### Motivation

Hello, world. I created new bot, u can chek this in my profile. Thanks

### Possible solution

_No response_

### Useful Skills

* Compiling Bitcoin Core from source
* Running the C++ unit tests and the Python functional tests
* ...


### Guidance for new contributors

Want to work on this issue?

For guidance on contributing, please read [CONTRIBUTING.md](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md) before opening your pull request.

✅ fanquake closed an issue: "Trading Tool"
(https://github.com/bitcoin/bitcoin/issues/31006)

:lock: fanquake locked an issue: "Trading Tool"
(https://github.com/bitcoin/bitcoin/issues/31006)

💬 Sjors commented on pull request "Don't zero-after-free `DataStream`: ~25% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/30987#issuecomment-2385580772)
I tested this PR on an AMD Ryzen 7950x machine with Ubuntu 24.04, with one local network peer and a gigabit internet connection.

```
bitcoind -dbcache=30000 -stopatheight=863000 -blocksdir=/magnetic/.bitcoin -addnode=local-network
```

Before: 5 hours 10 minutes
After: 4 hours and 55 minutes

Time includes 20 minutes to flush chainstate to disk during shutdown.

That's nowhere near a 25% difference and probably not statistically significant. Some configurations might do better than o
...

💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-2385586088)
Rebased and added release note.

@maflcko once the UI supports loading a snapshot, this PR should be in the same release (or earlier). But there's no PR implementing that yet, so no specific release to target.

I don't think there's any outstanding feedback (minor tweaks in iconography and wording can wait for a followup). Some people find it useful, others don't.

💬 epiccurious commented on pull request "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS":
(https://github.com/bitcoin/bitcoin/pull/30982#issuecomment-2385589171)
Post-merge ACK.

For added context, I believe the `codesign` step is only required on arm64 machines. From my testing, the failure doesn't reproduce on amd64 / x86_64 machines.

📝 MarnixCroes opened a pull request: "doc: add testnet4 section header for config file"
(https://github.com/bitcoin/bitcoin/pull/31007)
This PR adds the missing `testnet4` config section header documentation for the config file.

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitco
...

💬 maflcko commented on pull request "Fix unsigned integer overflows in interpreter":
(https://github.com/bitcoin/bitcoin/pull/24214#issuecomment-2385598890)
Rebased once more due to conflict in `test/sanitizer_suppressions/ubsan`.