Bitcoin Core Github
43 subscribers
122K links
Download Telegram
πŸ’¬ achow101 commented on pull request "kernel: handle null or empty directories in implementation":
(https://github.com/bitcoin/bitcoin/pull/33867#issuecomment-3555163702)
ACK 6657bcbdb4d0359c1843ca31fb3670c7c0c260d5
πŸ’¬ achow101 commented on pull request "clang-format: Set InsertNewlineAtEOF: true":
(https://github.com/bitcoin/bitcoin/pull/33896#issuecomment-3555168238)
ACK fa1bf6818f0910f997e5235a197ff51f2e18780d
πŸš€ achow101 merged a pull request: "kernel: handle null or empty directories in implementation"
(https://github.com/bitcoin/bitcoin/pull/33867)
πŸš€ achow101 merged a pull request: "clang-format: Set InsertNewlineAtEOF: true"
(https://github.com/bitcoin/bitcoin/pull/33896)
πŸ’¬ w0xlt 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#issuecomment-3555189629)
Thanks for the review @yuvicc and @purpleKarrot .

Great idea exposing the chain parameter class.
I’ve updated the PR to follow this approach and split it into three commits for better readability and easier reviewing.
I also had to update `BlockValidationState` so the reference parameter works correctly.
I kept `btck_BlockCheckFlags` to avoid unclear boolean parameters.
πŸ’¬ 151henry151 commented on issue "Load PSBT error: Unable to decode PSBT":
(https://github.com/bitcoin/bitcoin/issues/30070#issuecomment-3555230079)
I've started some work on this. I modified `WalletModel::prepareTransaction` in `src/qt/walletmodel.h` (line 99) and `src/qt/walletmodel.cpp` (line 150) to accept an optional `sign` parameter (defaults to `false`). In lines 206-208 of `walletmodel.cpp`, I changed the logic so it only signs when explicitly requested via this parameter, rather than always signing when private keys are available. Then in `src/qt/sendcoinsdialog.cpp`, I updated the call to `prepareTransaction` on line 293 to pass `s
...
πŸ’¬ ajtowns commented on pull request "wallet: Add separate balance info for non-mempool wallet txs":
(https://github.com/bitcoin/bitcoin/pull/33671#issuecomment-3555267523)
> Concept NACK-ish
>
> There's a lot of reasons that an unconfirmed transaction might be in the wallet but not the mempool, and I think giving an accurate balance of those transactions will be hard to get right. We absolutely do not want to mislead users into thinking they have more coins than they actually do, so I am hesitant to add something like this.

We currently mislead users into thinking they have less coins than they actually do (due to transactions in the wallet with long mempool
...
πŸ’¬ danielabrozzoni commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3555600586)
Hey, sorry this took a while. I tried rebasing onto #33785 but it wouldn't fix the warning.

In my last push (de4242f):
- I didn't rebase, to hopefully make re-reviewing easier
- I removed `const` as suggested in https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2509736434, I like it a bit better than the previous solution
- I squashed the fix inside the last commit instead pushing as a separate one

> Technically I think the repo policy is that every commit should build on all C
...
πŸ’¬ 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