Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 hebasto commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3241549612)
> I've done some experimenting with a test project on transifex to see what the actual effects this change would cause, and a potential workaround.
>
> In this test project, I copied over a couple fully translated languages to observe how the translated strings change when the source is updated. After rebasing this PR branch and doing `cmake --build build --target translate`, I uploaded the resulting modified `bitcoin_en.xlf` file. This resulted in 122 (~10% of strings) being marked untransla
...
💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2313401875)
Could check the return code and print something to clarify this is an error, similar to the tidy error above:

```sh
echo "^^^ ⚠️ Failure generated from clang-tidy"
false
```
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2313427999)
Merged these into a single step in fdf64e55324, as there was no clear distinction between the two steps.
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2313428760)
amedned in cc1735d7771
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2313429763)
Yes I'd like to keep this for a followup for now.
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2313430468)
Done, here and elsewhere
💬 willcl-ark commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3241633101)
Rebased on master to catch upstream changes, manually added changes to align with #31802 and #33099 which were merged while I was away. Hopefully I didn't miss any others.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3241642016)
`dbd76e68c3...2bd4714fa9`: rebase due to conflicts and address https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2274730057

I am back from some afk time.
💬 hebasto commented on pull request "stabilize translations by reverting old ids by text content":
(https://github.com/bitcoin/bitcoin/pull/33270#issuecomment-3241645029)
Since we are not using [ID-based](https://doc.qt.io/qt-6/linguist-id-based-i18n.html) translations, it seems reasonable to consider removing the `id` attributes from `trans-unit` elements in the XML file.
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3241685301)
Just some style fixups, but it looks like out-of-storage errors are happening:

```
/home/admin/actions-runner/_work/_temp/depends/work/build/arm-linux-gnueabihf/qt/6.7.3-634ec665c7f/qtbase/src/gui/util/qvalidator.h:163:2: fatal error: cannot write PCH file: No space left on device
```

re-ACK 8a7eb6ef2c226459739bdfe050b735a5a0d0b771 🕧

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

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisi
...
💬 maflcko commented on issue "Revisiting us self-hosting parts of our CI":
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3241685779)
To keep a list of follow-ups:

* https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2290159660 (merge-base issue)
* https://github.com/bitcoin/bitcoin/pull/33145 (silent merge conflict check)
* https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3234350722 (qa-assets pull)
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3241834149)
Rebased and addressed @maflcko's [comment](https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2313401875).
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2313578680)
Thanks! [Done](https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3241834149).
💬 maflcko commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-3241842331)
Are you still working on this?
💬 hebasto commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3241930685)
> If this doc change is desired by contributors, and the 9 acks suggested that it is, then...

... it can be accepted as is, but left untranslated until the version v31.0.

This is acceptable because:
1. We never promised, nor have we ever delivered, 100% translations for every language.
2. It's still easy for the user to translate the warning message themself.

I still hesitant about introducing last-minute changes to the translation pipeline / framework. We can revisit all related sugg
...
💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3242087818)
re-ACK 1d49346465de8edec85bb2d498859b4fbc346935 📶

<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: re-ACK 1d49346465de8edec85b
...
💬 ismaelsadeeq commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2313772649)
Done
💬 ismaelsadeeq commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2313787122)
This is a nice catch.
The reason is that all transactions the fee estimator sees with a fee rate below 1 sat/vB are added to the **1–1.1 sat/vB** fee rate bucket. When that bucket is the lowest with sufficient data, the average is taken, which allows it to pass.

I modified the change to be more accurate by mixing in some 1 sat/vB transactions. This way, it will fail on master and also serve as a test case for that edge case.

Hence I think we require this test I will resolve https://github
...
💬 Sjors commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2313830013)
A slightly different approach, which avoids confusing `NLMSG_DONE` with `done`:

```diff
diff --git a/src/common/netif.cpp b/src/common/netif.cpp
index 77246980c3..3f3b8fdcea 100644
--- a/src/common/netif.cpp
+++ b/src/common/netif.cpp
@@ -117,9 +117,10 @@ std::optional<CNetAddr> QueryDefaultGatewayImpl(sa_family_t family)
// Receive response.
char response[4096];
ssize_t total_bytes_read{0};
- bool done{false};
- bool multi_part{false};
- while (!done) {
+
...