💬 achow101 commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2091971018)
As-is is more legible, and this area of code changes significantly in #27865 where having it like it is now makes those changes easier to read.

💬 hodlinator commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2885100223)
### Re: One file per block

FWIW the idea of using one file per block gets me going too. :) It is slightly orthogonal but would be nice to avoid changing formats twice in short succession.

My bikeshed color: Since block hashes start with zeroes, maybe one could shard based off the last two bytes:

Genesis block ends up in something like:
`$DATADIR/blocks/e2/6f/000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f.dat`
Block [896819](https://mempool.space/block/000000000000000
...

💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2091989087)
Worked, force pushed [449cb9e](https://github.com/bitcoin/bitcoin/commit/449cb9e8985b350a96ddad74a9997ac95118edd0)

💬 davidgumberg commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2092020703)
Agreed

💬 mzumsande commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2091928728)
nit: I'd replace "here" with "before setting m_blockfiles_indexed" so the reason becomes more clear.

💬 mzumsande commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2092011943)
would be good to add a subtest that a run without `-assumevalid` will not connect the invalid block.

💬 mzumsande commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2091930195)
I'd prefer to call it `activate_all_chainstates` to not introduce a less precise term ("update").

💬 mzumsande commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2091983914)
This means that in case of a reindex, `ActivateBestChain()`, will be called twice now from `ImportBlocks`, but that shouldn't be a problem because the second call just won't do any work.

💬 mzumsande commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2092020053)
is there a reason we need to mine that many blocks, and not just a few more to make the test faster?

💬 mzumsande commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2092014654)
Comment needs updating, `-assumevalid` points to the last block, not the invalid one.

💬 davidgumberg commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#discussion_r2092025994)
Question: Why is it necessary to search for mt.exe here, but not in the `windows-native-dll` job above?

💬 fkorotkov commented on issue "Revisiting us self-hosting parts of our CI":
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-2885174918)
Cirrus CI founder here. Just wanted to add 2 cents that we now also have a service of self-hosted runners called [Cirrus Runners](https://cirrus-runners.app/) with fast machines and bigger caches. Main difference is also that it's fixed price for unlimited minutes per month hence we buy servers off traditional cloud like AWS/GCP/Azure.

💬 Drew72-ita commented on issue "LevelDB error: Corruption: block checksum mismatch didn't trigger reindex.":
(https://github.com/bitcoin/bitcoin/issues/30138#issuecomment-2885200231)
I recently encountered this exact issue on a production Raspibolt node. Bitcoin Core was stuck in a restart loop due to a sudden corrupted txindex file (*.ldb) (cosmic ray XD?) . It wasn’t immediately obvious what was going on—the logs showed a LevelDB error, but there was no clear indication that I needed to delete the index manually or restart with -reindex.

It took me quite some time to understand that a manual cleanup was required. At the very least, the log message should clearly explain t
...

🤔 ismaelsadeeq reviewed a pull request: "qa: feature_framework_startup_failures.py fixes & improvements (#30660 follow-up)"
(https://github.com/bitcoin/bitcoin/pull/32509#pullrequestreview-2845113491)
Concept ACK
thanks for following up.

💬 ismaelsadeeq commented on pull request "qa: feature_framework_startup_failures.py fixes & improvements (#30660 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/32509#discussion_r2092055590)
In "qa: Handle --timeout-factor=0" 1235a0df5ff6278d07dc95e7acc3ccb14f536286 whats the rationale behind multiplying by factor of 60?

Wondering if this is the right fix? if I pass timeout-factor=99998 the test will fail, but on master I dont think running the functional test with timeout-factor=99998 will trigger any failure.

💬 ismaelsadeeq commented on pull request "qa: feature_framework_startup_failures.py fixes & improvements (#30660 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/32509#discussion_r2092055972)
Nice this is much better I think.

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091927852)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091450477

I wouldn't necessarily expect a user looking for help to know whether they are supposed to type `bitcoin help` or `bitcoin --help` or something else, so it seems reasonable to support both or these, as long as they are both fit in with other syntax we support. `git` could be a model here as well since it also supports both `git --help` and `git help`.

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091881632)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091131888

> Shouldn't we throw when that happen?

Yes, that's a good idea, added this.

🤔 ryanofsky reviewed a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2844819831)
Thanks for the reviews!

Updated 7af6e1089ea264e870b26ac83e81e7aa374acbe1 -> a5ac43d98d1ad3ebed934f2c50208a85aae17e5e ([`pr/wrap.33`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.33) -> [`pr/wrap.34`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.34), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/wrap.33..pr/wrap.34)) just updating copyright and adding exception to handle an unexpected condition

re: https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2
...

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091922386)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091447427

I can believe that it might not be good to allow certain options with commands, but would be helpful to know some specific examples you think are problems?

I don't like suggested change very much because it seems to provide less information to users while making the logic more deeply nested and complicated, and doesn't seem to follow the pattern of trying to provide usage information when the command is misused.

Bu
...