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

💬 achow101 commented on pull request "contrib: Improve verify-commits.py to work with maintainers leaving":
(https://github.com/bitcoin/bitcoin/pull/27058#discussion_r1113610957)
I think it is reasonable to require that. I'm sure the people who would run this script can figure that out. I'll add a note to the docs.

💬 msgilligan commented on issue "Bech32 address generation in regtest does not have the prefix `tb`":
(https://github.com/bitcoin/bitcoin/issues/12314#issuecomment-1439145203)
Add **bitcoinj** (https://github.com/bitcoinj/bitcoinj) to the list of implementations that use `bcrt`. Please don't change this.

💬 achow101 commented on pull request "Remove MarcoFalke fingerprint, update trusted-git-root":
(https://github.com/bitcoin/bitcoin/pull/27135#issuecomment-1439154380)
ACK fab17f08e24f0db687dc25c5e10eb62293070048

The key removed matches the fingerprint of the key that I have for Marco Falke and is the same that has been used to sign commits.

With the key removed and the new trusted git root, all commits still verify.

⚠️ orenyomtov opened an issue: "Add support for sighash flags in PSBT (like SINGLE|ANYONECANPAY)"
(https://github.com/bitcoin/bitcoin/issues/27141)
**Is your feature request related to a problem? Please describe.**
When using Bitcoin Core's GUI to sign a PSBT that has a UTXO with `sighash=SINGLE|ANYONECANPAY`, it throws following error:
```
Specified sighash value does not match value stored in PSBT
```

**Describe the solution you'd like**
I'd like it to successfully sign PSBTs containing `sighash=SINGLE|ANYONECANPAY`

**Describe alternatives you've considered**
The current non-idea workaround is to use the console window as desc
...

👍 stickies-v approved a pull request: "wallet: SecureString to allow null characters"
(https://github.com/bitcoin/bitcoin/pull/27068)
re-ACK [4bbf5dd](https://github.com/bitcoin/bitcoin/commit/4bbf5ddd44bde15b328be131922123eaa3212a7e)

I verified that the only changes are as per review comments: updated commit title and slight phrasing tweaks to error messages.

💬 1440000bytes commented on issue "Add support for sighash flags in PSBT (like SINGLE|ANYONECANPAY)":
(https://github.com/bitcoin/bitcoin/issues/27141#issuecomment-1439230218)
Concept ACK

However GUI issues and PRs are maintained in https://github.com/bitcoin-core/gui