💬 josibake commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1603028373)
in https://github.com/bitcoin/bitcoin/pull/30079/commits/a6e81336897bfa472b79c82899584b2763af99e4 ("ignore all transaction with in block child"):

Shouldn't we also remove the child? Otherwise we will overestimate fees, unless I'm missing something? Example:

* Parent pays 1 sat/vb
* 10 sat/vb per tx is required to be competitive for the next block
* Child pays 20 sat/vb to try and get them + parent to 10 sat/vb
* Now in fee estimation, we see 20 sat/vb for a child that *could* have only
...

💬 josibake commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1603035220)
yeah, would recommend this be a separate PR, since there are likely multiple instances in other tests where we could replace the satToBtc conversion with a utility function in the test framework.

💬 hebasto commented on issue "Testsuite for Bitcoin Core 27.0.0 - FAIL: qt/test/test_bitcoin-qt":
(https://github.com/bitcoin/bitcoin/issues/30020#issuecomment-2114790520)
@dominicusadinfinitum

https://github.com/bitcoin/bitcoin/issues/30020#issuecomment-2090005933 ?

✅ laanwj closed a pull request: "test: Add large aligned vmov check for mingw"
(https://github.com/bitcoin/bitcoin/pull/29874)

💬 laanwj commented on pull request "test: Add large aligned vmov check for mingw":
(https://github.com/bitcoin/bitcoin/pull/29874#issuecomment-2114791346)
Closing for now.

📝 fanquake locked a pull request: "lint: fixed typo in test_runner causing linter warning"
(https://github.com/bitcoin/bitcoin/pull/30106)
introduced in https://github.com/bitcoin/bitcoin/commit/357ad110548d726021547d85b5b2bfcf3191d7e3

This typo is causing an error in the linter, fixing this should remove this warning

```
test/functional/test_runner.py:651: insuffient ==> insufficient
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```

[link to cirrus ci warning](https://cirrus-ci.com/task/5926490804584448?logs=lint
...

🤔 rkrux reviewed a pull request: "test: Added test to ensure log and failure happen when work is less than active chainstate"
(https://github.com/bitcoin/bitcoin/pull/30105#pullrequestreview-2060313054)
Make successful, all functional tests also. Asked a question and provided a suggestion.

@kevkevinpal Should we get rid of this TODO on line 26 now?
https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_assumeutxo.py#L26

💬 rkrux commented on pull request "test: Added test to ensure log and failure happen when work is less than active chainstate":
(https://github.com/bitcoin/bitcoin/pull/30105#discussion_r1603079655)
@kevkevinpal We are loading the snapshot of `n0` in `n0` itslelf? Should it not be loaded into `n1`?

👍 TheCharlatan approved a pull request: "wallet: Implement independent BDB parser"
(https://github.com/bitcoin/bitcoin/pull/26606#pullrequestreview-2060338479)
Re-ACK c0e0a0affea3e3cee9a8910de4bae55a9bdd24cf

💬 fanquake commented on pull request "build: Enable `thread_local` for MinGW-w64 builds":
(https://github.com/bitcoin/bitcoin/pull/30099#issuecomment-2114842252)
Now that the research has been done, it should be added to the commit message and PR description, so it's clear why this change might be safe, or the previous thread_local assumptions misguided. The commit message of `The previously mentioned erroneous behavior is not an issue anymore.` explains nothing.

💬 glozow commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1603098402)
> although unclear to me why we don't also remove the child transaction from fee estimation to avoid overestimating

A child is already ignored, new mempool txns with unconfirmed parents are never added

https://github.com/bitcoin/bitcoin/blob/dd42a5ddea6a72e1e9cad54f8352c76b0b701973/src/validation.cpp#L1274-L1279
https://github.com/bitcoin/bitcoin/blob/dd42a5ddea6a72e1e9cad54f8352c76b0b701973/src/policy/fees.cpp#L614-L615

💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1603108416)
Thanks for your review, Josie. I think you are missing something.

When the child arrives, it will have a mempool parent, and we currently do not consider all transactions with mempool parent for fee estimation.

So when processing a transaction in `processBlockTx` all transactions with mempool parent, will be missing and , and we will skip them.

Child transactions whose parents aren't in our mempool will also be missing, and we will skip them because orphan transactions aren't added to t
...

💬 rkrux commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1603130595)
Yeah, I can pick this up in a separate PR.

💬 hebasto commented on pull request "build: Enable `thread_local` for MinGW-w64 builds":
(https://github.com/bitcoin/bitcoin/pull/30099#issuecomment-2114912000)
> Now that the research has been done, it should be added to the commit message and PR description, so it's clear why this change might be safe, or the previous thread_local assumptions misguided. The commit message of `The previously mentioned erroneous behavior is not an issue anymore.` explains nothing.

Thanks! Done.

💬 theStack commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1603142578)
With the databases I tested with, this branch is always executed, as `inner_meta.last_page` is 2, and each processed page `curr_page` is >= 3. The reason that we don't notice is that there is a `throw` missing here. I assume we should do this instead:
```suggestion
if (curr_page > outer_meta.last_page) {
throw std::runtime_error("Page number is greater than database last page");
}
```

💬 hebasto commented on issue "`WalletCreate{Encrypted/Plain}` benchmark crash on Windows":
(https://github.com/bitcoin/bitcoin/issues/29816#issuecomment-2114972539)
cc @furszy as an author of https://github.com/bitcoin/bitcoin/pull/28894.

💬 TheCharlatan commented on pull request "kernel: De-globalize fReindex":
(https://github.com/bitcoin/bitcoin/pull/29817#discussion_r1603176078)
Ok, I think I get the different mechanics involved here now and think if initially wiping an index is going to change, it should be done in a separate pull request. Will revert this change.

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2115000215)
> I'm not sure how to be confident that the UpdateBlockTipSync / hashRecentRejectsChainTip would be correct and didn't miss an edge case.

I (locally) split the commit into (1) update on validation interface callback and asserting that `hashRecentRejectsChainTip` is equal to the chain tip whenever we call `AlreadyHaveTx` + (2) removing `hashRecentRejectsChainTip`. I think if we see that everything runs fine with (1) it's probably correct.

I've (locally) hit a bug though, so will figure out
...

💬 TheCharlatan commented on pull request "kernel: De-globalize fReindex":
(https://github.com/bitcoin/bitcoin/pull/29817#issuecomment-2115009784)
Updated b47bd959207e82555f07e028cc2246943d32d4c3 -> b47bd959207e82555f07e028cc2246943d32d4c3 ([noGlobalReindex](https://github.com/TheCharlatan/bitcoin/tree/noGlobalReindex) -> [noGlobalReindex](https://github.com/TheCharlatan/bitcoin/tree/noGlobalReindex), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalReindex..noGlobalReindex))

* Reverted previous change in init code that would no longer wipe the indexes when restarting in the middle of a reindex without the `-reindex` fl
...

💬 dominicusadinfinitum commented on issue "Testsuite for Bitcoin Core 27.0.0 - FAIL: qt/test/test_bitcoin-qt":
(https://github.com/bitcoin/bitcoin/issues/30020#issuecomment-2115020459)
@hebasto
where or how do I get to the debug.log?