💬 davidgumberg commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#issuecomment-2905941970)
ACK https://github.com/bitcoin/bitcoin/pull/32404/commits/53e9b71b2fd59c18b75e45e3a24c39182c20a59e

💬 davidgumberg commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2105490756)
non-blocking nit: `uint8_t` here seems unnecessary

📝 benthecarman opened a pull request: "rpc: Note in fundrawtransaction doc, fee rate is for package"
(https://github.com/bitcoin/bitcoin/pull/32607)
Accidentally made some transactions with a much higher fee rate than I wanted because I did not know this would do it for the package rather than the individual tx.

💬 davidgumberg commented on pull request "rpc: Note in fundrawtransaction doc, fee rate is for package":
(https://github.com/bitcoin/bitcoin/pull/32607#discussion_r2105492913)
```suggestion
"Only pay-to-pubkey, multisig, and P2SH versions thereof are currently supported for watch-only\n"
"Note that if specifying an exact fee rate, the resulting transaction may have a higher fee rate\n"
"if the transaction has unconfirmed inputs, this is because the wallet will attempt to make the\n"
"entire package have the given fee rate, not the resulting transaction.",
{
```

💬 roconnor-blockstream commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r2105493300)
Isn't it a bit odd to reserve `CHECKSUM_SIZE` extra bytes, when only `CreateChecksum` uses these extra bytes, but `VerifyChecksum` does not?

💬 benthecarman commented on pull request "rpc: Note in fundrawtransaction doc, fee rate is for package":
(https://github.com/bitcoin/bitcoin/pull/32607#discussion_r2105493581)
woops

💬 roconnor-blockstream commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r2105493939)
Nevermind @theuni commented about this above.

💬 gmaxwell commented on issue "compact block fingerprinting":
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-2905966115)
I wouldn't suggest bumping the HB limit at this time, as roundtripping to get transactions seems to be the long pole in the tent right now.

💬 hMsats commented on issue "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2905997778)
Couldn't find a commit error (luckily). Improved a few things on my server and now it seems to be working well:

Removed permitbaremultisig=0, datacarrier=0, increased my reduced cpu speed, give bitcoind more time before starting public electrum server Fulcrum and Core Lightning (CLN). Maybe 29.0 it a bit more demanding than 28.0. Sorry for the confusion and thanks for all the help!

✅ hMsats closed an issue: "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system"
(https://github.com/bitcoin/bitcoin/issues/32455)

💬 BitcoinMechanic commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2906118037)
> > We’ve already seen a clear signal that people don’t want this, with a noticeable migration from Bitcoin Core to Bitcoin Knots over the past three weeks.
>
> It’s clear we’ve agreed to disagree: people who prefer arbitrary filters will run Bitcoin Knots, and people who prefer harmless, consensus-valid transactions will run Bitcoin Core.

Part of the motivation for this PR is that large OP RETURNs are *less* harmful than fake pub keys, with the understanding that both are still harmful.

💬 achow101 commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2105646397)
I don't think that's necessary. Many functions make db changes internally and don't have such comments.

💬 achow101 commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2105647289)
I think documenting this is rather orthogonal, and especially documenting the coin unlock specifically since it is incidental and not the reason this function exists at all.

The reason we unlock is to not pollute the wallet with things that are meaningless; things that are spent don't need to be locked.

💬 achow101 commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2105647692)
`lockunspent` did not originally persist to disk. This is a (relatively) recent addition. The point of this is to refactor the code so that it can simplify future PRs, not change the existing behavior.

💬 achow101 commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2105648603)
We never automatically lock coins, all locked coins must be initiated by the user explicitly, whether that is `lockcoin` or they specify to lock inputs during funding. Locking automatically would result in users being unable to spend some of their funds which we generally want to avoid if there's no good reason to do that behavior.

💬 earonesty commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2906258968)
wouldn't it be reasonable to make the cap a "little bigger by default" as
a conservative change, and just to see if it causes an uptick in average
op_return sizes and uses as storage?

its not so urgent to allow 10kb op_returns

and data *carrier* limits are more about protecting the p2p and mempool
layers - not protecting from inclusion in blocks

the knock-on effects of arbitrary data at the p2p layer could be severe in
ways that have nothing to do with block storage

On Fri, May
...

👍 rkrux approved a pull request: "walletdb: Log additional exception error messages for corrupted wallets"
(https://github.com/bitcoin/bitcoin/pull/32598#pullrequestreview-2866176097)
ACK ad9a13fc424e9deb262e2b1d54bcdc7370263ea0

📝 koyaness opened a pull request: "Small improvement to the documentation"
(https://github.com/bitcoin/bitcoin/pull/32609)
Small improvement to the documentation to improve readability and understanding.

💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2906544041)
> `CNode` was supposed to become internal and not passed into `PeerManager` at all

Do you need any help with that?

💬 rkrux commented on pull request "wallet, rpc: clarify wallet version in getwalletinfo help":
(https://github.com/bitcoin/bitcoin/pull/32603#discussion_r2105749398)
Makes sense, done.