💬 hebasto commented on pull request "doc: Update translation process guide":
(https://github.com/bitcoin/bitcoin/pull/29414#discussion_r1488255979)
I think the second sentence should be reworked to refer to https://developers.transifex.com/docs/cli unconditionally, because our docs do not provide install instructions any more.

💬 ismaelsadeeq commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1488282848)
In implementation they are sorted in increasing fee rate and believe the comments should be ordered like as is? and also the 0 fee and size chuck sorts first.
```suggestion
* by decreasing size. The empty FeeFrac (fee and size both 0) sorts first. So for example, the
* following FeeFracs are in sorted order:
*
* - fee=0 size=0 (undefined feerate)
* - fee=2 size=1 (feerate 2)
* - fee=3 size=2 (feerate 1.5)
* - fee=1 size=1 (feerate 1)
* - fee=2 size=2 (feerate 1)
* - fee=2 siz
...

👍 ismaelsadeeq approved a pull request: "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2"
(https://github.com/bitcoin/bitcoin/pull/29242#pullrequestreview-1878540108)
Code review ACK 3a2c66cc4f0afad9aff300110b18a39981055d63

I've reviewed the following commits
- [x] [Add FeeFrac utils `d200d30431368c7637a3490509b9158844b69e0d` ](https://github.com/bitcoin/bitcoin/pull/29242/commits/d200d30431368c7637a3490509b9158844b69e0d)
- [x] [Add FeeFrace unit tests `860823fb93212fa78ab928589bd403da462be222`](https://github.com/bitcoin/bitcoin/pull/29242/commits/860823fb93212fa78ab928589bd403da462be222)
- [x] [Implement ImprovesFeerateDiagram `802a104d0e2f810d9756
...

💬 ismaelsadeeq commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1488285682)
Maybe add unit test for `CalculateFeerateDiagramsForRBF` also?

💬 ismaelsadeeq commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1488290016)
Why leave out commend version of f1 and f2.
```suggestion
int32_t s1 = provider.ConsumeIntegral<int32_t>();
if (s1 == 0) f1 = 0;
FeeFrac fr1(f1, s1);
assert(fr1.IsEmpty() == (s1 == 0));

int64_t f2 = provider.ConsumeIntegral<int64_t>();

```

💬 achow101 commented on pull request "rpc: Drop migratewallet experimental warning":
(https://github.com/bitcoin/bitcoin/pull/28037#issuecomment-1942094206)
> I tested yesterday a bdb testnet3 wallet with 1000 txs on spinning storage, and I aborted the migration after 20 minutes.

Can you profile that so we can see where it is actually spending the most time?

💬 maflcko commented on issue "Move from Static Dust Limit [330 / 546 sats] to Variable Dust Limit [= to TxFee]":
(https://github.com/bitcoin/bitcoin/issues/29423#issuecomment-1942121280)
Considering transactions as policy-invalid where any output amount is smaller than the total fee paid by the transaction is going to create issues for payment batching. See https://bitcoinops.org/en/payment-batching/. For example, if many recipients are paid in a large transaction, the fee may be higher than one of the recipients' amounts. If the payment creator is now forced to pay every recipient individually in a separate transaction, it will consume more block space overall and make the prob
...

🚀 achow101 merged a pull request: "wallet: batch erase procedures and improve 'EraseRecords' performance"
(https://github.com/bitcoin/bitcoin/pull/29403)

💬 mzumsande commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1488241966)
nit: I don't know if we have recomendations about this, but I find `/*deterministic=*/deterministic` a bit silly, might as well drop the named argument.

🤔 mzumsande reviewed a pull request: "test: create deterministic addrman in the functional tests"
(https://github.com/bitcoin/bitcoin/pull/29007#pullrequestreview-1878460162)
Code Review ACK 3f2bb7a230dfc3869391dd3944573c2e84c1a466

💬 mzumsande commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1488218367)
i had the same suggestion independently (we have functional tests that doesn't use regtest but testnet)

💬 theuni commented on pull request "bitcoin-config.h includes cleanup":
(https://github.com/bitcoin/bitcoin/pull/29404#issuecomment-1942165950)
@TheCharlatan Sure, agreed it's better than nothing. Not quite self-contained, but given that a few of us have independently arrived at what the correct diff should be, it's easy to tell if it's correct.

Will take that commit.

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1488424495)
Done

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1488424589)
Done

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1488424666)
Done

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#issuecomment-1942188367)
Rebased for silent merge conflict.

👍 jarolrod approved a pull request: "Update translation source file for v27.0 string freeze"
(https://github.com/bitcoin-core/gui/pull/793#pullrequestreview-1878773475)
ACK 3d1bb1a122a037e966c2fb2e2113f0440edb8d93

Have zero-diff with this pr

👋 furszy's pull request is ready for review: "wallet: optimize migration process, batch db transactions"
(https://github.com/bitcoin/bitcoin/pull/28574)

🤔 furszy reviewed a pull request: "wallet: optimize migration process, batch db transactions"
(https://github.com/bitcoin/bitcoin/pull/28574#pullrequestreview-1878791279)
> Starting to review, if you get a chance can you rebase this so that it's only the relevant outstanding commits?

Done. Rebased and reworked part of it.
I still need to accommodate some code and improve the commit descriptions. But, at least the changes are reviewable now.

🤔 furszy reviewed a pull request: "rpc: Drop migratewallet experimental warning"
(https://github.com/bitcoin/bitcoin/pull/28037#pullrequestreview-1878798243)
The last batching changes might help but.. it smells like you are facing another issue apart from those.