💬 sipa commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1468584354)
In commit "wallet: Add CoinGrinder coin selection algorithm":

Only one coin selection algorithm is run here (unless you count the brute forcing).

💬 furszy commented on issue "New crash in v26.0":
(https://github.com/bitcoin-core/gui/issues/785#issuecomment-1913324146)
It seems you are running a prune node. Please with the logs, provide your bitcoin.conf values (hiding the sensitive information).

💬 furszy commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1468635643)
This does not seem to be necessary. As `sendall()` doesn't make use of the internal coin selection procedures, wouldn't make more sense to return the weight in the command result and let the user decide whether to broadcast the tx based on it?

🤔 furszy reviewed a pull request: "net: don't lock cs_main while reading blocks in net processing"
(https://github.com/bitcoin/bitcoin/pull/26326#pullrequestreview-1847265131)
Re-ACK f9d4152.
Since my last review, this PR was only rebased and a comment was added.

⚠️ hebasto opened an issue: "Clang 14 emits `-Wunreachable-code` warnings"
(https://github.com/bitcoin/bitcoin/issues/29334)
```
% clang --version
Apple clang version 14.0.0 (clang-1400.0.29.202)
Target: x86_64-apple-darwin21.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
```

```
...
CXX wallet/libbitcoin_wallet_a-walletdb.o
wallet/walletdb.cpp:1518:15: warning: code will never be executed [-Wunreachable-code]
error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", fs::PathToString(path)));

...

💬 TheCharlatan commented on pull request "Add missed `config/bitcoin-config.h` header":
(https://github.com/bitcoin/bitcoin/pull/29333#issuecomment-1913340159)
What motivates changing only this single file and not the various other modules that iwyu reports to have unused or missing `bitcoin-config.h` includes?

✅ hebasto closed a pull request: "Add missed `config/bitcoin-config.h` header"
(https://github.com/bitcoin/bitcoin/pull/29333)

💬 hebasto commented on pull request "Add missed `config/bitcoin-config.h` header":
(https://github.com/bitcoin/bitcoin/pull/29333#issuecomment-1913342542)
> What motivates changing only this single file and not the various other modules that iwyu reports to have unused or missing `bitcoin-config.h` includes?

It was spotted during debugging another issue. Agree that it looks like a random diff.

✅ petertodd closed a pull request: "RBF: Require unconfirmed inputs to come from a single conflicting transaction"
(https://github.com/bitcoin/bitcoin/pull/29297)

💬 petertodd commented on pull request "RBF: Require unconfirmed inputs to come from a single conflicting transaction":
(https://github.com/bitcoin/bitcoin/pull/29297#issuecomment-1913382743)
Closing. Turns out the exploit this was meant to solve doesn't actually exist: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2024-January/022314.html

💬 petertodd commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1913477451)
@chrisguida Let me expand on my NACK: given how easy it is for miners to adopt profit-maximizing transaction policies, and how easy it is for people to relay profit-maximizing transactions to them, we should not try to filter out profitable transactions. That'll just make life more difficult for smaller miners and encourage the growth of out-of-band transaction submission mechanisms that only large miners can realistically profit from.

We'd all be better off focusing our energy on technical i
...

💬 BitcoinMechanic commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1913486716)
@petertodd

>we should not try to filter out profitable transactions

That is not what I or I believe others wish to do. The concern is the circumnavigating around sensible restrictions on arbitrary data storage, not presence of a relatively high fee.

>That'll just make life more difficult for smaller miners and encourage the growth of out-of-band transaction submission mechanisms

This is something that can only be offered by the larger pools which, as you point out can create centra
...

📝 BrandonOdiwuor opened a pull request: "test: Handle functional test disk-full error"
(https://github.com/bitcoin/bitcoin/pull/29335)
Fixes: https://github.com/bitcoin/bitcoin/issues/23099

Handle disk-full more gracefully in functional tests

💬 totient commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1913544845)
@petertodd Is it your view that all PR's should be ACK'd or NACK'd based on how much miners can profit from the PR?

There are a great many PR's I could imagine which could have an impact on miner profitability, but I don't think the Core client development path should depend on whether the community decides at any point in time to arbitrarily raise or lower miner profitability.

I don't understand how debating Bitcoin miner P&L statements is germane to optimizing the Core client codebase. P
...

🤔 mccrudd3n reviewed a pull request: "bitcoin-cli help detail to show full help for all RPCs"
(https://github.com/bitcoin/bitcoin/pull/29163#pullrequestreview-1847481729)
Concept ACK though I dont think this is needed due to the `./bitcoin-cli help` to view all cmds, and `./bitcoin-cli help [cmd]` to get additional information about that command

💬 petertodd commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1913560193)


On January 28, 2024 12:17:35 PM GMT+02:00, totient ***@***.***> wrote:
>@petertodd Is it your view that all PR's should be ACK'd or NACK'd based on how much miners can profit from the PR?

Mining is a zero sum game, so PRs that increase or decrease profitability evenly usually don't matter.

This PR however changes profitability unevenly, giving large miners an advantage by preventing small miners from getting access to profitable information: fee paying transactions that use bare multi
...

💬 josibake commented on pull request "indexes: Stop using node internal types and locking cs_main, improve sync logic":
(https://github.com/bitcoin/bitcoin/pull/24230#issuecomment-1913572083)
Concept ACK

Found this from https://github.com/bitcoin/bitcoin/blob/5fbcc8f0560cce36abafb8467339276b7c0d62b6/src/index/base.cpp#L87 while working with the indexing code.

💬 maflcko commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1468839187)
nit: In python the `()` are not needed. Also, could use f-strings?

💬 maflcko commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1468839064)
Where comes 50+16 from?

💬 BrandonOdiwuor commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1468841627)
I based the values based on the discussions on https://github.com/bitcoin/bitcoin/pull/23100#issuecomment-941115724 and https://github.com/bitcoin/bitcoin/pull/23100#discussion_r716825510 and