💬 l0rinc commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#discussion_r1901722976)
Is it important this is a Decimal?
```suggestion
raw_tx = n1.createrawtransaction([prevout], {getnewdestination()[2]: "24.99"})
```
Seems to pass as well - floating point values weren't defined to be this precise anyway

💬 l0rinc commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#discussion_r1901725082)
we have [many](https://github.com/bitcoin/bitcoin/blob/master/test/functional/rpc_signrawtransactionwithkey.py#L81) [other](https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_sendall.py#L291) [instances](https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_bumpfee.py#L146) where we've kept the floating point format - can we set up a linter if that's important instead of fixing only a subset?

💬 maflcko commented on pull request "ci: Enable DEBUG=1 for one GCC-12+ build to catch 117966 regressions":
(https://github.com/bitcoin/bitcoin/pull/31522#issuecomment-2569183122)
For reference, this conflicts with commit a1add80c80bc95afdd55652bb87379fb34150a5c from

> [#29881](https://github.com/bitcoin/bitcoin/pull/29881) (guix: use GCC 13 to build releases by fanquake)

However, this is fine, because any version 12, or above should be fine. I've edited the pull title to clarify GCC-12+.

💬 rkrux commented on pull request "BlockAssembler: return selected packages virtual size and fee":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1901746082)
https://en.cppreference.com/w/cpp/container/vector/emplace_back#:~:text=It%20demonstrates%20how%20emplace_back%20forwards%20parameters%20to%20the%20President%20constructor

This is handy!

🤔 rkrux reviewed a pull request: "BlockAssembler: return selected packages virtual size and fee"
(https://github.com/bitcoin/bitcoin/pull/30391#pullrequestreview-2529015468)
Nit: In the PR description, a draft PR is mentioned as the project. Can add this link for project issue: https://github.com/bitcoin/bitcoin/issues/30392

💬 rkrux commented on pull request "BlockAssembler: return selected packages virtual size and fee":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1901757147)
Getting thrown off a bit by the name here. Is it really a histogram at this point or just a vector of fee rates/fracs?
Going by this def: https://en.wikipedia.org/wiki/Histogram

Sorry for not raising this earlier.

💬 rkrux commented on pull request "BlockAssembler: return selected packages virtual size and fee":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1901761122)
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: https://github.com/bitcoin/bitcoin/pull/30157/files#diff-34fc4771c305399b6f7634acd8c5fd2d0b89115f998397d41676180d742c28daR10

📝 i-am-yuvi opened a pull request: "#31403 follow-up"
(https://github.com/bitcoin/bitcoin/pull/31598)
#31403 follow-up, see [comment](https://github.com/bitcoin/bitcoin/pull/31403#pullrequestreview-2498806354)

- Rename `invalid_call` parameter to `called_by_framework` in `generateblock`, `generatetoaddress` and `generatetodescriptor` mining methods to better express its intended usage.
- Add explicit assertion message clarifying that these functions should only be called by TestFramework itself to maintain proper node synchronization.

✅ i-am-yuvi closed a pull request: "#31403 follow-up"
(https://github.com/bitcoin/bitcoin/pull/31598)

📝 i-am-yuvi opened a pull request: "test: improve rogue calls in mining functions"
(https://github.com/bitcoin/bitcoin/pull/31599)
#31403 follow-up, see [comment](https://github.com/bitcoin/bitcoin/pull/31403#pullrequestreview-2498806354)

- Rename `invalid_call` parameter to `called_by_framework` in `generateblock`, `generatetoaddress` and `generatetodescriptor` mining methods to better express its intended usage.
- Add explicit assertion message clarifying that these functions should only be called by TestFramework itself to maintain proper node synchronization.

💬 richieoscar commented on issue " Cannot figure out how to use std::atomic error for MacOS Sequoia 15.2":
(https://github.com/bitcoin/bitcoin/issues/31579#issuecomment-2569259288)
@adyshimony, this solution also worked for me

Installed LLVM via homebrew brew install llvm
Clean build / delete dir to clean cache via git clean -fxd
Updated the build command to use the Homebrew-installed LLVM path, and also adding the LLVM library path:

cmake -DCMAKE_C_COMPILER=/opt/homebrew/opt/llvm/bin/clang \
-DCMAKE_CXX_COMPILER=/opt/homebrew/opt/llvm/bin/clang++ \
-DCMAKE_EXE_LINKER_FLAGS=-L/opt/homebrew/opt/ll
...

💬 i-am-yuvi commented on pull request "test: Call generate RPCs through test framework only":
(https://github.com/bitcoin/bitcoin/pull/31403#issuecomment-2569265861)
> The follow-up idea in [#31403 (review)](https://github.com/bitcoin/bitcoin/pull/31403#pullrequestreview-2498806354) is up-for-grabs, if someone is looking for something to work on

I've opened a follow-up PR #31599

🤔 ryanofsky reviewed a pull request: "doc: Clarify comments about endianness after #30526"
(https://github.com/bitcoin/bitcoin/pull/31596#pullrequestreview-2529115512)
Updated 13464c0c44645e0ed88d9d72c3ad697dca800e10 -> 3e0a992a3f0f2b15b7be5049dc4f3134b4b0bc40 ([`pr/docblob.1`](https://github.com/ryanofsky/bitcoin/commits/pr/docblob.1) -> [`pr/docblob.2`](https://github.com/ryanofsky/bitcoin/commits/pr/docblob.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/docblob.1..pr/docblob.2)) with suggestions

Thanks for the reviews!

💬 ryanofsky commented on pull request "doc: Clarify comments about endianness after #30526":
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901817595)
re: https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901612593

Thanks, took suggestion

💬 ryanofsky commented on pull request "doc: Clarify comments about endianness after #30526":
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901811249)
re: https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901643256

Thanks I basically just took your first sentence. I dropped the second sentence, because I think it's not essential, and the phrasing is potentially misleading because the the wtxid bytes are only reversed when they are sorted, not when they hashed after being sorted.

Should be ok not to mention here that you can sort little endian numbers by reversing the bytes.

💬 ryanofsky commented on pull request "doc: Clarify comments about endianness after #30526":
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901804778)
re: https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901633014

Thanks, agree that is clearer, took the suggestion but kept "represented"

💬 ryanofsky commented on pull request "doc: Clarify comments about endianness after #30526":
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901817428)
re: https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901655760

Thanks, I like "shown in byte-reversed hex" as a shorter alternative, and tried to incorporate your other ideas while not sounding ambiguous. If reviewers are happy with the new phrasing I'll add new commit changing other RPCs to use it. The other RPCs do seem ok as-is because they don't mention "little-endian hexadecimal", but it's good to be consistent.

🚀 ryanofsky merged a pull request: "refactor: Allow std::byte in (Read/Write)(LE/BE)"
(https://github.com/bitcoin/bitcoin/pull/31524)

📝 Sjors opened a pull request: "rpc: fix mintime field testnet4, expand timewarp attack test"
(https://github.com/bitcoin/bitcoin/pull/31600)
#30681 fixed the `curtime` field of `getblocktemplate` to take the timewarp rule into account. However I forgot to do the same for the `mintime` field, which was hardcoded to use `pindexPrev->GetMedianTimePast()+1`.

This PR adds a helper `GetMinimumTime()` and uses it for the `mintime` field.

It could be backported to v28. However I don't expect (testnet4) pools to use `mintime` in practice, since under normal mainnet circumstances this would produce blocks an hour in the past.


Additi
...

💬 hebasto commented on pull request "ci: Switch to latest macOS and Windows images":
(https://github.com/bitcoin/bitcoin/pull/31597#discussion_r1901853408)
> IIRC xcode 15 corresponds to clang-16, somewhat?

That's [correct](https://en.wikipedia.org/wiki/Xcode#Xcode_15.0_-_(since_visionOS_support)_2).