💬 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.

💬 MarcoFalke commented on pull request "github: Switch to yaml issue templates":
(https://github.com/bitcoin/bitcoin/pull/27025#discussion_r1132224642)
Looks like this wasn't addressed?

💬 vasild commented on pull request "Avoid integer overflow in CheckDiskSpace":
(https://github.com/bitcoin/bitcoin/pull/27235#issuecomment-1463633762)
Just for reference, like @MarcoFalke pointed above:

```
node1 stderr util/system.cpp:138:51: runtime error: unsigned integer overflow: 52428800 + 18446744073709551615 cannot be represented in type 'unsigned long'
#0 0x564a482b5088 in CheckDiskSpace(fs::path const&, unsigned long) src/./src/util/system.cpp:138:51
#1 0x564a4728dc59 in AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/./src/init.cpp:1639:14
#2 0x564a47256e6a in AppInit(node::NodeContext&, i
...

📝 TheCharlatan opened a pull request: "refactor: Split logging utilities from system.h"
(https://github.com/bitcoin/bitcoin/pull/27238)
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". These commits were originally authored by @empact and are taken from their parent PR #25152.

#### Context

There is an ongoing effort to decouple the `ArgsManager` used for command line parsing user-provided arguments from the libbitcoinkernel libr
...

💬 vasild commented on pull request "Avoid integer overflow in CheckDiskSpace":
(https://github.com/bitcoin/bitcoin/pull/27235#discussion_r1132241660)
Do you mean `SaturatingAdd(min_disk_space, additional_bytes)`? If yes, then in this case it would end up with `UINT64_MAX` and this function will always report that there is not enough disk space (because that number is so big, nobody has so much disk space).

In this case, I think neither `0` or `UINT64_MAX` is suitable. In manual pruning mode, I guess, it makes sense to assume that the user will delete some blocks but not much and thus the required disk space is as if in non-pruned mode - `A
...