Bitcoin Core Github
42 subscribers
126K links
Download Telegram
🤔 janb84 reviewed a pull request: "build: Introduce internal kernel library"
(https://github.com/bitcoin/bitcoin/pull/28690#pullrequestreview-3552815631)
ACK 4c387e938015de197860215d94b9c9aee9a82da8

Looks good to me, next to several builds & tests, I did a (subsection of a ) Guix build to see if there where any issues with that. It ran fine, see below.

my Guix Build Output

**Host architecture:** `aarch64`
**Commit:** `4c387e938015`

```shell
0eea1fa55868cf288544de481d7ae5e8285f068074536723e927c20b9ece0518 guix-build-4c387e938015/output/dist-archive/bitcoin-4c387e938015.tar.gz
bf648f3ef27667e011b2c4547987527111c3d5c286933d092ad826
...
🤔 sipa reviewed a pull request: "precalculate SipHash constant salt XORs"
(https://github.com/bitcoin/bitcoin/pull/30442#pullrequestreview-3552798362)
Approach ACK
💬 sipa commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2599180166)
In commit "optimization: introduce PresaltedSipHasher for repeated hashing"

Unrelated change, which does not match style guide.
💬 sipa commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2599216213)
In commit "refactor: extract shared SipHash state into SipSalt"

These could be doxygen comments (`/** Equivalent to ... */`).
💬 sipa commented on pull request "precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2599207202)
In commit "refactor: extract shared SipHash state into SipSalt"

Given that instances of this class are apprently used for the internal state in `CSipHasher` and `PresaltedSipHasher`, maybe it would be more appropriate to call it `SipHashState` or so?
💬 darosior commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2599232376)
I hadn't seen your other comment. Makes sense.
🤔 sipa reviewed a pull request: "validation: periodically flush dbcache during reindex-chainstate"
(https://github.com/bitcoin/bitcoin/pull/32414#pullrequestreview-3552937494)
utACK c1e554d3e5834a140f2a53854018499a3bfe6822
🤔 sipa reviewed a pull request: "refactor: C++20 operators"
(https://github.com/bitcoin/bitcoin/pull/33771#pullrequestreview-3552981158)
ACK 48840bfc2d7beeac0ddf56a3c26b243156ec8936
🤔 sipa reviewed a pull request: "netbase: Remove "tor" as a network specification"
(https://github.com/bitcoin/bitcoin/pull/34031#pullrequestreview-3553002765)
ACK 7efb18c38150344ca6f7efbcd8441792a2ea921f

14 major releases ought to be a long enough deprecation period.
🚀 fanquake merged a pull request: "refactor: C++20 operators"
(https://github.com/bitcoin/bitcoin/pull/33771)
💬 fanquake commented on pull request "net: Waste less time in socket handling":
(https://github.com/bitcoin/bitcoin/pull/34025#issuecomment-3628023796)
cc @theuni @vasild
💬 fanquake commented on pull request "netbase: Remove "tor" as a network specification":
(https://github.com/bitcoin/bitcoin/pull/34031#issuecomment-3628025038)
cc @laanwj
💬 pablomartin4btc commented on pull request "argsman, cli: GNU-style command-line option parsing (allows options after non-option arguments)":
(https://github.com/bitcoin/bitcoin/pull/33540#issuecomment-3628033533)
-<ins>_**Updates**_</ins>:
- Added @w0xlt's [fix](https://github.com/bitcoin/bitcoin/pull/33540#pullrequestreview-3490620415) on long (double dash --) options that [worked](https://github.com/bitcoin/bitcoin/pull/33540#pullrequestreview-3490254189) on `master`, adding a new test for it (the test would pass in `master` but fail without @w0xlt's changes).
- Refactored both `ArgsManager::ProcessOptionKey` (new function added in this PR) and `ArgsManager::ParseParameters` in separated commits maki
...
💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2599397406)
> I think hodlinator's solution adding requires clauses might avoid the need to specialize [#34006 (comment)](https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2592454740)

No, I don't think this will generally be more correct. `std::expected<void, B>::value()` exists, so hiding it will make it differ from the std version.

I think the only way to replicate all behavior of `std::expected` 100%, would be to implement it exactly the same way. I'd prefer not to write the void-specializa
...
🤔 sipa reviewed a pull request: "miniscript refactor: Remove unique_ptr-indirection"
(https://github.com/bitcoin/bitcoin/pull/31713#pullrequestreview-3553094316)
reACK d782bffc5461f7361ae410963466ae1577ef63df
💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2599407230)
It'll be used in the kernel, soon. So I'd rather avoid the churn. In any case, it will likely remain empty, and behavior should all be the same. So I think it doesn't matter and my preference would be to just keep this as-is for now.
💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2599410546)
ok, i'll try to fixup the second commit tomorrow, or in a follow-up, whichever happens earlier.
🤔 hebasto reviewed a pull request: "guix: reduce allowed exported symbols"
(https://github.com/bitcoin/bitcoin/pull/33950#pullrequestreview-3553159236)
> FWIW i cherry-picked commit [7b90b4f](https://github.com/bitcoin/bitcoin/commit/7b90b4f5bb10e2156709b07e3996f867e2421232) on top of [b354d1c](https://github.com/bitcoin/bitcoin/commit/b354d1ce5c47997aa2f93910e05c0f8daa8736eb) (the commit before #33181). A full guix build finished succesfully. So it must have been something earlier even!

The `_fini` and `_init` symbols have no longer been exported since commit b5fc6d46a3854c18f6e8dfc89540d24ef778caa2 in https://github.com/bitcoin/bitcoin/pull/
...
💬 ryanofsky commented on issue "Memory leak when using IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3628241016)
re: https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3611006053

>* `git clone https://github.com/stratum-mining/sv2-apps -b v0.1.0`

Thanks for the instructions. I've been experimenting with the v0.1.0 pool_sv2 app on regtest with a python script to generate transactions & connect blocks quickly to make a memory leak happen without needing to wait a long time.

So far, behavior has been mostly as expected, where generating lots of transactions without connecting new blocks causes bl
...
💬 Sjors commented on issue "Memory leak when using IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3628281213)
> if the python client connected blocks too quickly, the mining client seemed to fall behind and only release templates after a delay

There's a hardcoded 10(?) second grace period where the Template Provider (both implementations) hold on to templates after a new tip is connected. It's there for two reasons:

1. Trying to relay the block anyway (requires further node changes, since we currently honour the first-seen rule even when that means "our" block loses)
2. Giving pools a chance to verify
...
💬 theuni commented on issue "Net split meta issue":
(https://github.com/bitcoin/bitcoin/issues/33958#issuecomment-3628282838)

> In particular, I think good goals to have here are (not in priority order):
>
> 1. simplify the codebase so it's easier to understand and update
>
> 2. improve robustness of the codebase, so that if there are bugs, they have less impact
>
> 3. make the code higher performance, so that we can have more peers, and/or run on cheaper hardware
>

Goal #1 is a logical separation of concerns. Today, the many of interactions between `CConnman` and `PeerManager` are subtle and (I'd gue
...