💬 ryanofsky commented on pull request "rpc: add `revelant_blocks` to `scanblocks status`":
(https://github.com/bitcoin/bitcoin/pull/30713#discussion_r1904300535)
In commit "rpc: add relevant_blocks to scanblocks status" (4c9dc4bbe6b23ae7dba7badbdb6dd6c3d0f74e08)

It would be good to declare these next to `g_scanfilter_` variables so all global state is declared in one place, and so other functions besides `scanblocks` can see this information (other RPCs and maybe tests should be able to access it). Would suggest renaming `relevant_blocks` to `g_scanfilter_relevant_blocks` and `cs_relevant_blocks` to `g_scanfilter_relevant_blocks_mutex`. (The "critical
...

💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#issuecomment-2573467782)
Updated ad80e410b3bf8814cb5b997715ee8cb28f2a2a66 -> 54bdaaefbc683a61f3051dd90bbaf69de5d6cac5 ([kernel_cache_sizes_5](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_5) -> [kernel_cache_sizes_6](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernel_cache_sizes_5..kernel_cache_sizes_6))

* Addressed @hodlinator's [comment](https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1903999989), took sugg
...

💬 i-am-yuvi commented on pull request "qa: Improve framework.generate* enforcement (#31403 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31599#issuecomment-2573527165)
> Also, a rebase would be good

Done!

💬 i-am-yuvi commented on pull request "func test: Expand tx download preference tests":
(https://github.com/bitcoin/bitcoin/pull/31437#issuecomment-2573560025)
Concept ACK

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2573570012)
Rebased on the latest #31581.

I added a commit that on testnet3 and testnet4 returns a new template after 20 minutes, to take advantage of the min difficulty rule.

Since `miner_tests.cpp` uses mainnet, I created `testnet4_miner_tests.cpp` to cover this behaviour in the unit tests. A functional test could be added later using the testnet4 blocks added by #31583.

I also switched to using `NodeClock::now()` to be able to use mock time in tests.

💬 1440000bytes commented on issue "Fake bitcoin core website at the top of duckduckgo":
(https://github.com/bitcoin/bitcoin/issues/31602#issuecomment-2573668394)
I submitted an [abuse report](https://abuse.cloudflare.com/phishing) for this related domains to cloudflare because its being used for DNS. They have forwarded the report to the responsible hosting provider (IQWeb FZ-LLC).

I see a warning now when I open this link in the browser:


💬 theuni commented on issue "cmake inconsistently overriding `-O3` (sometimes)":
(https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2573690501)
I'd say this actually isn't a special case, rather it's fixing a logical bug in the current impl.

Currently, we essentially do: `echo "$CMAKE_CXX_FLAGS_RELEASE" | sed "s/-O3/-O2/"`.

Instead, I think we only want to do that if we're not overriding the user. Essentially the same as: https://github.com/bitcoin/bitcoin/blob/28.x/configure.ac#L298

So basically the user will always get -O2, unless they change the optims by setting them in the supported/documented variable: `-DCMAKE_CXX_FLAGS_
...

🤔 ismaelsadeeq reviewed a pull request: "mining: bugfix: Fix duplicate coinbase tx weight reservation"
(https://github.com/bitcoin/bitcoin/pull/31384#pullrequestreview-2532569808)
Thanks for your feedback @Sjors and your review @TheCharlatan

I've forced pushed to see [diff](https://github.com/bitcoin/bitcoin/compare/cd48e2ccaf24c9594f5987a29e02b924573134ed..ededdd13f3263df08f7878aa2514d3796436cbfb):
1. Add the `-maxcoinbaseweight` option with default as `8000`.
2. Address review comments from @TheCharlatan

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1904497444)
This is gone now.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1904447541)
done

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1904402308)
> Is there a material difference between this test block and the previous one that actually exercises the default coinbase weight?

There is no any material difference, removed.

> Could more exact weight accounting where the total weight is checked with assert_equal instead of assert_greater_than_or_equal in verify_block_template be useful?

It would be useful, but the reason why I am not is because there is a little disparity between the target weight and the actual weight.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1904495041)
I think it has.

I've even modified it to also use the correct weight and clarify the log.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1904497231)
fixed

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1904447430)
done.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1904495815)
This is fixed now.

💬 Eunovo commented on issue "assumevalid is not always applied when reindexing":
(https://github.com/bitcoin/bitcoin/issues/31494#issuecomment-2573704148)
I added a test reproducing this bug https://github.com/eunovo/bitcoin/commit/5e1ff5fe2c7374f97eb56454099c1b235c75e23c

💬 Eunovo commented on issue "assumevalid is not always applied when reindexing":
(https://github.com/bitcoin/bitcoin/issues/31494#issuecomment-2573707408)
> * always enable assumvalid while connecting blocks in the context of reindexing

Isn't it still valuable to check that the current block is an ancestor of the most work chain?

💬 LarryRuane commented on pull request "dbwrapper: Bump LevelDB max file size to 32 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2573707548)
@sipa:
> Concept ACK. Please update the PR title and description to reflect the new size.

If it is still possible post-merge, please update the description; it still says 128 MiB (the title has been updated), thanks.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1904514447)
fixed. thanks @pinheadmz

💬 theuni commented on issue "cmake inconsistently overriding `-O3` (sometimes)":
(https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2573712156)
Note that this does not address the depends problem. The issue there is that the "-Ox" flags are jumbled in with the rest of the flags.

To fix that, we need to grab the "-O" values out of the passed-in `CXXFLAGS` and tack the last one onto `CMAKE_CXX_FLAGS_RELWITHDEBINFO_INIT`. I'll push a commit for that to my branch.