💬 maflcko commented on pull request "ci: Switch to latest macOS and Windows images":
(https://github.com/bitcoin/bitcoin/pull/31597#discussion_r1901724818)
Not sure about this. This should be the minimum version supported. IIRC xcode 15 corresponds to clang-16, somewhat?

If you want to test the latest xcode version, you can add a different task here, or in a nightly repo.

Otherwise, the build may break silently for xcode 15 and users/devs will complain.

🤔 l0rinc reviewed a pull request: "qa: Ensure consistent use of decimals instead of floats"
(https://github.com/bitcoin/bitcoin/pull/31595#pullrequestreview-2528976834)
I'm not sure about this, aren't we just masking a bigger problem (i.e. treating floats as if they were real numbers).
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?

💬 l0rinc commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#discussion_r1901727037)
is this really better than before?
```suggestion
wallet.sendtoaddress(address=address, amount=1)
```
or
```suggestion
wallet.sendtoaddress(address=address, amount="1")
```

Both seem to work

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