Bitcoin Core Github
45 subscribers
118K links
Download Telegram
💬 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
...
💬 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
...
📝 fanquake opened a pull request: "[29.x] Backport logging ratelimiting"
(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?
💬 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.
💬 fanquake commented on pull request "log: rate limiting followups":
(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.
💬 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
...
🤔 janb84 reviewed a pull request: "Add functional test for IPC interface"
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3136209292)
ACK 088dc2c486af0ad6803d919d8114ab29d3cd652f (after base #31802)

This PR adds some functional tests for the IPC interface, setups the CI to use libmultiprocess and changes build documentation to match the changes in this PR.

Current state of the PR works on nix on macOS, builds & test run without trouble.

- build
- tested
- code review

small NIT if retouched, please see the remarks of DrahtBot's LLM linter.
💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206024287)
> If this PR is merged as-is now, and later -ipcbind support is added to bitcoind too, then we end up in a situation where we're literally shipping two feature-equivalent binaries (bitcoin-node and bitcoind),

I don't see how that would make sense. We should simply drop the libexec/bitcoin-node and libexec/bitcoin-gui binaries if they do not have distinguishing features at that point.

I feel like I keep pointing out difference between the `bin/` and `libexec/` directories, and you and Cory
...