💬 adamjonas commented on issue "Incorrect balance reported in getwalletinfo/getbalance":
(https://github.com/bitcoin/bitcoin/issues/21768#issuecomment-1462756998)
@Crypto2 can you attempt to reproduce with a updated version, please?

💬 Crypto2 commented on issue "Incorrect balance reported in getwalletinfo/getbalance":
(https://github.com/bitcoin/bitcoin/issues/21768#issuecomment-1462833684)
Yep I'll keep an eye on it if/when the network gets backed up like that again.

💬 furszy commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1131024834)
This can be a `CTransactionRef` (aka shared_ptr) and not a plain tx copy. Transaction data never changes.

Just need to adapt the constructor to call `GetSharedTx` instead.

💬 furszy commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1131070331)
In a0d54e6b:

`mempool.CalculateDescendants` returns the base parent transaction. So you could remove this line and add a comment to remember it.

💬 furszy commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1131091527)
In https://github.com/bitcoin/bitcoin/commit/a0d54e6b8f8afab070de48911f3b11d4cdf289ff:

nano nit:
Could prevent extra `GetConflictTx` calls by moving the conflicting tx block of code inside the `mempool.exists` block.
e.g.

```c++
for (const auto& outpoint : outpoints) {
if (!mempool.exists(GenTxid::Txid(outpoint.hash))) {
// This UTXO is either confirmed or not yet submitted to mempool.
// If it's confirmed, no bump fee is required.
// If it's not yet
...

💬 furszy commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1131618449)
This has been puzzling me for a while.

It is not clear to me why need to calculate the descendants once more if they are all in the cluster.
Couldn't return from `GatherCluster` a map of tx id and descendants instead?
Would need to first go up, to the outpoints first parents, and then start going down (basically the DAG in map format).

Another point is why continue traversing the children when the parent was marked to be removed? To double assert that the previous loop worked as expected
...

💬 adamjonas commented on issue "Incorrect balance reported in getwalletinfo/getbalance":
(https://github.com/bitcoin/bitcoin/issues/21768#issuecomment-1462853559)
Mind if we close since we don't have any reports from others? Can reopen if you can isolate it again.

💬 mzumsande commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1126752055)
I think that it would be good to mention the 500 entries DoS limit here, plus the fact that we also return an empty vector if that is limit is breached - since that's something possible future users of this function would want to know about.

💬 mzumsande commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1128592892)
Indentation is broken, here and below (I initially thought the for loop ended here).

💬 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_r1131669863)
I've updated the test with the full error message, also I've changed the text a bit:

`Multiple wallets are loaded. Please specify a wallet by requesting the RPC through the /wallet/<filename> URI path. Using "bitcoin-cli", add the "-rpcwallet=<filename>" option before the command (run "bitcoin-cli -h" for help or "bitcoin-cli listwallets" to see which wallets are currently loaded).`

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