Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 stratospher commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-3555608000)
Thank you all for the very helpful feedback on this! and sorry for getting late with the update.

I've pushed an [update](https://github.com/bitcoin/bitcoin/compare/27e90008835a4c980447d97e80910742e041dfaf..14ee29dedc36611f9d33a207b7c5b25a7c9b89ea) addressing all the review comments except @mzumsande's conceptual concern about whether
`v2onlyclearnet` should accept v2 inbounds as well so that the network doesn't face a shortage of listening capacity in the worst case scenario where a huge fra
...
💬 151henry151 commented on issue "Standardness policy rules for legacy Multisig script is incoherent":
(https://github.com/bitcoin/bitcoin/issues/33882#issuecomment-3555753329)
P2SH allows non-solvable redeem scripts with low sigop counts (PR #4365), while legacy scripts still require solvability. This inconsistency is hard to justify, especially since both rely on sigop limits for security.

Aligning legacy script redemption with P2SH policy would remove the asymmetry and fix the Counterparty UTXOs without special-casing, while preserving security through existing sigop limits.

If hybrid key bans are reconsidered (as ajtowns suggested), that could simplify further, b
...
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2544448081)
Stopping the SOCKS5 proxy works. Leaving it like that, but just to mention, another alternative that occurred to me would be to `self.nodes[0].setnetworkactive(state=False)` before the restart.
💬 laanwj commented on issue "guix build fails on RISC-V due to python-py-cpuinfo test failure":
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3556124760)
Another faliure, this time in `rust-1.75.0.drv.gz`:
```
malloc(): unaligned fastbin chunk detected
build process 6 exited with status 6
```
💬 Sjors commented on pull request "test: Fix race condition in IPC interface block progation test":
(https://github.com/bitcoin/bitcoin/pull/33880#issuecomment-3556318784)
re-utACK 2578e6fc0f4af35f389cd8ff59825c874e0b72ac
💬 Sjors commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#issuecomment-3556340864)
Got the same error when I rebased, working on a fix...
📝 Sjors converted_to_draft a pull request: "wallet: warn against accidental unsafe older() import"
(https://github.com/bitcoin/bitcoin/pull/33135)
[BIP 379](https://github.com/bitcoin/bips/blob/master/bip-0379.md) ([Miniscript](https://bitcoin.sipa.be/miniscript/)) allows relative height and time locks that have no consensus meaning in [BIP 68](https://github.com/bitcoin/bips/blob/master/bip-0068.mediawiki) (relative timelocks) / [BIP 112](https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki) (`CHECKSEQUENCEVERIFY`). This is (ab)used by some protocols, e.g. [by Lightning to encode extra data](https://delvingbitcoin.org/t/explorin
...
💬 maflcko commented on pull request "test: Fix race condition in IPC interface block progation test":
(https://github.com/bitcoin/bitcoin/pull/33880#issuecomment-3556353881)
lgtm ACK 2578e6fc0f4af35f389cd8ff59825c874e0b72ac
💬 Sjors commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#issuecomment-3556397690)
Rebased and improved the test error logging:

```cpp
// A safe boundary value should yield no warnings.
{
FlatSigningProvider keys;
std::string err;
auto descs = Parse("wsh(and_v(v:pk(0379e45b3cf75f9c5f9befd8e9506fb962f6a9d185ac87001ec44a8d3df8d4a9e3),older(65535)))", keys, err, /*require_checksum=*/false);
BOOST_REQUIRE_MESSAGE(!descs.empty(), err);
```

This fails with:

```
test/descriptor_tests.cpp:1265: Entering test case "descriptor_old
...
💬 Eunovo commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-3556555996)
Rebased on master. I updated the Miniscript implementation slightly due to changes from https://github.com/bitcoin/bitcoin/pull/31734
💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3556680990)
`6e0f3a4a58...47f4f65d0c`: rebase due to conflicts

Is the local failure you observe reproducible or sporadic? If it is reproducible maybe you can nail down which test is making the traffic? Previously to find the offending test I bisected the list of tests (used to nail down the traffic from `node_init_tests/init_test` which is fixed in this PR). Here is a write-only script to list all tests (681 currently) and run only e.g. from 1 to 340:

```sh
BUILD/bin/test_bitcoin $(BUILD/bin/test_bit
...
🤔 yuvicc reviewed a pull request: "kernel: add context‑free block validation API (`btck_check_block_context_free`) with POW/Merkle flags"
(https://github.com/bitcoin/bitcoin/pull/33908#pullrequestreview-3486594405)
Approach ACK
💬 yuvicc commented on pull request "kernel: add context‑free block validation API (`btck_check_block_context_free`) with POW/Merkle flags":
(https://github.com/bitcoin/bitcoin/pull/33908#discussion_r2545072096)
And then use `Handle` instead of `UniqueHandle` here:

```suggestion
class BlockValidationState : public Handle<btck_BlockValidationState, btck_block_validation_state_copy, btck_block_validation_state_destroy>, public BlockValidationStateApi<BlockValidationState>
```
💬 yuvicc commented on pull request "kernel: add context‑free block validation API (`btck_check_block_context_free`) with POW/Merkle flags":
(https://github.com/bitcoin/bitcoin/pull/33908#discussion_r2545063204)
I think we also expose `_copy` API here:

```suggestion

/**
* @brief Copies the block validation state.
*
* @param[in] block_validation_state Non-null.
* @return The copied block validation state.
*/
BITCOINKERNEL_API btck_BlockValidationState* BITCOINKERNEL_WARN_UNUSED_RESULT btck_block_validation_state_copy(
const btck_BlockValidationState* block_validation_state) BITCOINKERNEL_ARG_NONNULL(1);

```
💬 yuvicc commented on pull request "kernel: add context‑free block validation API (`btck_check_block_context_free`) with POW/Merkle flags":
(https://github.com/bitcoin/bitcoin/pull/33908#discussion_r2545075148)
Also here:

```suggestion
explicit BlockValidationState() : Handle{btck_block_validation_state_create()} {}
```
💬 fanquake commented on pull request "depends: update xcb-util packages to latest versions":
(https://github.com/bitcoin/bitcoin/pull/33851#discussion_r2545130316)
I think the issue was updating the packages in lockstep. I'll cherry-pick the two commits, with some adjustments, i.e to remove `--without-launchd`.
💬 fanquake commented on pull request "guix: build `bitcoin-qt` with static libxcb & utils":
(https://github.com/bitcoin/bitcoin/pull/33537#discussion_r2545150228)
There's a related report to libxcb here: https://gitlab.freedesktop.org/xorg/lib/libxcb/-/merge_requests/25.
👍 vasild approved a pull request: "refactor: remove incorrect lifetimebounds"
(https://github.com/bitcoin/bitcoin/pull/33870#pullrequestreview-3486800310)
ACK 99d012ec80a4415e1a37218fb4933550276b9a0a

Reviewing is learning, I love it! Thanks!
📝 maflcko opened a pull request: "clang-format: Set PackConstructorInitializers: CurrentLine"
(https://github.com/bitcoin/bitcoin/pull/33912)
Now that the minimum supported clang version is 17, the `PackConstructorInitializers` setting can be set to `CurrentLine` in the clang-format file. (https://releases.llvm.org/16.0.0/tools/clang/docs/ClangFormatStyleOptions.html#packconstructorinitializers)

The `CurrentLine` option will either put all constructor initializers on the current line if they fit. Otherwise, it will put each one on its own line.

The `CurrentLine` option is desirable over the current `BinPack` option, because:


...
💬 maflcko commented on pull request "refactor: remove incorrect lifetimebounds":
(https://github.com/bitcoin/bitcoin/pull/33870#discussion_r2545299017)
I don't think the tip can be nullptr in rpc code. This can just be a `CHECK_NONFATAL`, but this seems unrelated to lifetimebound stuff?