Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 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
...
👍 alfonsoromanz approved a pull request: "AssumeUTXO: Don't Assume m_chain_tx_count in GuessVerificationProgress"
(https://github.com/bitcoin/bitcoin/pull/30909#pullrequestreview-2307534418)
tACK 9be080abfa87543652c415af4e239a6ba4d2b2e1

The test passes locally, and I see that all the CI checks have passed, so it looks good to me.

It took me some time today to get familiar with the CMake-based build system, but I managed to get it working.

<img width="982" alt="Screenshot 2024-09-16 at 16 05 09" src="https://github.com/user-attachments/assets/bbe337ef-f789-49f0-a414-2661a17dbfb7">


Thanks!
💬 jonatack commented on pull request "AssumeUTXO: Don't Assume m_chain_tx_count in GuessVerificationProgress":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1761740170)
Question, is logging this enough, e.g. in the various RPCs (and CLI -getinfo) that return a "verificationprogress" field, are there any where it would be a good idea to describe the what/why of a returned 0.0 value.
💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2353722966)
> I have opened a separate PR #30909 with my suggested fix and your test commit cherry-picked on top of it @alfonsoromanz .

Thanks a lot @fjahr !

Should I modify this PR now to be on top of yours, including only the second test? Or would it be better to wait until your PR gets merged and then modify mine?
💬 achow101 commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#issuecomment-2353732588)
ACK a93c171faa7b4426823466e972c8f24260918bbf
💬 Sjors commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#issuecomment-2353739606)
@achow101 see #30510
🚀 achow101 merged a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440)
💬 achow101 commented on pull request "refactor: add clang-tidy `modernize-use-starts-ends-with` check":
(https://github.com/bitcoin/bitcoin/pull/30868#issuecomment-2353758047)
ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3
💬 theuni commented on pull request "build: speed up by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2353778200)
Looks like this is hitting a CMake bug: https://gitlab.kitware.com/cmake/cmake/-/issues/24058

Converting to draft while I investigate.
📝 theuni converted_to_draft 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
...