Bitcoin Core Github
42 subscribers
126K links
Download Telegram
πŸš€ achow101 merged a pull request: "log: print every script verification state change"
(https://github.com/bitcoin/bitcoin/pull/33336)
πŸ’¬ l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3444285774)
Let me summarize our offline discussions:

### Cache hierarchy

During block connection we're adding an extra temporary in-memory dbcache layer on top so that whatever happens during block connection doesn't end up polluting the big dbcache or leveldb.
I think we should take advantage of this and use the temporary top layer to collect the missing inputs there:
* if block validation fails we can just throw it out
* it's easier to test and benchmark, we can rerun the same operation without
...
πŸ’¬ l0rinc commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3444302074)
Thank you all for the reviews, appreciate the help.
Once we're branching for `30.1` I'd like to suggest we backport this.
πŸš€ achow101 merged a pull request: "fuzz: enhance wallet_fees by mocking mempool stuff"
(https://github.com/bitcoin/bitcoin/pull/33210)
πŸ’¬ furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#issuecomment-3444568607)
> Can't we use an already existing open source library instead of reinventing the wheel?

That's a good question. It's usually where we all start.
Generally, the project consensus is to avoid introducing new external dependencies (unless they’re maintained by us) to minimize potential attack vectors. This doesn’t mean we should reinvent everything, just that we need to be very careful about what we decide to include.

That being said, for the changes introduced in this PR, can argue that we
...
πŸ’¬ achow101 commented on pull request "clang-format: make formatting deterministic for different formatter versions":
(https://github.com/bitcoin/bitcoin/pull/32813#issuecomment-3444782149)
ACK 13f36c020f0329b5e975282b45292fdf2a495e31
πŸ’¬ theStack commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#issuecomment-3444789711)
Rebased on master (due to conflict with recently merged #32983, commit b63428ac9ce2c903670409b3e47b9f6730917ae8).
πŸš€ achow101 merged a pull request: "clang-format: make formatting deterministic for different formatter versions"
(https://github.com/bitcoin/bitcoin/pull/32813)
πŸ’¬ kevkevinpal commented on pull request "wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult":
(https://github.com/bitcoin/bitcoin/pull/32958#issuecomment-3444860797)
Just rebased the PR to [86d9a82](https://github.com/bitcoin/bitcoin/pull/32958/commits/86d9a82cc8f4c5dd873592ef3589fd2406bb513c) since @DrahtBot was upset
πŸ’¬ diegoviola commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-3444933735)
> from the current application

Application window/surface.
πŸ€” furszy reviewed a pull request: "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)"
(https://github.com/bitcoin/bitcoin/pull/29640#pullrequestreview-3378984656)
Tested ACK 0465574c127907df9b764055a585e8281bae8d1d.

Reverting the fix makes the introduced test fails (after ~60 test runs).
πŸ€” stringintech reviewed a pull request: "kernel: Introduce C header API"
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3369762303)
Comments on commits 3 (92f54852) through 6 (f9bf4d9b) - context and chainstate manager. Also includes a couple of follow-ups from previous review comments on logging.
πŸ’¬ stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2457664989)
Could this be simplified to wrap `CChainParams` directly instead? (to match the pattern where handles either wrap types directly or wrap `shared_ptr`?
<details>
<summary>diff</summary>

```diff
diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
index 10cf741bd1..49749b312a 100644
--- a/src/kernel/bitcoinkernel.cpp
+++ b/src/kernel/bitcoinkernel.cpp
@@ -481,7 +481,7 @@ struct btck_ScriptPubkey : Handle<btck_ScriptPubkey, CScript> {};
struct btck_LoggingConnection
...
πŸ’¬ stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2455010438)
```suggestion
/**
* Opaque data structure for holding options for creating a new kernel context.
*
* Once a kernel context has been created from these options, they may be
* destroyed. The options hold the notification callbacks and validation interface callbacks as well as the
* selected chain type until they are passed to the context. If no options are
```
πŸ’¬ stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2459437404)
Lines 365-366 have changes in commit 92f5485. It seems they should have been included in the previous commit - 3204110. Also line 770 in the same file.
πŸ’¬ stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2459999505)
Can we omit virtual methods here? (also for `ValidationInterface`)
<details>
<summary>diff</summary>

```diff
diff --git a/src/kernel/bitcoinkernel_wrapper.h b/src/kernel/bitcoinkernel_wrapper.h
index 98862b8d05..7fb7c2bc6f 100644
--- a/src/kernel/bitcoinkernel_wrapper.h
+++ b/src/kernel/bitcoinkernel_wrapper.h
@@ -812,21 +812,19 @@ template <typename T>
class KernelNotifications
{
public:
- virtual ~KernelNotifications() = default;
+ void BlockTipHandler(SynchronizationSt
...
πŸ’¬ stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2461486493)
Last line seems unnecessary.
πŸ’¬ stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2461385314)
In commit 3270c061 description: it is better to drop "It is only valid if that context remains in memory too" now that context is a `shared_ptr` shared among objects that depend on it (as the above doc correctly indicates).
πŸ’¬ stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2459690268)
Last line seems unnecessary.
πŸ’¬ stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2461974025)
Replying [this](https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2445236632) comment: shouldn't we have the following change to reflect that?

```diff
diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp
index 5e108f8949..6e4fc188fb 100644
--- a/src/test/kernel/test_kernel.cpp
+++ b/src/test/kernel/test_kernel.cpp
@@ -285,9 +285,9 @@ void CheckHandle(T object, T distinct_object)

// Copy constructor
T object2(distinct_object);
- BOOST_C
...