✅ fanquake closed an issue: "avoid using invalidateblock to directly test reorg behavior"
(https://github.com/bitcoin/bitcoin/issues/32531)
(https://github.com/bitcoin/bitcoin/issues/32531)
🚀 fanquake merged a pull request: "test: Fix reorg patterns in tests to use proper fork-based approach"
(https://github.com/bitcoin/bitcoin/pull/32587)
(https://github.com/bitcoin/bitcoin/pull/32587)
💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2565077473)
In commit "Simplify removeRecursive"
A possibility here, and below in `removeForReorg`, is to only add the child transactions themselves (not all their descendants) to a vector, in `TxGraph::Ref*` format, and then call `TxGraph::GetDescendantsUnion` once to find all their children to remove.
That would also make it acceptable, I think, to forego the epoch/`visited` check and just add everything, as `GetDescendantsUnion` will deduplicate anyway.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2565077473)
In commit "Simplify removeRecursive"
A possibility here, and below in `removeForReorg`, is to only add the child transactions themselves (not all their descendants) to a vector, in `TxGraph::Ref*` format, and then call `TxGraph::GetDescendantsUnion` once to find all their children to remove.
That would also make it acceptable, I think, to forego the epoch/`visited` check and just add everything, as `GetDescendantsUnion` will deduplicate anyway.
💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2565171834)
In commit "doc: add design notes for cluster mempool"
I think the theory thread is outdated now. Merging and postlinearization won't be very relevant post SFL, and the existence of optimal linearizations can probably be shown much more concisely when using results from the GGT paper.
It's probably worth creating a trimmed-down version with just definitions of clusters, chunks, and fee-size diagams, possibly to include directly in the document here, but that can be done as a follow-up I thi
...
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2565171834)
In commit "doc: add design notes for cluster mempool"
I think the theory thread is outdated now. Merging and postlinearization won't be very relevant post SFL, and the existence of optimal linearizations can probably be shown much more concisely when using results from the GGT paper.
It's probably worth creating a trimmed-down version with just definitions of clusters, chunks, and fee-size diagams, possibly to include directly in the document here, but that can be done as a follow-up I thi
...
💬 sipa commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2565156011)
In commit "doc: Update mempool-limits.md for cluster mempool".
Perhaps worth elaborating that it is not just parents and children, but also any combination thereof:
> A transaction's *cluster* is the set of all transactions that can be reached by following any combination of parent or child relationships. This includes all ancestors and descendants, but also any descendants of those ancestors and ancestors of those descendants, and so on.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2565156011)
In commit "doc: Update mempool-limits.md for cluster mempool".
Perhaps worth elaborating that it is not just parents and children, but also any combination thereof:
> A transaction's *cluster* is the set of all transactions that can be reached by following any combination of parent or child relationships. This includes all ancestors and descendants, but also any descendants of those ancestors and ancestors of those descendants, and so on.
💬 hebasto commented on pull request "Clear out space on CentOS, depends, gui GHA job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3581524339)
@willcl-ark
Please rebase.
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3581524339)
@willcl-ark
Please rebase.
🤔 hebasto reviewed a pull request: "Clear out space on CentOS, depends, gui GHA job"
(https://github.com/bitcoin/bitcoin/pull/33514#pullrequestreview-3511188692)
Concept ACK. A similar approach was used in https://github.com/hebasto/bitcoin-core-nightly/pull/91.
(https://github.com/bitcoin/bitcoin/pull/33514#pullrequestreview-3511188692)
Concept ACK. A similar approach was used in https://github.com/hebasto/bitcoin-core-nightly/pull/91.
💬 willcl-ark commented on pull request "Clear out space on CentOS, depends, gui GHA job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3581540604)
> @willcl-ark
>
> Please rebase.
Sorry, was waiting to see if @maflcko 's background task approach works before adopting it here. I tested it in my fork [here](https://github.com/willcl-ark/bitcoin/actions/runs/19703413734/job/56444984809) where it appears to have worked successfully.
So I have pushed e07e57368e9fab8ecfc140d44aef7db9b23c7ce0 here which:
- applies the clearout to all jobs (on `gha` runners`)
- runs the task in the background
- takes the suggested command from https:
...
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3581540604)
> @willcl-ark
>
> Please rebase.
Sorry, was waiting to see if @maflcko 's background task approach works before adopting it here. I tested it in my fork [here](https://github.com/willcl-ark/bitcoin/actions/runs/19703413734/job/56444984809) where it appears to have worked successfully.
So I have pushed e07e57368e9fab8ecfc140d44aef7db9b23c7ce0 here which:
- applies the clearout to all jobs (on `gha` runners`)
- runs the task in the background
- takes the suggested command from https:
...
💬 willcl-ark commented on pull request "Clear out space on CentOS, depends, gui GHA job":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2565200327)
taken this suggestion in e07e57368e9fab8ecfc140d44aef7db9b23c7ce0. Thanks!
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2565200327)
taken this suggestion in e07e57368e9fab8ecfc140d44aef7db9b23c7ce0. Thanks!
💬 willcl-ark commented on pull request "Clear out space on CentOS, depends, gui GHA job":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2565201088)
Not relevant any more since e07e57368e9fab8ecfc140d44aef7db9b23c7ce0
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2565201088)
Not relevant any more since e07e57368e9fab8ecfc140d44aef7db9b23c7ce0
💬 willcl-ark commented on pull request "Clear out space on CentOS, depends, gui GHA job":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2565201930)
removed in e07e57368e9fab8ecfc140d44aef7db9b23c7ce0
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2565201930)
removed in e07e57368e9fab8ecfc140d44aef7db9b23c7ce0
💬 maflcko commented on pull request "Clear out space on CentOS, depends, gui GHA job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3581553019)
lgtm ACK e07e57368e9fab8ecfc140d44aef7db9b23c7ce0
Missing `ci: ` prefix in the title?
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3581553019)
lgtm ACK e07e57368e9fab8ecfc140d44aef7db9b23c7ce0
Missing `ci: ` prefix in the title?
💬 fjahr commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#issuecomment-3581563308)
> In PR description:
>
> ```diff
> - primarily adds test coverage for some of the most prominent failure cases in the first commit.
> + primarily adds test coverage for some of the most prominent failure cases in the last commit.
> ```
Fixed as well
(https://github.com/bitcoin/bitcoin/pull/33636#issuecomment-3581563308)
> In PR description:
>
> ```diff
> - primarily adds test coverage for some of the most prominent failure cases in the first commit.
> + primarily adds test coverage for some of the most prominent failure cases in the last commit.
> ```
Fixed as well
✅ maflcko closed an issue: "Docs: "External signer" documentation is outdated. (plz close if unwanted request)"
(https://github.com/bitcoin/bitcoin/issues/31005)
(https://github.com/bitcoin/bitcoin/issues/31005)
💬 maflcko commented on issue "Docs: "External signer" documentation is outdated. (plz close if unwanted request)":
(https://github.com/bitcoin/bitcoin/issues/31005#issuecomment-3581571135)
ok, let's move the discussion to the pull request.
(https://github.com/bitcoin/bitcoin/issues/31005#issuecomment-3581571135)
ok, let's move the discussion to the pull request.
💬 maflcko commented on pull request "doc: update interface, --stdin flag, `signtx` (#31005)":
(https://github.com/bitcoin/bitcoin/pull/33765#issuecomment-3581579197)
Please use unique, descriptive commit message, or squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits.
Also, there are a few LLM linter comments. Not sure if they all apply, but the missing `a` article makes sene.
(https://github.com/bitcoin/bitcoin/pull/33765#issuecomment-3581579197)
Please use unique, descriptive commit message, or squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits.
Also, there are a few LLM linter comments. Not sure if they all apply, but the missing `a` article makes sene.
💬 fanquake commented on pull request "policy: Remove individual transaction <minrelay restriction":
(https://github.com/bitcoin/bitcoin/pull/33892#discussion_r2565262574)
LLM: s/includeing/including/
(https://github.com/bitcoin/bitcoin/pull/33892#discussion_r2565262574)
LLM: s/includeing/including/
💬 maflcko commented on pull request "ubsan: add another suppression for InsecureRandomContext":
(https://github.com/bitcoin/bitcoin/pull/33949#issuecomment-3581633296)
Not sure. Is this caused by removing `-g`? If yes, I am not sure this is something meaningful to support when running with sanitizers.
(https://github.com/bitcoin/bitcoin/pull/33949#issuecomment-3581633296)
Not sure. Is this caused by removing `-g`? If yes, I am not sure this is something meaningful to support when running with sanitizers.
💬 maflcko commented on pull request "ubsan: add another suppression for InsecureRandomContext":
(https://github.com/bitcoin/bitcoin/pull/33949#issuecomment-3581643069)
Going further, we could require `-fno-inline` for ubsan. But if you want to go the route in this pull, might as well just wildcard-suppress `InsecureRandomContext`?
(https://github.com/bitcoin/bitcoin/pull/33949#issuecomment-3581643069)
Going further, we could require `-fno-inline` for ubsan. But if you want to go the route in this pull, might as well just wildcard-suppress `InsecureRandomContext`?
💬 fjahr commented on pull request "contrib: Count entry differences in asmap-tool diff summary":
(https://github.com/bitcoin/bitcoin/pull/33939#discussion_r2565291756)
Ah, neat, I am sure I knew `num_addresses` existed. Updated the code to use it now and confirmed the result is the same as before for a diff between two different asmap files.
(https://github.com/bitcoin/bitcoin/pull/33939#discussion_r2565291756)
Ah, neat, I am sure I knew `num_addresses` existed. Updated the code to use it now and confirmed the result is the same as before for a diff between two different asmap files.