π¬ maflcko commented on pull request "contrib: Check build options for gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/32949#issuecomment-3068540607)
Thanks, but I'll close this for now:
* It is unclear how this fixes the issue, given that it about something else. Also, it is unclear what your goal is (try to fix the issue, or something else)
* It is unclear how this is different from `skip_missing_binaries`, which already exists.
* It is unclear how to test this, given that there are no steps to test.
* It is unclear how to review this, given that there is no description of the changes.
Feel free to work on this again, or anything e
...
(https://github.com/bitcoin/bitcoin/pull/32949#issuecomment-3068540607)
Thanks, but I'll close this for now:
* It is unclear how this fixes the issue, given that it about something else. Also, it is unclear what your goal is (try to fix the issue, or something else)
* It is unclear how this is different from `skip_missing_binaries`, which already exists.
* It is unclear how to test this, given that there are no steps to test.
* It is unclear how to review this, given that there is no description of the changes.
Feel free to work on this again, or anything e
...
π fanquake merged a pull request: "fix spelling in tor.md docs"
(https://github.com/bitcoin/bitcoin/pull/32961)
(https://github.com/bitcoin/bitcoin/pull/32961)
β
yuvicc closed a pull request: "[WIP] tracing: lock contention"
(https://github.com/bitcoin/bitcoin/pull/32952)
(https://github.com/bitcoin/bitcoin/pull/32952)
π¬ yuvicc commented on pull request "[WIP] tracing: lock contention":
(https://github.com/bitcoin/bitcoin/pull/32952#issuecomment-3068574786)
> I'm not sure if opening this draft PR at this stage is worth it. There is nothing to experiment with, CI fails, and the
> feedback from the original PR is still a TODO. I'd recommend you finish your research first, come up with something that > works, and then open a PR :)
I think you're right β I opened this draft PR a bit prematurely. I'll take your advice and focus on finishing my research and having something more concrete before opening this PR again.
Closing this for now. Thank
...
(https://github.com/bitcoin/bitcoin/pull/32952#issuecomment-3068574786)
> I'm not sure if opening this draft PR at this stage is worth it. There is nothing to experiment with, CI fails, and the
> feedback from the original PR is still a TODO. I'd recommend you finish your research first, come up with something that > works, and then open a PR :)
I think you're right β I opened this draft PR a bit prematurely. I'll take your advice and focus on finishing my research and having something more concrete before opening this PR again.
Closing this for now. Thank
...
π maflcko opened a pull request: "log: [refactor] Use info level for init logs"
(https://github.com/bitcoin/bitcoin/pull/32967)
Many of the normal, and expected init logs, which are run once after startup use the deprecated alias of `LogInfo`.
Fix that by using `LogInfo` directly, which is a refactor, except for a few log lines that also have `__func__` removed.
(Also remove the unused trailing `\n` char while touching those logs)
(https://github.com/bitcoin/bitcoin/pull/32967)
Many of the normal, and expected init logs, which are run once after startup use the deprecated alias of `LogInfo`.
Fix that by using `LogInfo` directly, which is a refactor, except for a few log lines that also have `__func__` removed.
(Also remove the unused trailing `\n` char while touching those logs)
π¬ stratospher commented on issue "Intermittent failure in rpc_invalidateblock.py assert_equal(self.nodes[0].getblockchaininfo()['headers'], 7) [ AssertionError: not(24 == 7)]":
(https://github.com/bitcoin/bitcoin/issues/32965#issuecomment-3068592237)
> 2025-07-12T13:43:21.8573662Z node1 2025-07-12T13:43:20.090914Z [msghand] [D:\a\bitcoin\bitcoin\src\net.cpp:3870] [void __cdecl CConnman::PushMessage(class CNode *,struct CSerializedNetMsg &&)] [net] sending block (260 bytes) peer=0
2025-07-12T13:43:21.8574419Z node0 2025-07-12T13:43:20.091379Z [msghand] [D:\a\bitcoin\bitcoin\src\net_processing.cpp:5939] [bool __cdecl `anonymous-namespace'::PeerManagerImpl::SendMessages(class CNode *)] [net] Requesting block 72876656a1468aa0784c5624a377651fb
...
(https://github.com/bitcoin/bitcoin/issues/32965#issuecomment-3068592237)
> 2025-07-12T13:43:21.8573662Z node1 2025-07-12T13:43:20.090914Z [msghand] [D:\a\bitcoin\bitcoin\src\net.cpp:3870] [void __cdecl CConnman::PushMessage(class CNode *,struct CSerializedNetMsg &&)] [net] sending block (260 bytes) peer=0
2025-07-12T13:43:21.8574419Z node0 2025-07-12T13:43:20.091379Z [msghand] [D:\a\bitcoin\bitcoin\src\net_processing.cpp:5939] [bool __cdecl `anonymous-namespace'::PeerManagerImpl::SendMessages(class CNode *)] [net] Requesting block 72876656a1468aa0784c5624a377651fb
...
π¬ fanquake commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2204337893)
LogWarning?
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2204337893)
LogWarning?
π¬ fanquake commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2204338168)
LogWarning?
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2204338168)
LogWarning?
π¬ maflcko commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2204346812)
thx, reverted. (I'll do the warn level in a follow-up, so that this is limited to just the info level)
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2204346812)
thx, reverted. (I'll do the warn level in a follow-up, so that this is limited to just the info level)
π stratospher opened a pull request: "test: fix intermittent failure in rpc_invalidateblock.py"
(https://github.com/bitcoin/bitcoin/pull/32968)
resolves #32965.
node1 (with 24 blocks) causes node0 (with 6 blocks + 1 extra header) to silently reorg. so disconnect node0 and node1 to avoid silent reorgs.
(https://github.com/bitcoin/bitcoin/pull/32968)
resolves #32965.
node1 (with 24 blocks) causes node0 (with 6 blocks + 1 extra header) to silently reorg. so disconnect node0 and node1 to avoid silent reorgs.
π¬ purpleKarrot commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3068613759)
NACK
After working with the API for a few days and reviewing the various language bindings listed in the PR summary, I found that the API requires some fundamental changes in order to reduce the amount of glue code required in language bindings and client code. I wrote a rather detailed analysis here: https://njump.me/naddr1qvzqqqr4gupzqrcxrljwdpfz2qn5a57hse6ez6pkd34pe0wpeskmktt2p62yeketqqvxy6t5vdhkjmntv4exuetv94shq6fdwfjhv6t9wuxrjull
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3068613759)
NACK
After working with the API for a few days and reviewing the various language bindings listed in the PR summary, I found that the API requires some fundamental changes in order to reduce the amount of glue code required in language bindings and client code. I wrote a rather detailed analysis here: https://njump.me/naddr1qvzqqqr4gupzqrcxrljwdpfz2qn5a57hse6ez6pkd34pe0wpeskmktt2p62yeketqqvxy6t5vdhkjmntv4exuetv94shq6fdwfjhv6t9wuxrjull
π¬ maflcko commented on pull request "test: fix intermittent failure in rpc_invalidateblock.py":
(https://github.com/bitcoin/bitcoin/pull/32968#discussion_r2204408981)
This means the remainder of the test, the state of the two nodes shouldn't affect each other. So the reconsiderblock+test
```
assert_equal(self.nodes[0].getbestblockhash(), blockhash_6)
assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 6)
assert_equal(self.nodes[0].getblockchaininfo()['headers'], 7)
```
Can be moved after this line to disconnect the nodes? This would bundle the tests more nicely, but no strong opinion.
(https://github.com/bitcoin/bitcoin/pull/32968#discussion_r2204408981)
This means the remainder of the test, the state of the two nodes shouldn't affect each other. So the reconsiderblock+test
```
assert_equal(self.nodes[0].getbestblockhash(), blockhash_6)
assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 6)
assert_equal(self.nodes[0].getblockchaininfo()['headers'], 7)
```
Can be moved after this line to disconnect the nodes? This would bundle the tests more nicely, but no strong opinion.
π¬ stratospher commented on issue "Intermittent failure in rpc_invalidateblock.py assert_equal(self.nodes[0].getblockchaininfo()['headers'], 7) [ AssertionError: not(24 == 7)]":
(https://github.com/bitcoin/bitcoin/issues/32965#issuecomment-3068706510)
# Reconsider the header
self.nodes[0].reconsiderblock(block.hash)
import time; time.sleep(200)
# Since header doesn't have block data, it can't be chain tip
# Check if it's possible for an ancestor (with block data) to be the chain tip
assert_equal(self.nodes[0].getbestblockhash(), blockhash_6)
assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 6)
assert_equal(self.nodes[0].getblockchaininfo()['headers'], 7)
I am not sure wh
...
(https://github.com/bitcoin/bitcoin/issues/32965#issuecomment-3068706510)
# Reconsider the header
self.nodes[0].reconsiderblock(block.hash)
import time; time.sleep(200)
# Since header doesn't have block data, it can't be chain tip
# Check if it's possible for an ancestor (with block data) to be the chain tip
assert_equal(self.nodes[0].getbestblockhash(), blockhash_6)
assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 6)
assert_equal(self.nodes[0].getblockchaininfo()['headers'], 7)
I am not sure wh
...
π¬ maflcko commented on pull request "test: fix intermittent failure in rpc_invalidateblock.py":
(https://github.com/bitcoin/bitcoin/pull/32968#issuecomment-3068710732)
lgtm ACK 4aa0e4d9ce6283665b1aac185da67543b8df606e
Also, left a nit/questsion
(https://github.com/bitcoin/bitcoin/pull/32968#issuecomment-3068710732)
lgtm ACK 4aa0e4d9ce6283665b1aac185da67543b8df606e
Also, left a nit/questsion
π¬ maflcko commented on issue "Intermittent failure in rpc_invalidateblock.py assert_equal(self.nodes[0].getblockchaininfo()['headers'], 7) [ AssertionError: not(24 == 7)]":
(https://github.com/bitcoin/bitcoin/issues/32965#issuecomment-3068720253)
You can test this by replacing the newly added `disconnect_nodes` in your fix with `self.nodes[0].reconsiderblock(block.hash)`.
(https://github.com/bitcoin/bitcoin/issues/32965#issuecomment-3068720253)
You can test this by replacing the newly added `disconnect_nodes` in your fix with `self.nodes[0].reconsiderblock(block.hash)`.
π fanquake approved a pull request: "cmake: Drop no longer necessary "cmakeMinimumRequired" object"
(https://github.com/bitcoin/bitcoin/pull/32954#pullrequestreview-3015664087)
ACK 12a6959892cb24b940b3579828f2066651572153 - has been unneeded since it was introduced (minimum was already 3.22).
(https://github.com/bitcoin/bitcoin/pull/32954#pullrequestreview-3015664087)
ACK 12a6959892cb24b940b3579828f2066651572153 - has been unneeded since it was introduced (minimum was already 3.22).
π fanquake merged a pull request: "cmake: Drop no longer necessary "cmakeMinimumRequired" object"
(https://github.com/bitcoin/bitcoin/pull/32954)
(https://github.com/bitcoin/bitcoin/pull/32954)
π¬ TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3068782737)
Thank you for the review @stringintech!
Rebased 690a5dac223ed18a65c9d9e6c535466cc3ad4511 -> 52bab146a5045899ea6800305fa6d9b4efdcc6bd ([kernelApi_42](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_42) -> [kernelApi_43](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_43), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_42..kernelApi_43))
Updated 52bab146a5045899ea6800305fa6d9b4efdcc6bd -> 267a7b3f321304f75e8c47e380da49ba9c64bc84 ([kernelApi_43](https://gith
...
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3068782737)
Thank you for the review @stringintech!
Rebased 690a5dac223ed18a65c9d9e6c535466cc3ad4511 -> 52bab146a5045899ea6800305fa6d9b4efdcc6bd ([kernelApi_42](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_42) -> [kernelApi_43](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_43), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_42..kernelApi_43))
Updated 52bab146a5045899ea6800305fa6d9b4efdcc6bd -> 267a7b3f321304f75e8c47e380da49ba9c64bc84 ([kernelApi_43](https://gith
...
π¬ maflcko commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#issuecomment-3068819299)
re-ACK 11546183c70def6c0aa539642fd1c9ada3d46840 π»
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 11546183c70def6c0aa5
...
(https://github.com/bitcoin/bitcoin/pull/32677#issuecomment-3068819299)
re-ACK 11546183c70def6c0aa539642fd1c9ada3d46840 π»
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 11546183c70def6c0aa5
...
β€1
π¬ maflcko commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2204495759)
exists in disk -> exists on disk [βin diskβ is nonstandard; use βon diskβ]
An alternative wording could be "if ... was persisted, clear it"
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2204495759)
exists in disk -> exists on disk [βin diskβ is nonstandard; use βon diskβ]
An alternative wording could be "if ... was persisted, clear it"