💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2115522891)
sorry forgot to link https://github.com/bitcoin/bitcoin/pull/30072 here, added to OP

🚀 ryanofsky merged a pull request: "blockstorage: Separate reindexing from saving new blocks"
(https://github.com/bitcoin/bitcoin/pull/29975)

✅ willcl-ark closed an issue: "dumpprivkey error"
(https://github.com/bitcoin/bitcoin/issues/30124)

💬 willcl-ark commented on issue "dumpprivkey error":
(https://github.com/bitcoin/bitcoin/issues/30124#issuecomment-2115538080)
You should be able to get the info you need using `bitcoin-cli listdescriptors true`, to include private descriptors.

If that doesn't work for you, you can create a legacy BDB wallet (using a newer version of the software) by following the v26.0 wallet [release notes](https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-26.0.md#wallet). Of particular importance is use of the `-deprecatedrpc=create_bdb` option to `bitcoind`, before running `bitcoin-cli createwallet ..
...

🤔 ryanofsky reviewed a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2061116999)
Updated b58156701ac132f87b8ef8da1c7d22158c804a81 -> 7689dfd6646054a0be8e62ccbf72d5403e28b548 ([`pr/rmutil.17`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.17) -> [`pr/rmutil.18`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.18), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/rmutil.17..pr/rmutil.18)) moving check-deps script.

---

re: https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2055052953

> The includes could be cleaned up a bit, especi
...

💬 ryanofsky commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1603561157)
re: https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1599843905

> I think rather than `test/` this should go into `contrib/devtools`, which contains functionally similar scripts like `symbol-check`, or `security-check`.

Good suggestion, it does fit in more with those tools, moved over there.

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1603602382)
Removed now, thanks

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1603602970)
Removed and added a scripted-diff commit to rename `EraseTxNoLock` to `EraseTxInternal`

💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1574716741)
While in here, maybe fix the typo

```suggestion
By default the cookie is stored in the data directory, but its location can be
```

💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1574722548)
We already print settings somewhere else, and this isn't where the actual permissions are being changed.

💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1574723868)
nit: "all" seems better than "others" here IMO

💬 hebasto commented on issue "Testsuite for Bitcoin Core 27.0.0 - FAIL: qt/test/test_bitcoin-qt":
(https://github.com/bitcoin/bitcoin/issues/30020#issuecomment-2115593433)
@dominicusadinfinitum
> where or how do I get to the debug.log?

In your data directory. Please consult the https://github.com/bitcoin/bitcoin/blob/master/doc/files.md#data-directory-layout.

💬 theuni commented on pull request "rpc: avoid copying into UniValue":
(https://github.com/bitcoin/bitcoin/pull/30115#issuecomment-2115593653)
> Could this be a case for a clang-tidy plugin?

I considered this, but I don't think so. UniValue copies are reasonable in many cases, but our use of them often lends itself to moving. So we can't detect and disable copies as a rule, and (I assume) if clang-tidy detect and suggest possible moves as optimizations, it would be offering a generic version of that already.

Here's an upstream discussion about it that has apparently gone stale: https://github.com/llvm/llvm-project/issues/53489

💬 hebasto commented on pull request "rpc: avoid copying into UniValue":
(https://github.com/bitcoin/bitcoin/pull/30115#issuecomment-2115594712)
Concept ACK.

🤔 vasild reviewed a pull request: "net: Replace libnatpmp with built-in PCP implementation"
(https://github.com/bitcoin/bitcoin/pull/30043#pullrequestreview-2061123015)
Concept ACK

💬 vasild commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1603565742)
The following patch uses a netlink socket to get the information from the kernel, that is supported on (at least) Linux and FreeBSD>=13.2:

<details>
<summary>[patch] get default gateway using a netlink socket</summary>

```diff
--- a/src/test/netbase_tests.cpp
+++ b/src/test/netbase_tests.cpp
@@ -1,25 +1,34 @@
// Copyright (c) 2012-2022 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licen
...

💬 hebasto commented on pull request "bench: enable wallet creation benchmarks on all platforms":
(https://github.com/bitcoin/bitcoin/pull/30122#issuecomment-2115606470)
Concept ACK.

💬 ajtowns commented on pull request "policy: Allow non-standard scripts with -acceptnonstdtxn=1 (test nets only)":
(https://github.com/bitcoin/bitcoin/pull/29843#issuecomment-2115614319)
Bumped past #29086

💬 ryanofsky commented on pull request "rpc: avoid copying into UniValue":
(https://github.com/bitcoin/bitcoin/pull/30115#issuecomment-2115622164)
> Could this be a case for a clang-tidy plugin?

I think the ideal thing to do for types that are potentially expensive to copy is to disable implicit copies, but provide explicit `T Copy() const` and `void CopyFrom(const T&)` methods so copies can be made where needed. (There was a `DISABLE_IMPLICIT_COPIES` macro proposed in #24641 that tried to do this, but it didn't seem possible to generalize the implementation so we never added it.) It think it would be probably be reasonable to allow uni
...

💬 instagibbs commented on pull request "policy: restrict all TRUC (v3) transactions to 25KvB":
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2115624871)
From a more general wallet perspective if we're assuming ~110vbytes of overhead per additional transaction required to match an input/output set, picking 10kvB as the new limit vs 100kvB results in 10*110=1,100vb, or ~1% additional overhead vs 100kvB. At 5kvB you'd be receiving ~2% penalty, and so on.

FWIW 10kvB also seems like plenty of headroom for practical LN channels with >60-HTLCs-per-direction, but below today's spec limit.