Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 hebasto commented on pull request "ci: run s390x job":
(https://github.com/bitcoin/bitcoin/pull/33436#issuecomment-3410327550)
> Maybe we could add the cross-compile only, to match Guix.

What does it mean? We do not build for the `s390x-linux-gnu` host in the Guix scripts?
💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3410342332)
> > Not sure why I get this failure ...
>
> It is making requests to the DNS server at `1111:1111::1.53`, trying to resolve `x9.dummySeed.invalid.`

Yes, I understand this, but I don't understand why the CI is green on this pull request here, but it fails locally when using podman. I guess it could be due to running as root inside the VM. Though, the failure seemingly not being reproducible on every run makes it even more odd.

My cmd history today (once it passed, once it failed):

```
...
💬 fanquake commented on pull request "cmake: Fix `FindQt` module":
(https://github.com/bitcoin/bitcoin/pull/32709#discussion_r2435498808)
Is this going to be addressed / reported upstream? I'd rather not undocumented work arounds, for suspected upstream issues (given that they mark all other dependencies as advanced), just to fix the output of a GUI tool.
💬 hebasto commented on pull request "Fix Wayland visual glitches":
(https://github.com/bitcoin-core/gui/pull/904#discussion_r2435501488)
This code was introduced in https://github.com/bitcoin-core/gui/pull/831.

cc @pablomartin4btc
💬 stickies-v commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2435508418)
> why wouldn't we want to be a bit spammy in this scenario?

I don't disagree, but it seems orthogonal to this PR which is addressing db corruption issues? It's okay for warning messages to not cover all edge cases, but I think it is good to try to avoid incorrect diagnostics or solutions if we can help it without too much complexity. `invalidateblock` is debug-only, but is also commonly referenced as a break-in-case-of-emergency RPC. With the current change, usage of the RPC is guaranteed to
...
💬 stickies-v commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2435514483)
Alternatively, I would be okay with (but not prefer) the log message to be generalized to highlight that db corruption or consensus failure are possible, and the `invalidateblock` RPC documentation updated to highlight that this warning will be flooded upon usage.
🤔 janb84 reviewed a pull request: "Update secp256k1 subtree to latest master"
(https://github.com/bitcoin/bitcoin/pull/33625#pullrequestreview-3344304291)
ACK

<details> <summary>Guix Build Output</summary>

**Host architecture:** `aarch64`
**Commit:** `879c21045eba`

```shell
22b8a342b9e47069d05d1fca40437ce2b19820e3f3c6b7ef17f2132a462500f9 guix-build-879c21045eba/output/aarch64-linux-gnu/SHA256SUMS.part
8f156c6e1cea62efb25007ab62ce02292eac014fe81f526249e4ff6724292a0c guix-build-879c21045eba/output/aarch64-linux-gnu/bitcoin-879c21045eba-aarch64-linux-gnu-debug.tar.gz
9afd57b78623c0ca553a6e8bf794baf8e31ee55b3dcbe9e265b02810cfaebb82
...
💬 maflcko commented on pull request "ci: run s390x job":
(https://github.com/bitcoin/bitcoin/pull/33436#issuecomment-3410381160)
I'd expect that cross-compilation in this CI task always succeeds, if all the other CI tasks (including the other cross-compile ones). For s390x the value really is in running the full tests to see any endian bugs.
💬 naiyoma commented on pull request "p2p: Mitigate GETADDR fingerprinting by setting address timestamps to a fixed value":
(https://github.com/bitcoin/bitcoin/pull/33498#issuecomment-3410382253)

> With this PR, if an address older than 10 days (but not older than 30 days) is part of a `GetAddr` answer, the receiving peer will _postdate_ its timestamp, giving it more time until it gets Terrible. There is a chaining effect, because the receiving peer will also relay it longer when it answers `GetAddr` requests itself, and so on. If this would happen frequently enough, the addr of the node that has left the network would always stay in the 10-30 day range and never cease to be relayed, a
...
👍 stickies-v approved a pull request: "[28.x] Backport & finalise 28.3"
(https://github.com/bitcoin/bitcoin/pull/33613#pullrequestreview-3344326684)
ACK 2dfb3a06902e9a98e00a422705afa002a3744545

No changes except for 1 CI change, which I (utACK) verified. And finalization.
💬 fanquake commented on pull request "ci: re-add Valgrind job to the CI":
(https://github.com/bitcoin/bitcoin/pull/33411#discussion_r2435545807)
Just dropped the exclusion.
💬 diegoviola commented on pull request "Fix Wayland visual glitches":
(https://github.com/bitcoin-core/gui/pull/904#discussion_r2435575477)
@hebasto Thanks for letting me know. I managed to reproduce the artifacts/flicker with the original code on sway and labwc (wlroots-based compositors). In both cases, getting rid of the flags fixed the issue.

I've not tested this on KWin and Mutter yet, I plan on doing that later today. Hopefully, we can come up with a solution that works everywhere.
💬 diegoviola commented on pull request "Fix Wayland visual glitches":
(https://github.com/bitcoin-core/gui/pull/904#discussion_r2435577485)
@hebasto Thanks for letting me know. I managed to reproduce the artifacts/flicker with the original code on sway and labwc (wlroots-based compositors). In both cases, getting rid of the flags fixed the issue.

I've not tested this on KWin and Mutter yet, I plan on doing that later today. Hopefully we can come up with a solution that works everywhere.
💬 marcofleon commented on pull request "[28.x] Backport & finalise 28.3":
(https://github.com/bitcoin/bitcoin/pull/33613#issuecomment-3410469971)
ACK 2dfb3a06902e9a98e00a422705afa002a3744545
💬 maflcko commented on pull request "ci: re-add Valgrind job to the CI":
(https://github.com/bitcoin/bitcoin/pull/33411#issuecomment-3410475429)
Again, this seems fine, and I don't want to raise an objection. However, the general question of how to deal with the false-positives (see my previous comment) here is still unanswered. IIUC, valgrind may not work under a given compiler (version). For example, currently it doesn't work under clang out of the box. There are workarounds needed, but they come with drawbacks:

* Add a suppression, but if this one is too broad, it decreases the value of running valgrind
* Use `-O1`, but valgrind i
...
💬 maflcko commented on pull request "Update GitHub Action to github-script@v8":
(https://github.com/bitcoin/bitcoin/pull/33634#issuecomment-3410485493)
> policy

I think the policy is "Ensure your workflows use the latest versions of actions that are running on Node 20. For more information, see [Using Versions for Actions](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#example-using-versioned-actions).". Source: https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/#for-actions-users

Something similar may happen with node-24. However, I am not familiar with GHA
...
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2435629619)
I did make a few changes (the linter alerted on a few lines) so please take a look again at what is here.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2435644194)
Oh, actually we need the weight further down for the check to see if the chunk will fit in the block, so I'm inclined to leave this as-is?
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2435654614)
Done.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2435668780)
Incorporated in a8be743aeb42ec8ab613f822989a11a2f2ce70ac