💬 Sjors commented on pull request "[do not merge] validation: assumeutxo params mainnet":
(https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-1941615713)
I plan to make a fresh snapshot after #26045 lands.

💬 maflcko commented on pull request "fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse`":
(https://github.com/bitcoin/bitcoin/pull/29413#issuecomment-1941644439)
rfm?

💬 maflcko commented on pull request "log: Nuke error(...)":
(https://github.com/bitcoin/bitcoin/pull/29236#discussion_r1487987333)
Yeah, but "error" is still applicable *within* the addrdb functions, as they can not continue and must return. So I think, ideally they'd return `Result<void>`, and the caller could downgrade the error to a warning and log it? Happy to review such a change, if someone creates it.

💬 maflcko commented on pull request "log: Nuke error(...)":
(https://github.com/bitcoin/bitcoin/pull/29236#discussion_r1487995172)
Why? This line is a failure to connect to a peer and the above are proxy errors. So keeping this as-is makes most sense. In any case, if there is a reason to change something unrelated in other lines, a separate pull request would be ideal.

🚀 fanquake merged a pull request: "fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse`"
(https://github.com/bitcoin/bitcoin/pull/29413)

💬 maflcko commented on pull request "log: Nuke error(...)":
(https://github.com/bitcoin/bitcoin/pull/29236#discussion_r1488015627)
If there are changes to other log lines, I'd say it would be best to do them in a separate pull request. The goal here is to be as close to a refactor as possible.

💬 maflcko commented on pull request "Move log messages: tx enqueue to mempool, allocation to blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27277#discussion_r1488036409)
e67ccf09ff3df3810e08d278739fa25335785767: How is this a refactor? You are changing the behavior from debug to trace, no?

Also, I am not sure about this change. This makes intermittent test failures harder to debug, since trace is disabled. Also, below Debug is still used, so this drops only one of two log lines.

💬 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