Bitcoin Core Github
44 subscribers
120K links
Download Telegram
👋 dergoegge's pull request is ready for review: "ci: Split out native fuzz jobs for macOS and windows (take 2)"
(https://github.com/bitcoin/bitcoin/pull/31221)
💬 marcofleon commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1829592727)
The upper target is the `nBits` value from the genesis block (0x1D00FFFF) and the lower one is the `nBits` from some mainnet block at a certain height (0x17058EBE). Using `SetCompact` on those values gives those two uint256 values. The work returned from the lower target multiplied by the longest possible chain (1600) in the test should be lower than `MinimumChainWork`.

I get 0x11fe50e0ab9cbcf864c0500 for the theoretical max amount of work a headers chain from this test could have. `MinimumC
...
💬 marcofleon commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1829593835)
Done.
💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2457548171)
I could remove another `sleep` import line after https://github.com/bitcoin/bitcoin/pull/30942 was merged which was what the linter complained about. Should be trivial to re-ACK: https://github.com/bitcoin/bitcoin/compare/352b8209aa5327f7d369e2acc4d87f9767389a6b..f04529fc40f2f7c8e4e63d892cdcbe6c91dfbf96
💬 fanquake commented on pull request "build: Unify `-logsourcelocations` format":
(https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2457600736)
Guix build:
```bash
67b7e95e0f1d45debad8a8cac4c2d349e3e01efcf62e0b109345896f296758d9 guix-build-788c1324f3d8/output/aarch64-linux-gnu/SHA256SUMS.part
cd3c67eaa5f6fadf0a73cdb1aa03563d720fc7fdae1b59aa495a0eda9d2695af guix-build-788c1324f3d8/output/aarch64-linux-gnu/bitcoin-788c1324f3d8-aarch64-linux-gnu-debug.tar.gz
5c1eb4ff827336a9d113497459981e213844b87e232c33fc00317ce20302f761 guix-build-788c1324f3d8/output/aarch64-linux-gnu/bitcoin-788c1324f3d8-aarch64-linux-gnu.tar.gz
dd78923f8fb7410a
...
👍 fanquake approved a pull request: "depends, doc: List packages required to build `qt` package separately"
(https://github.com/bitcoin/bitcoin/pull/31192#pullrequestreview-2416094490)
ACK 4747f030956d6876ec7cef4e2500a8579bc82146
🚀 fanquake merged a pull request: "depends, doc: List packages required to build `qt` package separately"
(https://github.com/bitcoin/bitcoin/pull/31192)
💬 fanquake commented on pull request "doc: Fix word order in developer-notes.md":
(https://github.com/bitcoin/bitcoin/pull/31220#issuecomment-2457604363)
ACK 44939e5de1b37ccd85ef31268219c5866bd3ee3f
🚀 fanquake merged a pull request: "doc: Fix word order in developer-notes.md"
(https://github.com/bitcoin/bitcoin/pull/31220)
🚀 fanquake merged a pull request: "doc: Use relative hyperlinks in release-process.md"
(https://github.com/bitcoin/bitcoin/pull/31206)
💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1829641670)
Sure, I will do it.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2457621199)
Rebased due to the conflict with the merged bitcoin/bitcoin#31192.
💬 Abdulkbk commented on pull request "test: cover edge case for lockunspent rpc":
(https://github.com/bitcoin/bitcoin/pull/31209#issuecomment-2457625789)
> Not sure about this. This seems to be adding coverage for code that shouldn't exist in the first place.
>
> The integer is passed to `COutPoint`, which only takes unsigned integers, so parsing the integer as such would be more consistent. The check would then be redundant and could be removed.

I did a quick test by commenting the check and you're right an error was thrown. However, the error is not as descriptive as the current implementation. The error without the check: "Invalid parame
...
👍 fanquake approved a pull request: "msvc: Update vcpkg manifest"
(https://github.com/bitcoin/bitcoin/pull/31186#pullrequestreview-2416130023)
ACK f6577b717416191e65ba8221be57f1a48d74e5d7
🚀 fanquake merged a pull request: "msvc: Update vcpkg manifest"
(https://github.com/bitcoin/bitcoin/pull/31186)
💬 fanquake commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#discussion_r1829664613)
> Overall, I think this is a reasonable approach because it pretty much just works with no fiddling. The main drawback is that over time the headers list will grow stale, so it'll require some attention now and then.

Not sure if here, but it'd seem useful to document some (minified) version of the PR description in regards to maintaining this going forward. If anything to prevent if from being too arbtrary.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2457652154)
Rebased due to the conflict with the merged bitcoin/bitcoin#31186.
💬 instagibbs commented on pull request "TxDownloadManager followups":
(https://github.com/bitcoin/bitcoin/pull/31190#issuecomment-2457652583)
is this ready for undraft?
👍 hodlinator approved a pull request: "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests"
(https://github.com/bitcoin/bitcoin/pull/30746#pullrequestreview-2416152348)
re-ACK 6fd185c035c1cc4dd961cf14a2087e97fb069440

(Hoping Drahtbot will react properly to this comment.. already ACKed the same PR version above).
💬 fanquake commented on pull request "depends: Specify CMake generator explicitly":
(https://github.com/bitcoin/bitcoin/pull/31171#issuecomment-2457662041)
> in environments where the CMAKE_GENERATOR variable is set.

Which environment/distro did you see this in?