Bitcoin Core Github
43 subscribers
122K links
Download Telegram
πŸ’¬ fanquake commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3485238772)
> This one simply addresses your comments https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3460836590 and https://github.com/bitcoin/bitcoin/pull/33593#pullrequestreview-3398698978.

Somewhat, but if this is the approach, it means the not only the compiler, but also the version of the headers being tested isn't going to match Guix?
πŸ’¬ maflcko commented on issue "ci: spurious linter failure?":
(https://github.com/bitcoin/bitcoin/issues/33773#issuecomment-3485279327)
This simply times out, because the Ubuntu PPAs are so slow/rate-limited/ddos'd:

The task starts at 8:50, but download only concludes after 9:06:

* https://github.com/bitcoin/bitcoin/actions/runs/19062940342/job/54446647440?pr=32876#step:4:1110

`#11 951.6 Fetched 173 MB in 6min 31s (443 kB/s)`

* https://github.com/bitcoin/bitcoin/actions/runs/19062940342/job/54446647440?pr=32876#step:4:298:

`#11 549.5 Fetched 181 MB in 7min 56s (380 kB/s)`


See https://github.com/bitcoin/bitcoin/pull/33744
...
πŸ’¬ 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,
+
...