Bitcoin Core Github
43 subscribers
122K links
Download Telegram
πŸš€ achow101 merged a pull request: "kernel: Move background load thread to node context"
(https://github.com/bitcoin/bitcoin/pull/30896)
πŸ€” jonatack reviewed a pull request: "refactor: move m_is_inbound out of CNodeState"
(https://github.com/bitcoin/bitcoin/pull/30233#pullrequestreview-2304335527)
Post-merge ACK 07f4cebe5780f1039541d989e64b70eccc5b4eb5
πŸ’¬ jonatack commented on pull request "refactor: move m_is_inbound out of CNodeState":
(https://github.com/bitcoin/bitcoin/pull/30233#discussion_r1759630416)
FWIW I tried adding an assert here while reviewing this pull and found it unexpectedly difficult. Happy to review if someone does it.

As a sanity check verified both unit and functional tests appear to fail if this line is changed or omitted.
πŸ’¬ sipa commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2350782260)
@achow101 I've added a commit that gets rid of `AutoFile::Get` entirely (based on @davidgumberg's d238c46a44292d5ed81d703089b66be333a68083).
πŸ’¬ petertodd commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1759707389)
@instagibbs What MARA told me was that their nodes do not enforce the dust limit. Notably, that means if Libre Relay or some other implementation removed the dust limit, transactions violating it would get to MARA and be mined.
πŸ’¬ ryanofsky commented on pull request "Fix crash when closing wallet":
(https://github.com/bitcoin-core/gui/pull/835#issuecomment-2350959839)
One way to test difference between the current fix and suggested fix is to delay the notification:

```diff
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -174,7 +174,10 @@ bool RemoveWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet
context.wallets.erase(i);
}
// Notify unload so that upper layers release the shared pointer.
- wallet->NotifyUnload();
+ std::thread([wallet]() {
+ std::this_thread::sleep_for(std::chrono::s
...
πŸ“ hebasto opened a pull request: "cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets"
(https://github.com/bitcoin/bitcoin/pull/30901)
This PR leverages the approach from the https://github.com/chaincodelabs/libmultiprocess project and introduces a new `target_data_sources()` function, which minimizes code needed to assign a `*.json` or `*.raw` data file to the `test_bitcoin` or `bench_bitcoin` target.
πŸ’¬ hebasto commented on pull request "cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets":
(https://github.com/bitcoin/bitcoin/pull/30901#issuecomment-2350962600)
Friendly ping @ryanofsky :)
πŸ’¬ hebasto commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#discussion_r1759725111)
The comment above no longer seems correct.
πŸ“ hebasto opened a pull request: "Remove Autotools packages from depends and CI"
(https://github.com/bitcoin/bitcoin/pull/30902)
This PR is a follow-up to https://github.com/bitcoin/bitcoin/pull/30752 and addresses https://github.com/bitcoin/bitcoin/pull/30752#discussion_r1758594864.
πŸ’¬ hebasto commented on pull request "guix: Drop unused autotools packages":
(https://github.com/bitcoin/bitcoin/pull/30752#discussion_r1759727460)
Thanks! Addressed in https://github.com/bitcoin/bitcoin/pull/30902.
πŸ’¬ fanquake commented on pull request "Remove Autotools packages from depends and CI":
(https://github.com/bitcoin/bitcoin/pull/30902#discussion_r1759727845)
I don't think this is correct. Given we still have depends packages that can output libtool archives (which we currently remove).
πŸ’¬ Sjors commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2350973678)
> when opening or updating a PR that only touches a markdown file, I'd use this

We might still want to run spell check and such things for documentation changes. Skipping unnecessary tasks could be done by CI itself. So I don't think that's an argument for this PR.
πŸ’¬ hebasto commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#discussion_r1759729804)
Adjusted in https://github.com/bitcoin/bitcoin/pull/30901.
πŸ’¬ hebasto commented on pull request "Remove Autotools packages from depends and CI":
(https://github.com/bitcoin/bitcoin/pull/30902#discussion_r1759730052)
Thanks! Reverted back.
πŸ’¬ jonatack commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2350987707)
> > when opening or updating a PR that only touches a markdown file, I'd use this
>
> We might still want to run spell check and such things for documentation changes.

Insofar as the spelling linter in the CI doesn’t raise, running it in the CI isn't very important -- it's a facultative aid that can be run locally by authors and reviewers. Thus, I'd use it on my own pulls to avoid wasting CI cycles where they aren't needed.
πŸ‘ pablomartin4btc approved a pull request: "qt: Translations update"
(https://github.com/bitcoin/bitcoin/pull/30899#pullrequestreview-2304483433)
ACK ae0529576147a1a5bee992574e2cefc8a1fa37d0

<details>
<summary>I've run the translation update tool on <code>master</code> and compared the changes with this branch.</summary>

```

./../bitcoin-maintainer-tools/update-translations.py

git diff --cached --name-only -- src/qt/locale
src/qt/locale/bitcoin_am.ts
src/qt/locale/bitcoin_bn.ts
src/qt/locale/bitcoin_de.ts
src/qt/locale/bitcoin_de_CH.ts
src/qt/locale/bitcoin_gl_ES.ts
src/qt/locale/bitcoin_ru.ts
src/qt/locale/bitcoin_sw.
...
πŸ’¬ naiyoma commented on pull request "p2p: Increase inbound capacity for block-relay only connections":
(https://github.com/bitcoin/bitcoin/pull/28463#discussion_r1759742652)
nit: I’m trying to understand something. According to the PR description, 50% of inbound slots are allocated for tx-relaying peers, but the commit message seems to imply that 50% are for block-relay-only peers. Does this mean that the inbound slots are equally split between tx-relaying and block-relay-only peers? If so, could the description be expanded for clarity?
πŸ’¬ pablomartin4btc commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2351018530)
Since you are here, you could update the translate instructions as well (I've tested them), if @hebasto agrees with the changes:

- `doc/translation_process.md`

```diff
@@ -18,8 +18,8 @@ We use automated scripts to help extract translations in both Qt, and non-Qt sou

To automatically regenerate the `bitcoin_en.ts` file, run the following commands:
```sh
-cd src/
-make translate
+cmake -B build -DWITH_BDB=ON -DBUILD_GUI=ON
+cmake --build build --target
...
πŸ“ hebasto opened a pull request: "cmake: Add `FindZeroMQ` module"
(https://github.com/bitcoin/bitcoin/pull/30903)
This PR introduces the `FindZeroMQ` module, which first attempts to find the `libzmq` library using CMake's `find_package()` and falls back to `pkg_check_modules()` if unsuccessful.

Resolves https://github.com/bitcoin/bitcoin/issues/30876.