💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430024540)
const nit
```suggestion
const auto peerman_info{node.peerman->GetInfo()};
```

💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430056990)
Wdyt about making the offsets typesafe with `std::chrono::seconds`?

<details>
<summary>git diff on 8b1c133921</summary>

```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 664992dcab..4a7243b82f 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -193,12 +193,12 @@ private:
static constexpr size_t N{199};

mutable Mutex m_mutex;
- std::array<int64_t, N> m_offsets GUARDED_BY(m_mutex){};
+ std::array<std::chrono::seconds, N>
...

💬 theDavidCoen commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1860335728)
Concept ACK.

These kind of parameters should default to false.
Node operators should be active users and decide which feature they want to enable for their node.

In this specific case, permitbaremultisig should default to zero or nodes would add and broadcast unconventional multisig transactions by default, and include also public keys out of the Bitcoin spectrum (uncompressed keys that are not points on the secp256k1 curve), which cannot be other than spam.
Node operators should be able
...

💬 TheCharlatan commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1860358206)
Concept ACK

💬 dergoegge commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430078828)
> I think the calculation is wrong: the average of the 2 middle elements should be taken in case of an even number of elements.

yes🤦

> UB: sorted_copy being an array of size N, I think we'd be sorting uninitialized values if m_index < N?

No, `m_offsets` is not uninitialized

💬 dergoegge commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430080537)
Good idea, but I'd prefer to leave that for a follow up

💬 fanquake commented on pull request "contrib: drop GCC MAX_VERSION to 4.3.0 in symbol-check":
(https://github.com/bitcoin/bitcoin/pull/28844#issuecomment-1860400750)
Guix Build (aarch64):
```bash
018360e32742367f372f48529ac321758dad7f64f42538807f06b2129e0d97fd guix-build-ff896d25819d/output/aarch64-linux-gnu/SHA256SUMS.part
8958317b7f79cb2839bed4f089c5c580be8dd3434563d2a797bb8f05905fbec5 guix-build-ff896d25819d/output/aarch64-linux-gnu/bitcoin-ff896d25819d-aarch64-linux-gnu-debug.tar.gz
71f0fd9abd4e354b87c4f670bdb17560f186f251d992f040c50239e3fbc783af guix-build-ff896d25819d/output/aarch64-linux-gnu/bitcoin-ff896d25819d-aarch64-linux-gnu.tar.gz
f82cf4
...

🚀 fanquake merged a pull request: "contrib: drop GCC MAX_VERSION to 4.3.0 in symbol-check"
(https://github.com/bitcoin/bitcoin/pull/28844)

🚀 fanquake merged a pull request: "fuzz: Limit p2p fuzz targets to MAX_PROTOCOL_MESSAGE_LENGTH"
(https://github.com/bitcoin/bitcoin/pull/29079)

🚀 fanquake merged a pull request: "build: Bump guix time-machine to unlock riscv64 metal"
(https://github.com/bitcoin/bitcoin/pull/29078)

✅ fanquake closed an issue: "./contrib/guix/guix-build does not work on riscv64"
(https://github.com/bitcoin/bitcoin/issues/29020)

📝 SatoshiNT0 opened a pull request: "bitcoin"
(https://github.com/bitcoin/bitcoin/pull/29106)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ fanquake closed a pull request: "bitcoin"
(https://github.com/bitcoin/bitcoin/pull/29106)

📝 fanquake locked a pull request: "bitcoin"
(https://github.com/bitcoin/bitcoin/pull/29106)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 fanquake commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1860456382)
If we are going to change it here, should also switch in `hash.cpp`:

https://github.com/bitcoin/bitcoin/blob/c840dea27edfcfc67beb756ca6eda27b319f93d5/src/hash.cpp#L12-L15

Also cc @theuni.

📝 kristapsk opened a pull request: "Fix spelling errors"
(https://github.com/bitcoin/bitcoin/pull/29107)
Found these when running lint tests locally.

```
src/rpc/util.h:405: falsy ==> falsely, false
src/rpc/util.h:408: falsy ==> falsely, false
src/test/fuzz/package_eval.cpp:214: non-existant ==> non-existent
src/test/span_tests.cpp:56: memeber ==> member
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```

Guess it's because I'm having different version of codespell?

In any case,
...

💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430148600)
> No, `m_offsets` is not uninitialized

The array is initialized, but the items are not, and afaik `int64_t` doesn't have a default constructor?

💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430149707)
Care to elaborate why? Since this is newly introduced code, seems sensible to do it right away?

🤔 ismaelsadeeq reviewed a pull request: "fuzz: make FuzzedDataProvider usage deterministic"
(https://github.com/bitcoin/bitcoin/pull/29043#pullrequestreview-1786902962)
>Good point, I guess since the spec is unspecified they have the right to change it but I would still be surprised if they do. I'm not sure if there is a trivial way to check if they ever have.

The order of evaluation issue is a characteristic of the C++ language standard. While the LLVM compiler might have a consistent order of evaluation, but generally compiler behavior could change, maybe we should not use the current LLVM behavior as validation for this but rather refer to the overall lan
...

💬 ismaelsadeeq commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1430148489)
Nit: use descriptive variable names for more clarity instead of single letters
<details>
<summary>see diff</summary>

```diff
diff --git a/src/test/fuzz/crypto.cpp b/src/test/fuzz/crypto.cpp
index aa478277e35..2fa2b109e55 100644
--- a/src/test/fuzz/crypto.cpp
+++ b/src/test/fuzz/crypto.cpp
@@ -23,8 +23,8 @@ FUZZ_TARGET(crypto)
std::vector<uint8_t> data = ConsumeRandomLengthByteVector(fuzzed_data_provider);
if (data.empty()) {
auto new_size = fuzzed_data_prov
...