💬 kristapsk commented on pull request "RPC: Add universal options argument to listtransactions":
(https://github.com/bitcoin/bitcoin/pull/22807#issuecomment-1864433342)
> Are you still working on this?
Not at the moment. Feel free to close. Will open a new PR in future if will get back to this.
(https://github.com/bitcoin/bitcoin/pull/22807#issuecomment-1864433342)
> Are you still working on this?
Not at the moment. Feel free to close. Will open a new PR in future if will get back to this.
💬 mononaut commented on issue "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837":
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1864484504)
I believe I just saw the same issue reproduce on two separate instances running on the same machine:
```
2023-12-20T10:28:46Z UpdateTip: new best=00000000000000000001036517697dfb912067d7f66f311d815a568b2c9dc166 height=822072 version=0x2d338000 log2_work=94.612828 tx=939105193 date='2023-12-20T10:28:26Z' progress=1.000000 cache=119.3MiB(745842txo)
2023-12-20T10:29:02Z New outbound peer connected: version: 70016, blocks=822072, peer=722 (block-relay-only)
2023-12-20T10:31:50Z New outbound pe
...
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1864484504)
I believe I just saw the same issue reproduce on two separate instances running on the same machine:
```
2023-12-20T10:28:46Z UpdateTip: new best=00000000000000000001036517697dfb912067d7f66f311d815a568b2c9dc166 height=822072 version=0x2d338000 log2_work=94.612828 tx=939105193 date='2023-12-20T10:28:26Z' progress=1.000000 cache=119.3MiB(745842txo)
2023-12-20T10:29:02Z New outbound peer connected: version: 70016, blocks=822072, peer=722 (block-relay-only)
2023-12-20T10:31:50Z New outbound pe
...
🤔 furszy reviewed a pull request: "wallet: Have the wallet store the key for automatically generated descriptors"
(https://github.com/bitcoin/bitcoin/pull/26728#pullrequestreview-1787001982)
Finished another round, only small findings.
(https://github.com/bitcoin/bitcoin/pull/26728#pullrequestreview-1787001982)
Finished another round, only small findings.
💬 furszy commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1432698232)
In 8bb7a8b0:
This is moved without an explanation. Could mention the rationale in the commit description.
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1432698232)
In 8bb7a8b0:
This is moved without an explanation. Could mention the rationale in the commit description.
💬 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.
(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.
(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.
(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.
(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.
(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 |
...
(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
```
(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
(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)
(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)?
(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?
(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)
(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`
(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.
(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.
(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`.
(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`.