Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🚀 fanquake merged a pull request: "test: use address_to_scriptpubkey instead of RPC call"
(https://github.com/bitcoin/bitcoin/pull/27349)
💬 fanquake commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#issuecomment-1488392041)
> resulting in improved performance for functional tests.

Generally good to provide some sort of benchmark/indication of the speedup when talking about performance.
🚀 fanquake merged a pull request: "guix: use GCC tool wrappers"
(https://github.com/bitcoin/bitcoin/pull/27345)
💬 dimitaracev commented on pull request "validation: Move warningcache to ChainstateManager and rename to m_warningcache":
(https://github.com/bitcoin/bitcoin/pull/27357#discussion_r1151809995)
No reason, moved it more down with 5526849 .
💬 dimitaracev commented on pull request "validation: Move warningcache to ChainstateManager and rename to m_warningcache":
(https://github.com/bitcoin/bitcoin/pull/27357#issuecomment-1488452250)
@MarcoFalke Thanks for the remainder, squashed them and it should be good now.
👋 MarcoFalke's pull request is ready for review: "ci: Use Cirrus CI dockerfile env"
(https://github.com/bitcoin/bitcoin/pull/27340)
📝 fanquake opened a pull request: "ci: use LLVM/clang-16 in ASAN job"
(https://github.com/bitcoin/bitcoin/pull/27360)
Similar to #27298. Working for me on `x86_64` and solves the issue I currently see with TSAN on `aarch64` with master (68828288e5d35c17848ab3da8cd231d1b69651c0):
```bash
crc32c/src/crc32c_arm64.cc:101:26: runtime error: load of misaligned address 0xffff84400406 for type 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment
0xffff84400406: note: pointer points here
b9 c5 22 00 01 01 1a 6c 65 76 65 6c 64 62 2e 42 79 74 65 77 69 73 65 43 6f 6d 70 61 72 61 74 6f
^
...
💬 MarcoFalke commented on pull request "ci: use LLVM/clang-16 in ASAN job":
(https://github.com/bitcoin/bitcoin/pull/27360#issuecomment-1488552377)
You will probably need to do the same for fuzz asan?
💬 MarcoFalke commented on pull request "ci: use LLVM/clang-16 in ASAN job":
(https://github.com/bitcoin/bitcoin/pull/27360#issuecomment-1488554684)
Also, need to edit Cirrus CI to do the same
💬 hebasto commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1151910811)
Considering switching s/`stdio.h`/`cstdio`/, does it make sense to s/`FILE`/`std::FILE`/ as well? Maybe as a follow up?
👍 hebasto approved a pull request: "refactor: Extract util/fs from util/system"
(https://github.com/bitcoin/bitcoin/pull/27254)
ACK 00e9b97f37e0bdf4c647236838c10b68b7ad5be3
💬 hebasto commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1151936439)
nit:

```diff
--- a/src/qt/intro.cpp
+++ b/src/qt/intro.cpp
@@ -6,16 +6,16 @@
#include <config/bitcoin-config.h>
#endif

-#include <chainparams.h>
#include <qt/intro.h>
#include <qt/forms/ui_intro.h>
-#include <util/fs.h>

#include <qt/guiconstants.h>
#include <qt/guiutil.h>
#include <qt/optionsmodel.h>

+#include <chainparams.h>
#include <interfaces/node.h>
+#include <util/fs.h>
#include <util/fs_helpers.h>
#include <util/system.h>
#include <validation.h>

...
💬 meglio commented on issue "signrawtransactionwithwallet fails with signed non-wallet inputs and breaks the existing signatures":
(https://github.com/bitcoin/bitcoin/issues/26385#issuecomment-1488621015)
Am I correct to assume this is still a bug awaiting a fix?
💬 fanquake commented on pull request "ci: use LLVM/clang-16 in ASAN job":
(https://github.com/bitcoin/bitcoin/pull/27360#issuecomment-1488669908)
> You will probably need to do the same for fuzz asan?

Will add.

> Also, need to edit Cirrus CI to do the same

Can we even do this for native_asan given there is no 2304-lto image listed here: https://cloud.google.com/compute/docs/images/os-details#ubuntu_lts ?
💬 dergoegge commented on pull request "p2p: Fill reconciliation sets and request reconciliation (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/26283#discussion_r1151979732)
```suggestion
inbounds_fanouted += pnode->IsInboundConn() && pnode->m_relays_txs && !m_txreconciliation->IsPeerRegistered(pnode->GetId());
```
Right? because other wise this includes erlay peers which we are not flooding to by default.
💬 dergoegge commented on pull request "p2p: Fill reconciliation sets and request reconciliation (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/26283#discussion_r1151986544)
`m_connman.GetNodeCount(ConnectionDirection::In)` includes all inbound connections, even ones that do want transactions relayed to them. IIUC, you want this to be "number of all inbound peers that sent fRelay=true" and the second member of the pair should be "number of all non-erlay inbound peers that sent fRelay=true"
💬 MarcoFalke commented on pull request "ci: use LLVM/clang-16 in ASAN job":
(https://github.com/bitcoin/bitcoin/pull/27360#issuecomment-1488682494)
Yeah, I suspect that debian:bookworm will be added earlier by google to the compute images. You can ask Sylvestre to add clang-16 to bookworm, I guess?
💬 dergoegge commented on pull request "validation: Move warningcache to ChainstateManager and rename to m_warningcache":
(https://github.com/bitcoin/bitcoin/pull/27357#issuecomment-1488686338)
Concept ACK
💬 MarcoFalke commented on pull request "ci: Use Cirrus CI dockerfile env":
(https://github.com/bitcoin/bitcoin/pull/27340#issuecomment-1488712858)
CI should finish green now
💬 apoelstra commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#issuecomment-1488767884)
@S3RK I agree that Sjors' PR does what we want (and more), up to the specific import format, but

* It has not been updated in 18 months and now has many conflicts
* It depends on two other unmerged PRs, one of which is marked draft
* It is itself marked draft
* It is 30 commits long and requires significant reviewer attention

Meanwhile this PR is narrow in scope, doesn't even touch the actual wallet code and therefore won't conflict with ongoing mega-refactors, and is currently up to da
...