Bitcoin Core Github
44 subscribers
122K links
Download Telegram
πŸ’¬ stringintech commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2489983149)
> but the remaining code won't be called anyways

But previously we would call `InvalidChainFound()` (and the rest of the code after the `m_chain.Contains(to_mark_failed)` check) on the last disconnected tip even if we were interrupted; right?
πŸ’¬ stringintech commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#issuecomment-3485329617)
Concept ACK
πŸ’¬ l0rinc commented on pull request "refactor: inline constant return values from `dbwrapper` write methods":
(https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3485386361)
thank you for the reviews.
rfm?
πŸ’¬ hebasto commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#discussion_r2490096100)
> I guess yaml anchors could be used, but this would increase the diff and review effort again when it is deleted. An alternative would be to use a matrix, but I guess this requires depending both cross-test tasks on both cross-compile tasks? I'd say the current version is fine, but no strong opinion.

You’re right. I considered both of these options while working on this PR and came to the same conclusions.
βœ… fanquake closed an issue: "ci: spurious linter failure?"
(https://github.com/bitcoin/bitcoin/issues/33773)
πŸ‘ stickies-v approved a pull request: "log: reduce excessive "rolling back/forward" messages during block replay"
(https://github.com/bitcoin/bitcoin/pull/33443#pullrequestreview-3415849302)
ACK 1fc7a81f1f5f30ba3ebe305ac2a520c7b4afb596
πŸ’¬ stickies-v commented on pull request "log: reduce excessive "rolling back/forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2490081033)
nit: these log statements might be useful for debugging. An alternative would be to use `LogPrintLevel` with varying log levels:

<details>
<summary>git diff on 1fc7a81f1f</summary>

```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index deba0711b9..4fbd9124cd 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4899,9 +4899,9 @@ bool Chainstate::ReplayBlocks()
LogError("RollbackBlock(): ReadBlock() failed at %d, hash=%s\n", pindexOld->nHeight,
...
πŸ’¬ stickies-v commented on pull request "log: reduce excessive "rolling back/forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2490123498)
nit: the duplicated condition checking and increased nesting can be avoided by using `std::call_once`
πŸ’¬ maflcko commented on pull request "ci: Fix lint runner selection (and docker cache)":
(https://github.com/bitcoin/bitcoin/pull/33744#issuecomment-3485496156)
Didn't look at the second commit, which is written in Bash, but I guess this is harmless and fine either way.

re-ACK 7632e0ba312a372259897c68fd7c7eb723df3738 πŸ“ž

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdY
...
πŸ‘ hebasto approved a pull request: "doc: update Guix INSTALL.md"
(https://github.com/bitcoin/bitcoin/pull/33574#pullrequestreview-3415958912)
ACK b4d0288c467f82a94041b51d10d38e66bb5c33ae.

I currently use Debian Sid and Ubuntu 25.10, and the updated documentation reflects my expectations for installing Guix on these systems.
πŸš€ hebasto merged a pull request: "doc: update Guix INSTALL.md"
(https://github.com/bitcoin/bitcoin/pull/33574)
βœ… hebasto closed an issue: "ci: Lint task caching does not work?"
(https://github.com/bitcoin/bitcoin/issues/33735)
πŸš€ hebasto merged a pull request: "ci: Fix lint runner selection (and docker cache)"
(https://github.com/bitcoin/bitcoin/pull/33744)
πŸ’¬ fanquake commented on pull request "ci: Fix lint runner selection (and docker cache)":
(https://github.com/bitcoin/bitcoin/pull/33744#issuecomment-3485571657)
Backported to `30.x` in #33609.
πŸ’¬ fanquake commented on pull request "Add createwallet, createwalletdescriptor, and migratewallet to history filter":
(https://github.com/bitcoin-core/gui/pull/901#issuecomment-3485578511)
Backported to `30.x` in https://github.com/bitcoin/bitcoin/pull/33609.
πŸ’¬ l0rinc commented on pull request "[30.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33609#issuecomment-3485583769)
Can you please add https://github.com/bitcoin/bitcoin/pull/33336 to the queue as well?
πŸ’¬ kristapsk commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-3485606391)
Concept ACK
πŸ’¬ stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3485634089)
re-ACK 6c7a34f
<details>
<summary>nits if/when you have to retouch</summary>

```diff
diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
index 8bba3cf1c0..07b59d9543 100644
--- a/src/kernel/bitcoinkernel.cpp
+++ b/src/kernel/bitcoinkernel.cpp
@@ -602,11 +602,11 @@ void btck_transaction_output_destroy(btck_TransactionOutput* output)
}

int btck_script_pubkey_verify(const btck_ScriptPubkey* script_pubkey,
- const int64_t amount,
+
...
πŸ’¬ hebasto commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3485712938)
> > This one simply addresses your comments [#33593 (comment)](https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3460836590) and [#33593 (review)](https://github.com/bitcoin/bitcoin/pull/33593#pullrequestreview-3398698978).
>
> Somewhat, but if this is the approach, it means that not only the compiler, but also the version of the headers being tested isn't going to match Guix? (I'm wondering if internalising the headers might be a better approach, as that would be more flexible, and,
...
πŸ’¬ optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3485734385)
CI failed, limits of the 3rd for cycle were off; fixed

```diff
int count_diff_smaller_16 = 0;
int max_diff = 0;
int total_diff = 0;
- for(int i{1}; i < n; ++i) {
+ for(size_t i{0}; i < skip_diffs.size(); ++i) {
auto diff = skip_diffs[i];
if (diff < 16) {
++count_diff_smaller_16;
```