💬 maflcko commented on pull request "Update doc/policy/README.md":
(https://github.com/bitcoin/bitcoin/pull/29095#issuecomment-1860150991)
So I guess it would be better to update the docs to refer to that, than to start listing more options here.

💬 aureleoules commented on pull request "rpc: Optimize serialization disk space of dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/26045#issuecomment-1860158880)
Rebased

I had to slightly change the tests in `feature_assumeutxo.py` because I changed the encoding format of the dump. I added 2 bytes to the offset because of the new `size` (2 bytes) field.

👋 aureleoules's pull request is ready for review: "rpc: Optimize serialization disk space of dumptxoutset"
(https://github.com/bitcoin/bitcoin/pull/26045)

💬 fanquake commented on issue "Performance decrease after tapscript miniscript":
(https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-1860166405)
cc @darosior @sipa

💬 maflcko commented on issue "Discussion: Upgrading to C++20":
(https://github.com/bitcoin/bitcoin/issues/23363#issuecomment-1860181524)
> I think you need an helper

Looks like you are right. I thought that it was possible to write a concept like

```cpp
template <class T>
concept ArrayLike = requires(T a) {
std::array<decltype(*a.begin()), a.size()>{};
};
```

But that doesn't work, because `a.size()` is unevaluated (https://eel.is/c++draft/expr.prim#req.general-2) and presumably template arguments must be evaluated?

💬 ajtowns commented on issue "Discussion: Upgrading to C++20":
(https://github.com/bitcoin/bitcoin/issues/23363#issuecomment-1860216234)
> > I think you need an helper
>
> Looks like you are right. I thought that it was possible to write a concept like
>
> ```c++
> template <class T>
> concept ArrayLike = requires(T a) {
> std::array<decltype(*a.begin()), a.size()>{};
> };
> ```
>
> But that doesn't work, because `a.size()` is unevaluated (https://eel.is/c++draft/expr.prim#req.general-2) and presumably template arguments must be evaluated?

`ArrayLike` sounds like `ranges::contiguous_range` ? https://en.cppref
...

💬 darosior commented on issue "Performance decrease after tapscript miniscript":
(https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-1860221553)
Without profiling my hunch is that this performance decrease comes from using a less efficient but more optimal algorithm for satisfying a `multi_a` fragment:
https://github.com/bitcoin/bitcoin/blob/3695ecbf680a66b718f97d504308578d001eec49/src/script/miniscript.h#L1178-L1205

The [previously algorithm](https://github.com/bitcoin/bitcoin/commit/4f473ea515bc77b9138323dab8a741c063d32e8f#diff-8a974828ccf5a554c068f5e859e62d1ab1e5010c66baaa6d6b83f42b26b219adL286-L299) would use the first `k` availa
...

💬 fanquake commented on pull request "build: Bump guix time-machine to unlock riscv64 metal":
(https://github.com/bitcoin/bitcoin/pull/29078#issuecomment-1860266173)
> Is there an easy way to print all affected/changed/bumped dependencies in the Bitcoin Core build graph?

A quick check is to compare the manifests of master vs the change, i.e:
```bash
diff guix-build-3695ecbf680a/var/profiles/x86_64-linux-gnu/manifest guix-build-fa87a2072b91/var/profiles/x86_64-linux-gnu/manifest
60c60
< "0.67"
---
> "0.68"
62c62
< "/gnu/store/4ck57lp0wdcslrc106c649yfm3fclmsl-moreutils-0.67")
---
> "/gnu/store/09jzmmg2h8cxf75imk0r1c3j0cx20
...

👍 fanquake approved a pull request: "build: Bump guix time-machine to unlock riscv64 metal"
(https://github.com/bitcoin/bitcoin/pull/29078#pullrequestreview-1786733471)
ACK fa87a2072b91c591d2714bc70488b395c22df95d

Guix build (aarch64):
```bash
d137d8dc94b3d734965b7e50fac273f704e122f969430e1b43bfe21c9af31bbe guix-build-fa87a2072b91/output/aarch64-linux-gnu/SHA256SUMS.part
42d66d0daf6067a74c3ba4e55c012712ae5cca02c9123ed3ef7fd9a8f09c16d4 guix-build-fa87a2072b91/output/aarch64-linux-gnu/bitcoin-fa87a2072b91-aarch64-linux-gnu-debug.tar.gz
3ccda6af99c1dd4b54533d073dca82d5f252759cede55a335220065dfe24ee7c guix-build-fa87a2072b91/output/aarch64-linux-gnu/bitco
...

👍 brunoerg approved a pull request: "fuzz: Limit p2p fuzz targets to MAX_PROTOCOL_MESSAGE_LENGTH"
(https://github.com/bitcoin/bitcoin/pull/29079#pullrequestreview-1786739591)
crACK fa769d3e41daec696452b8a0a8753ba511b0a4b5

🤔 stickies-v reviewed a pull request: "Nuke adjusted time (attempt 2)"
(https://github.com/bitcoin/bitcoin/pull/28956#pullrequestreview-1786684919)
Looks like a nice improvement, but still improving my understanding so comments are limited to code review now, and not the concept. It would be helpful if at least some of the commits had a bit more rationale in the description.

💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1430002494)
nit: ["AssertLockNotHeld() can be omitted if LOCK() is called unconditionally after it because LOCK() does the same check as AssertLockNotHeld() internally, for non-recursive mutexes"](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#threads-and-synchronization)

💬 stickies-v commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1429975769)
1. I think the calculation is wrong: the average of the 2 middle elements should be taken in case of an even number of elements.
2. UB: `sorted_copy` being an array of size `N`, I think we'd be sorting uninitialized values if `m_index < N`?
3. nit: the scope of the lock can be reduced

Potential fix addressing all 3:

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

```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 664992dcab..9ade704fe9 100644
--- a/src/
...

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