💬 zPapaBear commented on issue "Error when launching Bitcoin Core":
(https://github.com/bitcoin/bitcoin/issues/29995#issuecomment-2082960674)
> > alright, how can i know which file is corrupt and fix it?
>
> I think a `-reindex-chainstate` should be good enough.

alright, where can i run this command though?

👍 brunoerg approved a pull request: "fuzz: don't allow adding duplicate transactions to the mempool"
(https://github.com/bitcoin/bitcoin/pull/29990#pullrequestreview-2028770893)
ACK cc15c5bfd1eb3903de246c124702a7c66c687008

💬 zPapaBear commented on issue "Error when launching Bitcoin Core":
(https://github.com/bitcoin/bitcoin/issues/29995#issuecomment-2082984800)
> > alright, how can i know which file is corrupt and fix it?
>
> I think a `-reindex-chainstate` should be good enough.

still same launch result.

💬 hebasto commented on pull request "build: Assume HAVE_CONFIG_H, Add IWYU pragma keep to bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29494#issuecomment-2082991136)
While testing this PR I faced the problem that running linters [locally](https://github.com/bitcoin/bitcoin/tree/master/test/lint#running-locally) alters the ownership of some files in `src` to the `root` user. How to avoid such behaviour?

💬 maflcko commented on issue "Error when launching Bitcoin Core":
(https://github.com/bitcoin/bitcoin/issues/29995#issuecomment-2082999834)
Is this an external HDD? What is the filesystem?

💬 maflcko commented on pull request "build: Assume HAVE_CONFIG_H, Add IWYU pragma keep to bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29494#issuecomment-2083003826)
I didn't add the docker image, nor is it modified in this pull request. If you have questions about docker or the linter usage, a separate issue may be more appropriate.

📝 alfonsoromanz opened a pull request: "test: Assumeutxo: import snapshot in a node with a divergent chain"
(https://github.com/bitcoin/bitcoin/pull/29996)
This PR adds tests to cover two scenarios of loading a snapshot when the current chain tip is:

- Not an ancestor of the snapshot block but has less work
- Not an ancestor or a descendant of the snapshot block and has more work

In the second scenario, the snapshot block does not belong to the most-work chain anymore so I believe it covers this scenario too: `TODO: Valid snapshot file and snapshot block, but the block is not on the
most-work chain`. Therefore I deleted that TODO as w
...

💬 zPapaBear commented on issue "Error when launching Bitcoin Core":
(https://github.com/bitcoin/bitcoin/issues/29995#issuecomment-2083008473)
> Is this an external HDD? What is the filesystem?

Windows 10, internal HDD.

💬 maflcko commented on pull request "build: Assume HAVE_CONFIG_H, Add IWYU pragma keep to bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29494#issuecomment-2083011036)
If you want to run locally, my recommendation would be to use the `test/lint/test_runner`.

💬 hebasto commented on pull request "build: Assume HAVE_CONFIG_H, Add IWYU pragma keep to bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29494#issuecomment-2083018272)
> > How is it supposed to automatically catch cases when code changes make `#include <config/bitcoin-config.h>` unneeded in a header or source file?
>
> Yes, it is supposed to catch those. This is already the case and unrelated to this pull request. You can try it yourself. The output will be:
>
> ```
> ^^^
> None of the files use a symbol declared in the bitcoin-config.h header. However, they are including
> the header. Consider removing the unused include.
>
> ^---- ⚠️
...

💬 maflcko commented on issue "Error when launching Bitcoin Core":
(https://github.com/bitcoin/bitcoin/issues/29995#issuecomment-2083019057)
Which filesystem is the HDD that holds the datadir? For example, NTFS is a possible filesystem.

In any case, this error should not happen, unless there is a serious bug, or the data on the storage is corrupt. If `-reindex-chainstate` does not work, you can try `-reindex`, but they will only fight the symptom temporarily. The issue will come back at some point, because if it was a bug, it will remain unfixed, or if the storage is corrupt, it will also not be fixed by setting a flag in the sof
...

💬 maflcko commented on pull request "build: Assume HAVE_CONFIG_H, Add IWYU pragma keep to bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29494#issuecomment-2083023858)
> The linter silently removes the added `#include <config/bitcoin-config.h> // IWYU pragma: keep` lines with no message.

Again, this is not about the linter. This is an issue with the docker setup, which I didn't write, and which is not changed in this pull request. If the issue persists in the lint/test_runner, then let me know. Otherwise, this is a separate issue.



> To elaborate my concerns, adding `// IWYU pragma: keep` precludes the IWYU from notifying us about the cases when the `
...

💬 zPapaBear commented on issue "Error when launching Bitcoin Core":
(https://github.com/bitcoin/bitcoin/issues/29995#issuecomment-2083024584)
> Which filesystem is the HDD that holds the datadir? For example, NTFS is a possible filesystem.
>
> In any case, this error should not happen, unless there is a serious bug, or the data on the storage is corrupt. If `-reindex-chainstate` does not work, you can try `-reindex`, but they will only fight the symptom temporarily. The issue will come back at some point, because if it was a bug, it will remain unfixed, or if the storage is corrupt, it will also not be fixed by setting a flag in th
...

💬 alfonsoromanz commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#discussion_r1583270129)
Maybe the line that should be removed here is this one? `- TODO: A descendant of the snapshot block`. I believe the scenario where the snapshot block is not on the most-work chain is addressed here: https://github.com/bitcoin/bitcoin/pull/29996

👍 hebasto approved a pull request: "build: Assume HAVE_CONFIG_H, Add IWYU pragma keep to bitcoin-config.h includes"
(https://github.com/bitcoin/bitcoin/pull/29494#pullrequestreview-2028864535)
ACK fa4673c7dd465f18b1d7b2042262123992e846df, I have reviewed the code and it looks OK. Tested on Ubuntu 23.10.

📝 maflcko opened a pull request: "rpc: Remove index-based Arg accessor"
(https://github.com/bitcoin/bitcoin/pull/29997)
The index-based Arg accessor is redundant with the name-based one. It does not provide any benefit to the code reader, or otherwise, so remove it.

💬 maflcko commented on pull request "RPC: access RPC arguments by name":
(https://github.com/bitcoin/bitcoin/pull/29277#discussion_r1583297727)
Done in https://github.com/bitcoin/bitcoin/pull/29997

💬 maflcko commented on pull request "Lint: support running individual rust linters and improve subtree exclusion":
(https://github.com/bitcoin/bitcoin/pull/29965#discussion_r1583306274)
nit in 88b824a80b5c955285ef2ddc490e086bb40a890c: Given that you are adding features to the lint runner which are not trivially available in the docker way (above), it could make sense to move the docker option down by a section (only as a fallback), as it seems to have bugs anyway, according to https://github.com/bitcoin/bitcoin/pull/29494#issuecomment-2083018272

💬 stickies-v commented on pull request "doc: removed help text saying that peers may not connect automatically":
(https://github.com/bitcoin/bitcoin/pull/29994#discussion_r1583309265)
I don't think we should remove default ports or the I2P doc
```suggestion
argsman.AddArg("-port=<port>", strprintf("Listen for connections on <port> (default: %u, testnet: %u, signet: %u, regtest: %u). Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
```

🤔 stickies-v reviewed a pull request: "doc: removed help text saying that peers may not connect automatically"
(https://github.com/bitcoin/bitcoin/pull/29994#pullrequestreview-2028885329)
Concept ACK. According to https://bitnodes.io/dashboard/, ~85% of public nodes are running 23 or higher.