💬 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
...

💬 maflcko commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#discussion_r2567485967)
@151henry151 I don't think it is a good use of anyone's time to ask an LLM to fundamentally change the script policy in a tight agent loop that uses the public repo resources to run and get the CI result continuously.

💬 ajtowns commented on pull request "Adds non-mempool wallet balance to overview":
(https://github.com/bitcoin-core/gui/pull/911#issuecomment-3584610611)
Screenshot:

<img width="318" height="218" alt="non-mempool-gui" src="https://github.com/user-attachments/assets/885411f4-d01a-4574-a341-98b962c5c2ec" />

The negative sign for the non-memool stuff isn't really very clear, perhaps.

✅ maflcko closed a pull request: "leveldb: correct trailer boundary check in Reader::SkipToInitialBlock"
(https://github.com/bitcoin/bitcoin/pull/33955)

💬 maflcko commented on pull request "leveldb: correct trailer boundary check in Reader::SkipToInitialBlock":
(https://github.com/bitcoin/bitcoin/pull/33955#issuecomment-3584610788)
thx, but this will need to go upstream

✅ maflcko closed a pull request: "Align legacy script policy with P2SH policy in AreInputsStandard"
(https://github.com/bitcoin/bitcoin/pull/33926)

💬 maflcko commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3584663392)
Closing, to break the bot loop

💬 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3584727791)
Sorry about that -- not trying to falsely represent anything here. I thought I had a pretty good start with my changes to policy.cpp, but then I've been struggling with getting the tests working. I know that overuse of LLMs is frowned upon and I realize that accidentally committing and pushing a bunch of MD files with notes generated by an LLM is not a good look, but I'm going to keep trying to figure this out locally and hope to make a valuable contribution here. I also realize that I should ha
...

💬 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#discussion_r2567617676)
I didn't consider that I was burning through valuable resources, and I have certainly been using the LLM to help me troubleshoot and try to figure out why I can't get the tests to pass. Sorry for the abuse of resources and I'll use my own machine for tests until I get some good results.

💬 maflcko commented on pull request "test: check for output to stdout in `TestShell` test":
(https://github.com/bitcoin/bitcoin/pull/33951#issuecomment-3584759633)
lgtm ACK 52230a7f697fd99abdc4550d6a60737be024e246

💬 maflcko commented on pull request "qa: Remove no longer needed `feature_dirsymlinks.py`":
(https://github.com/bitcoin/bitcoin/pull/33924#issuecomment-3584772427)
Yeah, I guess there may not be a reduced path coverage inside Bitcoin Core right now, but will it always be that case in the future?

Is there some other benefit to removing the test?

👍 rkrux approved a pull request: "test: check for output to stdout in `TestShell` test"
(https://github.com/bitcoin/bitcoin/pull/33951#pullrequestreview-3514137749)
crACK 52230a7f697fd99abdc4550d6a60737be024e246

I didn't know about TestShell, looks quite handy!