💬 dergoegge commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#issuecomment-1875575611)
Coverage for `block_index`: https://dergoegge.github.io/bitcoin-coverage/pr28209/fuzz.coverage/index.html

📝 fanquake opened a pull request: "Update libsecp256k1 subtree for 0.4.1 release"
(https://github.com/bitcoin/bitcoin/pull/29169)
Updates our libsecp256k1 subtree for the 0.4.1 release: https://github.com/bitcoin-core/secp256k1/releases/tag/v0.4.1.

> The point multiplication algorithm used for ECDH operations (module ecdh) was replaced with a slightly faster one.

> Optional handwritten x86_64 assembly for field operations was removed because modern C compilers are able to output more efficient assembly. This change results in a significant speedup of some library functions when handwritten x86_64 assembly is enabled
...

💬 sipa commented on pull request "Update libsecp256k1 subtree for 0.4.1 release":
(https://github.com/bitcoin/bitcoin/pull/29169#issuecomment-1875614423)
Obvious ACK c13a17c6996442f04635bdf70ee8f06bf6854ff6 is obvious.

💬 fanquake commented on pull request "Update libsecp256k1 subtree for 0.4.1 release":
(https://github.com/bitcoin/bitcoin/pull/29169#issuecomment-1875616633)
cc @real-or-random @jonasnick

🤔 sr-gi reviewed a pull request: "Nuke adjusted time (attempt 2)"
(https://github.com/bitcoin/bitcoin/pull/28956#pullrequestreview-1800831281)
Concept ACK

💬 sr-gi commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1439751712)
Out of curiosity: are you grouping these two so you can query them together, given you'll need both of them in `getnetworkinfo` if peerman is defined?

💬 sr-gi commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1439717289)
In order to be consistent with how the code used to be what needs updating is the code and not the comment, doesn't it? Otherwise, we will be computing the median as low as with 4 elements when we used to need 5 before.

I don't know what the motivation to require 5 data points previously was (why 4 or 3?), but my impression by reading the comments in this PR is that we are trying no to change anyting with respect to how the median is computed and leave that for a followup

💬 sr-gi commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1439738186)
I think a version of this comment may still be relevant to motivate why we only sample outbound nodes

💬 sr-gi commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1439726269)
Came for this. 😕 indeed

💬 sr-gi commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1439793095)
I was going to suggest doing `sorted_copy[m_index / 2] + sorted_copy[m_index / 2 - 1]) / 2))` before seeing this comment since the current approach feels a bit strange. If we keep this I'd say at least add a comment pointing out this is to prevent overflow

💬 hebasto commented on pull request "Update libsecp256k1 subtree for 0.4.1 release":
(https://github.com/bitcoin/bitcoin/pull/29169#issuecomment-1875642123)
Shouldn't the subtree be synced to the `v0.4.1` tag instead of the current master branch?

💬 fanquake commented on pull request "Update libsecp256k1 subtree for 0.4.1 release":
(https://github.com/bitcoin/bitcoin/pull/29169#issuecomment-1875643011)
> Shouldn't the subtree be synced to the v0.4.1 tag instead of the current master branch?

Why? It makes 0 difference.

💬 fanquake commented on pull request "Update libsecp256k1 subtree for 0.4.1 release":
(https://github.com/bitcoin/bitcoin/pull/29169#issuecomment-1875644508)
I would also prefer not to introduce some policy about only using tags, or similar, because we want to retain the ability to update the subtree to any commit we'd like, at any point.

👍 hebasto approved a pull request: "Update libsecp256k1 subtree for 0.4.1 release"
(https://github.com/bitcoin/bitcoin/pull/29169#pullrequestreview-1802641219)
ACK c13a17c6996442f04635bdf70ee8f06bf6854ff6, I've updated the `secp256k1` to its current master branch (efe85c70a2e357e3605a8901a9662295bae1001f) locally and got zero diff with this PR.

However, the PR title is a bit misleading as it mentions "0.4.1 release", which implies (at least for me) syncing to the `v0.4.1` tag.

👍 TheCharlatan approved a pull request: "doc: Rework guix docs after 1.4 release"
(https://github.com/bitcoin/bitcoin/pull/28962#pullrequestreview-1802673759)
ACK fad444f6e1b1ce5c1572c773db4a9a098c7a0b96

💬 brunoerg commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#issuecomment-1875711592)
Nice one!

📝 fanquake opened a pull request: "contrib: add macho branch protection check"
(https://github.com/bitcoin/bitcoin/pull/29170)
Followup to https://github.com/bitcoin/bitcoin/pull/28459. Add a sanity check that `bti` instructions are present in the arm macho binary, similar to our x86_64 check for control flow.

Could do something similar for aarch64 linux in future, and maybe could use https://github.com/lief-project/LIEF/issues/975.

💬 TheCharlatan commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#issuecomment-1875751255)
Concept ACK

💬 luke-jr commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1365807093)
As `nodeServices` is incorrect after this call, it seems to make more sense to just leave it passing `nLocalServices` here and the other call site?

💬 luke-jr commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1365836836)
Maybe it should be on by default?