💬 pablomartin4btc commented on pull request "cli: add validation to cli side commands besides when it's used with -rpcwallet":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1131676111)
I've updated the error message as I've commented it [here](https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1131669863).

💬 pablomartin4btc commented on pull request "cli: add validation to cli side commands besides when it's used with -rpcwallet":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1131702758)
@andrewtoth I've updated the error message, check if that looks better for you.

💬 Crypto2 commented on issue "Incorrect balance reported in getwalletinfo/getbalance":
(https://github.com/bitcoin/bitcoin/issues/21768#issuecomment-1462931809)
Go for it :)

👍 theStack approved a pull request: "test: Use self.wait_until over wait_until_helper"
(https://github.com/bitcoin/bitcoin/pull/27226)
Code-review ACK faa671591f9c83ef0fb5afea151a1907c28f024b

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1131782831)
This may be easier to comprehend:
```
// Compare by min(ancestor feerate, individual feerate), then iterator.
//
// The ancestor feerate of a high-feerate tx should be reduced by its
// low-feerate ancestors, but the ancestor feerate of a low-feerate tx
// should not be increased by its high-feerate ancestors.
struct AncestorFeerateComparator
{
template<typename I>
bool operator()(const I& a, const I& b) const {
auto min_feerate = [](const MiniMinerMempoolEntry& e) ->
...

✅ adamjonas closed an issue: "Incorrect balance reported in getwalletinfo/getbalance"
(https://github.com/bitcoin/bitcoin/issues/21768)

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1463100208)
Thanks, I’m just seeing the review comments, I will continue looking into them tomorrow. I just pushed a documentation change for `CalculateBumpFees(…)`, after I spent large parts of the day figuring out _why_ the fuzzer had produced a crash and convincing myself that the fix was actually correct.

💬 arcivanov commented on issue "RPC `getblock` resulted in 500 and ReadBlockFromDisk: OpenBlockFile failed for FlatFilePos(nFile=-1, nPos=0)":
(https://github.com/bitcoin/bitcoin/issues/20978#issuecomment-1463197645)
I could check the logs to see if it's still the case.

👍 pablomartin4btc approved a pull request: "rest: add verbose and mempool_sequence query params for mempool/contents"
(https://github.com/bitcoin/bitcoin/pull/26207)
tested ACK 1ff5d61dfdaf8987e5619162662e4c760af76a43.

I've performed manual tests getting the node started with `-rest`, comparing results between the rest api call using curl with the different combinations of params and values vs. calling the `getrawtransacion` rpc command (using `bitcoin-cli`), which is essentially what the python functional tests do.

It works as expected and it's consistent with rpc.

💬 MarcoFalke commented on issue "Wallet: Reenable -fallbackfee by default for regtest and signet (and maybe testnet too)?":
(https://github.com/bitcoin/bitcoin/issues/20087#issuecomment-1463481470)
The issue didn't seem to attract much attention in the past. Also, the issue seems not important enough right now to keep it sitting around idle in the list of open issues.

Closing due to lack of interest and direction. Pull requests with improvements are always welcome.

✅ MarcoFalke closed an issue: "Wallet: Reenable -fallbackfee by default for regtest and signet (and maybe testnet too)?"
(https://github.com/bitcoin/bitcoin/issues/20087)

✅ MarcoFalke closed an issue: "Remove gArgs"
(https://github.com/bitcoin/bitcoin/issues/21005)

📝 MarcoFalke reopened a pull request: "rpc: Add submit option to generateblock"
(https://github.com/bitcoin/bitcoin/pull/18933)
When submit is turned off, a block can be generated and returned as hex, to be used for further tests. For example, it can be submitted on a different node, on a different interface (like p2p), or just never submitted and be used for other testing purposes.

📝 willcl-ark opened a pull request: "util: fix argsman dupe key error"
(https://github.com/bitcoin/bitcoin/pull/27236)
fixes #22638

If we find a duplicate key and error, clear `values` before returning so that WriteSettings will write an empty file, therefore clearing it.

This aligns with GUI behaviour added in 1ee6d0b.

The test added only checks that `values` is empty after a duplicate key is detected. This paves the way for the `abort` option in the GUI to properly clear `settings.json`, if the user selects the option, but the test does not currently check this entire mechanism (e.g. the file contents
...

📝 MarcoFalke opened a pull request: "2303 test block header throw 📞"
(https://github.com/bitcoin/bitcoin/pull/27237)
This adds a test, but is based on https://github.com/bitcoin/bitcoin/pull/18933, thus draft

💬 MarcoFalke commented on issue "RPC `getblock` resulted in 500 and ReadBlockFromDisk: OpenBlockFile failed for FlatFilePos(nFile=-1, nPos=0)":
(https://github.com/bitcoin/bitcoin/issues/20978#issuecomment-1463557219)
@adamjonas See the last commit in https://github.com/bitcoin/bitcoin/pull/27237 on how to reproduce.

💬 MarcoFalke commented on pull request "rpc: Add submit option to generateblock":
(https://github.com/bitcoin/bitcoin/pull/18933#issuecomment-1463561721)
> Do we want to skip that check as well?

Should be trivial to add in a follow-up with a one-line patch, if and when needed?

💬 MarcoFalke commented on pull request "rpc: fix logging RPC when "none" values are passed, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#discussion_r1132214754)
```suggestion
static void EnableOrDisableLogCategories(const UniValue& categories, bool enable)
{
```

nit: While touching this, can avoid the forth copy of the same thing here as well :)

💬 MarcoFalke commented on pull request "Avoid integer overflow in CheckDiskSpace":
(https://github.com/bitcoin/bitcoin/pull/27235#discussion_r1132218514)
Seems an odd way to write this when you can just use `SaturatingAdd`?

💬 MarcoFalke commented on pull request "Avoid integer overflow in CheckDiskSpace":
(https://github.com/bitcoin/bitcoin/pull/27235#issuecomment-1463614571)
> https://cirrus-ci.com/task/5059018708746240

Not sure about referring to an URL that will auto-delete in 90 days. It would be better to at least copy-paste the relevant lines, or even better, add a test case.