💬 marcofleon commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2225759355)
I opted for passing in a GenTxid to `FindTxForGetData` and doing the visit there over the templated version for a couple of reasons:

1. It feels like slightly cleaner to me.
2. We need a GenTxid anyway for `m_most_recent_block_txs`.
3. It avoids repeating the branching logic that is already present in `ToGenTxid`.

I’m keeping the functions in `txmempool` Txid/Wtxid, as after this PR there won’t be any GenTxids at all in the current mempool code, which is nice.

> below you are removing
...

💬 marcofleon commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2225760575)
Added.

💬 marcofleon commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2225761745)
Fixed.

💬 apoelstra commented on pull request "[POC] wallet: Enable non-electronic (paper-based) wallet backup with codex32":
(https://github.com/bitcoin/bitcoin/pull/33043#issuecomment-3108925454)
> For encrypted wallets, recovery requires both the codex32 seed and the original passphrase, enhancing the security.

If I understand you correctly, the intent is that an attacker needs the *wallet passphrase* in addition to the seed. But the seed is the seed -- once an attacker has it, it's game over. He doesn't need the passphrase or any other part of the wallet to sweep coins.

codex32 itself does not have any notion of passphrases or encryption.

👍 pinheadmz approved a pull request: "net: Fix Discover() not running when using -bind=0.0.0.0:port"
(https://github.com/bitcoin/bitcoin/pull/32757#pullrequestreview-3047773382)
ACK 1dc6b1e8b04363107b4b44cbf42f8a25f2efabfe

Built and tested on macos/arm64. Reviewed code, minimal changes since last ACK, addressing clean-ups in the test. Tested on mainnet and compared against master to check that local address was discovered when `-bind=0.0.0.0`

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 1dc6b1e8b04363107b4b44cbf42f8a25f2efabfe
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyTo
...

💬 glozow commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3108978032)
> Perhaps there should be a (continuously updated) BIP of transaction standardness rules? It can track which clients and versions support them. And list which ones are configurable in clients. And finally which clients have different defaults.

I think there are a lot of cases where a document would be helpful but BIP editors have been highly resistant to it, hence doc/policy.

🤔 marcofleon reviewed a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32969#pullrequestreview-3047900115)
ACK a9a71b840d729332d940ebdf2cba6b53b82aa792

Built and ran tests on 28.x branch locally. Skipped `--legacy-wallet` tests so lgtm 😌

📝 fanquake opened a pull request: "depends: disable builtin rules and suffixes."
(https://github.com/bitcoin/bitcoin/pull/33045)
This picks up #22126. Previously, this was more complicated to do, as depends packages (upnp, natpmp) used the rules being disabled. Those packages have since been removed, and we should be able to use the flag directly.

When there is no rule to build a target in the makefile, make looks for a builtin rule. When `-r` is specified make no longer performs this lookup.

E.g. the following in an excerpt from `make -d` output. Here, make looks for a rule to build `all`.
```bash
Considering tar
...

💬 fanquake commented on pull request "build: Disable make builtin rules.":
(https://github.com/bitcoin/bitcoin/pull/22126#issuecomment-3109111783)
Picked up in #33045.

💬 enirox001 commented on pull request "test: Do not pass tests on unhandled exceptions":
(https://github.com/bitcoin/bitcoin/pull/33001#issuecomment-3109152663)
Looks good to me—ACK faa3e68

I verified both versions: before the patch the test passed erroneously, but with your changes it now fails as intended. This will be invaluable for debugging by ensuring unhandled exceptions surface as test failures.

💬 l0rinc commented on pull request "refactor: inline constant return values from `dbwrapper` write methods":
(https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3109173946)
We could go the other way as well, enabling writes to only return a boolean and handle them everywhere.
I'm open to suggestions.

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2226003081)
This is a good point, if `coin_control.m_version` is not set and the preselected inputs have different versions, then an error would not be thrown. I have changed `m_version` so that it has a default value of `CTransaction::CURRENT_VERSION` instead of being optional.

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2226003455)
Thanks, I've updated the comment

💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2226004291)
I've added a test for this.

🚀 fanquake merged a pull request: "test: Do not pass tests on unhandled exceptions"
(https://github.com/bitcoin/bitcoin/pull/33001)

🚀 fanquake merged a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32969)

💬 w0xlt commented on pull request "[POC] wallet: Enable non-electronic (paper-based) wallet backup with codex32":
(https://github.com/bitcoin/bitcoin/pull/33043#issuecomment-3109229857)
Yes, you're right. I thought the passphrase deterministically modified the seed to create the BIP 32 Extended Key, but that's not the case.
https://github.com/bitcoin/bitcoin/blob/73e754bd01b0653d1fda2d947fcaed0742da81c3/src/wallet/wallet.cpp#L3542

So the passphrase doesn't provide any additional security in this case.

📝 w0xlt converted_to_draft a pull request: "[POC] wallet: Enable non-electronic (paper-based) wallet backup with codex32"
(https://github.com/bitcoin/bitcoin/pull/33043)
This PR introduces support for exporting and restoring wallet seeds using the [codex32](https://github.com/BlockstreamResearch/codex32) format, enabling non-electronic (paper-based) wallet backups.

To accomplish this, the patch ports the `codex32.{c,h}` implementation from Core Lightning to C++, integrating it with Bitcoin Core's libraries. Corresponding unit tests for codex32 encoding and decoding are also included.

Because Bitcoin Core wallets currently do not store the seed material by
...

📝 fanquake opened a pull request: "[29.x] test: Do not pass tests on unhandled exceptions"
(https://github.com/bitcoin/bitcoin/pull/33046)
Backports #33001 to `29.x`.

💬 fanquake commented on pull request "test: Do not pass tests on unhandled exceptions":
(https://github.com/bitcoin/bitcoin/pull/33001#issuecomment-3109248309)
Backported to `29.x` in #33046.

👍 pablomartin4btc approved a pull request: "[29.x] test: Do not pass tests on unhandled exceptions"
(https://github.com/bitcoin/bitcoin/pull/33046#pullrequestreview-3048147546)
ACK 411e15194b3a770ff455d413a0fe2495f0362297