Bitcoin Core Github
43 subscribers
122K links
Download Telegram
👍 hebasto approved a pull request: "[RFC] Switch and/or align debugging flags (back) to `-Og`"
(https://github.com/bitcoin/bitcoin/pull/29796#pullrequestreview-2307158543)
ACK 296def1f6fd936d72d49ce98f9473b4a5d2f9c4b, I have reviewed the code and it looks OK.

This PR effectively aligns flags between the depends build subsystem and the main build system. This change makes the current undocumented behaviour--where flags from depends override flags form the main build system--more flexible, and it can be considered in the ongoing [discussion](https://github.com/bitcoin/bitcoin/issues/30813).

Users will still be able to adjust their debugging experience by manua
...
💬 sdaftuar commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2353413849)
ACK 9ad2fe7e69e9e69949ebbb280a15756dc3301f09
🤔 sipa reviewed a pull request: "fuzz: Add HeadersSyncState target"
(https://github.com/bitcoin/bitcoin/pull/26662#pullrequestreview-1212243962)
Apparently I never submitted this feedback. Maybe useful for follow-ups.
💬 sipa commented on pull request "fuzz: Add HeadersSyncState target":
(https://github.com/bitcoin/bitcoin/pull/26662#discussion_r1044846771)
It'd be a bit easier to follow if instead of this conditional there were a `if (!result.request_more) break;`. The `requested_more` variable can then also go away, as it'd always be true any time it's checked. Lastly, it'd make it more obvious that `redownloaded_it` is always initialized in iterations after the first.
💬 sipa commented on pull request "fuzz: Add HeadersSyncState target":
(https://github.com/bitcoin/bitcoin/pull/26662#discussion_r1044834458)
Nit: `headers = std::move(*deser_headers);` is equivalent and matches intent better.
💬 ryanofsky commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change and removed libtools from CI":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1761528227)
> @ryanofsky `bitcoin-wallet` is built, if enabled, independently of the multiprocess flag (if I'm not confused); should `doc/multiprocess.md` describe this differently?

Probably previous description was a little confusing but current version c3d68e11191e036e154916f27f21200d2bf16756 which doesn't mention bitcoin-wallet looks good to me.

This readme was written as part of #10102 before bitcoin-wallet tool existed, so --enable-multiprocess option really did build a new binary that didn't exi
...
⚠️ 2charlehsbailevy opened an issue: "IMPORTANT! Join https://discord.gg/gruppe"
(https://github.com/bitcoin/bitcoin/issues/30910)
Hey there!
Please join our Discord server, we have to tell you something important.

https://discord.gg/gruppe
💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change and removed libtools from CI":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1761561450)
I went ahead and changed it back from `bitcoind` to `bitcoin-node` in [dc2e223](https://github.com/bitcoin/bitcoin/pull/30875/commits/dc2e2231036ec09fac4529d54881457fa9a7dc51)
💬 mzumsande commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1761565066)
done
💬 mzumsande commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1761565377)
Removed the line here and below
💬 mzumsande commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1761565931)
Done - even if it's a little bit repetitive, I agree it's good to have coverage for this.
💬 mzumsande commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1761566663)
Makes sense. I've added a call to `NotifyHeaderTip`.
💬 mzumsande commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1761569316)
I've reworked this by introducing `SetBlockFailureFlags` (as a counterpart to `ResetBlockFailureFlags` that removes the flags after reconsiderblock)
💬 mzumsande commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#issuecomment-2353514570)
[c9bf06a ](https://github.com/bitcoin/bitcoin/commit/c9bf06a531617fdd70b64e23572931af2b969828)to [5aa9160](https://github.com/bitcoin/bitcoin/commit/5aa9160698c4375afc8f242f8f1e3078e61b1713):
* rebased
* addressed feedback
* reworked commit f44a403ad27297c10256df11c3876b27c58dc1e4 (fix m_best_header tracking and BLOCK_FAILED_CHILD ) to set `BLOCK_FAILED_CHILD` for all blocks building on the invalid block, instead of just the ancestors of `m_best_header`
💬 mzumsande commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1761582712)
actually, that didn't work because `NotifyHeaderTip` can't be called with `cs_main` is held. Reverting this, I'll think about this a bit more.
🚀 glozow merged a pull request: "cluster mempool: optimized candidate search"
(https://github.com/bitcoin/bitcoin/pull/30286)
🚀 glozow merged a pull request: "fuzz: Test headers pre-sync through p2p"
(https://github.com/bitcoin/bitcoin/pull/30661)
💬 jonatack commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1761632501)
That confirms what I thought when suggesting https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1759260020. Thanks @ryanofsky for the quick reply.
📝 theuni opened a pull request: "build: speed up by flattening the dependency graph"
(https://github.com/bitcoin/bitcoin/pull/30911)
These changes speed up my build (default config/options/targets) by roughly 10%. I suspect the difference may be more significant in other build configs.

Before:
> $ time cmake --build build -j24
> real 3m26.932s

After:
> $ time cmake --build build -j24
> real 3m7.556s

Generally they allow for jobservers (either `make -jX` or `ninja`) to be better utilized. This can be verified using `top` while building and looking at the number of compiles running at any given time before/after th
...
💬 achow101 commented on pull request "[RFC] Switch and/or align debugging flags (back) to `-Og`":
(https://github.com/bitcoin/bitcoin/pull/29796#issuecomment-2353689353)
NACK using `-Og` for the main build.

Since I spend a considerable amount of time in gdb, I find this to be detrimental to debugability. One big issue that I'm seeing is that when stepping through line by line, some lines get skipped entirely, including lines that call other functions so it is no longer possible to step into those functions from that caller. Instead a breakpoint needs to be set on that function directly, but this is not always desirable. This can actually be seen in my [earlie
...