🤔 fjahr reviewed a pull request: "[Draft/POC] Add secp256k1-based HPKE (Hybrid Public Key Encryption) For Payjoin v2"
(https://github.com/bitcoin/bitcoin/pull/32617#pullrequestreview-3396562602)
Just skimmed a bit to start. Before spending significant time on reviewing this, I think it would be great if this could be cleaned up to the point that it can be taken out of draft status. Potentially also consider splitting it into multiple commits.

💬 fjahr commented on pull request "[Draft/POC] Add secp256k1-based HPKE (Hybrid Public Key Encryption) For Payjoin v2":
(https://github.com/bitcoin/bitcoin/pull/32617#discussion_r2475742295)
There is a 2.5 but no 3?

💬 fjahr commented on pull request "[Draft/POC] Add secp256k1-based HPKE (Hybrid Public Key Encryption) For Payjoin v2":
(https://github.com/bitcoin/bitcoin/pull/32617#discussion_r2475743528)
There are two 2s here

💬 fjahr commented on pull request "[Draft/POC] Add secp256k1-based HPKE (Hybrid Public Key Encryption) For Payjoin v2":
(https://github.com/bitcoin/bitcoin/pull/32617#discussion_r2475734567)
2018?

💬 w0xlt commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2476033021)
Should the height be validated here ?

💬 davidgumberg commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3465392926)
Rebased to address reviewer feedback.

@ajtowns I'm thinking of this now less as a mitigation for the fingerprinting vector, and more just defense-in-depth / closing off attack surface for compact blocks. E.g. [CVE-2024-35202](https://bitcoincore.org/en/2024/10/08/disclose-blocktxn-crash/) and [CVE-2024-52921](https://bitcoincore.org/en/2024/10/08/disclose-mutated-blocks-hindering-propagation/) are more trivial to exploit because you don't need an HB slot.

An HB slot might be a low bar for
...

💬 w0xlt commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2476043001)
Out of curiosity, why return a reference instead of an owned copy? It would avoid lifetime ambiguity

🤔 w0xlt reviewed a pull request: "kernel: Introduce C header API"
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3396973870)
Are the `assert` statements in `src/kernel/bitcoinkernel.cpp` really necessary, or could they be replaced with runtime validation?
Using `assert` makes the project incompatible with some constrained TEEs, such as SGX.

📝 polespinasa opened a pull request: "rpc: [PoC] Optionally print feerates in sat/vb"
(https://github.com/bitcoin/bitcoin/pull/33741)
Part of https://github.com/bitcoin/bitcoin/issues/32093

Returning feerates in BTC/kvB it can be very burdensome and is not good practice, as the most widely adopted units are sat/vB. This PR aims to show a PoC of how we could migrate to sat/vb in a backwards compatible manner.

The RPC affectd by this PR are `getmempoolinfo`, `getnetworkinfo`, `getwalletinfo`, `estimatesmartfee` and `estimaterawfee`.

Because of sub 1sat/vB environment we cannot rely on GetFee() because it internally uses
...

💬 polespinasa commented on issue "Migrate from BTC/kvB to sat/vB on RPC and startup options":
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-3465697269)
https://github.com/bitcoin/bitcoin/pull/33741 implements a PoC on how we can print feerates using sat/vB in a backwards compatible way.

Some similar approach could be used for the arguments, where the user can set feerates in sat/vB and it will be interpreted as it only if he also sets satvb flag to true.

That would create inconsistencies with the approach already taken by `fundrawtransaction`. But being honest I don't like the two arguments approach I think the UX is horrible and it can lead
...

🤔 w0xlt reviewed a pull request: "rpc: Optionally print feerates in sat/vb"
(https://github.com/bitcoin/bitcoin/pull/33741#pullrequestreview-3397062784)
Concept ACK

💬 furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#issuecomment-3465766818)
> Can you run the commits through clang-format-diff.py? There are a bunch of formatting issues in the first two commits.

Pushed.

💬 furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2476143122)
Pushed.

💬 davidgumberg commented on pull request "log: avoid collecting `GetSerializeSize` data when compact block logging is disabled":
(https://github.com/bitcoin/bitcoin/pull/33738#issuecomment-3465768705)
Thanks for catching this, the PR seems correct to me, why is it in draft?

💬 umrashrf commented on issue "Can I compile on OSX Tahoe?":
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3466146355)
I feel a bit stupid as I just did git pull and it brought some new changes.

Anyway I am still struggling to build successfully. @l0rinc you gave me a good tip. It's using /usr/local/include/boost/ instead of /opt/homebrew/Cellar/boost/1.89.0/include/boost/

Any idea how can switch boost?

💬 trevarj commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#discussion_r2476471723)
@hebasto This builds correctly with `guix build -f [save below to tmp file]`. The `native-inputs` override solves the issue with recursion.

```scheme
(use-modules
(guix packages)
(guix gexp)
(guix utils)
(guix build-system gnu)
(gnu packages mingw))

(define (override-msvcrt pkg)
(package
(inherit pkg)
(name (string-append (package-name pkg) "-ucrt"))
(arguments
(substitute-keyword-arguments (package-arguments pkg)
((#:configure-flags flags)

...

💬 umrashrf commented on issue "Can I compile on OSX Tahoe?":
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3466231229)
Oh looks like I made some progress by using:

```
rm -rf build
cmake -B build \
-DCMAKE_C_COMPILER=/opt/homebrew/opt/llvm/bin/clang \
-DCMAKE_CXX_COMPILER=/opt/homebrew/opt/llvm/bin/clang++ \
-DENABLE_IPC=OFF \
-DBUILD_GUI=OFF \
-DWITH_QRENCODE=OFF \
-DENABLE_WALLET=OFF
```

configure logs: https://gist.github.com/umrashrf/dabd3c476da9fa727add2db1522bf3c7

It failed but with a different error.

Build logs: https://gist.github.com/umrashrf/919fbed81177e5829b16bddea6220aae

✅ umrashrf closed an issue: "Can I compile on OSX Tahoe?"
(https://github.com/bitcoin/bitcoin/issues/33733)

💬 umrashrf commented on issue "Can I compile on OSX Tahoe?":
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3466251180)
Doing the following fixed the issue!

```
export LDFLAGS="-L$(brew --prefix llvm)/lib/c++"
export CXXFLAGS="-I$(brew --prefix llvm)/include"
```

Build success 100%

💬 maflcko commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3466299136)
```
# uname --machine && find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
riscv64
12cce8584ce64a7e9064e62ca7d1674e59edb8ebfaada9745074f3b873307472 guix-build-a99bbec14500/output/aarch64-linux-gnu/SHA256SUMS.part
c664f6e1c6fc89f822e81c1307ce278e6f53becae398e97b6b708ca2f8b93115 guix-build-a99bbec14500/output/aarch64-linux-gnu/bitcoin-a99bbec14500-aarch64-linux-gnu-debug.tar.gz
799d4a62daec4fece5c408875ee8fbffdf0856426e84b6
...

💬 TheCharlatan commented on pull request "log: avoid collecting `GetSerializeSize` data when compact block logging is disabled":
(https://github.com/bitcoin/bitcoin/pull/33738#issuecomment-3466336991)
Concept ACK