💬 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
...
✅ 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)
(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
...
(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
...
💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3205591544)
@theuni wrote:
> Please take a step back and just think like a user. v29 shipped with bitcoind. They'll now see bitcoin, bitcoind, and bitcoin-node.
As discussed above `bitcoin-node` is not in `PATH` since #31679. This has the nice benefit of being able to drop the binary if we go for a different approach later of adding `-ipcbind` to `bitcoind` instead, which is discussed in #30983 (not sure if it's a good idea).
Once we instruct miners to use `bitcoin -m node -ipcbind=unix`, ideally t
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3205591544)
@theuni wrote:
> Please take a step back and just think like a user. v29 shipped with bitcoind. They'll now see bitcoin, bitcoind, and bitcoin-node.
As discussed above `bitcoin-node` is not in `PATH` since #31679. This has the nice benefit of being able to drop the binary if we go for a different approach later of adding `-ipcbind` to `bitcoind` instead, which is discussed in #30983 (not sure if it's a good idea).
Once we instruct miners to use `bitcoin -m node -ipcbind=unix`, ideally t
...
📝 fanquake opened a pull request: "[29.x] Backport logging ratelimiting"
(https://github.com/bitcoin/bitcoin/pull/33225)
Backports:
* #32604
* #33011
* #33211
(https://github.com/bitcoin/bitcoin/pull/33225)
Backports:
* #32604
* #33011
* #33211
💬 fanquake commented on pull request "[29.x] Backport logging ratelimiting":
(https://github.com/bitcoin/bitcoin/pull/33225#discussion_r2287838255)
Adjust?
(https://github.com/bitcoin/bitcoin/pull/33225#discussion_r2287838255)
Adjust?
💬 fanquake commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#issuecomment-3205733317)
Backported in #33225.
(https://github.com/bitcoin/bitcoin/pull/32604#issuecomment-3205733317)
Backported in #33225.
💬 fanquake commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#issuecomment-3205733976)
Backported to 29.x in #33225.
(https://github.com/bitcoin/bitcoin/pull/33011#issuecomment-3205733976)
Backported to 29.x in #33225.
💬 fanquake commented on pull request "test: modify logging_filesize_rate_limit params":
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3205734127)
Backported to 29.x in #33225.
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3205734127)
Backported to 29.x in #33225.
💬 sipa commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3205872366)
> I think we both agree that change could be a followup for a future release and does not need to block the current PR, **but please let me know if I misunderstood and that is not the case, or if you changed your mind about this**.
I don't really see the point in that case.
My (mild) preference, if we are to have IPC in release binaries in 30.0, is to add `-ipcbind` support to `bitcoind`, and not include `bitcoin-node` in the release. This means everyone uses and tests the same binary, isn
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3205872366)
> I think we both agree that change could be a followup for a future release and does not need to block the current PR, **but please let me know if I misunderstood and that is not the case, or if you changed your mind about this**.
I don't really see the point in that case.
My (mild) preference, if we are to have IPC in release binaries in 30.0, is to add `-ipcbind` support to `bitcoind`, and not include `bitcoin-node` in the release. This means everyone uses and tests the same binary, isn
...