💬 hebasto commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2916853608)
There seems a silent conflict with https://github.com/bitcoin/bitcoin/pull/32396.
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2916853608)
There seems a silent conflict with https://github.com/bitcoin/bitcoin/pull/32396.
💬 dergoegge commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2112279755)
The return values for `GetHash` and `GetWitnessHash` need to be assigned to gtxid
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2112279755)
The return values for `GetHash` and `GetWitnessHash` need to be assigned to gtxid
💬 theStack commented on pull request "contrib: utxo_to_sqlite.py: add option to store txid/spk as BLOBs":
(https://github.com/bitcoin/bitcoin/pull/32621#issuecomment-2917055444)
> > I think I'd intuitively prefer `raw` for storing them in the hash order, since that's AFAICT the most common txid byte-string serialization
>
> Maybe `raw` and `rawle` then?
`raw`/`rawle` are a decent alternative that I'd slightly prefer over `rawhash`/`raw`, yes! Force-pushed accordingly. I'm fine with both though, so if other reviewers / potential users of that script feel strongly on `rawhash`/`raw`, I'm happy to revert.
> For future consideration `--spk=type` so that it only re
...
(https://github.com/bitcoin/bitcoin/pull/32621#issuecomment-2917055444)
> > I think I'd intuitively prefer `raw` for storing them in the hash order, since that's AFAICT the most common txid byte-string serialization
>
> Maybe `raw` and `rawle` then?
`raw`/`rawle` are a decent alternative that I'd slightly prefer over `rawhash`/`raw`, yes! Force-pushed accordingly. I'm fine with both though, so if other reviewers / potential users of that script feel strongly on `rawhash`/`raw`, I'm happy to revert.
> For future consideration `--spk=type` so that it only re
...
💬 ismaelsadeeq commented on pull request "ipc: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2917116354)
Not sure were to report, so I will drop this here.
While writing a custom ipc-client, I noticed that the IPC will freeze when you shutdown the node while the ipc-client is still running.
How to reproduce
- Apply the following patch to this branch:
<details>
<summary>diff</summary>
```diff
diff --git a/src/bitcoin-mine.cpp b/src/bitcoin-mine.cpp
index e31f666a68e..3b9e1b81964 100644
--- a/src/bitcoin-mine.cpp
+++ b/src/bitcoin-mine.cpp
@@ -16,6 +16,11 @@
#include <logging.h>
...
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2917116354)
Not sure were to report, so I will drop this here.
While writing a custom ipc-client, I noticed that the IPC will freeze when you shutdown the node while the ipc-client is still running.
How to reproduce
- Apply the following patch to this branch:
<details>
<summary>diff</summary>
```diff
diff --git a/src/bitcoin-mine.cpp b/src/bitcoin-mine.cpp
index e31f666a68e..3b9e1b81964 100644
--- a/src/bitcoin-mine.cpp
+++ b/src/bitcoin-mine.cpp
@@ -16,6 +16,11 @@
#include <logging.h>
...
🤔 w0xlt reviewed a pull request: "contrib: utxo_to_sqlite.py: add option to store txid/spk as BLOBs"
(https://github.com/bitcoin/bitcoin/pull/32621#pullrequestreview-2875935564)
reACK https://github.com/bitcoin/bitcoin/pull/32621/commits/1b10e882f3478d82095aa194f5941e02fb4a68a8
(https://github.com/bitcoin/bitcoin/pull/32621#pullrequestreview-2875935564)
reACK https://github.com/bitcoin/bitcoin/pull/32621/commits/1b10e882f3478d82095aa194f5941e02fb4a68a8
💬 w0xlt commented on pull request "contrib: utxo_to_sqlite.py: add option to store txid/spk as BLOBs":
(https://github.com/bitcoin/bitcoin/pull/32621#discussion_r2112440482)
Does `rawle` mean `raw little-endian`? If so, it would be better to be explicit in the description.
```suggestion
parser.add_argument('--txid', choices=['hex', 'raw', 'rawle'], default='hex', help='encode txid as hex, raw bytes (sha256 byteorder), or reversed raw bytes (litle-endian)')
```
Although `rev_raw` or something similar would also work fine.
(https://github.com/bitcoin/bitcoin/pull/32621#discussion_r2112440482)
Does `rawle` mean `raw little-endian`? If so, it would be better to be explicit in the description.
```suggestion
parser.add_argument('--txid', choices=['hex', 'raw', 'rawle'], default='hex', help='encode txid as hex, raw bytes (sha256 byteorder), or reversed raw bytes (litle-endian)')
```
Although `rev_raw` or something similar would also work fine.
🤔 w0xlt reviewed a pull request: "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer"
(https://github.com/bitcoin/bitcoin/pull/32606#pullrequestreview-2876056497)
ACK https://github.com/bitcoin/bitcoin/pull/32606/commits/1fc95e080040a9d038a8291eeecf645e0413a5c8
(https://github.com/bitcoin/bitcoin/pull/32606#pullrequestreview-2876056497)
ACK https://github.com/bitcoin/bitcoin/pull/32606/commits/1fc95e080040a9d038a8291eeecf645e0413a5c8
🤔 w0xlt reviewed a pull request: "log: Additional compact block logging"
(https://github.com/bitcoin/bitcoin/pull/32582#pullrequestreview-2876074131)
Approach ACK.
The first item "Adds a message to the beginning of PartiallyDownloadedBlock::InitData() so that that the logs indicate the amount of time it takes to populate a compact block from mempool transactions." could be improved by adding another message to the end of `PartiallyDownloadedBlock::InitData` indicating that execution has completed and how long it took.
(https://github.com/bitcoin/bitcoin/pull/32582#pullrequestreview-2876074131)
Approach ACK.
The first item "Adds a message to the beginning of PartiallyDownloadedBlock::InitData() so that that the logs indicate the amount of time it takes to populate a compact block from mempool transactions." could be improved by adding another message to the end of `PartiallyDownloadedBlock::InitData` indicating that execution has completed and how long it took.
💬 w0xlt commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#discussion_r2112525323)
Is this variable being used ?
(https://github.com/bitcoin/bitcoin/pull/32582#discussion_r2112525323)
Is this variable being used ?
💬 davidgumberg commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#discussion_r2112533752)
Thanks for catching this, it's printed below in the added `LogDebug()` message but I forgot to increment this.
(https://github.com/bitcoin/bitcoin/pull/32582#discussion_r2112533752)
Thanks for catching this, it's printed below in the added `LogDebug()` message but I forgot to increment this.
💬 davidgumberg commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#issuecomment-2917296094)
Rebased to address @w0xlt feedback.
(https://github.com/bitcoin/bitcoin/pull/32582#issuecomment-2917296094)
Rebased to address @w0xlt feedback.
🤔 w0xlt reviewed a pull request: "log: Additional compact block logging"
(https://github.com/bitcoin/bitcoin/pull/32582#pullrequestreview-2876111140)
ACK https://github.com/bitcoin/bitcoin/pull/32582/commits/2ff3a05fda11563fed303ef7a171f8edaeac111a
(https://github.com/bitcoin/bitcoin/pull/32582#pullrequestreview-2876111140)
ACK https://github.com/bitcoin/bitcoin/pull/32582/commits/2ff3a05fda11563fed303ef7a171f8edaeac111a
💬 instagibbs commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#discussion_r2112559081)
nit
```suggestion
unsigned int tx_requested_size = 0;
```
(https://github.com/bitcoin/bitcoin/pull/32582#discussion_r2112559081)
nit
```suggestion
unsigned int tx_requested_size = 0;
```
👍 instagibbs approved a pull request: "log: Additional compact block logging"
(https://github.com/bitcoin/bitcoin/pull/32582#pullrequestreview-2876135402)
ACK 2ff3a05fda11563fed303ef7a171f8edaeac111a
(https://github.com/bitcoin/bitcoin/pull/32582#pullrequestreview-2876135402)
ACK 2ff3a05fda11563fed303ef7a171f8edaeac111a
📝 brunoerg opened a pull request: "wallet: sqlite: there is no need to have exclusive locking when mocking"
(https://github.com/bitcoin/bitcoin/pull/32632)
For in-memory SQLite databases, exclusive locking is generally not necessary because in-memory dbs are private to the connection that created them.
(https://github.com/bitcoin/bitcoin/pull/32632)
For in-memory SQLite databases, exclusive locking is generally not necessary because in-memory dbs are private to the connection that created them.
💬 vicjuma commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#issuecomment-2917340365)
Concept ACK
This separates viewing and spending logic and will be efficient especially if **exporting** it to an _easily_ **importable** format
(https://github.com/bitcoin/bitcoin/pull/32489#issuecomment-2917340365)
Concept ACK
This separates viewing and spending logic and will be efficient especially if **exporting** it to an _easily_ **importable** format
📝 hebasto opened a pull request: "windows: Use predefined `RC_INVOKED` macro instead of custom one"
(https://github.com/bitcoin/bitcoin/pull/32633)
See: https://learn.microsoft.com/en-us/windows/win32/menurc/predefined-macros.
(https://github.com/bitcoin/bitcoin/pull/32633)
See: https://learn.microsoft.com/en-us/windows/win32/menurc/predefined-macros.
💬 strmfos commented on pull request "Replace dead gnome link notificator.cpp":
(https://github.com/bitcoin/bitcoin/pull/32629#discussion_r2112585490)
Agreed, the link doesn't directly help understand the code
Should I remove it?
(https://github.com/bitcoin/bitcoin/pull/32629#discussion_r2112585490)
Agreed, the link doesn't directly help understand the code
Should I remove it?
💬 enirox001 commented on pull request "p2p: protect addnode peers during IBD":
(https://github.com/bitcoin/bitcoin/pull/32051#issuecomment-2917377896)
Concept ACK and Code Review ACK.
I think the PR's increased timeout tolerance for addnode peers is valuable. Given my limited domain knowledge, I'd suggest test coverage on potential silent stalls to ensure blocks are efficiently re-requested
(https://github.com/bitcoin/bitcoin/pull/32051#issuecomment-2917377896)
Concept ACK and Code Review ACK.
I think the PR's increased timeout tolerance for addnode peers is valuable. Given my limited domain knowledge, I'd suggest test coverage on potential silent stalls to ensure blocks are efficiently re-requested
📝 hebasto opened a pull request: "build: Add resource file and manifest to `bitcoin.exe`"
(https://github.com/bitcoin/bitcoin/pull/32634)
This PR is a follow up to https://github.com/bitcoin/bitcoin/pull/31375, which:
1. Adds a resource file for `bitcoin.exe` for consistency with other Windows executables.
2. Adds an application manifest to `bitcoin.exe`, which has been required for release binaries since https://github.com/bitcoin/bitcoin/pull/32396.
(https://github.com/bitcoin/bitcoin/pull/32634)
This PR is a follow up to https://github.com/bitcoin/bitcoin/pull/31375, which:
1. Adds a resource file for `bitcoin.exe` for consistency with other Windows executables.
2. Adds an application manifest to `bitcoin.exe`, which has been required for release binaries since https://github.com/bitcoin/bitcoin/pull/32396.