💬 mzumsande commented on pull request "rpc: add 'getnetmsgstats' RPC":
(https://github.com/bitcoin/bitcoin/pull/28926#discussion_r1443508738)
Would be nice to have some validation of user input here - `getnetmsgstats '["foo","bar"]'` could return an error message, listing the supported dimensions.

💬 mzumsande commented on pull request "rpc: add 'getnetmsgstats' RPC":
(https://github.com/bitcoin/bitcoin/pull/28926#discussion_r1443492341)
nit: capital M, here and in `messageTypeFromIndex`

💬 mzumsande commented on pull request "rpc: add 'getnetmsgstats' RPC":
(https://github.com/bitcoin/bitcoin/pull/28926#discussion_r1443485004)
I agree: Code and constants that are only used in RPCs are not part of the protocol and therefore don't belong in `protocol.h` / `protocol.cpp`.

💬 achow101 commented on pull request "wallet: Allow users to create a wallet that encrypts all database records":
(https://github.com/bitcoin/bitcoin/pull/28142#issuecomment-1879421327)
Marking as up for grabs.

✅ achow101 closed a pull request: "wallet: Allow users to create a wallet that encrypts all database records"
(https://github.com/bitcoin/bitcoin/pull/28142)

✅ achow101 closed a pull request: "wallet: Have the wallet store the key for automatically generated descriptors"
(https://github.com/bitcoin/bitcoin/pull/26728)

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1879428075)
Superseded by #29130

I don't think there's a reason to come back to this design, but the branch won't be deleted so we can reopen if desired.

✅ achow101 closed a pull request: "wallet: rpc to add automatically generated descriptors"
(https://github.com/bitcoin/bitcoin/pull/25907)

💬 achow101 commented on pull request "wallet: rpc to add automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/25907#issuecomment-1879428850)
Superseded by #29130

I don't think there's a reason to come back to this design, but the branch won't be deleted so we can reopen if desired.

✅ achow101 closed a pull request: "wallet: reenable sethdseed for descriptor wallets"
(https://github.com/bitcoin/bitcoin/pull/29054)

💬 achow101 commented on pull request "wallet: reenable sethdseed for descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/29054#issuecomment-1879430043)
Superseded by #29136

💬 sanket1729 commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1879451346)
concept ACK. cc @danielabrozzoni @notmandatory (BDK group)

This is the list of [dependencies](https://crates.io/crates/bitcoinconsensus/reverse_dependencies) that rely on libbitcoinconsensus.

I believe most of them use libbitcoinconsensus for testing script execution, with the only exception of `floresta`? While I found it beneficial for cross-testing complex scripts, the absence of core policy rule checking became a notable drawback.

We've transitioned our projects to utilize the [bitc
...

💬 desi-bitcoiner commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1879545603)
Concept ACK

🤔 ismaelsadeeq reviewed a pull request: "rpc: validate fee estimation mode case insensitive"
(https://github.com/bitcoin/bitcoin/pull/29175#pullrequestreview-1807294664)
CI failure is unrelated?

💬 ismaelsadeeq commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#discussion_r1443655175)
This should test for a failed send RPC call because `estimatemode` is unset.

It is passing because `arg_conf_target` was not given, currently `estimatemode` is only validated when a confirmation target is passed.
I omit `fee_rate` because you should not pass both confirmation target and fee rate.

```suggestion
self.test_send(from_wallet=w0, to_wallet=w1, amount=1,
arg_estimate_mode=mode, arg_conf_target=1, add_to_wallet=False, expect_error = (-8, 'Specify e
...

💬 ismaelsadeeq commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#discussion_r1443657365)
`estimatemode` is validated when passed with confirmation target
```suggestion
res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1,
arg_conf_target=1, arg_estimate_mode=mode, add_to_wallet=False
)
assert_equal(res["complete"], True)

```

💬 ismaelsadeeq commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#discussion_r1443655347)
nit
```suggestion
assert_equal(res["complete"], True)
```

💬 ismaelsadeeq commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#discussion_r1443657718)
I think we should also return error when `estimatemode` is passed without confirmation target and a a test for the test case ?
Maybe in its own commit or a follow-up PR.
```suggestion
}
if (options["conf_target"].isNull() && !options["estimate_mode"].isNull()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "estimate_mode should be passed with conf_target");
}
```

💬 1440000bytes commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1879598180)
Since CVE ID is used to validate this as a [vulnerability](https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1848850110), I wanted to share that [cve.org](https://cve.org) has added "disputed" tag for CVE-2023-50428. This tag is used when there are differences of opinion about whether its a vulnerability based on the CVE program's definition.


A note has also been added to CVE by MITR
...

⚠️ dev7ba opened an issue: "Failed load mempool when restarting bitcoind"
(https://github.com/bitcoin/bitcoin/issues/29193)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

After a clean shutdown, bitcoind failed to load the majority of mempool transactions from mempool.dat. I keep a node up for months for webpage mempoolexplorer.com with options:
maxmempool=2000
mempoolfullrbf=1
mempoolexpiry=999999
to maximize the number of txs in my mempool.
The node is currently loading and saving mempool txs when shutdown ok (but with only a small number of them ins
...