💬 roconnor-blockstream commented on pull request "script: BIP341 txdata cannot be precomputed without spent outputs":
(https://github.com/bitcoin/bitcoin/pull/27122#issuecomment-1438917640)
> This was directly addressed in the comments earlier; should that be updated too?

> > This only works if spent_outputs was provided as well, but if it wasn't, actual validation will fail anyway.

I believe this comment is trying to say that (under the implicit circumstances that `force` is false) if the spend outputs are not provided, it will fail to detect if an input is using bip341. But if spent outputs are not provided, then bip341 cannot be validated anyways.

💬 Sjors commented on pull request "contrib: Improve verify-commits.py to work with maintainers leaving":
(https://github.com/bitcoin/bitcoin/pull/27058#discussion_r1113443957)
It seems so. If I remove the Homebrew version of Git (2.39.2) and fall back to Apple's default 2.37.1 the verification script fails on that commit:

```
% contrib/verify-commits/verify-commits.py 0cfbb17
Using verify-commits data from /Users/sjors/dev/bitcoin/contrib/verify-commits
usage: git merge-tree <base-tree> <branch1> <branch2>
Traceback (most recent call last):
File "contrib/verify-commits/verify-commits.py", line 191, in <module>
main()
File "contrib/verify-commits/ver
...

💬 achow101 commented on pull request "script: BIP341 txdata cannot be precomputed without spent outputs":
(https://github.com/bitcoin/bitcoin/pull/27122#issuecomment-1438942314)
ACK 95f12de92505522a32ba58acd5251c69e602d160

🚀 achow101 merged a pull request: "script: BIP341 txdata cannot be precomputed without spent outputs"
(https://github.com/bitcoin/bitcoin/pull/27122)

✅ achow101 closed an issue: "Add EnsureWalletIsUnlocked to rescanblockchain RPC?"
(https://github.com/bitcoin/bitcoin/issues/25702)

🚀 achow101 merged a pull request: "wallet: ensure the wallet is unlocked when needed for rescanning"
(https://github.com/bitcoin/bitcoin/pull/26347)

💬 sipa commented on pull request "fuzz: avoid redundant dup key checks when creating Miniscript nodes":
(https://github.com/bitcoin/bitcoin/pull/27117#issuecomment-1438974152)
ACK c1b7bd047f47dcd3eb6897adfaf9a55594deff5d

Ran fuzzing with miniscript_smart target, see https://github.com/bitcoin-core/qa-assets/pull/105.

👍 stickies-v approved a pull request: "wallet: SecureString to allow null characters"
(https://github.com/bitcoin/bitcoin/pull/27068)
ACK d73721b2be27afd9906ed362e286dd3d0a414413

2 non-blocking nits best to be ignored if no further force pushes are necessary.

---

> But that doesn't actually contain a null character. That contains a backslash followed by a 0, which some things may interpret as a null character, but not always, and not necessarily in a way that will affect us.

Thanks, I didn't realize this. I verified it both on cli and Qt and you're right. I agree that the chances of this actually being a problem a
...

💬 stickies-v commented on pull request "wallet: SecureString to allow null characters":
(https://github.com/bitcoin/bitcoin/pull/27068#discussion_r1113431868)
nit: slight rephrasing for tense consistency and highlighting that it's the old password that has the null character (also (partially) affects the other if-branch as well as the other sites where this message is raised).
```suggestion
tr("The old passphrase entered for the wallet decryption is incorrect. "
"It contains a null character (ie - a zero byte). "
```

💬 pinheadmz commented on pull request "wallet: be able to specify a wallet name and passphrase to migratewallet":
(https://github.com/bitcoin/bitcoin/pull/26595#discussion_r1113492544)
I think you are creating a wallet with the letter "t" in the name, but the path you pass to migratewallet has no "t":

`notloaded2` vs `noloaded2`

I think this isn't causing test failure because `test_unloaded_by_path()` is not added to the `run_test` list ...?

💬 furszy commented on pull request "wallet: be able to specify a wallet name and passphrase to migratewallet":
(https://github.com/bitcoin/bitcoin/pull/26595#discussion_r1113495150)
yeah, the test case is not being called. Nice catch.

⚠️ brunoerg opened an issue: "Add support for all networks in `deserialize_v2` in test_framework"
(https://github.com/bitcoin/bitcoin/issues/27140)
`deserialize_v2` function (`test_framework`) should be able to deseriablize all networks presented in BIP155 (e.g. onion, ipv6, etc), however, it only works for ipv4 and i2p. I noticed that when I was using `message-capture-parser` and realized that most `addrv2` messages weren't able to be deserialized.

💬 brunoerg commented on issue "Add support for all networks in `deserialize_v2` in test_framework":
(https://github.com/bitcoin/bitcoin/issues/27140#issuecomment-1439030594)
ooops, it added `feature` label but i think it should be `bug`

💬 jonatack commented on pull request "rpc, test: remove newline escape sequence from wallet warning fields":
(https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1439055704)
Updated for `restorewallet` as well.

💬 TheCharlatan commented on pull request "rpc, test: remove newline escape sequence from wallet warning fields":
(https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1439056879)
Concept ACK

> I believe only createwallet appends additional warning sentences and is affected by the fix.

I think they all do, check the code in `CWallet::Create`. E.g. by setting weird `mintxfee` and `maxapsfee` values:
```
./bitcoin-cli -signet restorewallet "w2" "w2"
{
"name": "w2",
"warning": "-mintxfee is set very high! This is the minimum transaction fee you pay on every transaction.\n-maxapsfee is set very high! This is the maximum transaction fee you pay (in addition to t
...

💬 john-moffett commented on pull request "wallet: SecureString to allow null characters":
(https://github.com/bitcoin/bitcoin/pull/27068#issuecomment-1439060748)
Thanks @stickies-v! Incorporated both suggestions.

range-diff:

```diff
@@ src/qt/askpassphrasedialog.cpp: void AskPassphraseDialog::accept()
- tr("The passphrase entered for the wallet decryption was incorrect. "
- "The passphrase contains a null character (ie - a zero byte). "
- "If this passphrase was set with a version of this software prior to 25.0,
...

💬 jonatack commented on pull request "rpc, test: remove newline escape sequence from wallet warning fields":
(https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1439069325)
Thanks @TheCharlatan, updated the OP and commit message.

💬 achow101 commented on pull request "wallet: be able to specify a wallet name and passphrase to migratewallet":
(https://github.com/bitcoin/bitcoin/pull/26595#discussion_r1113548649)
Nice catch! Fixed.

👍 john-moffett approved a pull request: "validation: Improve error handling when VerifyDB dosn't finish successfully"
(https://github.com/bitcoin/bitcoin/pull/25574)
ACK 0af16e7134459e0820ab95d751093876c1ec4c6d

💬 theuni commented on issue "`libbitcoinkernel`: Building `mingw-w64` dll's broken":
(https://github.com/bitcoin/bitcoin/issues/25008#issuecomment-1439139628)
@TheCharlatan I just rebased and pushed up a branch with a combo for dll's that seems to work.

Mind testing https://github.com/theuni/bitcoin/commits/fix-dll-exports ? I'll PR if it works for you.

Symbol visibility can be tested by marking a random function as dllexport. When dumped with `objdump -p`, you should see _only_ your selected exported symbols.