💬 glozow commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#issuecomment-1941768684)
Currently traveling but will push a rebase soon.

El El sáb, 10 feb 2024 a las 1:43, DrahtBot ***@***.***>
escribió:

> 🐙 This pull request conflicts with the target branch and needs rebase
> <https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#rebasing-changes>
> .
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/pull/29306#issuecomment-1936861861>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGAEGGO
...

💬 hebasto commented on issue "RFC: Migration from NSUserNotificationCenter to UNUserNotificationCenter on macOS":
(https://github.com/bitcoin-core/gui/issues/112#issuecomment-1941924302)
After a discussion in https://github.com/bitcoin/bitcoin/pull/29362, this issue has come to our attention again.

In response to concerns raised in https://github.com/bitcoin-core/gui/pull/114, it appears that migrating to the new `UNUserNotificationCenter` remains the only viable option. Therefore, I conducted further research and discovered the following:

The `UNUserNotificationCenter` requires two conditions to be met in order to be granted permissions from the OS:
1. The software must
...

💬 hebasto commented on pull request "build: Add missed definition for `AM_OBJCXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/29362#issuecomment-1941936259)
@theuni
> This warning seems legitimate. Looks to me like we should modernize this code rather than ignoring it, no?

I agree with you. More details regarding our options to "modernize this code" are posted in the dedicated https://github.com/bitcoin-core/gui/issues/112.

However, the goal of this PR is to fix the build system to prevent porting this bug into the new CMake-based build system. For the context, please refer to https://github.com/hebasto/bitcoin/pull/84#discussion_r1469371615
...

💬 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