Bitcoin Core Github
44 subscribers
121K links
Download Telegram
šŸ’¬ pablomartin4btc commented on pull request "refactor: unify container presence checks":
(https://github.com/bitcoin/bitcoin/pull/33094#discussion_r2244246962)
nit: not this one?
```suggestion
if (mapModifiedTx.contains(it) || inBlock.contains(it->GetSharedTx()->GetHash()) || failedTx.contains(it->GetSharedTx()->GetHash())) {
```
šŸ¤” pablomartin4btc reviewed a pull request: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33094#pullrequestreview-3073861902)
Concept ACK

This refactor replaces explicit `.count(x)` checks with `.contains(x)`, which only verifies that at least one instance exists (or none when `== 0`). Please double-check whether `.contains()` preserves the original behavior in some contexts where exactness matters ( !=1, ==1, etc). Also where the logic enforces that exactly one item is/ is not present — useful even in containers that don't allow duplicates (like `std::map` or `std::set`) because it clearly expresses the expected lo
...
šŸ¤” l0rinc reviewed a pull request: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33094#pullrequestreview-3073908954)
Thanks, split up the PR into 3 commits to simplify assessing the risks - hope this clarifies it, see the commit messages for further details.
šŸ’¬ l0rinc commented on pull request "refactor: unify container presence checks":
(https://github.com/bitcoin/bitcoin/pull/33094#discussion_r2244294569)
I had these at one point, but decided against modifying multi-containers - but this one seems fine, thanks, added.
šŸ‘‹ l0rinc's pull request is ready for review: "[IBD] flush UTXO set in batches proportional to `dbcache` size"
(https://github.com/bitcoin/bitcoin/pull/31645)
šŸ’¬ l0rinc commented on pull request "[IBD] flush UTXO set in batches proportional to `dbcache` size":
(https://github.com/bitcoin/bitcoin/pull/31645#issuecomment-3138505865)
Rebased, the PR is ready for review again!

The batch size for UTXO set writes is now calculated based on the maximum `dbcache` size to ensure that with the default values, memory usage doesn't increase, while reducing flushing time when there is enough memory available.
The change reduces the IBD time by a fixed amount, it's speeding up a critical part of saving the state for long-term storage.
šŸ’¬ nervana21 commented on pull request "qa: test that we do not disconnect a peer for submitting an invalid compact block":
(https://github.com/bitcoin/bitcoin/pull/33083#issuecomment-3138622075)
tACK [c157438](https://github.com/bitcoin/bitcoin/pull/33083/commits/c1574381168573c561ebddf1945d2debefb340f7)

I reproduced the errors as expected with the provided diffs. I validated that the master branch tests pass when the proposed commits are applied.
šŸ’¬ HowHsu commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3138628456)
> > Hi furszy, I thought the test I put already demonstrate the issue, though it is not proper to be included to the codebase.
>
> I agree that a test that copies / amends the actual production code is not great for inclusion, but why not include the test [furszy@f2b8a06](https://github.com/furszy/bitcoin-core/commit/f2b8a06060e2a313f35101454c7669d46e0edc38) from above that doesn't have this problem?

I see, sorry for somehow I missed that part. I thought the test furszy mentioned is my tes
...
šŸ’¬ nervana21 commented on pull request "qa: test that we do not disconnect a peer for submitting an invalid compact block":
(https://github.com/bitcoin/bitcoin/pull/33083#discussion_r2241408531)
Small nit

```suggestion
# The failure above was cached. Submitting the compact block again will return a cached
```
āœ… rkrux closed a pull request: "Musig2 fields followups"
(https://github.com/bitcoin/bitcoin/pull/32412)
šŸ’¬ rkrux commented on pull request "Musig2 fields followups":
(https://github.com/bitcoin/bitcoin/pull/32412#issuecomment-3138649267)
Closing this one now.
šŸ’¬ HowHsu commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3138661911)
> @HowHsu, are you going to include the test or a patch to reproduce the issue? I’m happy to ACK it either way.

Hi @furszy , sorry for the delay, I've picked your commit of the test, which I missed earlier.
šŸ’¬ Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2244450698)
Ah yes, in that case:

```
@param[out] has_hardened whether hardened derivation was found
```
šŸ’¬ maflcko commented on pull request "refactor: remove unused `ser_writedata16be` and `ser_readdata16be`":
(https://github.com/bitcoin/bitcoin/pull/33093#issuecomment-3138711573)
lgtm ACK 0431a690c3a498a1e728c9df34a132ac16177a04
šŸ’¬ luke-jr commented on pull request "doc: move `cmake -B build -LH` up in Unix build docs":
(https://github.com/bitcoin/bitcoin/pull/33088#issuecomment-3138752685)
(might make sense to mention `ccmake` too?)
šŸ’¬ instagibbs commented on pull request "qa: test that we do not disconnect a peer for submitting an invalid compact block":
(https://github.com/bitcoin/bitcoin/pull/33083#issuecomment-3138794818)
crACK c1574381168573c561ebddf1945d2debefb340f7
šŸ’¬ instagibbs commented on pull request "package validation: relax the package-not-child-with-unconfirmed-parents rule":
(https://github.com/bitcoin/bitcoin/pull/31385#issuecomment-3138821243)
ACK ea17a9423fb431a86d36927b02d3624f654fd867

I think the doc update gives a decent explanation how this can be used. Code cleanup is nice.
šŸ’¬ instagibbs commented on issue "doc: Mempool Policy documentation Outdated since TRUC":
(https://github.com/bitcoin/bitcoin/issues/32067#issuecomment-3138855557)
You are correct that this no longer holds with TRUC. The caveat can be added.
šŸ’¬ fanquake commented on pull request "[28.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33076#issuecomment-3138907944)
Guix Build (aarch64):
```bash
9ef8edcd797ec28b927c50bcab2bb1ab31f5990ff08671081e6a62391c22c089 guix-build-5492e1be3b40/output/aarch64-linux-gnu/SHA256SUMS.part
d269e0b8610943a4eb061088c139f9840ebbdeb3a1884a2d1f2d13967b01ddf6 guix-build-5492e1be3b40/output/aarch64-linux-gnu/bitcoin-5492e1be3b40-aarch64-linux-gnu-debug.tar.gz
4fc9874cd91344348602a91b6ab680293f6aa1afb5bab54147bdcfe3c69c8777 guix-build-5492e1be3b40/output/aarch64-linux-gnu/bitcoin-5492e1be3b40-aarch64-linux-gnu.tar.gz
899d2d
...