π andrewtoth opened a pull request: "contrib: add xor-blocks tool to obfuscate blocks directory"
(https://github.com/bitcoin/bitcoin/pull/32451)
I wrote a tool in Rust to xor the blocks directory with a random key. It was pointed out to me that there already exists some Rust code in contrib, so this might be a welcome addition to the toolkit here.
This lets you obfuscate the blocks blk.dat and rev.dat files if you synced with a version prior to v28.
It checks if a `xor.dat` file exists, and if it is zero it overwrites it with a non-zero random key. It then goes through each `*.dat` file in the blocks directory, checking if the firs
...
(https://github.com/bitcoin/bitcoin/pull/32451)
I wrote a tool in Rust to xor the blocks directory with a random key. It was pointed out to me that there already exists some Rust code in contrib, so this might be a welcome addition to the toolkit here.
This lets you obfuscate the blocks blk.dat and rev.dat files if you synced with a version prior to v28.
It checks if a `xor.dat` file exists, and if it is zero it overwrites it with a non-zero random key. It then goes through each `*.dat` file in the blocks directory, checking if the firs
...
π¬ ryanofsky commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2080204034)
> `Flush()` is deleted post #28710 as sqlite's `Flush()` is a no-op.
Thanks, makes sense. I had been looking to see if the Flush call was moved in the diff, but of course it's not because it doesn't exist anymore.
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2080204034)
> `Flush()` is deleted post #28710 as sqlite's `Flush()` is a no-op.
Thanks, makes sense. I had been looking to see if the Flush call was moved in the diff, but of course it's not because it doesn't exist anymore.
π¬ davidgumberg commented on pull request "doc: Add troubleshooting note about Guix on SELinux systems":
(https://github.com/bitcoin/bitcoin/pull/32442#discussion_r2080204499)
Fixed, thanks.
(https://github.com/bitcoin/bitcoin/pull/32442#discussion_r2080204499)
Fixed, thanks.
π¬ achow101 commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2080206413)
It is not. That part of the test no longer exists after #28710. I've updated the commit message.
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2080206413)
It is not. That part of the test no longer exists after #28710. I've updated the commit message.
π€ ismaelsadeeq reviewed a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2825834195)
Nice cleanup, although I have one comment on the new log of `stop_node`.
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2825834195)
Nice cleanup, although I have one comment on the new log of `stop_node`.
π¬ ismaelsadeeq commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080170811)
In "qa: Only allow calling TestNode.stop() after connecting" 9b24a403fae4b896ff7705519bd48c877b4e621b
I am not sure whether this new error message is better, I think I will prefer the previous one as there is a case where the test node has start without a cookie file, hence `wait_for_rpc_connection` should not even be called.
And when you call stop_node it will tell you that there is no RPC connection.
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080170811)
In "qa: Only allow calling TestNode.stop() after connecting" 9b24a403fae4b896ff7705519bd48c877b4e621b
I am not sure whether this new error message is better, I think I will prefer the previous one as there is a case where the test node has start without a cookie file, hence `wait_for_rpc_connection` should not even be called.
And when you call stop_node it will tell you that there is no RPC connection.
π¬ ismaelsadeeq commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080196383)
π₯²
```suggestion
subdir = sha1(expected_exception.encode('utf-8')).hexdigest()[:8]
```
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080196383)
π₯²
```suggestion
subdir = sha1(expected_exception.encode('utf-8')).hexdigest()[:8]
```
π¬ ismaelsadeeq commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080132673)
In "qa: Include ignored errors in RPC connection timeout" 6ad21b4c0114029d16d334bf8d437834708a295e
This is an example error after this change
```
AssertionError: [node 0] Unable to connect to bitcoind after 60s (ignored errors: {'missing_credentials': 240}, latest error: ValueError('No RPC credentials'))
```
I think it will better if it will be consistent
e.g
```
AssertionError: [node 0] Unable to connect to bitcoind after 60s (ignored errors: {'missing_credentials': 240}, lat
...
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080132673)
In "qa: Include ignored errors in RPC connection timeout" 6ad21b4c0114029d16d334bf8d437834708a295e
This is an example error after this change
```
AssertionError: [node 0] Unable to connect to bitcoind after 60s (ignored errors: {'missing_credentials': 240}, latest error: ValueError('No RPC credentials'))
```
I think it will better if it will be consistent
e.g
```
AssertionError: [node 0] Unable to connect to bitcoind after 60s (ignored errors: {'missing_credentials': 240}, lat
...
π¬ achow101 commented on pull request "contrib: remove bdb exception from FORTIFY check":
(https://github.com/bitcoin/bitcoin/pull/32448#issuecomment-2863864209)
ACK f9dfe8d5e0d3f628659702ab781b7919505c829f
(https://github.com/bitcoin/bitcoin/pull/32448#issuecomment-2863864209)
ACK f9dfe8d5e0d3f628659702ab781b7919505c829f
π achow101 merged a pull request: "contrib: remove bdb exception from FORTIFY check"
(https://github.com/bitcoin/bitcoin/pull/32448)
(https://github.com/bitcoin/bitcoin/pull/32448)
π¬ theStack commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2863866776)
Concept ACK
Did a quick test of the wrapper on OpenBSD 7.7 for the monolithic binaries, everything worked as expected so far.
I'm wondering if the `util/subprocess.h` module could be of any use here, especially considering the [windows-specific argument quoting code](https://github.com/bitcoin/bitcoin/blob/1b4ddb0c2d106605b731211d71f8cfe00f56596a/src/util/subprocess.h#L175) seems to look very simliar at a first glance?
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2863866776)
Concept ACK
Did a quick test of the wrapper on OpenBSD 7.7 for the monolithic binaries, everything worked as expected so far.
I'm wondering if the `util/subprocess.h` module could be of any use here, especially considering the [windows-specific argument quoting code](https://github.com/bitcoin/bitcoin/blob/1b4ddb0c2d106605b731211d71f8cfe00f56596a/src/util/subprocess.h#L175) seems to look very simliar at a first glance?
π¬ ryanofsky commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2863875857)
re: https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2861121747
> π§ At least one of the CI tasks failed. Task `fuzzer,address,undefined,integer, no depends`: https://github.com/bitcoin/bitcoin/runs/41829354924 LLM reason (β¨ experimental): The CI failure is caused by a dynamic-type-mismatch detected by UndefinedBehaviorSanitizer during fuzz testing.
This seems unrelated to the other CI failures, but it looks like failure is caused by a bug in the fuzz test:
https://github.com/
...
(https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2863875857)
re: https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2861121747
> π§ At least one of the CI tasks failed. Task `fuzzer,address,undefined,integer, no depends`: https://github.com/bitcoin/bitcoin/runs/41829354924 LLM reason (β¨ experimental): The CI failure is caused by a dynamic-type-mismatch detected by UndefinedBehaviorSanitizer during fuzz testing.
This seems unrelated to the other CI failures, but it looks like failure is caused by a bug in the fuzz test:
https://github.com/
...
π¬ achow101 commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2080218220)
In 86f05d81a12851ab68c33c2447a000cce52ee25c "wallet: init, don't error out when loading legacy wallets"
`output` is overwritten here, given the previous line, I assume it's supposed to be appended to?
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2080218220)
In 86f05d81a12851ab68c33c2447a000cce52ee25c "wallet: init, don't error out when loading legacy wallets"
`output` is overwritten here, given the previous line, I assume it's supposed to be appended to?
π¬ furszy commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2080266825)
upps, yeah. Fixed.
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2080266825)
upps, yeah. Fixed.
π¬ Bitcoin3554 commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#issuecomment-2863960189)
alguΓ©m ai ta usando o btc3 versΓ£o 3.0 https://github.com/Bitcoin3554/Bitcoin3.0
(https://github.com/bitcoin/bitcoin/pull/32451#issuecomment-2863960189)
alguΓ©m ai ta usando o btc3 versΓ£o 3.0 https://github.com/Bitcoin3554/Bitcoin3.0
π ismaelsadeeq approved a pull request: "doc: warn that CheckBlock() underestimates sigops"
(https://github.com/bitcoin/bitcoin/pull/31624#pullrequestreview-2826075627)
ACK a04f17a1882407db09b0a07338e12877ac1d9e92
Thanks for taking my suggestion and providing the context.
----
I will prefer if the size check is also clarified in same PR.
Combining the two in same PR might be better to not miss writing the follow-up.
(https://github.com/bitcoin/bitcoin/pull/31624#pullrequestreview-2826075627)
ACK a04f17a1882407db09b0a07338e12877ac1d9e92
Thanks for taking my suggestion and providing the context.
----
I will prefer if the size check is also clarified in same PR.
Combining the two in same PR might be better to not miss writing the follow-up.
π€ musaHaruna reviewed a pull request: "wallet, rpc: Change `OutputType` items from `string` into compile-time constants `string_view`"
(https://github.com/bitcoin/bitcoin/pull/32432#pullrequestreview-2826091815)
Concept ACK [2fee8f4](https://github.com/bitcoin/bitcoin/pull/32432/commits/2fee8f4886e1421099dfb8ba959b9d0c7e4b0aa8)
After reading the comment [#32429](https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076251627)
The PR is addresses the comment very well.
Overall code looks good to me. The additional documentation added to the outputtype.h file help makes the code clearer. A little suggestion which you can feel free to ignore. Since PR is also making updates to the documentat
...
(https://github.com/bitcoin/bitcoin/pull/32432#pullrequestreview-2826091815)
Concept ACK [2fee8f4](https://github.com/bitcoin/bitcoin/pull/32432/commits/2fee8f4886e1421099dfb8ba959b9d0c7e4b0aa8)
After reading the comment [#32429](https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076251627)
The PR is addresses the comment very well.
Overall code looks good to me. The additional documentation added to the outputtype.h file help makes the code clearer. A little suggestion which you can feel free to ignore. Since PR is also making updates to the documentat
...
π€ mzumsande reviewed a pull request: "refactor: validation: mark CheckBlockIndex as const"
(https://github.com/bitcoin/bitcoin/pull/32364#pullrequestreview-2826150227)
Code Review ACK 3e6ac5bf772751c66cdcd015dcb7e6ce4ea2cc77
(https://github.com/bitcoin/bitcoin/pull/32364#pullrequestreview-2826150227)
Code Review ACK 3e6ac5bf772751c66cdcd015dcb7e6ce4ea2cc77
π¬ ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#issuecomment-2864081597)
Rebased to fix merge conflict.
(https://github.com/bitcoin/bitcoin/pull/29278#issuecomment-2864081597)
Rebased to fix merge conflict.
π ryanofsky approved a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2826013832)
Code review ACK 10845cd7cc89fc83b4ee844dc577b391720bba9c. Only changes since last review were adding a new commit tweaking assert_raises_message(), extending the new test to have a self-check, and to pass through all options to child tests instead of a hardcoded list of options. I left some cleanup suggestions below but they are not important.
I think I will go ahead and merge this with 2 current acks and many stale ones, since this had has a lot of heavy review already and it only affects t
...
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2826013832)
Code review ACK 10845cd7cc89fc83b4ee844dc577b391720bba9c. Only changes since last review were adding a new commit tweaking assert_raises_message(), extending the new test to have a self-check, and to pass through all options to child tests instead of a hardcoded list of options. I left some cleanup suggestions below but they are not important.
I think I will go ahead and merge this with 2 current acks and many stale ones, since this had has a lot of heavy review already and it only affects t
...