Bitcoin Core Github
42 subscribers
126K links
Download Telegram
πŸ’¬ plebhash commented on issue "Mining interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/33777#issuecomment-3575893233)
we're doing a SRI release this week that is planned to include https://github.com/stratum-mining/sv2-apps/pull/59, and it's meant to support v30

but it will come with two major limitations, namely:
- the initial iteration of `bitcoin_core_sv2` crate is marked with a bunch of `todo`s that are workarounds for the lack of `interruptWait`, and are being tracked at https://github.com/stratum-mining/sv2-apps/issues/81
- running it on systems with low RAM for extended periods of time can lead to crash
...
βœ… hebasto closed a pull request: "test: Introduce `SUPPRESS_ABORT_MESSAGE` environment variable"
(https://github.com/bitcoin/bitcoin/pull/32409)
πŸ“ hebasto reopened a pull request: "test: Introduce `SUPPRESS_ABORT_MESSAGE` environment variable"
(https://github.com/bitcoin/bitcoin/pull/32409)
This PR adds a way to suppress the abort message box when running `test_bitcoin.exe` and `fuzz.exe` built with the debug runtime library on Windows.

Otherwise, a failing `assert()` triggers the `abort` routine, which displays a message box and causes a timeout in CI.

Here are CI jobs for the "Debug" configuration:
- on the master branch: https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/14759346479/job/41435857332
- with this PR: https://github.com/hebasto/bitcoin-core-night
...
πŸ’¬ yuvicc commented on pull request "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState":
(https://github.com/bitcoin/bitcoin/pull/33856#issuecomment-3575923284)
Thanks for the review @TheCharlatan.
- Addressed [comment](https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2554084325) on using post-condition checks.
- Dropped the change to use separate state for reporting errors and not invalidity as suggested by @TheCharlatan [comment](https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2554092568)
πŸ€” rkrux reviewed a pull request: "Added rescan option for import descriptors"
(https://github.com/bitcoin/bitcoin/pull/31668#pullrequestreview-3505373212)
Concept ACK 6e523bda47854333d267f53cfa5292fd2eb40ec7

Agree with the intent to avoid rescanning if the user specifies so.
πŸ’¬ rkrux commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2560222785)
I think the test cases can be greatly simplified with the following diff:

```diff
diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py
index 49c41b0a1f..588d7725dc 100755
--- a/test/functional/wallet_importdescriptors.py
+++ b/test/functional/wallet_importdescriptors.py
@@ -25,6 +25,7 @@ from test_framework.descriptors import descsum_create
from test_framework.util import (
assert_equal,
assert_raises_rpc_error,
+ assert_greater_th
...
πŸ’¬ rkrux commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2560225153)
Would be good to add a small test case for this newly added condition as well.
πŸ’¬ yuvicc commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3576089382)
Rebased.
πŸ“ stickies-v opened a pull request: "kernel: don't use assert to handle invalid user input"
(https://github.com/bitcoin/bitcoin/pull/33943)
Crashing with `assert` for invalid user input is problematic for a library, e.g. this is impossible for FFI users to recover from, forcing them to re-implement the error handling logic client-side. See e.g. the [go](https://github.com/stringintech/go-bitcoinkernel/blob/88105c69b32c48919a45bb07fc05ca2c79d2728b/kernel/script_pubkey.go#L91-L102) and [.net](https://github.com/janb84/BitcoinKernel.NET/blob/4ba1ff802fe9a3b7c0af0012e6fdc33e5bc4f961/src/BitcoinKernel.Core/ScriptVerification/ScriptVerifi
...
πŸ’¬ instagibbs commented on pull request "txgraph: drop move assignment operator":
(https://github.com/bitcoin/bitcoin/pull/33862#issuecomment-3576309301)
ACK ade0397f59f2fb59ab0e4ebb39869ac343cc54ee
πŸ’¬ fanquake commented on pull request "doc: document capnproto and libmultiprocess deps in 29.x":
(https://github.com/bitcoin/bitcoin/pull/33623#issuecomment-3576316460)
Concept ACK - @willcl-ark did you want to address any of the suggestions here?
πŸ’¬ lucasbalieiro commented on issue "Memory leak when using IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3576336813)
here are some insights from the experiments I’ve been running over the past few days.

When I first reported this issue (https://github.com/stratum-mining/sv2-apps/pull/59#issuecomment-3568252007], my server was also running a few utility tools that I typically use during long test sessions.

To eliminate the β€œmaybe it’s something else eating the memory” hypothesis, I started a fresh session with **only** the following running:

* `sv2-apps`
* Bitcoin Core v30 (mainnet)
* The essential system pr
...
πŸ‘ instagibbs approved a pull request: "test: Fix reorg patterns in tests to use proper fork-based approach"
(https://github.com/bitcoin/bitcoin/pull/32587#pullrequestreview-3505760522)
reACK 2a64851c34fcf9955a673eb77463f68ab583a7bd

`git range-diff master b1581924a79e120f166719943646484ba00ff21b 2a64851c34fcf9955a673eb77463f68ab583a7bd`
πŸ’¬ hebasto commented on pull request "test: Introduce `SUPPRESS_ABORT_MESSAGE` environment variable":
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-3576356937)
> > @maflcko
> > > No objection, but assert/Assert seems to be used widely in the codebase, so shouldn't this be done for all test binaries, or none? Otherwise the same assert could lead to inconsistent behavior, depending on which test binary ran into it.
> >
> >
> > Thanks! Reworked.
>
> Thx, but it seems it would also need to be applied to bitcoind (or really all executables)?

It is already implemented for `bitcoind.exe` here:https://github.com/bitcoin/bitcoin/blob/5336bcd5784925c
...
πŸ’¬ Sjors commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#issuecomment-3576359913)
I restructured the implementation and commits a bit.


The `TxTemplateMap` now lives on the `NodeContext` rather than `MinerImpl` (interface). This reflects the fact that we want to track the global memory footprint instead of per client. It's a lightweight member `template_tx_refs` which should be easy to fold into a block template manager later.

It also made it easier to move `GetTemplateMemoryUsage` from `interface.cpp` to `miner.cpp`, where it's more reusable.

This in turn let me sp
...
πŸš€ fanquake merged a pull request: "txgraph: drop move assignment operator"
(https://github.com/bitcoin/bitcoin/pull/33862)
πŸ“ instagibbs converted_to_draft a pull request: "policy: don't CheckEphemeralSpends on reorg"
(https://github.com/bitcoin/bitcoin/pull/33616)
Similar reasoning to https://github.com/bitcoin/bitcoin/pull/33504

During a deeper reorg it's possible that a long sequence of dust-having transactions that are connected in a linear fashion. On reorg, this could cause each subsequent "generation" to be rejected. These rejected transactions may contain a large amount of competitive fees via normal means.

PreCheck based `PreCheckEphemeralSpends` is left in place because we wouldn't have relayed them prior to the reorg.

based on #32587 to
...
πŸ’¬ instagibbs commented on pull request "policy: don't CheckEphemeralSpends on reorg":
(https://github.com/bitcoin/bitcoin/pull/33616#issuecomment-3576395944)
marking as draft for now until it becomes dependency-less and I've looked over the tests some more
πŸ‘‹ instagibbs's pull request is ready for review: "policy: Remove individual transaction <minrelay restriction"
(https://github.com/bitcoin/bitcoin/pull/33892)