Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 maflcko commented on pull request "ci: Run unit tests parallel with functional tests":
(https://github.com/bitcoin/bitcoin/pull/33000#issuecomment-3484400040)
Closing for now, but I may implement this in another way later.
💬 maflcko commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3484508481)
review ACK 5d784bebaff5e3acc0b5180ee51d9a16aec0e356 🎒

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 5d784bebaff5
...
💬 maflcko commented on pull request "refactor: use options struct for signing and PSBT operations":
(https://github.com/bitcoin/bitcoin/pull/32876#discussion_r2489242068)
nit: now that the clang minimum is clang-17, you could revert the workaround again, if you want
💬 Sjors commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3484608786)
> maybe it would be about time to expand the scope since 28.x is no longer maintained.

Yes. Staying one major version behind seems acceptable to me.

@gmaxwell wrote:

> I do not believe luke-jr's comments on twitter explaining the current behavior as a product of ordinarily excluding 'new' versions is particularly credible.

Do you still believe that after his comment above?
💬 Sjors commented on pull request "refactor: use options struct for signing and PSBT operations":
(https://github.com/bitcoin/bitcoin/pull/32876#discussion_r2489308021)
Done!
💬 Sjors commented on pull request "refactor: use options struct for signing and PSBT operations":
(https://github.com/bitcoin/bitcoin/pull/32876#issuecomment-3484657001)
Rebased past #33555 to get rid of the ancient clang workaround, see https://github.com/bitcoin/bitcoin/pull/32876#discussion_r2489242068.
👍 willcl-ark approved a pull request: "doc: update Guix INSTALL.md"
(https://github.com/bitcoin/bitcoin/pull/33574#pullrequestreview-3415100841)
ACK b4d0288c467f82a94041b51d10d38e66bb5c33ae

Changes here are fine. "various versions of Debian" is arguably a bit generous if it's planned to be dropped from (all?) upcoming versions, but it's not incorrect and the repology link provides up-to-date info in any case.
💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3484908644)
Changes applies:
- Fix lint issue
- Added check on the median as well
- some minor formatting
💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489540239)
Indeed. Inline is only a hint, not crucial for modern compliers.
💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489550775)
I did that for the longer values, for shorter values with only a few ones is not really need.
💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489552705)
Added median check as well. Much lower than the avg!
💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489553665)
Added explanatory comments
💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489555855)
Oops, fixed!
💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489557279)
Done
💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489560183)
All cycles go from 1 to n-1; 0 is the genesis.
💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2489569251)
A fuzz test for this functionality (essentially a one-liner) seems a bit too much.
🚀 fanquake merged a pull request: "test: cover invalid codesep positions for signature in taproot"
(https://github.com/bitcoin/bitcoin/pull/32301)
⚠️ Sjors opened an issue: "ci: spurious linter failure?"
(https://github.com/bitcoin/bitcoin/issues/33773)
Run is marked as cancelled but looks fine:

- https://github.com/bitcoin/bitcoin/actions/runs/19062940342/job/54446647440?pr=32876
🚀 fanquake merged a pull request: "ci: Update Clang in "tidy" job"
(https://github.com/bitcoin/bitcoin/pull/33445)