💬 yancyribbens commented on pull request "FUZZ: Test that BnB finds best solution":
(https://github.com/bitcoin/bitcoin/pull/32894#discussion_r2222321444)
> UTXOs are ordered in descending effective value by BnB, so I don’t think the UTXO with largest value being likely to be generated first is a problem.

I was thinking of testing this more as you would a black box.

👍 maflcko approved a pull request: "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline"
(https://github.com/bitcoin/bitcoin/pull/32279#pullrequestreview-3042653949)
I haven't tested IBD/reindex performance.

review ACK f7d9f4510d537b280808e1e8e203c9445c8ad4df 🍴

<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=
truste
...

💬 maflcko commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2222294318)
nit in 3a9c80f7b4807e9abd9f2da36e8ee479ecf00d19:

`GetPubKey` does not need to be wrapped in `CPubKey{}`. You could just store a `dummy_pubkey` for all test cases to re-use.

👍 theStack approved a pull request: "Update secp256k1 subtree to latest master"
(https://github.com/bitcoin/bitcoin/pull/33036#pullrequestreview-3042764955)
ACK 336b8be37b2260d8e902b93f1761265a0aefa496

🤔 danielabrozzoni reviewed a pull request: "p2p: rename GetAddresses -> GetAddressesUnsafe"
(https://github.com/bitcoin/bitcoin/pull/32994#pullrequestreview-3042695024)
last push (1cb23997033c395d9ecd7bf2f54787b134485f41):
- rename `GetAddressesUncached` -> `GetAddressesUnsafe`
- improve documentation

💬 danielabrozzoni commented on pull request "p2p: rename GetAddresses -> GetAddressesUnsafe":
(https://github.com/bitcoin/bitcoin/pull/32994#discussion_r2222322327)
Fair enough, I think `Uncached` scares whoever knows about the getaddr cache, but if you don't have that context it's not immediately obvious.

I changed it to `GetAddresses`/`GetAddressesUnsafe` :)

💬 danielabrozzoni commented on pull request "p2p: rename GetAddresses -> GetAddressesUnsafe":
(https://github.com/bitcoin/bitcoin/pull/32994#discussion_r2222349382)
This is nice, thank you! I have included it and added you as a co-author of the last commit

💬 darosior commented on pull request "doc: Add release notes for 32521 (MAX_TX_LEGACY_SIGOPS)":
(https://github.com/bitcoin/bitcoin/pull/33037#issuecomment-3102529838)
Ack faa2f3b1afe706c6d44fa1949dd97c9e6df5c9d1

Thank you for taking care of that.

-------- Original Message --------
On 7/22/25 7:24 AM, Sjors Provoost wrote:

> Sjors left a comment [(bitcoin/bitcoin#33037)](https://github.com/bitcoin/bitcoin/pull/33037#issuecomment-3102303178)
>
> ACK [faa2f3b](https://github.com/bitcoin/bitcoin/commit/faa2f3b1afe706c6d44fa1949dd97c9e6df5c9d1)
>
> —
> Reply to this email directly, [view it on GitHub](https://github.com/bitcoin/bitcoin/pull/33037#issuecomment-
...

💬 darosior commented on pull request "[29.x] Backport #32521":
(https://github.com/bitcoin/bitcoin/pull/33013#issuecomment-3102584840)
Added a commit with the suggested release note. While i was there i also changed my nym for my name in the credits.

💬 brunoerg commented on pull request "test: check tx is final when there is no locktime":
(https://github.com/bitcoin/bitcoin/pull/33030#issuecomment-3102631463)
> Makes sense. utACK [7180b82](https://github.com/bitcoin/bitcoin/commit/7180b82420c0f303140a93ca635f5ac806bea77d). Could add a comment explaining this to the test as it may not be immediately obvious to a reader.

Thank you, I just added a comment for this.

💬 maflcko commented on pull request "test: check tx is final when there is no locktime":
(https://github.com/bitcoin/bitcoin/pull/33030#issuecomment-3102678470)
lgtm ACK 065e42976a70738770af256da810ddc1316a4496

👍 darosior approved a pull request: "test: check tx is final when there is no locktime"
(https://github.com/bitcoin/bitcoin/pull/33030#pullrequestreview-3042985591)
utACK 065e42976a70738770af256da810ddc1316a4496

💬 darosior commented on pull request "test: check tx is final when there is no locktime":
(https://github.com/bitcoin/bitcoin/pull/33030#discussion_r2222521762)
nit, don't retouch this for me, but my point with the comment wasn't to describe the line below (anyone can just read block height is 0 and locktime is 0) but to describe *why* them both being 0 is a special case worth exercising, as you did in OP.

💬 naiyoma commented on pull request "test: add test cases to wallet_signer.py":
(https://github.com/bitcoin/bitcoin/pull/33020#discussion_r2222554679)
done https://github.com/bitcoin/bitcoin/commit/38a6293fa89104f3d6f1907d04c2e4c488157307

💬 naiyoma commented on pull request "test: add test cases to wallet_signer.py":
(https://github.com/bitcoin/bitcoin/pull/33020#issuecomment-3102747234)

> For the third commit [74fb47f](https://github.com/bitcoin/bitcoin/commit/74fb47f071451145b550062e382416db9388433d): I suggest deleting the commented out code. It's not doing anything useful for the test, and I can't remember why I wrote it.

@Sjors, thanks for the review.
I have also deleted this part https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_signer.py#L235

💬 Sjors commented on pull request "test: add test cases to wallet_signer.py":
(https://github.com/bitcoin/bitcoin/pull/33020#issuecomment-3102748924)
ACK da318fe53fa954cc3eadd7c39e17eb3da7c3e09e

💬 stickies-v commented on pull request "p2p: rename GetAddresses -> GetAddressesUnsafe":
(https://github.com/bitcoin/bitcoin/pull/32994#issuecomment-3102815266)
re-ACK 1cb23997033c395d9ecd7bf2f54787b134485f41

💬 maflcko commented on pull request "test: functional test for incomplete PSBT with additional signatures required":
(https://github.com/bitcoin/bitcoin/pull/33035#issuecomment-3102843496)
@marshallshen is the code llm generated?

💬 fanquake commented on pull request "Update secp256k1 subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/33036#issuecomment-3102874210)
Guix Build (x86_64 & aarch64):
```bash
483e252d112c73c25ef86b8f9ba2bd2acd27b4209a55dd2b0c819badaa7e5123 guix-build-336b8be37b22/output/aarch64-linux-gnu/SHA256SUMS.part
53c2f3879c967deb4706cdf12bf54798a7ef4d3bf54701b6695abe5cd50c7435 guix-build-336b8be37b22/output/aarch64-linux-gnu/bitcoin-336b8be37b22-aarch64-linux-gnu-debug.tar.gz
818f15221bd362f772b9b7c4e07842dbf6965678eb1dd0c86b8d15031b725731 guix-build-336b8be37b22/output/aarch64-linux-gnu/bitcoin-336b8be37b22-aarch64-linux-gnu.tar.g
...

💬 stickies-v commented on pull request "Update secp256k1 subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/33036#issuecomment-3102989198)
I think the 0.7.0 release has been updated to fix wrong 0.7.1 references, I'm getting a slight diff when pulling the subtree. Current HEAD for https://github.com/bitcoin-core/secp256k1/releases/tag/v0.7.0 is https://github.com/bitcoin-core/secp256k1/commit/a660a4976efe880bae7982ee410b9e0dc59ac983, whereas this PR is using https://github.com/bitcoin-core/secp256k1/commit/b9313c6e1a6082a66b4c75777e18ca4b176fcf9d with [diff](https://github.com/bitcoin-core/secp256k1/compare/b9313c6e1a6082a66b4c7577
...