💬 purpleKarrot commented on pull request "CPack":
(https://github.com/bitcoin/bitcoin/pull/33455#discussion_r2432813653)
Yes, ideally we should not rename the generated file but control how the generated file will be named in the first place.
(https://github.com/bitcoin/bitcoin/pull/33455#discussion_r2432813653)
Yes, ideally we should not rename the generated file but control how the generated file will be named in the first place.
💬 fanquake commented on pull request "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2":
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3406873353)
Guix Build (aarch64)
```bash
3de417ac1dade848d8b1609adf72c0faefcaf5acf03199259aa3aeb83e6a863f guix-build-59c4898994bd/output/aarch64-linux-gnu/SHA256SUMS.part
c6eda45fce2b34940ea53caa2acf0c1436122f89b85c0bc727834360f78a40f5 guix-build-59c4898994bd/output/aarch64-linux-gnu/bitcoin-59c4898994bd-aarch64-linux-gnu-debug.tar.gz
41584667134bfc5c35851d5005692c447d3fc529a310c7868030d6383d8bd840 guix-build-59c4898994bd/output/aarch64-linux-gnu/bitcoin-59c4898994bd-aarch64-linux-gnu.tar.gz
5c6d784
...
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3406873353)
Guix Build (aarch64)
```bash
3de417ac1dade848d8b1609adf72c0faefcaf5acf03199259aa3aeb83e6a863f guix-build-59c4898994bd/output/aarch64-linux-gnu/SHA256SUMS.part
c6eda45fce2b34940ea53caa2acf0c1436122f89b85c0bc727834360f78a40f5 guix-build-59c4898994bd/output/aarch64-linux-gnu/bitcoin-59c4898994bd-aarch64-linux-gnu-debug.tar.gz
41584667134bfc5c35851d5005692c447d3fc529a310c7868030d6383d8bd840 guix-build-59c4898994bd/output/aarch64-linux-gnu/bitcoin-59c4898994bd-aarch64-linux-gnu.tar.gz
5c6d784
...
✅ fanquake closed a pull request: "ci: run s390x job"
(https://github.com/bitcoin/bitcoin/pull/33436)
(https://github.com/bitcoin/bitcoin/pull/33436)
💬 fanquake commented on pull request "ci: run s390x job":
(https://github.com/bitcoin/bitcoin/pull/33436#issuecomment-3406887518)
Agree that this specific job, isn't currently worth adding. Maybe we could add the cross-compile only, to match Guix.
(https://github.com/bitcoin/bitcoin/pull/33436#issuecomment-3406887518)
Agree that this specific job, isn't currently worth adding. Maybe we could add the cross-compile only, to match Guix.
💬 fanquake commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-3406905683)
Ran into this locally (https://github.com/bitcoin/bitcoin/commit/e14451ac87339ed61b8c872f027184a978dd96eb):
```bash
276/277 - p2p_i2p_ports.py failed, Duration: 81 s
stdout:
2025-10-15T14:58:36.321373Z TestFramework (INFO): PRNG seed is: 5343123104196887417
2025-10-15T14:58:36.321958Z TestFramework (INFO): Initializing test directory /ci_container_base/ci/scratch/test_runner/test_runner_₿_🏃_20251015_145347/p2p_i2p_ports_16
2025-10-15T14:58:37.956907Z TestFramework (INFO): Ensure we don't try t
...
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-3406905683)
Ran into this locally (https://github.com/bitcoin/bitcoin/commit/e14451ac87339ed61b8c872f027184a978dd96eb):
```bash
276/277 - p2p_i2p_ports.py failed, Duration: 81 s
stdout:
2025-10-15T14:58:36.321373Z TestFramework (INFO): PRNG seed is: 5343123104196887417
2025-10-15T14:58:36.321958Z TestFramework (INFO): Initializing test directory /ci_container_base/ci/scratch/test_runner/test_runner_₿_🏃_20251015_145347/p2p_i2p_ports_16
2025-10-15T14:58:37.956907Z TestFramework (INFO): Ensure we don't try t
...
💬 fanquake commented on pull request "Update libmultiprocess subtree in 30.x branch":
(https://github.com/bitcoin/bitcoin/pull/33519#issuecomment-3406955326)
> should we wait a bit longer?
For anything in particular? There are backports that are blocked on this, so it seems more useful to pull this now, and unblock them. It can always be pulled again later if there is a reason to do so.
(https://github.com/bitcoin/bitcoin/pull/33519#issuecomment-3406955326)
> should we wait a bit longer?
For anything in particular? There are backports that are blocked on this, so it seems more useful to pull this now, and unblock them. It can always be pulled again later if there is a reason to do so.
💬 pinheadmz commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2432972842)
good catch thanks I'll update this arg with `const` here and in the lambda
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2432972842)
good catch thanks I'll update this arg with `const` here and in the lambda
👋 fanquake's pull request is ready for review: "ci: add Valgrind fuzz"
(https://github.com/bitcoin/bitcoin/pull/33461)
(https://github.com/bitcoin/bitcoin/pull/33461)
💬 furszy commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2432988211)
only const? why not passing the ref too
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2432988211)
only const? why not passing the ref too
💬 pinheadmz commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2433006469)
oops thanks, updating...
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2433006469)
oops thanks, updating...
💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2433196004)
I think I can just drop this first commit entirely. We don't actually care to not set the coins we fetch as dirty.
In the happy path, all these coins will be spent immediately after `ConnectBlock`, so they will be set to dirty anyways.
In the unhappy path where the valid proof-of-work block is found to be invalid, the dirty coins we added will just cause the coins to be overwritten by the same data in the db at the next flush.
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2433196004)
I think I can just drop this first commit entirely. We don't actually care to not set the coins we fetch as dirty.
In the happy path, all these coins will be spent immediately after `ConnectBlock`, so they will be set to dirty anyways.
In the unhappy path where the valid proof-of-work block is found to be invalid, the dirty coins we added will just cause the coins to be overwritten by the same data in the db at the next flush.
💬 hebasto commented on pull request "depends: Switch from multilib to platform-specific toolchains":
(https://github.com/bitcoin/bitcoin/pull/32162#issuecomment-3407265710)
My Guix build:
```
aarch64
d542a4060802978bee153978d6bc08069a1cc4ad27a45f55fcad46ce1988fe98 guix-build-c09c045da4c4/output/aarch64-linux-gnu/SHA256SUMS.part
874f7b024f4b9284fa693e8b8d0209f808d80ed512b4f28706774cd04c3f6565 guix-build-c09c045da4c4/output/aarch64-linux-gnu/bitcoin-c09c045da4c4-aarch64-linux-gnu-debug.tar.gz
b371efca582cab23f182eea3c68bd264e3c720899284bf4689f7743b42e09bf2 guix-build-c09c045da4c4/output/aarch64-linux-gnu/bitcoin-c09c045da4c4-aarch64-linux-gnu.tar.gz
a46ae223
...
(https://github.com/bitcoin/bitcoin/pull/32162#issuecomment-3407265710)
My Guix build:
```
aarch64
d542a4060802978bee153978d6bc08069a1cc4ad27a45f55fcad46ce1988fe98 guix-build-c09c045da4c4/output/aarch64-linux-gnu/SHA256SUMS.part
874f7b024f4b9284fa693e8b8d0209f808d80ed512b4f28706774cd04c3f6565 guix-build-c09c045da4c4/output/aarch64-linux-gnu/bitcoin-c09c045da4c4-aarch64-linux-gnu-debug.tar.gz
b371efca582cab23f182eea3c68bd264e3c720899284bf4689f7743b42e09bf2 guix-build-c09c045da4c4/output/aarch64-linux-gnu/bitcoin-c09c045da4c4-aarch64-linux-gnu.tar.gz
a46ae223
...
💬 maflcko commented on pull request "ci: re-add Valgrind job to the CI":
(https://github.com/bitcoin/bitcoin/pull/33411#discussion_r2433256383)
Excluding a test that is expected to pass under valgrind (without even documenting why it is excluded) also makes the test less useful when run locally, assuming that it covers all tests
(https://github.com/bitcoin/bitcoin/pull/33411#discussion_r2433256383)
Excluding a test that is expected to pass under valgrind (without even documenting why it is excluded) also makes the test less useful when run locally, assuming that it covers all tests
💬 fanquake commented on pull request "ci: re-add Valgrind job to the CI":
(https://github.com/bitcoin/bitcoin/pull/33411#discussion_r2433277209)
Happy to document that it is excluded because it is very slow compared to all other tests. Also happy to re-enable it, if we are fine to add ~13 minutes of runtime to this job, for a single slow test (it'd be about on par with ASAN at that point, so I guess still reasonable).
(https://github.com/bitcoin/bitcoin/pull/33411#discussion_r2433277209)
Happy to document that it is excluded because it is very slow compared to all other tests. Also happy to re-enable it, if we are fine to add ~13 minutes of runtime to this job, for a single slow test (it'd be about on par with ASAN at that point, so I guess still reasonable).
💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2433141523)
Is there a reason we are checking for a null hash and return `nullptr` instead of an assertion? I see the c++ wrapper is throwing a runtime error in case of a `nullptr` return value for this (through the `Handle` constructor).
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2433141523)
Is there a reason we are checking for a null hash and return `nullptr` instead of an assertion? I see the c++ wrapper is throwing a runtime error in case of a `nullptr` return value for this (through the `Handle` constructor).
💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2433069372)
Here "the chain" might suggest the active chain, but the function appears to work regardless - it simply returns the parent block tree entry via the `pprev` pointer, even if the block is no longer in the active chain (assuming block tree entries remain valid for the chainstate manager's lifetime).
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2433069372)
Here "the chain" might suggest the active chain, but the function appears to work regardless - it simply returns the parent block tree entry via the `pprev` pointer, even if the block is no longer in the active chain (assuming block tree entries remain valid for the chainstate manager's lifetime).
💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2433243223)
Since `CBlockIndex` already has a calculated hash, can we return a non-owned `btck_BlockHash` here? (like `btck_transaction_get_txid()` for example)
A related question: why `GetBlockHash()` method in `CBlockIndex` returns a copied hash (unlike `GetHash()` in `CTransaction`)?
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2433243223)
Since `CBlockIndex` already has a calculated hash, can we return a non-owned `btck_BlockHash` here? (like `btck_transaction_get_txid()` for example)
A related question: why `GetBlockHash()` method in `CBlockIndex` returns a copied hash (unlike `GetHash()` in `CTransaction`)?
💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2433161387)
```suggestion
typedef void (*btck_ValidationInterfacePowValidBlock)(void* user_data, btck_Block* block, const btck_BlockTreeEntry* entry);
typedef void (*btck_ValidationInterfaceBlockConnected)(void* user_data, btck_Block* block, const btck_BlockTreeEntry* entry);
typedef void (*btck_ValidationInterfaceBlockDisconnected)(void* user_data, btck_Block* block, const btck_BlockTreeEntry* entry);
```
nit: for consistency.
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2433161387)
```suggestion
typedef void (*btck_ValidationInterfacePowValidBlock)(void* user_data, btck_Block* block, const btck_BlockTreeEntry* entry);
typedef void (*btck_ValidationInterfaceBlockConnected)(void* user_data, btck_Block* block, const btck_BlockTreeEntry* entry);
typedef void (*btck_ValidationInterfaceBlockDisconnected)(void* user_data, btck_Block* block, const btck_BlockTreeEntry* entry);
```
nit: for consistency.
💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2433200559)
```suggestion
* @brief Returns the block height where the transaction that created this coin was included in the active chain.
```
nit
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2433200559)
```suggestion
* @brief Returns the block height where the transaction that created this coin was included in the active chain.
```
nit
💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2433285220)
Good to mention about the lifetime of the returned pointer in the doc; also for `btck_transaction_out_point_get_txid()` (it seems we have been explicit about this for the rest of the returned const ptrs in the api).
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2433285220)
Good to mention about the lifetime of the returned pointer in the doc; also for `btck_transaction_out_point_get_txid()` (it seems we have been explicit about this for the rest of the returned const ptrs in the api).