✅ achow101 closed an issue: "dumpprivkey error"
(https://github.com/bitcoin/bitcoin/issues/30129)

💬 achow101 commented on issue "dumpprivkey error":
(https://github.com/bitcoin/bitcoin/issues/30129#issuecomment-2116532819)
As the error message says, your wallet is not compatible with the `dumpprivkey` command. @edilmedeiros has already laid out your alternatives, but there is no way to make your specific wallet work with `dumpprivkey`.

💬 ygcool commented on issue "dumpprivkey error":
(https://github.com/bitcoin/bitcoin/issues/30129#issuecomment-2116537647)
thank.
Solved it:

`bitcoind -deprecatedrpc=create_bdb -daemon`

`bitcoin-cli -named createwallet "walletname" descriptors=false`

`bitcoin-cli getnewaddress`

`bitcoin-cli dumpprivkey "address"`

🤔 tdb3 reviewed a pull request: "rpc: introduce getversion RPC"
(https://github.com/bitcoin/bitcoin/pull/30112#pullrequestreview-2062178448)
Concept ACK.

Great work. This seems like an improvement over whitelisting `getnetworkinfo` and reduces exposed info.

https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2114646750
> getrpcversion maybe? The reason i bring it up is that it needs to be restrictive enough in scope to just the RPC interface version, not wallet version, not P2P versions, etc.

I generally agree with this comment that the scope of this new RPC should be constrained where it makes sense. A more specif
...

💬 tdb3 commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#discussion_r1604241946)
Thinking out loud about this one: Are there instances where `long` would be insufficient for determining if the client/node is a release? In the examples provided in the opening post, it seems like this could be determined by lack of commit hash and rc designation. It definitely would be convenient for the RPC user to have a bool provided (and prevents burdening the user with extra parsing or interpretation), so I see value in keeping this field.

💬 stratospher commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#issuecomment-2116717090)
> I think it'd be better to have multiple classes that build from v2_p2p and modify what's needed, even if the file gets larger.

that's a great suggestion @sr-gi! it's possible to design those classes in such a way that it avoids code duplication and the file is almost the same size!

> Turns out if you removed both changes, the tests still passes, but I'm not sure why.

because we don't send a version message to ensure that the disconnection happens in the v2 handshake phase. in your ca
...

💬 josibake commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-2116882706)
reACK https://github.com/bitcoin/bitcoin/commit/d51fbab4b32d56765e8faab6ad01245fb259b0ca

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2117004469)
Added a (mostly untested for now) Windows implementation of `QueryDefaultGateway`, if someone could test this it'd be very helpful. i will try in WINE later.

💬 josibake commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1604550196)
ah, my bad. I had even read that comment that @glozow linked to earlier but it didn't click for me that by the time we get to the block we've already processed transactions and excluded the child transaction. I think a comment explaining what you just explained here would help a lot, maybe at the call site of `removeParentTxs`?

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

👍 TheCharlatan approved a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2062665306)
Re-ACK 1c26d10b23bb7a7d0204b4a8b44a50fad89f2a2a

✅ laanwj closed a pull request: "[PoC] qt, depends: Add wayland support without build-time nor fixed run-time deps"
(https://github.com/bitcoin/bitcoin/pull/29959)

💬 laanwj commented on pull request "[PoC] qt, depends: Add wayland support without build-time nor fixed run-time deps":
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2117037519)
Closing for now. It exists as a PoC, if anyone is truly interested in wayland support in bitcoin core's binary releases, and interested in helping test and review, let me know. Right now, it seems something people mostly ask about just to ask about.

💬 willcl-ark commented on pull request "[PoC] qt, depends: Add wayland support without build-time nor fixed run-time deps":
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2117042080)
Thanks @laanwj

Perhaps we close #19950 with the same rationale?

💬 laanwj commented on pull request "[PoC] qt, depends: Add wayland support without build-time nor fixed run-time deps":
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2117053619)
> Perhaps we close https://github.com/bitcoin/bitcoin/issues/19950 with the same rationale?

i'm not sure. Pretty sure it's something that people will open new issues about endlessly and then forget about them. Might as well leave this one open so there's context.

💬 laanwj commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2117084796)
> I'm partial to getnodeversion or getclientversion, since RPC is implicitly versioned by the major version of the client/node

The problem with those, imo, is that `client` and `node` are both words associated with general networking. They might as well return P2P protocol version information. If this call is intended to return RPC versions and capabilities, then that should probably be explicit in the name and documentation. But maybe i'm misunderstanding the intent.

> If in the future RP
...

💬 glozow commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1604604969)
Agree there's room for improvement clarity-wise.

Perhaps a good approach is to have each flag control 1 part of the validation logic (even if 1 flag implies another flag)
- whether you call `LimitMempoolSize()` within `Finalize()`
- whether all conflicts should be rejected immediately
- whether `CheckFeeRate` should be skipped in `PreChecks`
- whether sibling eviction can be considered
- whether package RBF can be considered

The `ATMPArgs` constructors should decide the flags based on
...

🤔 glozow reviewed a pull request: "test: expand LimitOrphan and EraseForPeer coverage"
(https://github.com/bitcoin/bitcoin/pull/30082#pullrequestreview-2058508184)
lgtm

💬 glozow commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1601941249)
Note you've only jumped 1 second here

💬 glozow commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1601942742)
Would be nice if the constants were moved to txorphanage.h so we don't need the magic number here. Could be useful in other tests as well.