Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 glozow commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#discussion_r1945343151)
(I added that instead)
💬 glozow commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#discussion_r1945346739)
So `m_total_orphan_weight`, `PeerOrphanInfo::m_total_weight`?

Are `BytesFromPeer` and `TotalOrphanBytes` ok? Or do we want `WeightFromPeer` and `TotalOrphanWeight`? Or maybe `UsageByPeer` and `TotalOrphanUsage`?
💬 furszy commented on pull request "Wallet: don't underestimate the fees when spending a Taproot output":
(https://github.com/bitcoin/bitcoin/pull/26573#issuecomment-2640881605)
Concept ACK. Will review soon.
🤔 Prabhat1308 reviewed a pull request: "test: test_inv_block, use mocktime instead of waiting"
(https://github.com/bitcoin/bitcoin/pull/31811#pullrequestreview-2599781783)
tACK [2706c5b](https://github.com/bitcoin/bitcoin/pull/31811/commits/2706c5b7c8eee7ffd8c3b23a8012f346165ddb93)

Speedup is observed when running the test with the diffs.

ACK with the use of Mocktime as the test is independent from the use of wall-clock time. This can be seen by the use of `setmocktime()` in other functions in the same file as well .
💬 instagibbs commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#discussion_r1945358694)
I like `Usage` :+1:
💬 hebasto commented on pull request "Fix MSVC ccache reporting (Fixes #31771)":
(https://github.com/bitcoin/bitcoin/pull/31813#issuecomment-2640895236)
@vijayabhaskar78

> ### Testing
>
> ```shell
> cmake -B build -G Ninja -DCMAKE_CXX_COMPILER_LAUNCHER=ccache
> ```

All CI job failed. Does this work locally for you?

What are cache hit rates in the following scenarios:
- different build trees
- different git work-trees

?

(for more details, please see https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2331096969 and https://github.com/bitcoin/bitcoin/pull/30861)
💬 davidgumberg commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1945362703)
would it make sense to move this check into `assert_is_sqlite()`?
💬 glozow commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#discussion_r1945367787)
m_total_orphan_usage, m_total_usage, UsageByPeer, TotalOrphanUsage?
🚀 hebasto merged a pull request: "Prepare "Open Transifex translations for v29.0" release step"
(https://github.com/bitcoin/bitcoin/pull/31809)
💬 sipa commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#discussion_r1945350493)
Is the `try_emplace` necessary here? Given that we're immediately decrementing `peer_info.m_total_size` after, I think the assumption is that the `m_peer_orphanage_info` object for peer `peer` must already exist here, so a `find` should suffice?
💬 sipa commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#discussion_r1945376879)
It looks like there is no accounting for the number of announcers per peer? Is the idea that we use the `m_work_set.size()` as overestimate? If so, can the `SanityCheck` verify that for every `(wtxid, peer)` announcement pair, a `wtxid` entry exists in `peer`'s `m_work_set`?
💬 theuni commented on pull request "build: set build type and per-build-type flags as early as possible":
(https://github.com/bitcoin/bitcoin/pull/31711#issuecomment-2640929632)
Ok, updated description. It's much simpler now and should be very straightforward to review.
💬 glozow commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#discussion_r1945393736)
The work set isn't really tied to `m_orphans`, as it can include orphans that have been erased, so I don't think that would make sense.

I'm planning to make `m_orphan_list` a per-peer thing and use the `size()` of that as the announcement count per peer. I will add a loop to `SanityCheck()` then to ensure it is consistent with `m_orphans`.
💬 maflcko commented on issue "Networking tests fail on emulated big-endian systems":
(https://github.com/bitcoin/bitcoin/issues/31812#issuecomment-2640966745)
I suspect this is a macOS issue, because on my local CI system using Linux the s390x task was last reported to have passed on Feb 2nd.
💬 glozow commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#discussion_r1945412927)
done
💬 glozow commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#discussion_r1945413178)
good point, done
💬 hebasto commented on issue "Release Schedule for 29.0":
(https://github.com/bitcoin/bitcoin/issues/31029#issuecomment-2640982819)
> ## 2025-02-06 🚧
>
> * Open Transifex translations for `v29.0`
>
> * Soft translation string freeze (no large or non-critical string changes until release)
>
> * Finalize and close translations for `v27.0`

First set of tasks completed.

See the announcement on Transifex: https://app.transifex.com/bitcoin/communication/d:df052ee8-69d4-4739-a529-ba835673a9f0/.
👍 theuni approved a pull request: "depends: Avoid using the `-ffile-prefix-map` compiler option"
(https://github.com/bitcoin/bitcoin/pull/31800#pullrequestreview-2599895291)
utACK 407062f2ac93624f350e9e8a4f641c882a2aaf2f
👍 hodlinator approved a pull request: "logging: Ensure -debug=0/none behaves consistently with -nodebug"
(https://github.com/bitcoin/bitcoin/pull/31767#pullrequestreview-2599809742)
ACK 1f4aff75c308226d58a45460982f75287e15cb81

### Concept

Having `-debug=0` and `-debug=none` perform the same effect as `-nodebug` in discarding all preceding categories makes more sense, instead of disabling all categories including the ones following them.

### Testing

As well as running the new test as-is, I also verified stated behavior of `-nodebug` prior to the C++ change by modifying included test.

### Suggestions

Only found nits, the `last_negated` calculation in particu
...
💬 hodlinator commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1945367933)
nit: I tried to replicate what [ChatGPT suggested to ryanofsky](https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1930956563), but `std::views` wouldn't compile (didn't look into modifying my setup). Was able to combine it with your version though into:
```suggestion
auto last_negated = std::find_if(categories.rbegin(), categories.rend(),
[](const std::string& cat) { return cat == "0" || cat == "none"; });
```