📝 jonatack opened a pull request: "rpc, test: remove newline escape sequence from wallet warning fields"
(https://github.com/bitcoin/bitcoin/pull/27138)
This PR extends our test coverage to demonstrate the issue, then removes newline escape sequences in the wallet `warning` field in RPCs `createwallet`, `loadwallet`, and `unloadwallet`.

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

before

```
$ ./src/bitcoin-cli -signet createwallet w1 false false "" false false
{
"name": "w1",
"warning": "Empty string given as passphrase, wallet will not be encrypted.\nWallet created successfully
...

💬 1440000bytes commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1438895432)
Most of the contributors in this repo have no experience about windows including windows servers. Sorry for being rude or straight but its a fact. I love linux but if you support multiple OS which is expected then please understand its different from a user's perspective.

Luke Dashjr will be the last person to ask if I need to add something in core for windows.

I have worked on windows for a long time and love linux. You can ignore my suggestion and NACK. Merge it as other PRs however I kn
...

💬 pinheadmz commented on pull request "docs: add ramdisk guide for running tests on OSX":
(https://github.com/bitcoin/bitcoin/pull/27124#discussion_r1113408160)
@brunoerg

Not sure I understand. The volume is named "ramdisk" and is mounted at `/Volumes/ramdisk`. The path `/Volumes/ramdisk` does not exist before the command is executed.

💬 roconnor-blockstream commented on pull request "script: BIP341 txdata cannot be precomputed without spent outputs":
(https://github.com/bitcoin/bitcoin/pull/27122#issuecomment-1438911071)
> The immediately following comment seems wrong too, as the branch is gated by a !scriptWitness.IsNull() test?

> > Note that this branch may trigger for scriptPubKeys that aren't actually segwit but in that case validation will fail as SCRIPT_ERR_WITNESS_UNEXPECTED anyway.

I believe this comment is saying it will trigger in the case the the scriptPubKey is OP_1 followed by a series of opcodes of an appropriate length, but is not a single push opcode, and a witness is (illegally) provide
...

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