👍 furszy approved a pull request: "wallet: ensure the wallet is unlocked when needed for rescanning"
(https://github.com/bitcoin/bitcoin/pull/26347)
Tested ACK 2e098439

No longer crashes due the deadlock.

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

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

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

💬 1440000bytes commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1438807616)
> Most users won't know what "signet" means, and it isn't self-explanatory like "testnet"...

Most users wont use it and it just helps devs and power users on windows. They know what signet means. Everyone else clicking on it is same as downloading a malware instead of bitcoin core which is not something this repo needs to solve. First match in search is always "bitcoin core (mainnet)" and even that would open other network if bitcoin.conf has other network set.

So this NACK makes no sense
...

📝 roconnor-blockstream opened a pull request: "Raise PRNG seed log to INFO."
(https://github.com/bitcoin/bitcoin/pull/27137)
Some build infrastructure, such as Nix, will delete failed builds by default, keeping only the log of the failed build.

For flaky tests, it would be very helpful to have the PRNG seed in the default log in order to redo the failed test.

By simply raising the PRNG seed logging to INFO, we can, by default, record the seed in the log of every build.

💬 achow101 commented on pull request "github: Switch to yaml issue templates":
(https://github.com/bitcoin/bitcoin/pull/27025#discussion_r1113341899)
It looks like it's different depending on your repo permissions.

For me, the big green button says "View Policy" and clicking on it goes to https://github.com/willcl-ark/bitcoin/security/policy. For a repo I own/admin/maintain, it shows the page similar to the one in your screenshot.

💬 ajtowns commented on pull request "doc: clarify that LOCK() does AssertLockNotHeld() internally":
(https://github.com/bitcoin/bitcoin/pull/27116#discussion_r1113346615)
If you're moving locks around (`<condition>` changes), then you should be reviewing whether `AssertLockNotHeld` is still appropriate either way?

💬 brunoerg commented on pull request "docs: add ramdisk guide for running tests on OSX":
(https://github.com/bitcoin/bitcoin/pull/27124#issuecomment-1438840188)
Tested these steps on my macOS machine and it worked as expected:
```sh
➜ bitcoin-core-dev git:(master) ✗ diskutil erasevolume HFS+ ramdisk $(hdiutil attach -nomount ram://8388608)
Started erase on disk5
Unmounting disk
Erasing
Initialized /dev/rdisk5 as a 4 GB case-insensitive HFS Plus volume
Mounting disk
Finished erase on disk5 (ramdisk)

➜ bitcoin-core-dev git:(master) ✗ ./test/functional/test_runner.py --cachedir=/Volumes/ramdisk/cache --tmpdir=/Volumes/ramdisk/tmp
Temporary te
...

💬 brunoerg commented on pull request "docs: add ramdisk guide for running tests on OSX":
(https://github.com/bitcoin/bitcoin/pull/27124#discussion_r1113366560)
doubt:
```
To create a 4 GiB RAM disk named "ramdisk" at `/Volumes/ramdisk/`:
```

Does it create a RAM disk named "ramdisk" at `/Volumes/`, resulting in `/Volumes/ramdisk/` or it creates a RAM disk named "ramdisk" into `/Volumes/ramdisk/`?

💬 kristapsk commented on pull request "docs: add ramdisk guide for running tests on OSX":
(https://github.com/bitcoin/bitcoin/pull/27124#issuecomment-1438851708)
Concept ACK

💬 achow101 commented on pull request "contrib: Improve verify-commits.py to work with maintainers leaving":
(https://github.com/bitcoin/bitcoin/pull/27058#discussion_r1113391015)
Looking at the [source](https://github.com/git/git/blob/master/builtin/merge-tree.c), I believe it uses `ort`.

💬 Sjors commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1438875758)
I don't see any harm in adding the word "test". A user who searches "bitcoin" will see the "signet" entry in their search results and wonder what it is, perhaps even worry about it. Without the word "test" the only two ways to figure out are to google "what is signet" or to launch the application.

I don't expect most users to know what "mainnet" is. They obviously know what Bitcoin is, but they may not know that one or more test networks exist.

💬 MarcoFalke commented on pull request "contrib: Improve verify-commits.py to work with maintainers leaving":
(https://github.com/bitcoin/bitcoin/pull/27058#discussion_r1113395319)
So the minimum git version will be 2.38? https://github.com/git/git/commit/1f0c3a29da3515d88537902cd267cc726020eea5

Might be good to test on commit 0cfbb171bd6f61a4edba913e281553b9bdf08d4a

💬 achow101 commented on issue "signrawtransactionwithkey command shouldn't output the "Witness program was passed an empty witness" error for a TapRoot transaction":
(https://github.com/bitcoin/bitcoin/issues/27017#issuecomment-1438882233)
`signrawtransactionwithkey` does not currently support taproot (and it is not clear whether it ever will) as there is not enough information provided with just a private key in order to be able to sign such transactions.

💬 1440000bytes commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1438890512)
> I don't expect most users to know what "mainnet" is. They obviously know what Bitcoin is, but they may not know that one or more test networks exist.

If someone doesnt know what is mainnet they wont use bitcoin core.

📝 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