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

💬 fanquake commented on pull request "Update secp256k1 subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/33036#issuecomment-3103000113)
> whereas this PR is using https://github.com/bitcoin-core/secp256k1/commit/b9313c6e1a6082a66b4c75777e18ca4b176fcf9d

Yes that is what I say in the PR description.

💬 murchandamus commented on pull request "FUZZ: Test that BnB finds best solution":
(https://github.com/bitcoin/bitcoin/pull/32894#discussion_r2222739458)
Isn’t that sufficient to demonstrate that it is doing that?

💬 marshallshen commented on pull request "test: functional test for incomplete PSBT with additional signatures required":
(https://github.com/bitcoin/bitcoin/pull/33035#issuecomment-3103097218)
@kevkevinpal - thanks! I will use it to ensure it does increase the test coverage.
@maflcko - I used LLM to help me understand the codebase and write parts of the test, my workflow is:
- understand structure of the TestFramework
- have a goal of what to test (in this case, PSBT is incomplete if some signature is missing)
- reusing the existing test artifacts, and try to write the test to match my goal above

Please let me know if there's anything I am missing as I'm still new to the codeba
...

👍 stickies-v approved a pull request: "Update secp256k1 subtree to latest master"
(https://github.com/bitcoin/bitcoin/pull/33036#pullrequestreview-3043431053)
> Yes that is what I say in the PR description.

Sorry, the link to the 0.7.0 release notes led me to believe the intention was to upgrade to the latest release instead of master, but I now understand (and as indicated in the PR title) that's not the usual update process used for secp256k1.

---

ACK 336b8be37b2260d8e902b93f1761265a0aefa496

Verified that the subtree pull matches https://github.com/bitcoin-core/secp256k1/commit/b9313c6e1a6082a66b4c75777e18ca4b176fcf9d, and briefly skimme
...

👍 theStack approved a pull request: "test: add test cases to wallet_signer.py"
(https://github.com/bitcoin/bitcoin/pull/33020#pullrequestreview-3043484317)
ACK da318fe53fa954cc3eadd7c39e17eb3da7c3e09e

nit: could update the PR title to also indicate that also (commented out) tests get removed

🤔 marcofleon reviewed a pull request: "[29.x] Backport #32521"
(https://github.com/bitcoin/bitcoin/pull/33013#pullrequestreview-3043558663)
ACK 716b4362c0b93dc666464088ed81742c4abf7732

Made sure the changes here match https://github.com/bitcoin/bitcoin/pull/32521. All unit and functional tests pass on 29.x.

💬 enirox001 commented on pull request "test: check tx is final when there is no locktime":
(https://github.com/bitcoin/bitcoin/pull/33030#issuecomment-3103441542)
ACK 065e429: Valuable test case that explicitly demonstrates `IsFinalTx` behavior when nLockTime is 0, including at genesis block height.

💬 Sjors commented on pull request "Update secp256k1 subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/33036#issuecomment-3103533914)
We have in the past updated to release tags, see e.g. #31216. Though that seems mostly a matter of how you phrase the commit title.