💬 rkrux commented on issue "Add `sat_to_btc()` and conversely `btc_to_sat()` util functions in functional tests":
(https://github.com/bitcoin/bitcoin/issues/31345#issuecomment-2512243262)
@le0nAg Thanks for showing interest in this, there is an open PR for it here: https://github.com/bitcoin/bitcoin/pull/31356
Maybe coordinate with @andremralves and work on the same PR?

💬 theuni commented on pull request "build: Set shared linker flags in toolchain file":
(https://github.com/bitcoin/bitcoin/pull/31395#issuecomment-2512332764)
Post-merge utACK a8e04704f930de168fdddbb1c040dd70fbbe8ff7

👍 danielabrozzoni approved a pull request: "refactor: Move GuessVerificationProgress into ChainstateManager"
(https://github.com/bitcoin/bitcoin/pull/31393#pullrequestreview-2473684721)
light tACK fa5ad4ed2b36fa483385670a210eb8564bc53c9b - i'm not too familiar with the code, but the change is reasonable. Tests are passing locally.

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2512564803)
@ryanofsky that one and https://github.com/chaincodelabs/libmultiprocess/issues/122 are the only two I'm aware of at the moment.

📝 mzumsande opened a pull request: "validation: stricter internal handling of invalid blocks"
(https://github.com/bitcoin/bitcoin/pull/31405)
Historically, some fields in validation were set opportunistically by "best effort":
- The `BLOCK_FAILED_CHILD` status (which means that the block index has an invalid predecessor)
- `m_best_header` (the most-work header not known to be invalid).

This means that there were known situations in which these fields were not set when they should've been, or set to wrong values. This was tolerated because the fields were not used for anything consensus-critical and triggering these situations inv
...

💬 maflcko commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1866522517)
IIRC chmod does not work on Windows, so a workaround may be needed for the test failure.

Not sure about the `wallet_migration.py` failure. Another workaround may be needed there.

Also, the `wallet_migration` test won't be running after https://github.com/bitcoin/bitcoin/pull/31248, so the previous releases tests should probably be run as well here.

💬 maflcko commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1866523690)
(Functional tests could be handled in a follow-up, if you want)

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1866581222)
Exactly, the existing code is inadvertently testing the wrong thing (it's invalid, but not for the reason the test seems to aim for).

💬 maflcko commented on pull request "ci: Skip broken Wine64 tests by default":
(https://github.com/bitcoin/bitcoin/pull/31284#issuecomment-2512820195)
... https://cirrus-ci.com/task/5540900196057088?logs=ci#L4684

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1866653074)
I don't think we care that much about the exact errors.

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1866659349)
Ok, changed to `LogInfo`.

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1866651913)
I have kept one (in `p2p_segwit.py`), and added a comment about it.

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1866659053)
I have changed it to using `(Script validation error in block: %s", state.ToString())` in both locations, which is close to what the final commit uses too. It feels wrong not to include the detailed information computed in `CScriptCheck::operator()` in this commit already.

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1866660122)
I have changed it to `mandatory_result`, as it may not exactly match the consensus rules.

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1866659604)
Done, more detailed information won't hurt.

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1866656706)
Done, using `LogInfo` for the new code now.

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1866655340)
It would work here, because the unit test only starts a single checker thread, but in general, checkqueue cannot guarantee that what it returns is the *first* error because of thread synchronization. So I'd rather not test for behavior we cannot in general guarantee.

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1866653394)
Good idea, added.

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1866656047)
Done.

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#issuecomment-2513004004)
Rebased, and addressed most comments.