💬 stratospher commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394151442)
ah I see how it could be confusing, clarified it to "since the previous commit removes BLOCK_FAILED_CHILD usage in InvalidateBlock"
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394151442)
ah I see how it could be confusing, clarified it to "since the previous commit removes BLOCK_FAILED_CHILD usage in InvalidateBlock"
💬 stratospher commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394152350)
good point. I've removed it since `BLOCK_FAILED_MASK = 96` is never written to disk but only used while reading BlockStatus values. if 96 exists on disk, it is the responsibility of `BLOCK_FAILED_VALID` and `BLOCK_FAILED_CHILD` and we have dealt with those cases.
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394152350)
good point. I've removed it since `BLOCK_FAILED_MASK = 96` is never written to disk but only used while reading BlockStatus values. if 96 exists on disk, it is the responsibility of `BLOCK_FAILED_VALID` and `BLOCK_FAILED_CHILD` and we have dealt with those cases.
💬 stratospher commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394152594)
done.
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2394152594)
done.
💬 trevarj commented on pull request "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2":
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3355880463)
> Hopefully Guix makes a release tag again one day.
This is coming soon, there have been talks of it.
In the meantime, you can simply `guix pull` and receive the freshest version (from master). If you don't want master, you can edit your Guix channels (`~/.config/guix/channels.scm`) and pin the commit to something "stable".
I'm not certain that would cause an issue though. To me it just sounds like unstable substitute servers. For context, if you try installing Guix System it could tak
...
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3355880463)
> Hopefully Guix makes a release tag again one day.
This is coming soon, there have been talks of it.
In the meantime, you can simply `guix pull` and receive the freshest version (from master). If you don't want master, you can edit your Guix channels (`~/.config/guix/channels.scm`) and pin the commit to something "stable".
I'm not certain that would cause an issue though. To me it just sounds like unstable substitute servers. For context, if you try installing Guix System it could tak
...
👍 stickies-v approved a pull request: "[28.x] backports + 28.3rc1"
(https://github.com/bitcoin/bitcoin/pull/33476#pullrequestreview-3288706062)
re-ACK 9968b15937cbc2071c3474bd83f9c9fb4bb3a970
Getting the same bitcoin.conf example.
(https://github.com/bitcoin/bitcoin/pull/33476#pullrequestreview-3288706062)
re-ACK 9968b15937cbc2071c3474bd83f9c9fb4bb3a970
Getting the same bitcoin.conf example.
💬 stickies-v commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2394279590)
Oh I see. The fact that test functions are slightly renamed combined with this patch of code being ~duplicated tripped me up a bit. Thanks for the clarification, can be marked as resolved.
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2394279590)
Oh I see. The fact that test functions are slightly renamed combined with this patch of code being ~duplicated tripped me up a bit. Thanks for the clarification, can be marked as resolved.
🤔 sipa reviewed a pull request: "validation: remove BLOCK_FAILED_CHILD"
(https://github.com/bitcoin/bitcoin/pull/32950#pullrequestreview-3288768556)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32950#pullrequestreview-3288768556)
Concept ACK
⚠️ venpisey12 opened an issue: "0968812058"
(https://github.com/bitcoin/bitcoin/issues/33513)
(https://github.com/bitcoin/bitcoin/issues/33513)
✅ willcl-ark closed an issue: "0968812058"
(https://github.com/bitcoin/bitcoin/issues/33513)
(https://github.com/bitcoin/bitcoin/issues/33513)
💬 sipa commented on issue "Mempool Expiry eviction might remove txs that could be mined in the next block":
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3356193889)
@davidgumberg I like the idea of expiring based on how many times a transaction was expected to be mined, but wasn't. E.g. every block, or every 10 minutes, or on a Poisson timer, run the block building code and increase a counter in the mempool for every transaction. Whenever the counter reaches a certain configurable value, expire. Note that that may well mean that nothing ever expires, if everything gets mined once it's spent enough time near the top of the mempool - but that may be ok, if it
...
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3356193889)
@davidgumberg I like the idea of expiring based on how many times a transaction was expected to be mined, but wasn't. E.g. every block, or every 10 minutes, or on a Poisson timer, run the block building code and increase a counter in the mempool for every transaction. Whenever the counter reaches a certain configurable value, expire. Note that that may well mean that nothing ever expires, if everything gets mined once it's spent enough time near the top of the mempool - but that may be ok, if it
...
💬 sipa commented on pull request "p2p: Use network-dependent timers for inbound inv scheduling":
(https://github.com/bitcoin/bitcoin/pull/33464#issuecomment-3356266112)
utACK 0f7d4ee4e8281ed141a6ebb7e0edee7b864e4dcf
As a follow-up, I think we can give every outbound onion connection a separate network_id, because they all look distinct to external observers.
(https://github.com/bitcoin/bitcoin/pull/33464#issuecomment-3356266112)
utACK 0f7d4ee4e8281ed141a6ebb7e0edee7b864e4dcf
As a follow-up, I think we can give every outbound onion connection a separate network_id, because they all look distinct to external observers.
💬 sipa commented on pull request "[IBD] precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2394589493)
In commit "optimization: Cache PresaltedSipHasher in CBlockHeaderAndShortTxIDs"
Why is this optional? It looks like it's always initialized in the constructor.
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2394589493)
In commit "optimization: Cache PresaltedSipHasher in CBlockHeaderAndShortTxIDs"
Why is this optional? It looks like it's always initialized in the constructor.
💬 m3dwards commented on pull request "ci: fix buildx gha cache authentication on forks":
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3356349768)
> But IMO exporting all variables prefixed with `ACTIONS_` to the CI env should not ever be problematic (and my insulate us a little in case new variables are introduced).
Which is what I think happened here to make this break in the first place so I'd support exporting all `ACTIONS_` vars.
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3356349768)
> But IMO exporting all variables prefixed with `ACTIONS_` to the CI env should not ever be problematic (and my insulate us a little in case new variables are introduced).
Which is what I think happened here to make this break in the first place so I'd support exporting all `ACTIONS_` vars.
💬 sipa commented on pull request "p2p: Mitigate GETADDR fingerprinting by setting address timestamps to a fixed value":
(https://github.com/bitcoin/bitcoin/pull/33498#issuecomment-3356355027)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33498#issuecomment-3356355027)
Concept ACK
🤔 sipa reviewed a pull request: "net: improve the interface around FindNode() and avoid a recursive mutex lock"
(https://github.com/bitcoin/bitcoin/pull/32326#pullrequestreview-3289213901)
utACK c9ffb6d7104f016ab89057bd57cd128ac03fd1fc
(https://github.com/bitcoin/bitcoin/pull/32326#pullrequestreview-3289213901)
utACK c9ffb6d7104f016ab89057bd57cd128ac03fd1fc
💬 sipa commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2394630697)
In commit "net: avoid recursive m_nodes_mutex lock in DisconnectNode()"
Nit: the `auto` here made me worried this was making a copy of the `CNode` data structure. Maybe make the `CNode*` type explicit here?
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2394630697)
In commit "net: avoid recursive m_nodes_mutex lock in DisconnectNode()"
Nit: the `auto` here made me worried this was making a copy of the `CNode` data structure. Maybe make the `CNode*` type explicit here?
💬 willcl-ark commented on issue "[29.x] guix build failure on ppc with xcb":
(https://github.com/bitcoin/bitcoin/issues/33488#issuecomment-3356499067)
Ah I see.
FWIW I am using the same guix version via nix but can't reproduce :(
```
[100%] Built target check-symbols
-- Install configuration: "RelWithDebInfo"
-- Installing: /distsrc-base/distsrc-f6d49d0a0995-powerpc64-linux-gnu/installed/bitcoin-f6d49d0a0995/bin/bitcoin-wallet
-- Installing: /distsrc-base/distsrc-f6d49d0a0995-powerpc64-linux-gnu/installed/bitcoin-f6d49d0a0995/share/man/man1/bitcoin-wallet.1
-- Installing: /distsrc-base/distsrc-f6d49d0a0995-powerpc64-linux-gnu/installed/bitco
...
(https://github.com/bitcoin/bitcoin/issues/33488#issuecomment-3356499067)
Ah I see.
FWIW I am using the same guix version via nix but can't reproduce :(
```
[100%] Built target check-symbols
-- Install configuration: "RelWithDebInfo"
-- Installing: /distsrc-base/distsrc-f6d49d0a0995-powerpc64-linux-gnu/installed/bitcoin-f6d49d0a0995/bin/bitcoin-wallet
-- Installing: /distsrc-base/distsrc-f6d49d0a0995-powerpc64-linux-gnu/installed/bitcoin-f6d49d0a0995/share/man/man1/bitcoin-wallet.1
-- Installing: /distsrc-base/distsrc-f6d49d0a0995-powerpc64-linux-gnu/installed/bitco
...
📝 willcl-ark opened a pull request: "Clear out space on centos job"
(https://github.com/bitcoin/bitcoin/pull/33514)
Fixes #33293
Clear out space on the centos job be deleteing unnecessary files.
Raised in #33293 which pointed to a solution like https://github.com/google/oss-fuzz/commit/b7f04d782277638a67bc44865de445977eed4708 which is adapted slightly here.
Only runs when cache provider (runner) is `gha`, and on the CentOS job.
A run on my fork can be seen here: https://github.com/willcl-ark/bitcoin/actions/runs/18130629028/job/51596196163#step:6:2
(https://github.com/bitcoin/bitcoin/pull/33514)
Fixes #33293
Clear out space on the centos job be deleteing unnecessary files.
Raised in #33293 which pointed to a solution like https://github.com/google/oss-fuzz/commit/b7f04d782277638a67bc44865de445977eed4708 which is adapted slightly here.
Only runs when cache provider (runner) is `gha`, and on the CentOS job.
A run on my fork can be seen here: https://github.com/willcl-ark/bitcoin/actions/runs/18130629028/job/51596196163#step:6:2
💬 willcl-ark commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3356522227)
Maintaining an action in this repo which only caters to forks is not ideal, but unless GH increases the space on runners, or provides a "slim" image variant, then it might be the best there is for now.
As it doesn't run in this repo it should at least never break our CI.
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3356522227)
Maintaining an action in this repo which only caters to forks is not ideal, but unless GH increases the space on runners, or provides a "slim" image variant, then it might be the best there is for now.
As it doesn't run in this repo it should at least never break our CI.
💬 vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3356581222)
`0c718da693...73071b0cdb`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3356581222)
`0c718da693...73071b0cdb`: rebase due to conflicts