💬 hebasto commented on pull request "Renamed UniValue::__pushKV to UniValue::pushKVEnd.":
(https://github.com/bitcoin/bitcoin/pull/27822#issuecomment-1575641606)
See https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#scripted-diffs for changes like this.

💬 theStack commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1216930591)
Thanks, done.

💬 theStack commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#issuecomment-1575651619)
Thanks for reviewing! Addressed the nit.

> Just a thought that came to my mind while reviewing it:
>
> * Seems like these test coverage uses just 1 node, if we'd stop node1 right before line 81, it wouldn't make the test to fail. So, it seems that `-minrelaytxfee=0` is just to make `send_self_transfer` - which calls `sendrawtransaction` - not fail (even if we might not have any connection).

Yes indeed, this sub-test works with a single node (it follows the design of previously introduce
...

💬 joostjager commented on issue "Allow accepting non-standard transactions on mainnet via local rpc":
(https://github.com/bitcoin/bitcoin/issues/27768#issuecomment-1575661768)
For me the relevant part of this issue is enabling the annex, which is already discussed in several other places. Closing issue.

✅ joostjager closed an issue: "Allow accepting non-standard transactions on mainnet via local rpc"
(https://github.com/bitcoin/bitcoin/issues/27768)

💬 glozow commented on pull request "Avoiding unnecessary std::string copy in ArgsManager::GetPathArg argument list":
(https://github.com/bitcoin/bitcoin/pull/27812#issuecomment-1575663213)
Thank you for your contribution. We have a large amount of PRs requiring review attention, so closing this. Please see our contributing guidelines on [refactoring](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refactoring) and [getting started](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#getting-started).

✅ glozow closed a pull request: "Avoiding unnecessary std::string copy in ArgsManager::GetPathArg argument list"
(https://github.com/bitcoin/bitcoin/pull/27812)

💬 kevkevinpal commented on pull request "wallet: Add tracing for sqlite statements":
(https://github.com/bitcoin/bitcoin/pull/27801#issuecomment-1575665937)
ACK https://github.com/bitcoin/bitcoin/commit/ff9d961bf38b24f8f931dcf66799cbc468e473df

💬 theStack commented on pull request "test: avoid sporadic MINIMALDATA failure in feature_taproot.py (fixes #27595)":
(https://github.com/bitcoin/bitcoin/pull/27631#issuecomment-1575671961)
Added a comment, as suggested by @instagibbs. Let me know what you think, suggestions very welcome.

@Sjors: thought about that approach too, but it seems that the devil is in the details; in your example, passing a single zero-byte would now wrongly translate to a OP_0 operation, which pushes a zero-length array instead. Concept ACK on fixing the test framework's CScript class to always correctly emit minimal-encoded scripts, but that seems to be a bigger operation that ideally also includes
...

📝 mzumsande opened a pull request: "init: return error when block index is non-contiguous, fix feature_init.py file perturbation"
(https://github.com/bitcoin/bitcoin/pull/27823)
When the block index database is non-contiguous due to file corruption (i.e. it contains indexes of height `x-1` and `x+1`, but not `x`), bitcoind can currently crash with an assert in `BuildSkip()` / `GetAncestor()` during `BlockManager::LoadBlockIndex()`:
```
bitcoind: chain.cpp:112: const CBlockIndex* CBlockIndex::GetAncestor(int) const: Assertion `pindexWalk->pprev' failed.
```
This PR changes it such that we instead return an `InitError` to the user.

I stumbled upon this because I no
...

💬 mzumsande commented on pull request "p2p, rpc: Manual block-relay-only connections with addnode":
(https://github.com/bitcoin/bitcoin/pull/24170#issuecomment-1575678278)
Seems like other proposals such as #27213 and #27509 that rely on improving automatic behavior have more support compared to this one (which needs manual setup by the node operator). Closing for now.

✅ mzumsande closed a pull request: "p2p, rpc: Manual block-relay-only connections with addnode"
(https://github.com/bitcoin/bitcoin/pull/24170)

💬 Brotcrunsher commented on pull request "init: return error when block index is non-contiguous, fix feature_init.py file perturbation":
(https://github.com/bitcoin/bitcoin/pull/27823#discussion_r1217034392)
Wouldn't it be better to check for `!=` instead for `>`?

💬 mzumsande commented on pull request "init: return error when block index is non-contiguous, fix feature_init.py file perturbation":
(https://github.com/bitcoin/bitcoin/pull/27823#discussion_r1217039090)
I think that would be incorrect, because there will be multiple indexes for a given height in case of stale blocks / forks.

💬 Brotcrunsher commented on pull request "init: return error when block index is non-contiguous, fix feature_init.py file perturbation":
(https://github.com/bitcoin/bitcoin/pull/27823#discussion_r1217039358)
Actually it can't - it's sorted. But still the indices coudl be the same.

💬 Brotcrunsher commented on pull request "init: return error when block index is non-contiguous, fix feature_init.py file perturbation":
(https://github.com/bitcoin/bitcoin/pull/27823#issuecomment-1575685598)
Little disclaimer: It's my first Review for Bitcoin Core. So treat my input with care, I might have no clue what I am talking about.

What I don't like about this change is that previously the user at least had a chance of finding out what's wrong, without debugging the code. Now he won't have any info of what went wrong other than "Error loading block database", which can have multiple other reasons. Is there a way for us to communicate this better? Maybe a log? Or can we throw here? The sad
...

👍 theStack approved a pull request: "wallet: Add tracing for sqlite statements"
(https://github.com/bitcoin/bitcoin/pull/27801#pullrequestreview-1461441581)
ACK ff9d961bf38b24f8f931dcf66799cbc468e473df

💬 ariard commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1575919359)
> To be clear, 0conf users have to "upgrade" every single time mempool policies/conditions change for any non-trivial amount of hashing power. Pretty much any mempool policy change can be exploited to double-spend unconfirmed transactions. That's why the only entities with any hope of relying on them are large, centralized, transaction processors with significant engineering resources. The nVersion=3 proposal is not special in this regard.

Not all the mempool policies/condition changes are eq
...

💬 MikeBH9944 commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-1575959653)
jmy blocks

💬 MikeBH9944 commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-1575959693)
m