Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 ryanofsky commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#issuecomment-3308386265)
re: https://github.com/bitcoin/bitcoin/pull/33388#issuecomment-3307790546

> The swallowing of the 2nd and subsequent exceptions seems to assume that one has control and has implemented the same policy in all destructors in the program.

That's not true, and wanted to be clear I wasn't advocating the policy generally. I was just pointing out that letting this particular destructor throw as it was designed to do, instead of aborting, is easy to support in practice with the noexcept(false) std
...
💬 l0rinc commented on pull request "log: show reindex progress in `ImportBlocks`":
(https://github.com/bitcoin/bitcoin/pull/33353#discussion_r2360199070)
It's just an approximation - or do you think `Reindexing block file blk%05u.dat (5/123)...` would be clearer? We have progress in blocks as well, they're all just approximations...
💬 Raimo33 commented on pull request "log: show reindex progress in `ImportBlocks`":
(https://github.com/bitcoin/bitcoin/pull/33353#discussion_r2360208827)
i think percentage is fine. it just means % of blocks instead of % of time
📝 fanquake opened a pull request: "depends: Boost 1.89.0"
(https://github.com/bitcoin/bitcoin/pull/33428)
Update [Boost to 1.89.0](https://www.boost.org/releases/1.89.0/) in depends.
📝 marcofleon opened a pull request: "fuzz: reduce iterations in slow targets"
(https://github.com/bitcoin/bitcoin/pull/33429)
This PR builds on top of the first two commits of https://github.com/bitcoin/bitcoin/pull/33425 to see if there's a speed up of the fuzz with MSan CI job.
💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2359472683)


```Suggestion
// The level must match the level the Cluster occurs in (call only valid with non-empty cluster).
```
💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2359625033)
a940e46bb06cf40f65772516d67f78cbd87c6b52

noting we aren't tracking BlockBuilderImpl memory usage which only has growth in `m_excluded_clusters`

(N.B. I don't think we need ordered set for m_excluded_clusters)
👍 instagibbs approved a pull request: "cluster mempool: control/optimize TxGraph memory usage"
(https://github.com/bitcoin/bitcoin/pull/33157#pullrequestreview-3239814693)
ACK 66401148ddcba04c27751e39e53fc41e6deb5d14

Did not validate the numbers, non-blocking comments only
💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2360237618)
66401148ddcba04c27751e39e53fc41e6deb5d14

micro-nit: couldn't figure out what `tot` meant for a second... "to t.... size?"
```Suggestion
DepGraphIndex total_size = max_size;
```
💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2360179765)
89b7997e7414074da3b0a35a17d86a2dfe24f378

For these functions(and the later singleton ones) can we `Assume(graph_idx >= 0)`
💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2359361516)
fdcb5f17bd100eb2c1765c13bbb8367d03fc7ea5

micro-nit for readability

```Suggestion
MakeAcceptable(*cluster, level);
```
💬 marcofleon commented on pull request "fuzz: reduce iterations in slow targets":
(https://github.com/bitcoin/bitcoin/pull/33429#issuecomment-3308577557)
There should be no loss of coverage when running on the inputs in qa-assets.

For `tx_pool_standard` I saw a speed up from 1068 to 720 seconds with this change. I think the loop in `initialize_tx_pool` is still slowing it down? I would need to look into some more, but I left that as is for now.
💬 Sjors commented on pull request "Update libmultiprocess subtree to fix intermittent mptest hang":
(https://github.com/bitcoin/bitcoin/pull/33412#issuecomment-3308650255)
ACK c49a43591f88dc199cc04e76f3cfcb7ba136f1a6
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2360439278)
Fixed name
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2360439282)
No problem, I put them back to the beginning
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2360439251)
I think this is trivial to review.
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2360439503)
This seemed like the time to simplify these, but I don't mind, reverted.
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2360440096)
I did this deliberately, but thinking a bit more I think we can make it work. Pushed something similar, let me know what you think.
👍 darosior approved a pull request: "p2p: Increase tx relay rate"
(https://github.com/bitcoin/bitcoin/pull/28592#pullrequestreview-3241205204)
utACK b81f37031c8f2ccad9346f1b65ee0f8083c44796. A value lower than 14 makes more sense to me, but 14 is an improvement on today's 7.

[Mainnet.observer](https://mainnet.observer/charts/transactions-per-day/) has historical statistics that can help guide our decision. Back when the 7 tx/s was picked, there was around 2.3 tx/s (when averaged over days). In this regard, doubling the default now that there is around 4.6 tx/s makes sense. However i think a lower value than 14 tx/s would be more ap
...
💬 achow101 commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33416#issuecomment-3308904174)
Should also backport #31407