Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 instagibbs commented on pull request "TxGraph: change m_excluded_clusters":
(https://github.com/bitcoin/bitcoin/pull/33469#issuecomment-3325464372)
h/t @marcofleon for validation of changes
📝 fanquake converted_to_draft a pull request: "TxGraph: change m_excluded_clusters"
(https://github.com/bitcoin/bitcoin/pull/33469)
Change BlockBuilderImpl's m_excluded_clusters to unordered set since ordering is not used.

Change the set to a set of sequence numbers for a modest stability increase under fuzz testing.

This shouldn't be merged until https://github.com/bitcoin/bitcoin/pull/33157 is merged.
💬 mzumsande commented on pull request "test: fix p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/33121#discussion_r2373401843)
done
💬 janb84 commented on pull request "guix: Added guix-shasums script for gathering and formatting build output checksums":
(https://github.com/bitcoin/bitcoin/pull/33465#issuecomment-3325505399)
## Guix Build Output

**Host architecture:** `aarch64`
**Commit:** `a6cdf0368194`

### File Checksums

| SHA256 | FILE |
|--------------------------------------------------------------------|-------------|
| `631ef0d74ff8260c57cb0bfd5d5592735eb659852b97bd6312ede60297a25354` | `guix-build-a6cdf0368194/output/aarch64-linux-gnu/SHA256SUMS.part` |
| `8b444b56d08047c199635bd150bca809aade5458df4db4f1ed99939b998371cd` | `guix-
...
💬 fanquake commented on pull request "guix: Added guix-shasums script for gathering and formatting build output checksums":
(https://github.com/bitcoin/bitcoin/pull/33465#issuecomment-3325518038)
~0. I'm not sure if 90 lines of bash, is better than documenting and using the one-liner? Note that the markdown also bloats the size of the comment dramatically (https://github.com/bitcoin/bitcoin/pull/33465#issuecomment-3325505399), compared to what is currently used (https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-31968446200 (and makes it harder to copy paste).
🤔 achow101 reviewed a pull request: "[28.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/33415#pullrequestreview-3259502806)
It looks like 3eab8b724044dc321f70e5eed66b149713158a04 was skipped in the backport? It doesn't seem like there are significant conflicts and it makes 2e756e8b02d30e9baacd36d4e519ab64421778ba easier to review.

> IIUC we skipped backporting https://github.com/bitcoin/bitcoin/pull/30125 as it's quite involved (also see https://github.com/bitcoin/bitcoin/pull/31422#pullrequestreview-2607160362)

It backports without issue to 28.x. For 27.x, the second commit is complicated, but I think it could
...
💬 achow101 commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#discussion_r2373386807)
In f8006d9df08597bc2f4e85d32bff677c27c36ced "[prep/test] make wallet_fundrawtransaction's minrelaytxfee assumption explicit"

`settxfee` is not deprecated in 28.x, so including this change is unnecessary.
👍 willcl-ark approved a pull request: "[28.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/33415#pullrequestreview-3259576347)
ACK 1b1f359fc43385cb4d465b15754dac84eef06873

#33106 backport is naturally a bit tricky to review, especially as I didn't review the original PR.

That said, `git-range-diff` helped out a fair bit e.g. `git range-diff e5f896bb1f052fb8c7811c6024cb49143b427512..ba84a25deec0b3b9b94ee51b373e715fec995791 6090af0d350..2e756e8b02d` uses the range-diff algo on the two commit ranges, which is pretty neat! It helped me identify the "massaging" that was needed for the backport, which looks fine to me.
...
💬 willcl-ark commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#issuecomment-3325544285)
> It looks like [3eab8b7](https://github.com/bitcoin/bitcoin/commit/3eab8b724044dc321f70e5eed66b149713158a04) was skipped in the backport? It doesn't seem like there are significant conflicts and it makes [2e756e8](https://github.com/bitcoin/bitcoin/commit/2e756e8b02d30e9baacd36d4e519ab64421778ba) easier to review.

Ah, I was just complaining about this offline, and now see it in the range-diff too. That would have indeed made it easier to review.

```
6: 3eab8b72404 < -: ----------- [
...
💬 glozow commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#issuecomment-3325559708)
> It looks like https://github.com/bitcoin/bitcoin/commit/3eab8b724044dc321f70e5eed66b149713158a04 was skipped in the backport? It doesn't seem like there are significant conflicts and it makes https://github.com/bitcoin/bitcoin/commit/2e756e8b02d30e9baacd36d4e519ab64421778ba easier to review.

My bad - I missed that commit and it didn't hit me that there was a separate commit while I was resolving the conflicts.
💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/33445#discussion_r2373459060)
> This seems more like a bug that should be reported upstream...

https://github.com/llvm/llvm-project/issues/160394
💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/33445#discussion_r2373473699)
> ... it would seem good to have a comment saying it is a temporary workaround.

Thanks! Done.
💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3325580515)
The [feedback](https://github.com/bitcoin/bitcoin/pull/33445#discussion_r2372684495) from @ryanofsky has been addressed.
💬 hebasto commented on pull request "guix: Added guix-shasums script for gathering and formatting build output checksums":
(https://github.com/bitcoin/bitcoin/pull/33465#issuecomment-3325607240)
> ~0. I'm not sure if 90 lines of bash, is better than documenting and using the one-liner? Note that the markdown also bloats the size of the comment dramatically ([#33465 (comment)](https://github.com/bitcoin/bitcoin/pull/33465#issuecomment-3325505399)), compared to what is currently used ([#33185 (comment)](https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-31968446200) (and makes it harder to copy paste).

Same from me.
💬 darosior commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3325610461)
> AFAIK there is already a PR for rc2 (see #33424, #32275) and I personally don't think it's worth holding up the release for this PR.

+1. I ACK'd the change because i believe it is technically correct to say we won't be removing the option anytime soon, but i don't think a small documentation change is worth holding up the release for.
👍 hebasto approved a pull request: "[30.0] Final changes + rc2"
(https://github.com/bitcoin/bitcoin/pull/33424#pullrequestreview-3259681241)
ACK 7ebdfa2173b90faad71057d37b6b71c462dd3ea1, I applied all backports locally without conflicts and obtained a zero diff with this PR branch.
💬 trevarj commented on pull request "guix: Added guix-shasums script for gathering and formatting build output checksums":
(https://github.com/bitcoin/bitcoin/pull/33465#issuecomment-3325704017)
Firstly, thanks for sharing that one-liner. I looked everywhere for it and ended up doing it manually 😊

Approach NACK from me as well, since I think the compact code block is more legible due to not having newlines in the markdown table cells. I don't mine the one-liner in its own script, but having it documented seems necessary.
💬 fanquake commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3325790255)
> AFAIK there is already a PR for rc2 (see https://github.com/bitcoin/bitcoin/pull/33424, https://github.com/bitcoin/bitcoin/issues/32275)

Given it's been 2 weeks since rc1, and there's going to be an rc3 in either cas, I'm going to tag an rc2 shortly, and when it's decided what to do here, it could be incorported into rc3.
🚀 fanquake merged a pull request: "[30.0] Final changes + rc2"
(https://github.com/bitcoin/bitcoin/pull/33424)