💬 hebasto commented on pull request "build: Enable -DCMAKE_EXPORT_COMPILE_COMMANDS=1":
(https://github.com/bitcoin/bitcoin/pull/33187#issuecomment-3204097881)
Some random guy suggests setting an environment variable: https://youtu.be/xNHKTdnn4fY?t=1535 :)
(https://github.com/bitcoin/bitcoin/pull/33187#issuecomment-3204097881)
Some random guy suggests setting an environment variable: https://youtu.be/xNHKTdnn4fY?t=1535 :)
💬 hodlinator commented on pull request "build: Enable -DCMAKE_EXPORT_COMPILE_COMMANDS=1":
(https://github.com/bitcoin/bitcoin/pull/33187#issuecomment-3204431479)
> Some random guy suggests setting an environment variable: https://youtu.be/xNHKTdnn4fY?t=1535 :)
We shouldn't expect the average Bitcoin Core contributor to consume 90min CMake videos but it's great that you do.
He also suggests every tool should support compile_commands.json and any issue/suggestion to do so would be massively upvoted.
(Adjacent issue: I never build within my IDE, so even if it were to set the environment variable automatically, I would not have benefited from that).
...
(https://github.com/bitcoin/bitcoin/pull/33187#issuecomment-3204431479)
> Some random guy suggests setting an environment variable: https://youtu.be/xNHKTdnn4fY?t=1535 :)
We shouldn't expect the average Bitcoin Core contributor to consume 90min CMake videos but it's great that you do.
He also suggests every tool should support compile_commands.json and any issue/suggestion to do so would be massively upvoted.
(Adjacent issue: I never build within my IDE, so even if it were to set the environment variable automatically, I would not have benefited from that).
...
💬 hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2287189020)
Base version for this PR: https://github.com/bitcoin/bitcoin/blob/fa05a726c225dc65dee79367bb67f099ae4f99e6/src/test/headers_sync_chainwork_tests.cpp#L95-L98
That comment would indeed be more useful before the first `ProcessNextHeaders`-call. Will correct if I push again.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2287189020)
Base version for this PR: https://github.com/bitcoin/bitcoin/blob/fa05a726c225dc65dee79367bb67f099ae4f99e6/src/test/headers_sync_chainwork_tests.cpp#L95-L98
That comment would indeed be more useful before the first `ProcessNextHeaders`-call. Will correct if I push again.
💬 davidgumberg commented on pull request "build: Enable -DCMAKE_EXPORT_COMPILE_COMMANDS=1":
(https://github.com/bitcoin/bitcoin/pull/33187#issuecomment-3204539562)
I think that knobs should be set in a way that as much as is reasonable relieves users from the burden of having to pore over [documentation](https://cmake.org/cmake/help/latest/manual/cmake-variables.7.html) to understand what each option does before proceeding successfully, and I maintain that no one has or ever would complain if this was enabled[^1], in this project or in any other project, and many would enjoy living life in sweet ignorance of `CMAKE_EXPORT_COMPILE_COMMANDS=1` while reaping
...
(https://github.com/bitcoin/bitcoin/pull/33187#issuecomment-3204539562)
I think that knobs should be set in a way that as much as is reasonable relieves users from the burden of having to pore over [documentation](https://cmake.org/cmake/help/latest/manual/cmake-variables.7.html) to understand what each option does before proceeding successfully, and I maintain that no one has or ever would complain if this was enabled[^1], in this project or in any other project, and many would enjoy living life in sweet ignorance of `CMAKE_EXPORT_COMPILE_COMMANDS=1` while reaping
...
💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-3204696862)
Thank you for the review @l0rinc!
Rebased 254d0a75b50b0eaf91003ea8a0534981ec740090 -> fc07ce3718b5b8cc168ab634885e0317b9621e8c ([blocktreestore_2](https://github.com/TheCharlatan/bitcoin/tree/blocktreestore_2) -> [blocktreestore_3](https://github.com/TheCharlatan/bitcoin/tree/blocktreestore_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/blocktreestore_2..blocktreestore_3))
Updated fc07ce3718b5b8cc168ab634885e0317b9621e8c -> d35ceaeb463bc836ac4fc4bd6dd4f387647f33fb ([blocktre
...
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-3204696862)
Thank you for the review @l0rinc!
Rebased 254d0a75b50b0eaf91003ea8a0534981ec740090 -> fc07ce3718b5b8cc168ab634885e0317b9621e8c ([blocktreestore_2](https://github.com/TheCharlatan/bitcoin/tree/blocktreestore_2) -> [blocktreestore_3](https://github.com/TheCharlatan/bitcoin/tree/blocktreestore_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/blocktreestore_2..blocktreestore_3))
Updated fc07ce3718b5b8cc168ab634885e0317b9621e8c -> d35ceaeb463bc836ac4fc4bd6dd4f387647f33fb ([blocktre
...
💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285423317)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285423317)
Fixed.
💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285417562)
Yes, removed.
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285417562)
Yes, removed.
💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285428314)
It would be nice if `GetSerializeSize` would be a constexpr. But yes, moving this out.
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285428314)
It would be nice if `GetSerializeSize` would be a constexpr. But yes, moving this out.
💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285424999)
Taken, thanks!
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285424999)
Taken, thanks!
💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285429465)
Done.
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285429465)
Done.
💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285420384)
Done.
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285420384)
Done.
💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285792571)
It would be, but we use the positions with the result of `ftell`, which returns a signed type, so I made them signed too.
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285792571)
It would be, but we use the positions with the result of `ftell`, which returns a signed type, so I made them signed too.
💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285303148)
This should not change across re-orgs. Only the `Chain` class mutates during re-orgs, the block index remains unchanged.
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285303148)
This should not change across re-orgs. Only the `Chain` class mutates during re-orgs, the block index remains unchanged.
💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285416813)
Done.
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285416813)
Done.
💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285420051)
Yes, raw enums are also not portable. This is better, but also required a small change to allow serialization.
(https://github.com/bitcoin/bitcoin/pull/32427#discussion_r2285420051)
Yes, raw enums are also not portable. This is better, but also required a small change to allow serialization.
💬 furszy commented on issue "Indexes stuck on unknown best block after unclean shutdown":
(https://github.com/bitcoin/bitcoin/issues/33208#issuecomment-3205061071)
> I think [@fjahr](https://github.com/fjahr) meant to calculate the MuHash object directly from the current utxo set (as in `bitcoin-cli gettxoutsetinfo 'muhash'` without an coinstatsindex), which takes ~8 minutes on my laptop and doesn't involve reading blocks from disk - still a bit messy though.
ha true, that's interesting.
(https://github.com/bitcoin/bitcoin/issues/33208#issuecomment-3205061071)
> I think [@fjahr](https://github.com/fjahr) meant to calculate the MuHash object directly from the current utxo set (as in `bitcoin-cli gettxoutsetinfo 'muhash'` without an coinstatsindex), which takes ~8 minutes on my laptop and doesn't involve reading blocks from disk - still a bit messy though.
ha true, that's interesting.
👍 dergoegge approved a pull request: "test: modify logging_filesize_rate_limit params"
(https://github.com/bitcoin/bitcoin/pull/33211#pullrequestreview-3135588711)
utACK 5dda364c4b1965da586db7b81de8be90b6919414
(https://github.com/bitcoin/bitcoin/pull/33211#pullrequestreview-3135588711)
utACK 5dda364c4b1965da586db7b81de8be90b6919414
✅ fanquake closed an issue: "ci: failure in `logging_tests`"
(https://github.com/bitcoin/bitcoin/issues/33195)
(https://github.com/bitcoin/bitcoin/issues/33195)
🚀 fanquake merged a pull request: "test: modify logging_filesize_rate_limit params"
(https://github.com/bitcoin/bitcoin/pull/33211)
(https://github.com/bitcoin/bitcoin/pull/33211)
💬 hodlinator commented on pull request "index: Don't commit state in BaseIndex::Rewind":
(https://github.com/bitcoin/bitcoin/pull/33212#discussion_r2287582650)
Right before the kill, an extra `getblockcount()`-call returns `start_height + 2`, same value for `getindexinfo().best_block_height`.
After the kill, we jump back exactly 2 blocks to `start_height`. I guess this is due to the clean shutdown in `restart_node()` on line 319 in the preceding test code above having fully committed that state to disk.
IIUC, could add a comment to that affect:
```suggestion
# Indexes reset to the point we last committed them to disk - during the clean
...
(https://github.com/bitcoin/bitcoin/pull/33212#discussion_r2287582650)
Right before the kill, an extra `getblockcount()`-call returns `start_height + 2`, same value for `getindexinfo().best_block_height`.
After the kill, we jump back exactly 2 blocks to `start_height`. I guess this is due to the clean shutdown in `restart_node()` on line 319 in the preceding test code above having fully committed that state to disk.
IIUC, could add a comment to that affect:
```suggestion
# Indexes reset to the point we last committed them to disk - during the clean
...