💬 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

💬 instagibbs commented on pull request "Fuzz: Check individual and package transaction invariants":
(https://github.com/bitcoin/bitcoin/pull/28764#discussion_r1382150706)
missing some `return` statements in this function

📝 instagibbs opened a pull request: "test: bugfix CheckPackageMempoolAcceptResult return all error strings"
(https://github.com/bitcoin/bitcoin/pull/28788)
Noticed on follow-up testing work https://github.com/bitcoin/bitcoin/pull/28764/files#r1382150706

💬 achow101 commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1382141691)
In 770a73c66a48434fa48f594709dbbf5831dc1d5d "wallet: track mempool conflicts"

nit: `snake_case` for member variables.

Additionally, this is tracking these conflicts at the `CWallet` level. However I wonder if it might be better to track it on a transaction level, i.e. have `CWalletTx` have the set of txids.

💬 ariard commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1793036667)
I think it’s good to communicate on the mailing list in which version (27.0 or 28.0) full-rbf might be turn on by default. I advocated such setting in the past as early as 0.24, though at the time some 0-conf transactions applications and services were migrating their stuff in consequence.

💬 achow101 commented on pull request "doc: Add offline signing tutorial":
(https://github.com/bitcoin/bitcoin/pull/28363#issuecomment-1793039287)
ACK 3c208cc05ea9efb145c956e70f80efd8b027ff33

📝 maflcko opened a pull request: "fuzz: Avoid utxo_total_supply timeout (take 2)"
(https://github.com/bitcoin/bitcoin/pull/28789)
Looks like this still may take a long time to run large fuzz inputs. Thus, reduce it further, but still allow it to catch the regression, if re-introduced:

```diff
diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp
index f949655909..4bdd15c5ee 100644
--- a/src/consensus/tx_check.cpp
+++ b/src/consensus/tx_check.cpp
@@ -40,7 +40,7 @@ bool CheckTransaction(const CTransaction& tx, TxValidationState& state)
std::set<COutPoint> vInOutPoints;
for (const auto& txin
...

💬 petertodd commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1793068038)
Well it's certainly not going to be in 26, as we're releasing it now.

If we actually merge this soonish I'll make a post saying it will come out in 27.0. I can also write some release notes if people want them.

Right now Antpool is the largest pool, and it's still mining full-rbf, along with all the others mentioned above. (Luxor recently turned it back on; as Nick alluded to, looks like they unintentionally had it turned off) So IIRC we're at ~40% of hash power mining it.

On November
...

💬 petertodd commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1793071684)
Conveniently, here's an example of Luxor mining a full-rbf double spend 20 minutes ago: https://mempool.space/tx/8c2a54f92237a66662083fdd3a85d4d7b1399f9301edf434eaf72219b8440f82

On November 3, 2023 5:12:32 PM GMT-03:00, Antoine Riard ***@***.***> wrote:
>I think it’s good to communicate on the mailing list in which version (27.0 or 28.0) full-rbf might be turn on by default. I advocated such setting in the past as early as 0.24, though at the time some 0-conf transactions applications and se
...

💬 maflcko commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1793371057)
```
test/functional/wallet_backwards_compatibility.py:29:1: F401 'test_framework.wallet_util.WalletUnlock' imported but unused