Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 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).
💬 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.