💬 TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1792927186)
Updated fc5e4dea6ffa21191e852a79103a2dfc9c241df1 -> a20c1d2b81a702f97267d4f098f13ed80e9320cf ([simplifyMemPoolInteractions_6](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_6) -> [simplifyMemPoolInteractions_7](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/simplifyMemPoolInteractions_6..simplifyMemPoolInteractions_7))

* Addressed @glozow's [comment](https://github.com/bitcoin/bi
...

💬 hashbender commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1792957112)
It would be nice if it was on by default

📝 theStack opened a pull request: "init: completely remove `-zapwallettxes` (remaining hidden option)"
(https://github.com/bitcoin/bitcoin/pull/28787)
The `-zapwallettxes` functionality has been removed in v0.21.0 (see commit 3340dbadd38f5624642cf0e14dddbe6f83a3863b / PR #19671), with the parameter being kept as hidden option, to inform users via an exit error that `abandontransaction` should be used instead.

As any guides that still suggest to use `-zapwallettxes` would refer to a Bitcoin Core version that is EOL since many years (i.e. <= v0.20.x), it is highly unlikely that the error caused by the option is still relevant for any user, he
...

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1382115534)
Thank you removed

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1382115704)
Done

💬 sr-gi commented on pull request "net: improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#issuecomment-1792979468)
> tACK [0420f9](https://github.com/bitcoin/bitcoin/commit/0420f99f429ce2382057e101859067f40de47be0)

Thanks for reviewing! Would you mind sharing what did you test and how?

@vasild would you mind re-acking this when you have some time?

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1382119240)
Thank you @glozow
Now using two structs `RemovedMempoolTransactionInfo` for this callback and `NewMempoolTransactionInfo` for `TransactionAddedToMempool callback.

Since they have similar fields, I created a base struct with all the similar fields so that they can both inherit 79803d3912747eb96153357f1cc33c3f8cf21eb6 and a469d937dbbadedeb58e0d5d1c44ed7881a7ebb9

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1382119456)
Fixed

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1382119621)
Added

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1382119946)
Thank you, this is removed in latest push

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1382120053)
Fixed

🤔 mzumsande reviewed a pull request: "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full"
(https://github.com/bitcoin/bitcoin/pull/27600#pullrequestreview-1713405725)
> Hoping to get some more feedback from @mzumsande and @naumenkogs to proceed, or abandon

Personally, I'm not sure if the use case (running a node with `-maxconnections < 40` and at the same time wanting certain inbound peers, or wanting a really high number of whitelisted inbounds peers at the same time) is common enough to introduce an extra permission just for that. In more typical cases, `noban` should be sufficient.
But if others think it is I'm not against it.

A related use case I c
...

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1382126244)
Thank you

This is fixed.
1. a469d937dbbadedeb58e0d5d1c44ed7881a7ebb9 introduces `NewMempoolTransactionInfo` with all the necessary fields and updates `TransactionAddedToMempool` callback parameter.
2. ac82f58e45421aa5aec1d3e6e992d1c369472a10 updates the fee estimator from `CValidationInterface` notifications and manage delegating `validForFeeEstimation` to `CBlockPolicyEstimator::processTransaction` together the cleanups.

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1382126431)
Done

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1382126780)
Fixed

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#issuecomment-1793005174)
Thanks for your review @glozow and @TheCharlatan
Forced pushed from 6d676c69195dc9032c4558987fb88f4c4b71092c to 09436b21e84cc6cd979fe4942ba1c415c4bc91be
[Compare changes](https://github.com/bitcoin/bitcoin/compare/6d676c69195dc9032c4558987fb88f4c4b71092c..09436b21e84cc6cd979fe4942ba1c415c4bc91be)

- Rework the commits in the order of this [review comment](https://github.com/bitcoin/bitcoin/pull/28368#pullrequestreview-1704427495) which is more cleaner and less verbose.
- Addressed @glozow
...

🤔 instagibbs reviewed a pull request: "validation: return more helpful results for reconsiderable fee failures and skipped transactions"
(https://github.com/bitcoin/bitcoin/pull/28785#pullrequestreview-1713411958)
LGTM with a few suggestions https://github.com/bitcoin/bitcoin/pull/28785/commits/6b0dc669886c07a81fdfbf2d1a71a77ca3bac1c3

💬 instagibbs commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1382139875)
`Wtxid`? :point_right: :point_left:

💬 instagibbs commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1382138716)
```suggestion
// Failed for fee reasons. Provide the effective feerate and which txn was included.
```

💬 instagibbs commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1382126634)
Might make sense to add a comment to the effect that we don't want to reject fetching this same tx in a future package