💬 furszy commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1430231352)
In 2e840bcfc03:

Would be good to cleanup `m_hd_key` when `m_hd_crypted_key` is set and vice versa. Just as a safety measure in case this method is called twice pre and post encryption.

💬 furszy commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1432718349)
In 2e2545c3a:

Missing check for `LoadActiveHDPubKey()` output.

💬 furszy commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1430209041)
in 8bb7a8b06:

nit: as `ACTIVEHDKEY` is unique, we don't need to call `LoadRecords` here. Could call a bare `Read()` or implement a `GetActiveHDKey()` at the `WalletBatch` level.

💬 furszy commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1432692273)
In 8bb7a8b06:

`EXCLUSIVE_LOCKS_REQUIRED` annotation is not required.

💬 furszy commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1432727652)
In ce5495a4b27f:

Need to check `SetActiveHDKey()` output. If it fails, the procedure should fail.

🤔 m3dwards reviewed a pull request: "Enable HW-accelerated implementations of SHA256 for MSVC builds"
(https://github.com/bitcoin/bitcoin/pull/24773#pullrequestreview-1790921033)
Tested e94ae81 on Windows 11 with VS2022. CPU: i7-4870HQ and saw big speed improvement.

On master:

```
$ ./src/bench_bitcoin.exe -filter=SHA256D64.*

| ns/byte | byte/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 11.57 | 86,459,102.90 | 0.0% | 0.01 | `SHA256D64_1024_AVX2 using the 'standard' SHA256 implementation`
| 11.89 | 84,074,406.67 |
...

💬 m3dwards commented on pull request "Enable HW-accelerated implementations of SHA256 for MSVC builds":
(https://github.com/bitcoin/bitcoin/pull/24773#discussion_r1432730913)
NIT: From what I understand we are enabling the special CPU instructions using intrinsics and not inline assembly. This makes the comment a little misleading and makes the symbol unfortunately named.

Perhaps the symbol could be?

```suggestion
/* Define this symbol to enable CPU SHA Extensions */
#define USE_SHA_EXTENTIONS
```

or?

```suggestion
/* Define this symbol to enable CPU Extensions */
#define USE_INTRINSICS
```

📝 BrandonOdiwuor opened a pull request: "test: Add test case for speding bare multisig"
(https://github.com/bitcoin/bitcoin/pull/29120)
Fixes https://github.com/bitcoin/bitcoin/issues/29113

✅ maflcko closed a pull request: "RPC: Add universal options argument to listtransactions"
(https://github.com/bitcoin/bitcoin/pull/22807)

💬 Sjors commented on issue "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837":
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1864535003)
@mononaut do you know if these nodes were connected via p2p encryption (BIP324)?

💬 maflcko commented on pull request "test: Add test case for speding bare multisig":
(https://github.com/bitcoin/bitcoin/pull/29120#discussion_r1432759454)
why re-assign?

👋 ajtowns's pull request is ready for review: "versionbits refactoring"
(https://github.com/bitcoin/bitcoin/pull/29039)

📝 shuoer86 opened a pull request: "test: fix typo"
(https://github.com/bitcoin/bitcoin/pull/29121)
fix a typo: `invald wallet` -> `invalid wallet`

💬 c0deright commented on issue "Old wallet.dat: Error reading wallet database: keymeta found with unexpected path / All keys read correctly, but transaction data or address metadata may be missing or incorrect":
(https://github.com/bitcoin/bitcoin/issues/29109#issuecomment-1864618020)
Let me know if I can help with debugging this any further.

🤔 furszy reviewed a pull request: "sqlite: Disallow writing from multiple `SQLiteBatch`s"
(https://github.com/bitcoin/bitcoin/pull/29112#pullrequestreview-1790997039)
Found possible deadlock causes and made a test to exercise one of them: https://github.com/furszy/bitcoin-core/commit/20344b91a03259eb33652863700562f5ef198128. Feel free to pull it here.

💬 furszy commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#discussion_r1432778465)
In 4d846b7b448:

This will produce a deadlock.
if the statement execution fails, the semaphore will remain taken forever. Blocking all future statements execution.
Same will occur on `ExecStatement`.

💬 furszy commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#discussion_r1432833047)
This can also cause a deadlock when the statement execution fails.

The semaphore is not released after a `TxnBegin` failure which will block forever if the software continues and executes a single write or another `TxnBegin` attempt. Which could happen as `TxnBegin` failure could be temporary.

💬 furszy commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#discussion_r1432814176)
Need to check if `TxnBegin()` was already called on this object prior to acquiring the semaphore. Otherwise two `TxnBegin()` calls will cause a deadblock. Can throw a logic exception error if it happens.

💬 josibake commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1432851243)
We are using either `subtract_fee_from_outputs` or `subtractFeeFromOutputs`, depending on which one is present. I think handling that logic in the `ParseOutputs` function is better. Since the options object is passed by reference there isn't any performance difference in passing a reference to the entire options object vs passing a single key.

💬 josibake commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1432851596)
Good catch!