💬 fanquake commented on pull request "cmake: Proactively avoid use of `SECP256K1_DISABLE_SHARED`":
(https://github.com/bitcoin/bitcoin/pull/33101#issuecomment-3136918668)
cc @theuni @purpleKarrot

👋 ishaanam's pull request is ready for review: "wallet, rpc: add v3 transaction creation and wallet support"
(https://github.com/bitcoin/bitcoin/pull/32896)

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#issuecomment-3136976112)
I have added more tests and this PR is ready for review.

👍 willcl-ark approved a pull request: "ci: Only pass documented env vars"
(https://github.com/bitcoin/bitcoin/pull/33002#pullrequestreview-3072361931)
ACK 3333d3f75f8917cf8c183984e9b81e2d7a447ca5

Seems reasonable to only export documented vars into the container.

I tried to think if there was a "cleaner" way to do this without creating a new file, but couldn't think of anything so maintainable as this simple python.

💬 maflcko commented on pull request "ci: allow for any libc++ intrumentation & use it for TSAN":
(https://github.com/bitcoin/bitcoin/pull/33099#issuecomment-3137022944)
Looks like the flags are correctly picked up: https://cirrus-ci.com/task/6536414633918464?logs=ci#L4866

`[10:46:11.059] C++ compiler flags .................... -fsanitize=thread -nostdinc++ -nostdlib++ -isystem /cxx_build/include/c++/v1 -L/cxx_build/lib -Wl,-rpath,/cxx_build/lib -lc++ -lc++abi -lpthread -Wno-unused-command-line-argument -O2 -g -std=c++20 -fPIC -fdebug-prefix-map=/ci_container_base/src=. -fmacro-prefix-map=/ci_container_base/src=. -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -Wstack-
...

💬 achow101 commented on issue "ci: failure in wallet_migration.py":
(https://github.com/bitcoin/bitcoin/issues/33096#issuecomment-3137168459)
I should have a fix in a few minutes.

📝 achow101 opened a pull request: "test: Use the same mocktime when migrating in wallet_migration.py"
(https://github.com/bitcoin/bitcoin/pull/33104)
In some test cases, the caller of `migrate_and_get_rpc` needs to know the mocktime that `migrate_and_get_rpc` will be using, otherwise checks for the timestamp in the backup filename may fail if the test is slow. We can ensure the mocktime is known in such cases by allowing the mocktime to be passed into `migrate_and_get_rpc`.

The CI failure can be reproduced locally by adding a sleep to `migrate_and_get_rpc`:
```diff
diff --git a/test/functional/wallet_migration.py b/test/functional/wallet
...

💬 achow101 commented on issue "ci: failure in wallet_migration.py":
(https://github.com/bitcoin/bitcoin/issues/33096#issuecomment-3137203812)
#33104

💬 maflcko commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#issuecomment-3137228995)
review ACK 94b39ce73831acc4c94c7f0d1347d5991b27ef0b 🎲

<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 94b39ce73831
...

💬 maflcko commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2243409022)
nit in the second commit: Why remove this comment, but not the "Or ..." continuation below. Better to keep both or none?

👍 theuni approved a pull request: "depends: hard-code necessary c(xx)flags rather than setting them per-host"
(https://github.com/bitcoin/bitcoin/pull/32584#pullrequestreview-3072776246)
ACK 9954d6c833381a44e1ea34d182ffe4d61b65d2ba

💬 benthecarman commented on pull request "BIP-348 (OP_CHECKSIGFROMSTACK) (regtest only)":
(https://github.com/bitcoin/bitcoin/pull/32247#discussion_r2243475748)
nit: this is between EvalChecksigPreTapscript and EvalChecksigTapscript which can be a little confusing/annoying

🚀 glozow merged a pull request: "refactor: GenTxid type safety followups"
(https://github.com/bitcoin/bitcoin/pull/33005)

🤔 glozow reviewed a pull request: "rpc: add optional blockhash to waitfornewblock, unhide wait methods in help"
(https://github.com/bitcoin/bitcoin/pull/30635#pullrequestreview-3072870120)
utACK c6e2c31c55123cc97b4400bcbf3c37a39b067a22

🚀 glozow merged a pull request: "rpc: add optional blockhash to waitfornewblock, unhide wait methods in help"
(https://github.com/bitcoin/bitcoin/pull/30635)

👍 theuni approved a pull request: "cmake: Proactively avoid use of `SECP256K1_DISABLE_SHARED`"
(https://github.com/bitcoin/bitcoin/pull/33101#pullrequestreview-3072902914)
Nice. I've been wanting to scope the secp `add_subdirectory` for a while now.

utACK d5d95035bc6ef135476d7f734c574b38d6faaf70. Reviewed with `git diff --color-moved=dimmed-zebra --color-moved-ws=allow-indentation-change`

💬 glozow commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#issuecomment-3137441579)
> Thank you, fixed

And fixed the one a few lines after too (thanks @marcofleon)

💬 darosior commented on pull request "p2p: never check tx rejections by txid":
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3137446026)
Yes, we could do that today. I suppose the minimal way to do so today on master would be to simply check whether we are spending any Segwit input while the transaction has no witness after `CheckInputScripts` failed:
<details>
<summary>Expand patch</summary>

```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 09e04ff0ddb..8f86b630ef5 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1254,13 +1254,17 @@ bool MemPoolAccept::PolicyScriptChecks(const ATMPArgs& arg
...

💬 glozow commented on pull request "p2p: never check tx rejections by txid":
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3137458271)
> However it seemed cleaner to me to get rid of the WITNESS_STRIPPED edge case detection in the first place, which was always meant to go?

I didn't realize it was always meant to go - how do you know that?

Fwiw, I do think that being able to detect `WITNESS_STRIPPED` without triple validation is the main thing we want, so would prioritize that kind of solution. At the same time, my main point in the OP is that being able to cache `INPUTS_NOT_STANDARD` isn't very helpful to us in practice,
...

💬 theuni commented on pull request "kernel: create monolithic kernel static library":
(https://github.com/bitcoin/bitcoin/pull/33077#issuecomment-3137474121)
> (On the other side, I'm also not sure about the benefits of this PR. PR description links to a lot of issues and might be clearer if it just described the problem it solves, or explained if the main movitation is simplifying cmake code or reducing the number of installed files or something else.)

At a high level, this simply encapsulates the kernel library. The shared lib already works this way, this PR would just align the static lib's behavior. The other dependencies (crypto, leveldb, se
...