💬 hebasto commented on pull request "ci: Switch to latest macOS and Windows images":
(https://github.com/bitcoin/bitcoin/pull/31597#issuecomment-2569350641)
> not sure about this. Is there a reason to do this?

https://github.com/bitcoin/bitcoin/blob/6aa0e70ccbd5491ec9d7c81892820f3341ccd631/.github/workflows/ci.yml#L79

and

https://github.com/bitcoin/bitcoin/blob/6aa0e70ccbd5491ec9d7c81892820f3341ccd631/.github/workflows/ci.yml#L151

Alternatively, these comments should be removed.

💬 hebasto commented on pull request "ci: Switch to latest macOS and Windows images":
(https://github.com/bitcoin/bitcoin/pull/31597#discussion_r1901859242)
> Not sure about this. This should be the minimum version supported.

Reverted back to Xcode 15.

💬 ryanofsky commented on pull request "refactor: Allow std::byte in (Read/Write)(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1901862936)
Thanks sipa and maflcko. I didn't realize you were allowed to pass explicit template arguments to functions declared without template parameters, but it makes sense given the equivalence shown. It is also interesting to see ASTs of these functions in godbolt, even though I did understand that clang-tidy could see when template parameters were referenced.

Upshot seems to be that it would be possible to write a clang-tidy linter that would not force you to write broken code (because template ar
...

💬 hebasto commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#issuecomment-2569366414)
> Also, can we update any one of the CI tasks to use the latest python to make sure we don't have to fix these manually all the time?

That's the plan for the [nightly builds](https://github.com/hebasto/bitcoin-core-nightly). The working branch is [250102-decimal-demo](https://github.com/hebasto/bitcoin-core-nightly/tree/250102-decimal-demo).

💬 hebasto commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#discussion_r1901870785)
I agree, but I'm not familiar with the Python linter capabilities.

💬 maflcko commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#discussion_r1901893390)
It should be trivial to write a python function that accepts json (value/dict/array) and fails if any value is float. The function could be run in authproxy

💬 sipa commented on pull request "refactor: Allow std::byte in (Read/Write)(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1901896336)
@ryanofsky I was just commenting on (lack of) semantic difference between the different syntaxes. Personally I have a slight preference for the shorter notations, but no opinion on whether the project should adopt/encourage/enforce that.

💬 maflcko commented on pull request "ci: Switch to latest macOS and Windows images":
(https://github.com/bitcoin/bitcoin/pull/31597#issuecomment-2569430341)
> Alternatively, these comments should be removed.

Makes sense to remove the comment and explain that the xcode version is supposed to denote the minimum supported version to compile from xcode. I presume this is equal to https://github.com/bitcoin/bitcoin/blame/6aa0e70ccbd5491ec9d7c81892820f3341ccd631/doc/release-notes-empty-template.md#L46, where 13.0+, means 13+. I assume that anyone on 13.0 is able to, and must upgrade to the latest 13.x anyway, so that the minimum requirement is not an i
...

💬 fanquake commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#issuecomment-2569453153)
It'd be good if you could explain what changed in Python 3.12+, and why this is only relevant on NetBSD. (a nightly build isn't strictly needed given many devs already use the latest version of Python locally).

👍 ryanofsky approved a pull request: "test: have miner_tests use Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31581#pullrequestreview-2529323093)
Code review ACK 04249682e381f976de6ba56bb4fb2996dfa194ab, just minor suggested changes (renames, comments, BOOST_REQUIREs) since last review and some more extra clarifications and checks added to the CreateNewBlock_validity test. The CreateNewBlock_validity changes seem clear and easy to understand now.

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901902305)
done

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901906865)
deleted

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901895281)
The conditions aren't exactly the same since `AddAnnouncer` also returns false if we already HaveTxFromPeer. We'd need to make the return result tri-state.

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901883025)
I've moved that test to its own commit

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901887146)
good point, added

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901867183)
removed

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901887320)
deleted now that parent_txids is gone

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901905478)
Fwiw makes sense, but this function doesn't exist anymore, so marking resolved

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901882259)
Done

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1901891303)
added