Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 purpleKarrot commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2359730526)
The original error had the message string in quotes. I think this should be preserved because it is easier to debug when the string contains whitespace at the end.
💬 dergoegge commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#issuecomment-3307965154)
Added this to the release notes: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/v30.0-Release-Notes-Draft
💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2359774840)
I don't think we need the cpu protection now that we're using txgraph to do things (versus all the pointer chasing that would happen before), and only left the 500 size limit to preserve existing behavior. But I can move this back into the for-loop if people have concerns.

I believe that after cluster mempool, mini miner can be reworked to eliminate this function altogether. With the disclaimer that I haven't thought about it in a while, I put together a branch demonstrating this a while
...
👋 fanquake's pull request is ready for review: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33416)
💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2359949673)
Thanks, done.
💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2359951971)
Ah nice catch, I completely missed this. Done.
💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2359953034)
Thanks, I believe I've fixed all of the camel case.
💬 b-l-u-e commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#discussion_r2359990114)
Thanks for the feedback I will make changes
💬 john-moffett commented on pull request "rpc: Always return per-wtxid entries in submitpackage tx-results":
(https://github.com/bitcoin/bitcoin/pull/33427#issuecomment-3308365732)
> I thought "unknown-not-validated" would be better since it matches the error string for skipped transactions, but I think it's better to have separate messages for "nothing was validated because the package was ill-formed" vs "this particular transaction wasn't validated."

Right now the code can only ever return the former, as both the RPC [code](https://github.com/bitcoin/bitcoin/blob/74fa028da1ea38c5348f988464074899684cebcf/src/rpc/mempool.cpp#L1054) and the actual [acceptance code](https
...
💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#issuecomment-3308381089)
The latest push adds two commits:
- a5619ea631bd8b93b4ef02a20abb8c1c0705d8e4 "test: add setup_validation_interface_no_scheduler to TestOpts"
- Disables the scheduler completely if set. This is needed because this harness creates a `TestingSetup` inside the fuzz test and scheduling a [promise](https://github.com/bitcoin/bitcoin/blob/74fa028da1ea38c5348f988464074899684cebcf/src/test/util/setup_common.cpp#L249) can be non-deterministic as the scheduler's `serviceQueue` may start with a non-empt
...
💬 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)`