💬 willcl-ark commented on pull request "doc: document capnproto and libmultiprocess deps in 29.x":
(https://github.com/bitcoin/bitcoin/pull/33623#discussion_r2434100605)
Nice, thanks!
I double-checked the tag points to the correct commit and tested a depends build using your package suggestion which all worked fine.
Have therefore gone ahead with updating the depends package and _dependencies.md_ in bf236d7a8af0826efc39a04113b79f96fe05f0c7 as you suggest.
(https://github.com/bitcoin/bitcoin/pull/33623#discussion_r2434100605)
Nice, thanks!
I double-checked the tag points to the correct commit and tested a depends build using your package suggestion which all worked fine.
Have therefore gone ahead with updating the depends package and _dependencies.md_ in bf236d7a8af0826efc39a04113b79f96fe05f0c7 as you suggest.
🤔 5girlstaken-arch reviewed a pull request: "ci: Properly include $FILE_ENV in DEPENDS_HASH"
(https://github.com/bitcoin/bitcoin/pull/33581#pullrequestreview-3342555102)
00000020
a6f5e4d3c2b1a0f9e8d7c6b5a4938271605f4e3d2c1b0a000000000000000000
04f15068366fba1de4c323bb6ca4fe58c7d968d81688abf68380bddf46fd4ef0
a08fee68
6a5d0f17
00010007
(https://github.com/bitcoin/bitcoin/pull/33581#pullrequestreview-3342555102)
00000020
a6f5e4d3c2b1a0f9e8d7c6b5a4938271605f4e3d2c1b0a000000000000000000
04f15068366fba1de4c323bb6ca4fe58c7d968d81688abf68380bddf46fd4ef0
a08fee68
6a5d0f17
00010007
💬 5girlstaken-arch commented on pull request "ci: Properly include $FILE_ENV in DEPENDS_HASH":
(https://github.com/bitcoin/bitcoin/pull/33581#issuecomment-3408539754)
3L3WW6b3c9ArSc35geweMZan2YpYkfCXAP
(https://github.com/bitcoin/bitcoin/pull/33581#issuecomment-3408539754)
3L3WW6b3c9ArSc35geweMZan2YpYkfCXAP
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434195677)
Looks good, will include -- thank you for the test coverage!
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434195677)
Looks good, will include -- thank you for the test coverage!
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434207347)
Ok it's been a really long time since I've thought about this, but I believe the issue was that I needed epoch markers to be movable so that I could make CTxMemPoolEntry movable.
I don't exactly remember why CTxMemPoolEntry needs to be movable but I believe it has something to do with inheriting from TxGraph::Ref. I guess Ref's can't be copied, but they can be moved?
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434207347)
Ok it's been a really long time since I've thought about this, but I believe the issue was that I needed epoch markers to be movable so that I could make CTxMemPoolEntry movable.
I don't exactly remember why CTxMemPoolEntry needs to be movable but I believe it has something to do with inheriting from TxGraph::Ref. I guess Ref's can't be copied, but they can be moved?
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434211174)
Actually, in fairness I think there might be some platforms where the space usage may be different -- I think I ran into that at some point in the history of this PR with one of our CI jobs, but I don't recall the details now. (Even still, I think saving the map lookup is likely worth it.)
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434211174)
Actually, in fairness I think there might be some platforms where the space usage may be different -- I think I ran into that at some point in the history of this PR with one of our CI jobs, but I don't recall the details now. (Even still, I think saving the map lookup is likely worth it.)
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434216116)
I think the reason I didn't is probably because `addChunks()` has multiple places where it returns, so it seemed a little messy to have to invoke `StopBlockBuilding()` within that function. But if you think it's worth it, I can rework the function and push the calls down into it.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434216116)
I think the reason I didn't is probably because `addChunks()` has multiple places where it returns, so it seemed a little messy to have to invoke `StopBlockBuilding()` within that function. But if you think it's worth it, I can rework the function and push the calls down into it.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434220844)
In this commit, `chunk_feerate.fee` is used further down, but this gets cleaned up further in #33591 with the introduction of `CFeeRate::GetFeePerVSize`, at which point this could be inlined. I'll make that change in #33591.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434220844)
In this commit, `chunk_feerate.fee` is used further down, but this gets cleaned up further in #33591 with the introduction of `CFeeRate::GetFeePerVSize`, at which point this could be inlined. I'll make that change in #33591.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434234855)
Will take this doc change in #33591.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434234855)
Will take this doc change in #33591.
📝 fjahr opened a pull request: "wallet: Expand MuSig test coverage and follow-ups"
(https://github.com/bitcoin/bitcoin/pull/33636)
This is a follow-up to #29675 and primarily adds test coverage for some of the most prominent failure cases in the first commit.
The following commits address a few left-over nit comments that didn't make it in before merge.
(https://github.com/bitcoin/bitcoin/pull/33636)
This is a follow-up to #29675 and primarily adds test coverage for some of the most prominent failure cases in the first commit.
The following commits address a few left-over nit comments that didn't make it in before merge.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434241333)
Good point! Taking your change.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434241333)
Good point! Taking your change.
💬 fjahr commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-3408681878)
I have opened a PR with my tests and the left-over nits here: #33636
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-3408681878)
I have opened a PR with my tests and the left-over nits here: #33636
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434242379)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434242379)
Fixed.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434242823)
Done, thanks.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2434242823)
Done, thanks.
💬 l0rinc commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2434247283)
Rebased, added explanation to commit message
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2434247283)
Rebased, added explanation to commit message
💬 l0rinc commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2434247408)
Added explanation to commit message
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2434247408)
Added explanation to commit message
💬 l0rinc commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#issuecomment-3408692812)
Rebased after https://github.com/bitcoin/bitcoin/pull/32313.
(https://github.com/bitcoin/bitcoin/pull/33602#issuecomment-3408692812)
Rebased after https://github.com/bitcoin/bitcoin/pull/32313.
💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#issuecomment-3408731231)
Rebased after https://github.com/bitcoin/bitcoin/pull/32313, the PR is simpler and better tested this way.
(https://github.com/bitcoin/bitcoin/pull/33512#issuecomment-3408731231)
Rebased after https://github.com/bitcoin/bitcoin/pull/32313, the PR is simpler and better tested this way.
👍 andrewtoth approved a pull request: "net_processing: rename RelayTransaction to better describe what it does"
(https://github.com/bitcoin/bitcoin/pull/33565#pullrequestreview-3342719761)
ACK 84b2ad03349bf6af3d6057f3dbcdf84c17f11bcb
(https://github.com/bitcoin/bitcoin/pull/33565#pullrequestreview-3342719761)
ACK 84b2ad03349bf6af3d6057f3dbcdf84c17f11bcb
👍 l0rinc approved a pull request: "net_processing: rename RelayTransaction to better describe what it does"
(https://github.com/bitcoin/bitcoin/pull/33565#pullrequestreview-3312759976)
Left a few code comment suggestions, I think the newly added one is a bit imprecise, but I also don't mind merging as is if others don't think they're serious.
ACK 84b2ad03349bf6af3d6057f3dbcdf84c17f11bcb
(https://github.com/bitcoin/bitcoin/pull/33565#pullrequestreview-3312759976)
Left a few code comment suggestions, I think the newly added one is a bit imprecise, but I also don't mind merging as is if others don't think they're serious.
ACK 84b2ad03349bf6af3d6057f3dbcdf84c17f11bcb