Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 tcharding commented on issue "Stale code (that has no effect)":
(https://github.com/bitcoin/bitcoin/issues/31558#issuecomment-2560408311)
cc @achow101
🤔 tdb3 reviewed a pull request: "descriptor: remove unreachable verification for `pkh`"
(https://github.com/bitcoin/bitcoin/pull/31555#pullrequestreview-2521284624)
Approach ACK
Left a small comment, but I don't feel very strongly about it.
💬 tdb3 commented on pull request "descriptor: remove unreachable verification for `pkh`":
(https://github.com/bitcoin/bitcoin/pull/31555#discussion_r1896229349)
Seems like it might be clearer and more defensive to check that we're entering with one of the appropriate contexts rather than check that we aren't entering in one of the inappropriate contexts (albeit currently the only inappropriate one).

```diff
- Assume(ctx != ParseScriptContext::P2WPKH);
+ Assume(ctx == ParseScriptContext::TOP || ctx == ParseScriptContext::P2SH || ctx == ParseScriptContext::P2WSH || ctx == ParseScriptContext::P2TR);

```

https://github.com/bitcoin/bitcoin/b
...
⚠️ zarei673mo opened an issue: "Bitcoincore"
(https://github.com/bitcoin/bitcoin/issues/31559)
pinheadmz closed an issue: "Bitcoincore"
(https://github.com/bitcoin/bitcoin/issues/31559)
💬 davidgumberg commented on pull request "lint: Move assertion linter into lint runner":
(https://github.com/bitcoin/bitcoin/pull/31435#issuecomment-2560532713)
crACK https://github.com/bitcoin/bitcoin/commit/e8f0e6efaf555a7d17bdb118464bd572bd5f3933


Tested manually with the following diff:

<details>

<summary>git diff</summary>

```diff
diff --git a/src/coins.cpp b/src/coins.cpp
index 24a102b0bc..000068c7e2 100644
--- a/src/coins.cpp
+++ b/src/coins.cpp
@@ -21,6 +21,7 @@ std::unique_ptr<CCoinsViewCursor> CCoinsView::Cursor() const { return nullptr; }

bool CCoinsView::HaveCoin(const COutPoint &outpoint) const
{
+ BOOST_ASSERT(1
...
📝 theStack opened a pull request: "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script"
(https://github.com/bitcoin/bitcoin/pull/31560)
This PR is based on #27432 and slightly modifies the `dumptxoutset` RPC to allow writing the UTXO set dump into a [named pipe](https://askubuntu.com/a/449192), so that the output data can be consumed by another process, see #31373. Taking use of this with the utxo-to-sqlite.py tool (introduced in #27432), creating an UTXO set in SQLite3 format is possible on the fly and becomes a one-liner with a newly introduced script `dump_to_sqlite.sh`. E.g. for signet:
```
$ ./contrib/utxo-tools/dump_to_s
...
⚠️ TwistedCrafts opened an issue: "Bitcoinnode
"
(https://github.com/bitcoin-core/gui/issues/849)
Need some focus pointer
👍 i-am-yuvi approved a pull request: "test: Avoid intermittent error in assert_equal(pruneheight_new, 248)"
(https://github.com/bitcoin/bitcoin/pull/31468#pullrequestreview-2521622917)
Code Review ACK fa0998f0a028161fe7264d0e81af36ffdcb43384

I was thinking of using a batch syncing process instead of one-by-one to be memory efficient, but it ended up being a lot slower. Anyway, nice work, @maflcko.

Here are the test results:

```
Temporary test directory at /var/folders/jb/wlbrz0t95vl58wzxjt75wqmh0000gn/T/test_runner_₿_🏃_20241224_132141
Remaining jobs: [feature_index_prune.py]
1/1 - feature_index_prune.py passed, Duration: 34 s

TEST
...
💬 Sjors commented on issue "rfc: DATUM mining interface requirements":
(https://github.com/bitcoin/bitcoin/issues/31002#issuecomment-2560811173)
> Seems like it would be trivial to add this to GBT.

Agreed.

> Seems like this would be better suited to a PR on the datum_gateway repo.

My only goal here is to figure out if the Mining interface needs anything for Datum that it doesn't need for Stratum v2. Whether you actually use it is up to you of course.
💬 Sjors commented on pull request "rpc: Add signet_challenge field to getblockchaininfo and getmininginfo":
(https://github.com/bitcoin/bitcoin/pull/31531#discussion_r1896523539)
Indeed the general pattern seems to be that for RPC we use `argument_name` whereas for startup arguments we use `-argumentname`.
💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2560911214)
`5766bbefa9...b448b01494`: avoid moving `GetRandomNodeEvictionCandidates()` as suggested in https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1858648731 and https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1745496410
💬 L508726 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/ce6df7df9bab2405cfe7d6e382f5682cf30de476#r150696862)
Oky my bro some how it will detamine
💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1896603609)
Dropped the move.
💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1896604403)
done, also suggested in https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1745496410
fanquake closed an issue: "Bitcoinnode
"
(https://github.com/bitcoin-core/gui/issues/849)
:lock: fanquake locked an issue: "Bitcoinnode
"
(https://github.com/bitcoin-core/gui/issues/849)
💬 maflcko commented on pull request "test: Avoid intermittent error in assert_equal(pruneheight_new, 248)":
(https://github.com/bitcoin/bitcoin/pull/31468#issuecomment-2560956747)
> I was thinking of using a batch syncing process instead of one-by-one to be memory efficient, but it ended up being a lot slower. Anyway, nice work, @maflcko.

For fun, I had written another async alternative that reads the next block while the current block is submitted, which was faster than this pull by a few seconds, but I think trivial code is more important in tests than the fastest solution.
💬 maflcko commented on issue "Stale code (that has no effect)":
(https://github.com/bitcoin/bitcoin/issues/31558#issuecomment-2560984394)
Removing the code yields a test failure for negative feerates for me:

```
test/amount_tests.cpp(64): error: in "amount_tests/GetFeeTest": check feeRate.GetFee(8) == CAmount(-1) has failed [0 != -1]

*** 1 failure is detected in the test module "Bitcoin Core Test Suite"
```


I've documented that in fa2da2cb607ba359231fccc9635abe7c8616de56 .

Though, I find it a bit odd to treat negative and positives feerates differently. So the code should probably still be removed and the ceil shou
...