💬 fanquake commented on pull request "guix: use GCC 12.3.0 to build releases":
(https://github.com/bitcoin/bitcoin/pull/27897#issuecomment-1677177179)
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=65151 landed in Guix. So I've updated the time-machine to include that change, and removed the commit dropping supported for `powerpc64-linux-gnu`. The last TODO here is resolve the determinism issue with the powerpc64le-linux-gnu build.

💬 hebasto commented on pull request "ci: Run "macOS 11.0 [gui, no tests] [jammy]" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28265#discussion_r1293363847)
Yes, it will happen from time to time. But not every run :)

There is an alternative to that approach, which is using Container Registry for images.

💬 stratospher commented on pull request "crypto: BIP324 ciphersuite follow-up":
(https://github.com/bitcoin/bitcoin/pull/28267#discussion_r1293365684)
done. added it in a new commit.

💬 fanquake commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#issuecomment-1677184316)
> Some changes here are also in https://github.com/bitcoin/bitcoin/pull/28008, but either or both can go in.

@sipa could you update the PR dscription in regards to this, now that #28008 has been merged.

cc @stratospher you might be interested in reviewing here?

💬 samyan commented on issue "Regtest mode loses unspents after day":
(https://github.com/bitcoin/bitcoin/issues/28262#issuecomment-1677188980)
> -mocktim

Still the same.

💬 fanquake commented on pull request "crypto: BIP324 ciphersuite follow-up":
(https://github.com/bitcoin/bitcoin/pull/28267#discussion_r1293378301)
Note that we keep our includes, and stardard library headers separate, so `span.h` should be added above. We also try use the cpp headers, [so cstdint over stdint.h](https://en.cppreference.com/w/cpp/header/cstdint) (regardless of what IWYU suggests).
```suggestion
#include <cstdint>
```

🚀 fanquake merged a pull request: "bitcoin-tidy: fix macOS build"
(https://github.com/bitcoin/bitcoin/pull/28258)

💬 fanquake commented on pull request "Break up script/standard.{h/cpp}":
(https://github.com/bitcoin/bitcoin/pull/28244#discussion_r1293380343)
We can just drop any `// For ...` comments entirely

💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1293390811)
> Haven't really looked at implementation yet, but can documentation be updated to say what happens with type mismatches?

This sound like a bug in the source code, so I don't think it makes sense to document unreachable or unexpected code paths. Generally, in the RPC server non-fatal bugs are expected to throw an internal bug message.

I haven't looked at the code, but types of arguments passed in by the user should be checked before *m_fun* is run. *m_fun*, and thus, `Arg()` isn't run when
...

💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1293392813)
> I think compile time RPC type checks would require templatising `RPCArg` by what is currently `m_type` (and `m_inner` perhaps), changing `RPCHelpMan` to be templatised based on a `std::tuple<...>` of `RPCArg<X>`s, and likewise for `RPCMethodImpl` which then gives the actual RPC implementation compile time access to its expected argument types.

I haven't looked at this, but some `RPCHelpMan` have run-time inputs, so I am not sure if they can be trivially converted to compile-time inputs. May
...

💬 theStack commented on pull request "test: refactor: support sending funds with outpoint result":
(https://github.com/bitcoin/bitcoin/pull/28264#issuecomment-1677224812)
Force-pushed with the feedback https://github.com/bitcoin/bitcoin/pull/28264#discussion_r1293224263 tackled and adapted the PR description accordingly.

💬 fanquake commented on pull request "build: Pointer Authentication and Branch Target Identification for aarch64 Linux (Guix)":
(https://github.com/bitcoin/bitcoin/pull/24123#issuecomment-1677226187)
Rebased onto #27897, which simplifies the actual changes here, and dropped no-longer needed commits.

💬 fanquake commented on pull request "build: Pointer Authentication and Branch Target Identification for aarch64 Linux (Guix)":
(https://github.com/bitcoin/bitcoin/pull/24123#discussion_r1293395019)
Made this Guix-only for now.

💬 fanquake commented on pull request "build: LLVM 16 & LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1677230335)
Re-rebased on #27897, and updated 6f6552688233e35a56f7a4c428c99c657150d313 to include `lld-15` & `lld-as-ld-wrapper-15`. Using lld-as-ld-wrapper seems to be the best way to make build systems (like Qts) work with LLD.

💬 fanquake commented on pull request "guix: Use LTO to build releases":
(https://github.com/bitcoin/bitcoin/pull/25391#issuecomment-1677251367)
Rebased on #27897. Simplifed / squashed down the changes here. Updated the PR description. The current (non-conceptual) blocker is that Qt for Windows doesn't really work under LTO... See 8031e400af66bf5267201069a7c062cb6a9d02ea.

💬 dergoegge commented on issue "meta: Isolated fuzzing of net processing":
(https://github.com/bitcoin/bitcoin/issues/27502#issuecomment-1677257433)
> What is this going to fuzz exactly? Is this intended to be abstracted away from validation/mempool behaviours (ie, stubbing them out)?

Mostly all code in `net_processing.cpp` (and relevant modules like orphanage, tx tracker) but excluding net, validation or mempool logic. Stubbing the latter out gives us control over testing net processing behavior related to these adjacent modules by being able to mock their behavior and state.

I'm not saying that we shouldn't fuzz the other modules, ju
...

💬 dergoegge commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1293423583)
... and set self.mocktime when ever setmocktime is called?

💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1293454810)
> > Also, the value will be nullptr before and after the call to HandleRequest
>
> Does this also hold if `m_fun` throws an exception?

The object does not exist after an exception. It will be destroyed/deallocated for each RPC call, see https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1285851447

💬 dergoegge commented on pull request "[no merge, meta] refactor: net/net processing split":
(https://github.com/bitcoin/bitcoin/pull/28252#issuecomment-1677329965)
> Being immediately dismissed out of hand feels pretty frustrating.

I apologize, that wasn't my intention.

I want to make it clear though that this PR is about continuing previous decoupling work that has seemingly dropped of peoples radar when previous contributors switched focus or left the project. I do not think the previous work went in the wrong direction nor that this breaks potentially multi threading the message processing code in the future.

If you think this design is somehow
...

💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1293469146)
I've made `bool no_default` a function `CheckNoDefault`, because this is exactly what needs to be checked. Let me know if you still disagree.