💬 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.

💬 furszy commented on pull request "test: raise explicit error if any of the needed release binaries is missing":
(https://github.com/bitcoin/bitcoin/pull/31462#discussion_r1904521854)
+1. Something like this would be helpful. I've received a few messages sharing these logs without knowing what to do next.

👍 ryanofsky approved a pull request: "test: Add test for rpcwhitelistdefault"
(https://github.com/bitcoin/bitcoin/pull/29858#pullrequestreview-2532726241)
Code review ACK fcf0ead6cd358fd35909e1102bdae2c176b0ace6. I left suggestions below, but none are important and they can be ignored. I think it is better to have this test coverage for this than not to have it, and no need to spend too much time iterating on the PR just to improve test code quality which is already decent.

Note that this PR does not provide any coverage for the case where rpcwhitelistdefault setting is unset, only for the cases where it is explicitly set to 0 and explicitly se
...

💬 ryanofsky commented on pull request "test: Add test for rpcwhitelistdefault":
(https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1904534581)
In commit "test: Add test for rpcwhitelistdefault" (fcf0ead6cd358fd35909e1102bdae2c176b0ace6)

Since this code is changing configuration that affects all future tests I think it would be less confusing if it were moved out of this individual test method and into the main `run_test` method. Otherwise it is not clear that calling this particular test method has side effects for calls to future test methods.

💬 ryanofsky commented on pull request "test: Add test for rpcwhitelistdefault":
(https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1904497634)
In commit "test: Add test for rpcwhitelistdefault" (fcf0ead6cd358fd35909e1102bdae2c176b0ace6)

I think it would be good to change this line to if `user[2] is not None:` and change the new user's whitelist above from `""` to `None`. This should be clearer and less confusing because `""` looks like an empty whitelist, not an unspecified whitelist, and "" is already used elsewhere in the test to check empty whitelists. `None` would be clearer than `""` as a way to represent unspecified whitelists
...

💬 ryanofsky commented on pull request "test: Add test for rpcwhitelistdefault":
(https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1904493624)
In commit "test: Add test for rpcwhitelistdefault" (fcf0ead6cd358fd35909e1102bdae2c176b0ace6)

This function mostly contains moved code, but is confusing because it is referencing a hardcoded index in lists that are not defined here. Would suggest replacing this with a shorter version:

```python
def get_permissions(whitelist):
return [perm for perm in whitelist.replace(" ", "").split(",") if perm]
```

And replacing `get_permissions(user)` with `get_permissions(user[2])` at callsit
...