💬 dergoegge commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2388163491)
> They are unrelated to the checksum, so if they are failing, it seems like this patch is incomplete and some part is still checking the magic or checksum, no?

I'm guessing that we need to make sure that (if we are fuzzing) `V1Transport::SetMessageToSend` creates valid checksums/magic-bytes according to the new fuzzing mode check.

✅ itornaza closed a pull request: "refactor: Appropriate re-naming of MAX_OPCODE after tapscript"
(https://github.com/bitcoin/bitcoin/pull/30953)

💬 itornaza commented on pull request "refactor: Appropriate re-naming of MAX_OPCODE after tapscript":
(https://github.com/bitcoin/bitcoin/pull/30953#issuecomment-2388190304)
@darosior, highly respecting your opinion and taking note of your suggestions, I am close this pr.

💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2388224233)
> Can you explain why the asserts and sanity checks would be failing? They are unrelated to the checksum, so if they are failing, it seems like this patch is incomplete and some part is still checking the magic or checksum, no?

Some previously valid magic or checksum could be now be considered invalid. But I removed the commit that bypasses the asserts and changed the verification to:
```
#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
if ((hdr.pchMessageStart[0] & 1) != 0 && hdr.pchM
...

✅ fanquake closed an issue: "ci: Intermittent issue in feature_fee_estimation.py in sanity_check_rbf_estimates (Block sync timed out after 2400s)"
(https://github.com/bitcoin/bitcoin/issues/30990)

✅ fanquake closed an issue: "Intermittent failure in feature_fee_estimation.py in sanity_check_rbf_estimates: est_feerate = node.estimatesmartfee(2)["feerate"] (KeyError: 'feerate')"
(https://github.com/bitcoin/bitcoin/issues/30640)

🚀 fanquake merged a pull request: "test: add missing sync to feature_fee_estimation.py"
(https://github.com/bitcoin/bitcoin/pull/31016)

💬 stickies-v commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1784260961)
I used `std::string` at first too, but then it was inconsistent with the rest of the documentation, which I think is even more unclear. I'll reconsider updating all `std::string` references in this block if I have to force-push again unless you think this is blocking?

💬 hebasto commented on issue "cmake: passing options from depends build to main build":
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2388309124)
This discussion might be related: https://discourse.cmake.org/t/is-there-a-downside-for-a-toolchain-to-set-vars-in-cache-instead-of-init/12034.

💬 stickies-v commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2388310577)
> It would be cleaner _in the implementation_ to use `util::Result` instead of overwriting the returned value with the error message

I don't think that's a good approach for our codebase, even if there was minimal code churn. All format strings are known at compile-time, so any formatting errors are programming errors. Handling that with `util::Result` would be both a cumbersome interface and bad practice imo. We don't need to let callsites handle formatting errors, they should never occur, b
...

✅ maflcko closed an issue: "Intermittent timeout in tsan feature_init.py"
(https://github.com/bitcoin/bitcoin/issues/30586)

💬 maflcko commented on issue "Intermittent timeout in tsan feature_init.py":
(https://github.com/bitcoin/bitcoin/issues/30586#issuecomment-2388314370)
Closing for now, but this can be reopened when it happens again the next time.

🚀 fanquake merged a pull request: "doc: add testnet4 section header for config file"
(https://github.com/bitcoin/bitcoin/pull/31007)

💬 fanquake commented on pull request "wallet: Filter-out "send" addresses from `listreceivedby*`":
(https://github.com/bitcoin/bitcoin/pull/25973#issuecomment-2388319477)
Some of this was picked up in #30972.

💬 maflcko commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2388324332)
I agree that `util::Result` doesn't make any sense here. Apart from the reasons mentioned, it would also make it harder to switch to `std::format`, as well as being confusing, because no other format lib I am aware of is doing that.

> * using the `tfm::format_raw` functions (only for tinyformat implementation or tests)

In the PR description: I think this should also say `or bitcoin-cli`.

💬 maflcko commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1784285299)
If `Tinyformat handles string parameters.` is too controversial, I think it can be removed in the follow-up that removes the linter.

Otherwise, if this is replaced with `std::string`, someone else is going to suggest to mention `std::string_view` as well.

💬 stickies-v commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2388345105)
> In the PR description: I think this should also say `or bitcoin-cli`.

`bitcoin-cli` wasn't using the `const std::string&` overload which that bulletpoint is about. It is mentioned in the next bulletpoint about compile-time checks:

> Callsites that for some reason don't pass the compile-time checks (such as in `bitcoin-cli.cpp`) can use `tfm::format_raw`.

💬 MrSuddenJoy commented on issue "RFC: Multiprocess binaries and packaging options":
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-2388347821)
@ryanofsky Not entirely sure this is way to go imho.

As long as everything works well ( = as expected ) its all ok, but as soon as things go south, problems (especially ones that are hard to solve/complex) start to accumulate/pile up. And thats exactly whats root cause of many bad things, namely frustration, burn-out and things like this.

Having `meta-package` has proven (to me) to be perfect cure for problems described above. If something breaks, its dead simple to: localise where (in cod
...

💬 maflcko commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#discussion_r1784296416)
maybe I am missing something, but this looks still wrong. I am reading this as fuzzing adding *additional* constraints, which seems the inverse of what we want to achieve.

Also, this could be impossible to satisfy, if `!(m_magic_bytes[0]&1)` holds. What am I missing?

💬 Sjors commented on issue "depends: llvm-ranlib (etc): "No such file or directory" on Intel macOS 15.0":
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2388352764)
I get it every time on master and any branch I test on.

Here's the initial log from a clean run on fc642c33ef28829eda0119a0fe39fd9bc4b84051 on Intel macOS 15.0:

```
git clean -dfx
cd depends
make
/bin/sh: command -v llvm-ranlib: No such file or directory
/bin/sh: command -v llvm-strip: No such file or directory
/bin/sh: command -v llvm-nm: No such file or directory
/bin/sh: command -v llvm-objdump: No such file or directory
/bin/sh: command -v dsymutil: No such file or directory

...