Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 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.
💬 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
...
💬 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
...
💬 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.
💬 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.
💬 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.
💬 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!
💬 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.
💬 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.
💬 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.
💬 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.
💬 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.
💬 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.
💬 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.
👍 dergoegge approved a pull request: "test: modify logging_filesize_rate_limit params"
(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)
🚀 fanquake merged a pull request: "test: modify logging_filesize_rate_limit params"
(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
...
Eunovo closed a pull request: "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict"
(https://github.com/bitcoin/bitcoin/pull/29680)
💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3205520649)
re: https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3200849464

> ### RFM?
>
> With all that, I feel PR has had enough to discussion and review to be ready for merge given its impact.

I think this is still ready for merge but obviously want to give this more time after the discussion yesterday. I would like to merge this today though, if this is reasonable.

To summarize the discussion yesterday I added the following sections to the [review summary](https://github.com/bitcoi
...