💬 darosior commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#issuecomment-2214720674)
Rebased this, taking advantage of https://github.com/bitcoin/bitcoin/pull/28960. I've also been investigating alternative approaches.
I first tried to move from `fmemopen` toward the more flexible `memfd_create`. It avoided the need for some of the filesystem mocks (which were necessary before because you can't call `fileno` on a `FILE*` created with `fmemopen`). This allowed to drop one commit. Further, this removed the need from creating 128MiB in-memory blk files. This is in turn makes it
...
(https://github.com/bitcoin/bitcoin/pull/29158#issuecomment-2214720674)
Rebased this, taking advantage of https://github.com/bitcoin/bitcoin/pull/28960. I've also been investigating alternative approaches.
I first tried to move from `fmemopen` toward the more flexible `memfd_create`. It avoided the need for some of the filesystem mocks (which were necessary before because you can't call `fileno` on a `FILE*` created with `fmemopen`). This allowed to drop one commit. Further, this removed the need from creating 128MiB in-memory blk files. This is in turn makes it
...
📝 Sjors opened a pull request: "Introduce waitTipChanged() mining interface and replace RPCNotifyBlockChange"
(https://github.com/bitcoin/bitcoin/pull/30409)
This continues the work in #30200 so that a future Stratum v2 Template Provider (see #29432) can avoid accessing node internals. It needs to know when a new block arrives in order to push new templates to connected clients.
In order to ensure the new method works as intended, the `waitfornewblock`, `waitforblock` and `waitforblockheight` RPC methods are refactored to use it. This allows removing `RPCNotifyBlockChange`.
The second commit adds tests for the methods that are about to be refac
...
(https://github.com/bitcoin/bitcoin/pull/30409)
This continues the work in #30200 so that a future Stratum v2 Template Provider (see #29432) can avoid accessing node internals. It needs to know when a new block arrives in order to push new templates to connected clients.
In order to ensure the new method works as intended, the `waitfornewblock`, `waitforblock` and `waitforblockheight` RPC methods are refactored to use it. This allows removing `RPCNotifyBlockChange`.
The second commit adds tests for the methods that are about to be refac
...
💬 fjahr commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-2214748299)
I think this may be ready for closing since all remaining TODOs have been addressed IMO. There are three remaining PRs that still need to get merged but I think we can expect that nobody will start a new PR based on the TODOs comment anymore.
For reference, the remaining PRs are #30403, #30320, #29996. See also https://github.com/bitcoin/bitcoin/pull/30403#issuecomment-2213758446
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-2214748299)
I think this may be ready for closing since all remaining TODOs have been addressed IMO. There are three remaining PRs that still need to get merged but I think we can expect that nobody will start a new PR based on the TODOs comment anymore.
For reference, the remaining PRs are #30403, #30320, #29996. See also https://github.com/bitcoin/bitcoin/pull/30403#issuecomment-2213758446
💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669007098)
I'm assuming that it's only used for stronger invariants checks in the fuzz/test harness
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669007098)
I'm assuming that it's only used for stronger invariants checks in the fuzz/test harness
💬 fjahr commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-2214756801)
Or rather, the problem is that the issue here is not specific and so none of the issues can close this issue because it's unclear which will be the last one to get merged. I will recommend in the PRs that they mention the issue here as related to so it's at least clear what is being worked on where.
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-2214756801)
Or rather, the problem is that the issue here is not specific and so none of the issues can close this issue because it's unclear which will be the last one to get merged. I will recommend in the PRs that they mention the issue here as related to so it's at least clear what is being worked on where.
💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669012557)
Good catch! Yes, since the full blocks are never being submitted to any node (just the headers to node 1), nothing failed. Fixed now.
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669012557)
Good catch! Yes, since the full blocks are never being submitted to any node (just the headers to node 1), nothing failed. Fixed now.
💬 ryanofsky commented on pull request "Fix issues with CI on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2214759087)
@maflcko is your "lgtm ACK" more of a partial ACK, or do you think this is good to merge? I think practically speaking this looks ok to merge, but I wouldn't want to merge it myself if I'm the only full reviewer.
I guess there are also number of possible followups to this PR if it is merged:
- Maybe preventing github actions from running twice on PRs, [#29274 (comment)](https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2188840483).
- Maybe restoring ability to skip cirrus ARM job
...
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2214759087)
@maflcko is your "lgtm ACK" more of a partial ACK, or do you think this is good to merge? I think practically speaking this looks ok to merge, but I wouldn't want to merge it myself if I'm the only full reviewer.
I guess there are also number of possible followups to this PR if it is merged:
- Maybe preventing github actions from running twice on PRs, [#29274 (comment)](https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2188840483).
- Maybe restoring ability to skip cirrus ARM job
...
💬 fjahr commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669014412)
Could also mention #28648 as related in description because of this.
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669014412)
Could also mention #28648 as related in description because of this.
💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669014970)
Done, in a slightly modified way. The alternative chain is just created in python, node 0 is only used to fetch the previous block it builds on.
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669014970)
Done, in a slightly modified way. The alternative chain is just created in python, node 0 is only used to fetch the previous block it builds on.
💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669015125)
took those
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669015125)
took those
💬 fjahr commented on pull request "Assumeutxo: bugfix on loadtxoutset with a divergent chain + test":
(https://github.com/bitcoin/bitcoin/pull/29996#issuecomment-2214762372)
nit: Could mention #28648 as related in the PR description.
(https://github.com/bitcoin/bitcoin/pull/29996#issuecomment-2214762372)
nit: Could mention #28648 as related in the PR description.
💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669015915)
Removed both TODOs.
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669015915)
Removed both TODOs.
💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#issuecomment-2214767684)
[29ab88d ](https://github.com/bitcoin/bitcoin/commit/29ab88dfe1c43af3620480c99cba844aa414023c)to [ab478c5](https://github.com/bitcoin/bitcoin/commit/ab478c5fa16427b496e8a93e4780770d4c270214):
Addressed feedback by @fjahr (thanks!) and rebased.
(https://github.com/bitcoin/bitcoin/pull/30320#issuecomment-2214767684)
[29ab88d ](https://github.com/bitcoin/bitcoin/commit/29ab88dfe1c43af3620480c99cba844aa414023c)to [ab478c5](https://github.com/bitcoin/bitcoin/commit/ab478c5fa16427b496e8a93e4780770d4c270214):
Addressed feedback by @fjahr (thanks!) and rebased.
👋 stickies-v's pull request is ready for review: "C++20 std::views::reverse"
(https://github.com/bitcoin/bitcoin/pull/28687)
(https://github.com/bitcoin/bitcoin/pull/28687)
💬 stickies-v commented on pull request "C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-2214772241)
With https://github.com/bitcoin/bitcoin/pull/30263 merged, this PR is now ready for review 🥳
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-2214772241)
With https://github.com/bitcoin/bitcoin/pull/30263 merged, this PR is now ready for review 🥳
📝 achow101 converted_to_draft a pull request: "wallet: Remove IsMine from migration code"
(https://github.com/bitcoin/bitcoin/pull/30328)
The legacy wallet `IsMine` code will be removed with the legacy wallet, but is still partially needed for migration. Instead of using `IsMine` directly in migration, equivalent checks are performed by migration.
Builds on #26596
(https://github.com/bitcoin/bitcoin/pull/30328)
The legacy wallet `IsMine` code will be removed with the legacy wallet, but is still partially needed for migration. Instead of using `IsMine` directly in migration, equivalent checks are performed by migration.
Builds on #26596
👍 instagibbs approved a pull request: "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip"
(https://github.com/bitcoin/bitcoin/pull/30111#pullrequestreview-2163961806)
reACK 17d41e7a547f16f0dd3802dac73a63772ca000b2
biggest change is changing the new function to `ActiveTipChange` and making ibd a no-op
reviewed via `git range-diff master 0e0c422 17d41e7`
(https://github.com/bitcoin/bitcoin/pull/30111#pullrequestreview-2163961806)
reACK 17d41e7a547f16f0dd3802dac73a63772ca000b2
biggest change is changing the new function to `ActiveTipChange` and making ibd a no-op
reviewed via `git range-diff master 0e0c422 17d41e7`
💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1669031104)
fixed
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1669031104)
fixed
💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1669032119)
thanks accepted your suggestion with a slight modification.
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1669032119)
thanks accepted your suggestion with a slight modification.
💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1669032477)
yes, this is gone now.
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1669032477)
yes, this is gone now.