💬 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

💬 maflcko commented on pull request "test: ensure output is large enough to pay for its fees":
(https://github.com/bitcoin/bitcoin/pull/29283#discussion_r1468842089)
```
test 2024-01-27T00:59:20.405000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 131, in main
self.run_test()
File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functiona
...

💬 maflcko commented on issue "Clang 14 emits `-Wunreachable-code` warnings":
(https://github.com/bitcoin/bitcoin/issues/29334#issuecomment-1913576851)
It is sometimes possible for older compilers to print false positive warnings.

This warning can safely be ignored.

💬 hebasto commented on issue "Clang 14 emits `-Wunreachable-code` warnings":
(https://github.com/bitcoin/bitcoin/issues/29334#issuecomment-1913579542)
> It is sometimes possible for older compilers to print false positive warnings.

I agree.

> This warning can safely be ignored.

It might require some efforts if building routines involve `-Werror`.

💬 ns-xvrn commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1913598137)
@petertodd : I would suggest you to pay more attention to how Utreexo will work. It may not be as easy to alleviate the problems of UTXO bloat as you think. It might be good for mobile phone nodes if it successfully bootstraps via bridge nodes, assuming that entire network can transition to that quickly and securely might be a very bad assumption. It will also come at the cost of increased latency which you're often concerned about but conveniently ignore here.
We might have to do both i.e. wo
...

💬 tdb3 commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1913628864)
ACK. Great work. I support the near-term goal of adding more correct handling of JSON-RPC 2.0 and the longer-term goal of deprecating 1.x, culminating in the eventual removal of 1.x support (and cleanup from refactoring out support of 1.x).

Noting findings/checks for archival purposes:
Cloned, checked out the PR branch (98031401c744ea3c2b62927fabf27c22b639c3cf), built, ran all functional tests (all passed), then executed bitcoind against a private signet. Issued RPC requests with curl. I
...

🤔 tdb3 reviewed a pull request: "Support JSON-RPC 2.0 when requested by client"
(https://github.com/bitcoin/bitcoin/pull/27101#pullrequestreview-1847563884)
ACK. Great work. I support the near-term goal of adding more correct handling of JSON-RPC 2.0 and the longer-term goal of deprecating 1.x, culminating in the eventual removal of 1.x support (and cleanup from refactoring out support of 1.x).

Noting for archival purposes:
Cloned, checked out the PR branch (98031401c744ea3c2b62927fabf27c22b639c3cf), built, ran all functional tests (all passed), then executed bitcoind against a private signet. Issued RPC requests with curl (getblockchaininfo
...

💬 tdb3 commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1468881262)
nit:
Minor non-compliance to the JSON RPC 2.0 spec (`rpc call with an empty Array` in https://www.jsonrpc.org/specification#examples). Providing an empty array results in an empty array provided in the response, but should result in a single error response (e.g. `"jsonrpc": "2.0", "error": {"code": -32600, "message": "Invalid Request"}, "id": null}`).

A check for `valRequest.size() == 0` before entering the for loop in 236 could enable error response (rather than empty response array). P
...