Bitcoin Core Github
44 subscribers
122K links
Download Telegram
🚀 fanquake merged a pull request: "depends: Bump to capnproto-c++-1.0.1"
(https://github.com/bitcoin/bitcoin/pull/28735)
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#issuecomment-1793825982)
Rebased on master to fix silent conflict with new I2P tests.

@maflcko thanks. Can you help me understand why this only broke the previous builds task?
👍 pinheadmz approved a pull request: "doc: Add offline signing tutorial"
(https://github.com/bitcoin/bitcoin/pull/28363#pullrequestreview-1714045182)
ACK 3c208cc05ea9efb145c956e70f80efd8b027ff33

Confirmed only updates since last review addressed achow101's nits re: `send` and `blank`

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 3c208cc05ea9efb145c956e70f80efd8b027ff33
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmVH8gIACgkQ5+KYS2KJ
yToV3Q//TyBSa0j3/cGSVnT7dNZREnfh2ecKV5IueWBK1OnS3Bae1eLFS9hikmuG
gjxgRJFcrQYnwrqst72fkXlhXWnldam9NRsMV7LB8e5Ca
...
💬 ryanofsky commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1793830954)
> I don't think the CI win64-cross build has multiprocess enabled

Thanks, I forgot it would only be built if that flag is enabled
📝 theStack opened a pull request: "wallet: cache descriptor ID to avoid repeated descriptor string creation"
(https://github.com/bitcoin/bitcoin/pull/28799)
Right now a wallet descriptor is converted to it's string representation (via `Descriptor::ToString`) repeatedly at different instances:
- on finding a `DescriptorScriptPubKeyMan` for a given descriptor (`CWallet::GetDescriptorScriptPubKeyMan`, e.g. used by the `importdescriptors` RPC); the string representation is created once for each spkm in the wallet and at each iteration again for the searched descriptor (`DescriptorScriptPubKeyMan::HasWalletDescriptor`)
- whenever `DescriptorScriptPubKe
...
💬 cmdruid commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1793929086)
Regtest is not an option if you want to host your own test network for other users to test your software, as anyone can grief the network by generating blocks. Default testnet/signet is too slow for actual testing.

I'd like the ability to host a test network that outside users can use, but not grief, while having fast block times (or even no time restriction at all) so that I can incorporate it into CI/CD for different projects.

This seems like a real obvious tool to have for development o
...
💬 Sjors commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1793951200)
re-utACK b1b12d1b88492fe66b26243760689c7f040c67c2
👍 Sjors approved a pull request: "wallet: prevent bugs from invalid transaction heights with asserts, comments, and refactoring"
(https://github.com/bitcoin/bitcoin/pull/28546#pullrequestreview-1714193806)
utACK f06016d77d848133fc6568f287bb86b644c9fa69
💬 Sjors commented on pull request "wallet: prevent bugs from invalid transaction heights with asserts, comments, and refactoring":
(https://github.com/bitcoin/bitcoin/pull/28546#discussion_r1382736556)
262a78b13365e5318741187fde431c4974539494: could expand this comment:

```
// Find block by hash and fill in the state height.
```
⚠️ Sjors opened an issue: "test: wallet_miniscript.py fails with thread sanitizer "
(https://github.com/bitcoin/bitcoin/issues/28800)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Using clang version 15.0.7 on Ubuntu and enabling the thread sanitizer, the `wallet_miniscript.py --descriptors` test consistently fails with a timeout.

### Expected behaviour

Don't timeout.

I could of course increate the timeouts, but this is the only test with that issue.

As an aside, the log output contains extremely long descriptors which should probably be truncated.

### Step
...
💬 Sjors commented on issue "test: wallet_miniscript.py fails with thread sanitizer ":
(https://github.com/bitcoin/bitcoin/issues/28800#issuecomment-1794029975)
I'll try if @theStack's #28799 helps.
💬 Sjors commented on pull request "wallet: cache descriptor ID to avoid repeated descriptor string creation":
(https://github.com/bitcoin/bitcoin/pull/28799#issuecomment-1794115914)
This fixes #28800 for me and run _way_ faster under `--with-sanitizers=thread`.

ACK 5e6bc6d830664a5afeb5d5bd7e7b3818a01376b7
⚠️ Sjors opened an issue: "p2p_segwit.py fails with thread sanitizer"
(https://github.com/bitcoin/bitcoin/issues/28801)
### Current behaviour

Using clang version 15.0.7 on Ubuntu and enabling the thread sanitizer, the `p2p_segwit.py --descriptors` test consistently fails.

It fails during the `Subtest: test_wtxid_relay (Segwit active = True)`.

It's different from #28800 in that this isn't a timeout. The subtests fails after about 5 seconds.

```
Node returned unexpected exit code (66) vs (0) when stopping
```

### Expected behaviour

Don't fail.

### Steps to reproduce

```
./autogen
./confi
...
💬 Sjors commented on issue "p2p_segwit.py fails with thread sanitizer":
(https://github.com/bitcoin/bitcoin/issues/28801#issuecomment-1794142983)
Some warnings from the sanitiser: https://gist.github.com/Sjors/607c3426136db1c91b89acea3c62affb
Sjors closed an issue: "p2p_segwit.py fails with thread sanitizer"
(https://github.com/bitcoin/bitcoin/issues/28801)
💬 Sjors commented on issue "p2p_segwit.py fails with thread sanitizer":
(https://github.com/bitcoin/bitcoin/issues/28801#issuecomment-1794150128)
Closing this. When I first saw this failure it was while running multiple tests in parallel. That was run by a script which _does_ use supressions. But when I tried to reproduce I forgot the suppressions. Running the indidivual test with suppressions on doesn't fail. And when I ran the test suite in parallel again it also didn't fail. So I think it was just a random failure.

I'll try to get the logs from the parallel run next time.
💬 Daniel600 commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1794185915)
> I think it’s good to communicate on the mailing list in which version (27.0 or 28.0) full-rbf might be turn on by default. I advocated such setting in the past as early as 0.24, though at the time some 0-conf transactions applications and services were migrating their stuff in consequence.

HI @ariard - has this PR been decided to be merged already? What would the timeline to expect if it will be released with 27.0 or 28.0? (i.e When should we expect 27.0 or 28.0 to released?)

At GAP600
...
💬 TheCharlatan commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1794248918)
Re https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1669359482

> What are the steps to reproduce?

Run the fuzzer on the fuzz test introduced in this PR seeded with Sjors' inputs from his [qa repo PR](https://github.com/bitcoin-core/qa-assets/pull/140) and compiled with bdb enabled.

> Outside of that, my recommendations would be to use bdb-5.3

The bugs persist even with bdb 5.3.
💬 Sjors commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1794251948)
The first commit ee79f05f1fef57bfbea24333a46e525385d02319 introduces a dangling reference warning for me. gcc 13.1.0 on Ubuntu 23.04.

```
./configure --enable-suppress-external-warnings --disable-fuzz-binary --without-gui
make
...

validation.cpp: In member function ‘PackageMempoolAcceptResult {anonymous}::MemPoolAccept::AcceptPackage(const Package&, ATMPArgs&)’:
validation.cpp:1488:36: warning: possibly dangling reference to a temporary [-Wdangling-reference]
1488 | const
...
💬 YellowRoseCx commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1794275787)
> > I think it’s good to communicate on the mailing list in which version (27.0 or 28.0) full-rbf might be turn on by default. I advocated such setting in the past as early as 0.24, though at the time some 0-conf transactions applications and services were migrating their stuff in consequence.
>
> HI @ariard - has this PR been decided to be merged already? What would the timeline to expect if it will be released with 27.0 or 28.0? (i.e When should we expect 27.0 or 28.0 to released?)
>
> At G
...