💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2106654018)
Thanks! Fixed in 1e2b26e4f8.

💬 maflcko commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-2908773615)
review ACK 1e2b26e4f8498a08072104b12759d91ef8b410db 👤

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 1e2b26e4f849
...

💬 maflcko commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2106734127)
nit: Can drop the 100 blocks and just use `TestingSetup`.

💬 maflcko commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#issuecomment-2908845905)
review ACK 8117c16fd7a0a5334fe63efaff94ea4e1d3cf851 🌋

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 8117c16fd7a0
...

💬 maflcko commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2106750204)
It is "just" 20kB, but I think we'd want to avoid adding binary blobs to the repo at this point. At a minimum, it would be good to have a reason to do this.

The alternative of just using a previous release to create it on-demand has many benefits:

* It works on Windows as well
* It doesn't bloat the git history by 20+ kB forever
* It is easier to review and easier to adapt

💬 maflcko commented on pull request "test: fix and augment block tests of invalid_txs":
(https://github.com/bitcoin/bitcoin/pull/32591#issuecomment-2908905046)
review ACK 8fcd6845052354fad80ae7e5feda3f6a2e441e12 🔶

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 8fcd68450523
...

📝 w0xlt opened a pull request: "[Draft/POC] Add secp256k1-based HPKE (Hybrid Public Key Encryption) For Payjoin v2"
(https://github.com/bitcoin/bitcoin/pull/32617)
This PR introduces an implementation of Hybrid Public Key Encryption (HPKE) using a secp256k1-based Diffie-Hellman KEM (per RFC 9180 and "secp256k1-based DHKEM for HPKE" specification). It provides the core cryptographic component needed to enable the Payjoin v2 protocol.

This is an exploratory PR intended to kickstart discussion on adding Payjoin v2 support to Bitcoin Core – feedback and reviews are very welcome.

Payjoin v2 makes use of a protocol called Oblivious HTTP (OHTTP) to strip cl
...

📝 w0xlt converted_to_draft a pull request: "[Draft/POC] Add secp256k1-based HPKE (Hybrid Public Key Encryption) For Payjoin v2"
(https://github.com/bitcoin/bitcoin/pull/32617)
This PR introduces an implementation of Hybrid Public Key Encryption (HPKE) using a secp256k1-based Diffie-Hellman KEM (per RFC 9180 and "secp256k1-based DHKEM for HPKE" specification). It provides the core cryptographic component needed to enable the Payjoin v2 protocol.

This is an exploratory PR intended to kickstart discussion on adding Payjoin v2 support to Bitcoin Core – feedback and reviews are very welcome.

Payjoin v2 makes use of a protocol called Oblivious HTTP (OHTTP) to strip cl
...

✅ rkrux closed a pull request: "doc: update CWallet::SignTransaction doc to mention SIGHASH_DEFAULT"
(https://github.com/bitcoin/bitcoin/pull/32411)

📝 rkrux converted_to_draft a pull request: "rpc, doc: clarify wallet version in getwalletinfo help"
(https://github.com/bitcoin/bitcoin/pull/32603)
I noted in review of a previous PR here: https://github.com/bitcoin/bitcoin/pull/32553#pullrequestreview-2853743852.

Relaying the same information in the RPC help that's present in a code comment: https://github.com/bitcoin/bitcoin/blob/af65fd1a333011137dafd3df9a51704fd319feb4/src/wallet/wallet.h#L809-L810

<!--
*** Please remove the following help text before submitting: ***

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

GUI-related pull requests
...

💬 maflcko commented on pull request "build: add a depends dependency provider":
(https://github.com/bitcoin/bitcoin/pull/32595#discussion_r2106854007)
nit: If we do this, may as well hop to https://packages.debian.org/bookworm/cmake 3.25 and g++-12?

💬 purpleKarrot commented on pull request "build: add a depends dependency provider":
(https://github.com/bitcoin/bitcoin/pull/32595#issuecomment-2908984486)
Thank you for working on a dependency provider!

I think core should provide *two* dependency providers, which will give users *three* different ways to build:

1. Not using a dependency provider: CMake will produce an error when dependencies are not found on the system. This will be the preferred way for distro packagers.
2. A fallback provider: This will use system packages when they are found and fall back to compiling vendored depends. Using this provider, `find_package` will always suc
...

📝 Krellan opened a pull request: "Including exception what() in Runaway dialog box"
(https://github.com/bitcoin-core/gui/pull/876)
If an exception happens, including e.what() in the text passed to the dialog box, so that the user can see it. This is in addition to any text from getWarnings() that might already be present, separated by newlines if so.

This will make it easier for users to find what went wrong and get the help they need, instead of having to tell the user to go back and dig through the debug.log file.

💬 polespinasa commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2909182955)
> I fail to see how showing a value lower than 1.0 on regtest if your tip is stale is "really confusing".

Because normally you don't have a regtest environment running 24/7 mining blocks. You turn on and off the environment for testing. The use of timestamps doesn't make sense, there is no "possible not known" blocks to take into account.

💬 fjahr commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2107034420)
> This all looks good to me, but I am not sure if embedding the database in our functional test suite is a good idea. I don't think it is too much data, but I am not sure how stable it is in terms of file formats. Is this really compatible with all posix systems? Then again it could just be removed once/if it starts becoming unreliable.

> It is "just" 20kB, but I think we'd want to avoid adding binary blobs to the repo at this point. At a minimum, it would be good to have a reason to do this.
...

💬 furszy commented on pull request "wallet: re-activate "AmountWithFeeExceedsBalance" error":
(https://github.com/bitcoin/bitcoin/pull/25269#issuecomment-2909243696)
Updated per feedback. Thanks murchandamus!

💬 furszy commented on pull request "wallet: re-activate "AmountWithFeeExceedsBalance" error":
(https://github.com/bitcoin/bitcoin/pull/25269#discussion_r2107038658)
good catch!, fixed. This also revealed that the optional field `CoinsResult::total_effective_amount` has never being optional (it was being set to 0 during construction).

💬 furszy commented on pull request "wallet: re-activate "AmountWithFeeExceedsBalance" error":
(https://github.com/bitcoin/bitcoin/pull/25269#discussion_r2107039968)
sure, added.

💬 maflcko commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2909269900)
Regtest is only used for testing and heavily mockable, so one should be cautious to put too much meaning into a value that even on mainnet is informational-only at best. I am happy to drop commit faf6304bdfdf228354b4072b72f4c0ef90fdaade, if the test is too confusing. Also note, generally it is best to start a code-related discussion thread in the related piece of code inline. This ensures that discussion is bundled nicely in one thread, and can be resolved/skimmed/reopened independent of the mai
...

💬 maflcko commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2107076197)
> Or maybe you would discard the corruption test and only do the happy path test?

Yeah, I wonder if we can reliably test anything other than the happy path anyway. Generally, once UB happened, there is no reliable way to recover from it. While in practise here, most likely you'll end up with negative values, I don't think this is guaranteed by the C++ standard and probably not something to be able to rely on.

Taking a step back, the size of the index is probably small and it is not too har
...