Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1440256210)
> I don't know much about how the CI works, maybe it's possible to install a newer clang version? I'm using 15.0.7

Ah, that makes sense.

Maybe keep this PR diff CI-driven? I assume that bumping tools versions in a CI task will force us to fix more warnings, not just those you have already [mentioned](https://github.com/bitcoin/bitcoin/pull/26642#pullrequestreview-1302031747).
💬 roconnor-blockstream commented on pull request "test: Replace 0xC0 constant":
(https://github.com/bitcoin/bitcoin/pull/27143#issuecomment-1440257802)
Updated. I've left `VALID_LEAF_VERS` alone because, logically speaking, the definition of the valid tapleaf versions (defined in BIP-341) is independent of the choice of which tapleaf version to assign to tapscript (defined in BIP-342).
📝 theuni opened a pull request: "kernel: add missing include"
(https://github.com/bitcoin/bitcoin/pull/27144)
This syncs the cs_main definition/declaration.

Noticed when experimenting with the external visibility of `cs_main`.

Specifically, this is needed for the following to work as intended:
```c++
__attribute__ ((visibility ("default"))) extern RecursiveMutex cs_main;
```
💬 pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1114563559)
Thanks Jon I believe I addressed all the style nits and reduced the number of generated blocks as requested in ec7dab684f172c7df37b3a9fe16250bedc2b775f. Adding test coverage for flush to disk is next...
💬 theuni commented on issue "`libbitcoinkernel`: Building `mingw-w64` dll's broken":
(https://github.com/bitcoin/bitcoin/issues/25008#issuecomment-1440326006)
@TheCharlatan symbol visibility is really complex, but it boils down to 2 main choices on our side, see here: https://github.com/bitcoin/bitcoin/issues/25008#issuecomment-1113661684

We can either ship a bunch of dll's (kernel, libsecp256k1, libstdc++, libpthread, etc)
_or_
We can roll everything up into 1 dll and have it just work.

For now, I propose we do the latter. However, this means we need tight control over our symbol visibility. If you're not familiar, see here for the canonical
...
👍 willcl-ark approved a pull request: "docs: add ramdisk guide for running tests on OSX"
(https://github.com/bitcoin/bitcoin/pull/27124)
ACK cb7be3a237c7e5a3d09f24259be015557dc72bca

Tested on my Macbook, worked as expected and speeded up the test suite noticeably.
💬 jamesob commented on pull request "assumeutxo: keep cache when flushing snapshot (#17487 followup)":
(https://github.com/bitcoin/bitcoin/pull/27008#discussion_r1114659889)
Sounds good, fixed.
💬 jamesob commented on pull request "assumeutxo: keep cache when flushing snapshot (#17487 followup)":
(https://github.com/bitcoin/bitcoin/pull/27008#discussion_r1114653412)
Good point! Fixed.
💬 achow101 commented on pull request "prune, import: allow pruning to work during loadblock import":
(https://github.com/bitcoin/bitcoin/pull/24957#issuecomment-1440481217)
re-ACK c4981e7f63a3e0aeec1ef3dec040261e993dd724
💬 jamesob commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#discussion_r1114701299)
For what it's worth, in future PRs I think this possibility is eliminated, since calls like `ActiveChainstate().ActivateBestChain()` in ProcessNewBlock are replaced with specific chainstate references.
fanquake closed an issue: "migratewallet help is misleading about encrypted wallets"
(https://github.com/bitcoin/bitcoin/issues/27048)
🚀 fanquake merged a pull request: "wallet: be able to specify a wallet name and passphrase to migratewallet"
(https://github.com/bitcoin/bitcoin/pull/26595)
👍 theStack approved a pull request: "test: Replace 0xC0 constant"
(https://github.com/bitcoin/bitcoin/pull/27143)
ACK c3b4b5a142b204ceeca4e9b1ca1e2ff41ddd1308
💬 jamesob commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#issuecomment-1440503591)
Rebased and reworded the commit message that @achow101 pointed out was out of date. Also fixed two minor items that @ariard noted.

In attempt to encourage re-ACKers, here is the diff (including rebase items):

```diff
diff <(git diff --no-color au.complete.10~9..au.complete.10) <(git diff --no-color au.complete.11~9..au.complete.11)
109c109
< index 60acb614b4..d36184fed7 100644
---
> index ba1024d22e..cd82d8743c 100644
138c138
< - if (nPruneTarget == std::numeric_limits<uint64_t>:
...
💬 jamesob commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#discussion_r1114711213)
Fixed.
💬 jamesob commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#discussion_r1114710206)
Fixed.
💬 jamesob commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#discussion_r1114710366)
Good for a follow-up.
💬 jamesob commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#discussion_r1114711271)
Fixed.
🚀 fanquake merged a pull request: "test: Raise PRNG seed log to INFO"
(https://github.com/bitcoin/bitcoin/pull/27137)