💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2566578365)
to minimize the diff, this comment for `PickTxForSend` should be adjusted in a previous commit where it was introduced (not in 129d67ebafdd63c4a48fc6c8f45c21f729cd8cf3)

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2566558339)
could we move this in a separate commit as well, it's not immediate obvious to me why this is safe to do

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2566595639)
we likely need a release notes for this, many people would want to know about it

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2566503863)
I prefer making the code easy to focus on the important stuff, so that the glue code fades into the background. Here the debug logging is dominating the method.
I'm more interested here in seeing e.g. why `pnode.GetLocalNonce()` is inlined during this change.

Instead of multilining it, can we use `pnode.LogIP(fLogIPs)` here as well as we do in other places:
```suggestion
LogDebug(BCLog::NET, "send version message: version %d, blocks=%d, txrelay=%d, peer=%d%s", PROTOCOL_VERSION, my_heig
...

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2566603668)
```suggestion
"via the Tor network. This conceals the transaction's origin. The transaction\n"
```

💬 fjahr commented on pull request "RFC: bench: Add multi thread benchmarking":
(https://github.com/bitcoin/bitcoin/pull/33740#issuecomment-3583484232)
> Not sure whether the concept of script validation has to creep into the benchmarking parameters, maybe we could generalize it to `-thread-count` or `-par` or something.

Thanks, happy to change this and address the other comments. But aside from that do you think this is generally interesting enough to be considered merging? I was unsure and since I haven't worked much on benchmarking I would like to get some concept-ack-ish feedback from the benchmarking wg :)

⚠️ multicryptobank opened an issue: "[BIP-352] Silent Payments Implementation"
(https://github.com/bitcoin/bitcoin/issues/33957)
Hi.

We are keen on 'playing with'/testing Silent Payments in Bitcoin Core and thus contributing to it.
There seems to be a myriad of relevant PRs (#28201, #28122, #27827 and so on) as well as (open) issues which confuse us.
Which one is the latest one? How many (and which ones) must be implemented? In which order?

Are there any Windows desktop wallets we could use for testing purposes?

Sorry about so many questions.

Thanks in advance.

@josibake @Eunovo

💬 pinheadmz commented on issue "[BIP-352] Silent Payments Implementation":
(https://github.com/bitcoin/bitcoin/issues/33957#issuecomment-3583520845)
Follow https://github.com/bitcoin/bitcoin/issues/28536

✅ pinheadmz closed an issue: "[BIP-352] Silent Payments Implementation"
(https://github.com/bitcoin/bitcoin/issues/33957)

⚠️ theuni opened an issue: "Net split meta issue"
(https://github.com/bitcoin/bitcoin/issues/33958)
Note: This is just a thought dump for now.. an attempt at a high-level roadmap based on an initial POC and the feedback I've gathered thus far. I will edit this meta issue frequently as a real roadmap takes shape.

The goal of this project to cleanly separate the net/net_processing layers, so that neither depends on the implementation details of the other.

In order to achieve that, we'll need to de-tangle the two subsystems (namely `CConnman` and `PeerManager`) and connect them instead via abst
...

💬 fjahr commented on pull request "Rollback for dumptxoutset without invalidating blocks":
(https://github.com/bitcoin/bitcoin/pull/33477#issuecomment-3583625311)
> > It might be possible to do it faster and with less disk usage for relatively short rollbacks via a two step process:
>
> I actually had a similar idea early on but then stayed with the simpler approach. I will try this out with a POC and check the performance impact.

I tried to a few different takes on the delta-based idea including some vibe coding tests. [Here](https://github.com/fjahr/bitcoin/commit/a86bbfbd6f0d6e440ff8c5c303e392e51e3eb60b) is the latest one, something is definitely
...

💬 fjahr commented on pull request "contrib: Count entry differences in asmap-tool diff summary":
(https://github.com/bitcoin/bitcoin/pull/33939#discussion_r2566789785)
I will look into this if I have to retouch. Usually the total count isn't that interesting of a number to me in analysis. But I can see it's a nice to have maybe with some percentage value as well. The calculation could certainly be much faster if the file is text based, then it's just a line count. But for the binary it's trickier of course and I am not sure we can do better.

💬 davidgumberg commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#issuecomment-3583996749)
Tested ACK 3e01b5d0e7be3d

This fixes #31873.

|sha256 | os:ver | python | zlib |
|----------------------------------------------------------------|--------|--------|------|
|95b00dc41fa090747dc0a7907a5031a2fcb2d7f95c9584ba6bccdb99b6e3d498|archlinux:latest|3.13.7|1.3.1|
|19e29377936086e7c8079c1047f6536e9bd53a46e180a3f49441ef38aaed900d|rockylinux:8|3.6.8|1.2.11|
|95b00dc41fa090747dc0a7907a5031a2fcb2d7f95c9584ba6bccdb99b6e3d498|rocky
...

💬 yuvicc commented on pull request "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState":
(https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2567043007)
I don't think the behavior of `ProcessNewBlock` is changed here. It's just a change in variable name (bg_state -> activation_state) see [comment](https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2547248797), we can keep the change as is if you prefer that way?

💬 yuvicc commented on pull request "kernel: don't use assert to handle invalid user input":
(https://github.com/bitcoin/bitcoin/pull/33943#issuecomment-3584097189)
Concept ACK on fixing input validation. Agree with @sedited here, not sure if we want to add more error handling to the API. I found the secp256k1 approach using `ARG_CHECK` macros through callback to be more straightforward and appropriate.

💬 gmaxwell commented on pull request "fix assumevalid is ignored during reindex":
(https://github.com/bitcoin/bitcoin/pull/33854#issuecomment-3584137905)
I think minimumchainwork being configurable may violate the underlying security assumption that made me feel comfortable writing assumevalid in the first place.

The concern is that attackers (or developers operating under coercion) could target victims with false statements that due to some "network incident" they must start nodes with -assumevalid=<badchainhash> (and use DOS attacks to prevent syncing an honest chain). AV security assumption there is that the bad chain needs to have two wee
...

💬 ajtowns commented on pull request "wallet: Add separate balance info for non-mempool wallet txs":
(https://github.com/bitcoin/bitcoin/pull/33671#issuecomment-3584302808)
Okay, so when a "transfer to self" tx paying 1310 sats in fees from 100 BTC of inputs doesn't hit the mempool, it currently looks like:

```json
{
"mine": {
"trusted": 5949.99999220,
"untrusted_pending": 0.00000000,
"immature": 3200.00000780
}
}
```

vs when the tx does hit the mempool:

```json
{
"mine": {
"trusted": 6049.99997910,
"untrusted_pending": 0.00000000,
"immature": 3200.00000780
}
}
```

That is, there's 99.9999869 BTC missing f
...

📝 yuvicc opened a pull request: "test: use ForkGenerator to deduplicate reorg test code (#32587 follow-up)"
(https://github.com/bitcoin/bitcoin/pull/33959)
Deduplicate reorg test code by introducing `ForkGenerator` utility class in `blocktools.py`.

### Changes
- Add `ForkGenerator` class with `prepare_fork()` / `trigger_reorg()` / `reset()` methods
- Removes duplicated `trigger_reorg()` methods across tests
- Update mempool tests to use the new utility

For now I am keeping it as draft as we need to address other tests to eliminate use of `invalidate_block` for reorg scenario. Also, optional args for non-empty forks to test mix-and-
...

💬 yuvicc commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3584312813)
I've opened a draft follow-up PR #33959 to address the duplication of `trigger_reorg` code.
CC @instagibbs @theStack

🤔 maflcko reviewed a pull request: "Align legacy script policy with P2SH policy in AreInputsStandard"
(https://github.com/bitcoin/bitcoin/pull/33926#pullrequestreview-3513898487)
Please note that contributors are required to fully understand their authored code themselves.

Asking others for review, to feed that into an LLM, does not count as understanding the code.

If they wanted to use an LLM, they could just do so themselves.

Please focus on creating high-quality, original content that demonstrates a clear understanding of the project's requirements and goals. Also, see the [contributing guidelines](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refa
...