💬 paplorinc commented on pull request "refactor: reserve memory allocation for transaction outputs":
(https://github.com/bitcoin/bitcoin/pull/30093#issuecomment-2160989550)
Moved the benchmark out to https://gist.github.com/paplorinc/812007eef71d5285be0654375ea3e03e

💬 ajtowns commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2160992009)
> I think that could reduce the difficulty by up to a factor of 16 (if you are willing to wait up to eight weeks), but I don’t see how someone needing to manually intervene and most likely still needing an ASIC mitigates the potential liveness issue here.

If someone's attacking testnet with 4x more hashpower than the rest of the network (ie, 80% hash rate), can't they just do a 50% attack and mine empty blocks? If you're trying to run a chain with minority hashpower you need to do something l
...

💬 ryanofsky commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1635022608)
re: https://github.com/bitcoin/bitcoin/pull/29996/files#r1633915935

> my proposed fix would be something like [mzumsande@edb2b69](https://github.com/mzumsande/bitcoin/commit/edb2b69a16889552ddd40b71a491e7722d9b4e12) (feel free to cherry-pick/ adjust as you like).

Nice find! Would suggest opening a separate PR so it is easier to understand the problem and fix. And maybe it is possible to come up with a simpler test for this problem specifically, like by adding an assert in FindNextBlocks()
...

💬 theuni commented on pull request "refactor: reserve memory allocation for transaction outputs":
(https://github.com/bitcoin/bitcoin/pull/30093#discussion_r1635062430)
One last nit: because it's not modified after creation, `const auto& txout`

💬 paplorinc commented on pull request "refactor: reserve memory allocation for transaction outputs":
(https://github.com/bitcoin/bitcoin/pull/30093#discussion_r1635065616)
Done, thanks

💬 ryanofsky commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1635083945)
re: https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1634607244

> To clarify the meaning of the Todo:
>
> > Interesting starting states could be loading a snapshot when the current chain tip is:
> >
> > * TODO: Not an ancestor of the snapshot block but has less work
>
> Particularly "but has less work" could mean A) less work than the tip of the chain that includes the snapshot or B) less work than the snapshot block itself. I believe A is the more interesting and intended s
...

👍 TheCharlatan approved a pull request: "depends: remove `FORCE_USE_SYSTEM_CLANG`"
(https://github.com/bitcoin/bitcoin/pull/30201#pullrequestreview-2110816913)
Nice, ACK 7cbfd7a7ce45ac68d6041f42f468862f5c193d8c


Guix builds (aarch64):
```
f8487a885a04a4b3c273b2d1ba3ef5e100026f16b03c08e866dbf4cd468d0802 guix-build-7cbfd7a7ce45/output/aarch64-linux-gnu/SHA256SUMS.part
b3da7604bc7302213d2864bddbccc54ede6374711377b89b2d17a989d2e9e64d guix-build-7cbfd7a7ce45/output/aarch64-linux-gnu/bitcoin-7cbfd7a7ce45-aarch64-linux-gnu-debug.tar.gz
d2340b2c084faf01a0db8ea0c077ba9f8c51ef23b680396d88001b33d126788b guix-build-7cbfd7a7ce45/output/aarch64-linux-gnu/
...

💬 mzumsande commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1635106225)
> Sounds good to me in principle but before doing 3 ensure that the node's tip is actually the tip of the divergent chain (...)

Maybe I misunderstand, but that seems overly complicated. I assume we're talking about the scenario "Not an ancestor of the snapshot block but has less work":
In my local test, I just gave the node the headers of the snapshot chain, and then used `-generate` to mine a divergent chain from the old tip. Number of blocks doesn't really matter. The other chain (with the
...

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1635107884)
Nice, agree that's better. However e4e14fedc9622ca7cfc40af4f2aa70ed4bb7daa6 is now unnecessary and can be dropped.

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1635108539)
I think so...

🚀 fanquake merged a pull request: "depends: remove `FORCE_USE_SYSTEM_CLANG`"
(https://github.com/bitcoin/bitcoin/pull/30201)

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1635112115)
Aha yes. Can resolve.

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1635117157)
even better. dropped.

💬 theuni commented on pull request "build: use `-no_exported_symbols` on macOS":
(https://github.com/bitcoin/bitcoin/pull/29072#issuecomment-2161096437)
Does ` __attribute__ ((visibility ("default")))` override this? Otherwise I assume we'd need to make sure this flag doesn't make it to shared libs.

💬 theuni commented on pull request "build: use `-no_exported_symbols` on macOS":
(https://github.com/bitcoin/bitcoin/pull/29072#issuecomment-2161099989)
> Does ` __attribute__ ((visibility ("default")))` override this? Otherwise I assume we'd need to make sure this flag doesn't make it to shared libs.

Oh whoops, already asked and answered. Nevermind!

👍 tdb3 approved a pull request: "assumeutxo: Check snapshot base block is not in invalid chain"
(https://github.com/bitcoin/bitcoin/pull/30267#pullrequestreview-2110894428)
ACK cbc604f6a7739bf22a5d811b6366453860d59f4c
Seems like an improvement to avoid expending resources loading a snapshot that won't be part of a valid chain.

Built and ran unit and functional tests (including `feature_assumeutxo`). All passed.

Tested that `feature_assumeutxo` would fail with:
```c++
bool start_block_invalid = false;
```
Test failed as expected.

💬 tdb3 commented on pull request "assumeutxo: Check snapshot base block is not in invalid chain":
(https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1635144969)
> If the base block of the snapshot is marked invalid or part of an invalid chain

This existing test seems to focus on the base block being part of an invalid chain. Is it also worth adding an explicit test for an invalid base block itself (or would it be overkill)?

💬 alfonsoromanz commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1635147026)
> Particularly "but has less work" could mean A) less work than the tip of the chain that includes the snapshot or B) less work than the snapshot block itself. I believe A is the more interesting and intended scenario because B only seems to be a state a node can be in if the snapshot block is invalid or its ancestors are unknown which should lead to a much earlier error.

I interpreted it as B) less work than the snapshot block itself.

Regarding A): If the new node (divergent chain) has le
...

📝 theuni opened a pull request: "utils: add missing VecDeque include"
(https://github.com/bitcoin/bitcoin/pull/30268)
Noticed when testing `VecDeque` with no other includes.

For libc++, need type_traits for `std::is_trivially_destructible_v`.

👍 theuni approved a pull request: "refactor: reserve memory allocation for transaction outputs"
(https://github.com/bitcoin/bitcoin/pull/30093#pullrequestreview-2110966158)
utACK db2a31c339ad697f1828cde162f4df2231c9b0f1