💬 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

💬 glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#issuecomment-2569511364)
Thanks for the reviews @mzumsande @instagibbs! Just got back from holiday, addressed all the comments. Biggest change was recalculating missing parents each time we add a new orphan reso candidate. I also wrote a test for https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1881018172

💬 ismaelsadeeq commented on pull request "BlockAssembler: return selected packages virtual size and fee":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1901970599)

> Is it really a histogram at this point or just a vector of fee rates/fracs?

It is a vector of package fee frac's, which can be used to build a histogram.

> Might be helpful to add a comment here telling that the values in this vector are sorted. That's what I assumed while going through the `CalculatePercentiles` function here.

Yes, it is sorted by the order in which the packages are selected in the block template. I believe this is obvious because we are emplacing the fee fr
...

✅ achow101 closed a pull request: "[28.x] Finalize 28.1"
(https://github.com/bitcoin/bitcoin/pull/31582)

💬 achow101 commented on pull request "[28.x] Finalize 28.1":
(https://github.com/bitcoin/bitcoin/pull/31582#issuecomment-2569523096)
Closing in favor of https://github.com/bitcoin/bitcoin/pull/31594

💬 ismaelsadeeq commented on pull request "BlockAssembler: return selected packages virtual size and fee":
(https://github.com/bitcoin/bitcoin/pull/30391#issuecomment-2569523210)
> Nit: In the PR description, a draft PR is mentioned as the project. Can add this link for project issue: #30392

Added, and addressed your comments!