💬 maflcko commented on pull request "fuzz: Improve fuzzing stability for minisketch harness":
(https://github.com/bitcoin/bitcoin/pull/29064#discussion_r1424406512)
When touching this, maybe move the Assert into the helper?

💬 1ma commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1852569550)
> > When the mempool is clogged with this kind of TXs, users trying to use Bitcoin as money are priced out in droves, because they can't pay 98% of their UTXOs in fees, so they can only compete by moving orders of magnitude more value than inscriptions. Therefore when inscriptions are setting the floor at $10, they're not just pricing out people trying to transact $10 onchain, but also $20, $50, $100.. maybe up to $500.
>
> I think the linked X thread's analysis and your analysis are both wro
...

🤔 mzumsande reviewed a pull request: "p2p: Fill reconciliation sets (Erlay)"
(https://github.com/bitcoin/bitcoin/pull/28765#pullrequestreview-1778050698)
Code Review ACK f56ec8a3b086b0f2f8a0fbde861447e40ffbc3d9

One thing I'm unsure about is how the way we call `ShouldFanoutTo()` multiple times for each transaction would might affect performance.
If we have 120 reconciling peers, and get a dump of `MAX_SET_SIZE=3000` transactions, I think we'd call this function `360000` times within a short timeframe. I wonder how long that would take, maybe we could add a benchmark for this? (could be done a follow-up)

💬 mzumsande commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1424331514)
I think that `deterministic_randomizer` could be passed by reference

📝 furszy opened a pull request: "bench: wallet, fix change position out of range error"
(https://github.com/bitcoin/bitcoin/pull/29065)
Fixes #29061. Only the benchmark is affected.

Since #25273, the behavior of 'inserting change at a random position'
is instructed by passing ´std::nullopt´ instead of -1.

Also, added missing documentation about the meaning of
'change_pos=std::nullopt' inside 'CWallet::CreateTransaction()'

📝 hebasto opened a pull request: "doc: Bump minimum required Boost version due to migration to C++20"
(https://github.com/bitcoin/bitcoin/pull/29066)
Boost versions <1.73 have C++20-specific bugs that were fixed in the following commits:
- https://github.com/boostorg/signals2/commit/15fcf213563718d2378b6b83a1614680a4fa8cec
- https://github.com/boostorg/test/commit/495c095dc063052ce54f2fe9217fe0fc69ced5f1

I tested [`libboost1.71-dev`](https://packages.ubuntu.com/focal/libboost1.71-dev) in Ubuntu 20.04 and Boost 1.71, 1.72, 1.73 in our depends build system.

Closes https://github.com/bitcoin/bitcoin/issues/29063.

💬 hebasto commented on issue "build: GCC 10.5 fails to compile `test_bitcoin` using Boost.Test 1.71":
(https://github.com/bitcoin/bitcoin/issues/29063#issuecomment-1852587644)
> Going to do more thorough tests before submitting a PR.

Found one more fixed bug. See #29066.

💬 furszy commented on issue "Benchmark WalletCreateTxUsePresetInputsAndCoinSelection crashes due to #25273":
(https://github.com/bitcoin/bitcoin/issues/29061#issuecomment-1852591428)
Created #29065 solving this.
Only the benchmark has been affected. The GUI side finding was just a false-positive; ugly code that we should clean up sooner rather than later.

💬 shovas commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1852591613)
Just wanted to chime in here and say that this is a critical blocker for anyone with a real wallet where this happens

I'm in an bad loop of loading wallet, 'beyond pruned data...download entire blockchain', download entire blockchain, abort on bad block, ...?? Has happened on two computers where Bitcoin Core has been working great for over a year.

💬 furszy commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1852600138)
> Just wanted to chime in here and say that this is a critical blocker for anyone with a real wallet where this happens
>
> I'm in an bad loop of loading wallet, 'beyond pruned data...download entire blockchain', download entire blockchain, abort on bad block, ...?? Has happened on two computers where Bitcoin Core has been working great for over a year.

In order to get this functionality merged, we need to get #27837 merged. Which requires #28120 and #28170. Which are under review currentl
...

💬 maflcko commented on pull request "doc: Bump minimum required Boost version due to migration to C++20":
(https://github.com/bitcoin/bitcoin/pull/29066#issuecomment-1852604458)
> https://github.com/boostorg/signals2/commit/15fcf213563718d2378b6b83a1614680a4fa8cec

This one could be fixed alternatively (in theory) by nuking signals2, but I don't think this will happen any time soon.

💬 maflcko commented on pull request "doc: Bump minimum required Boost version due to migration to C++20":
(https://github.com/bitcoin/bitcoin/pull/29066#issuecomment-1852604898)
lgtm ACK 1e0ed3bc0a99c08384642ad81ed4771bc98208b0

💬 eragmus commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1852632772)
> The fee rate was left out of the screenshots because it's not relevant to the analysis, but your example proves it's right all the same.

Okay, thanks for clarifying, now I understand the precise point being argued.


> That inscription is spending an $6.68 UTXO, and the transactor is spending a combined value of $2315. Both are equally competitive in the currently broken fee market (because they were included in the same block) despite the 2 orders of magnitude of difference between their val
...

💬 shovas commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1852635635)
Is there a workaround in the meantime?

📝 maflcko opened a pull request: " test: Treat msg_version.relay as unsigned, Remove `struct` packing in messages.py"
(https://github.com/bitcoin/bitcoin/pull/29067)
`struct` has many issues in messages.py:

* For unpacking, it requires to specify the length a second time, even when it is already clear from the `f.read(num_bytes)` context.
* For unpacking, it is designed to support a long format string and returning a tuple of many values. However, except for 3 instances in `messages.py`, usually only a single value is unpacked and all those cases require an `[0]` access.
* For packing and unpacking of a single value, the format string consists of charac
...

💬 furszy commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1852665616)
> Is there a workaround in the meantime?

Would be good to know why your node is aborting on a bad block. Maybe you have a storage issue?
Could you share the logs? (here or in a new issue is fine, just tag me when you do it).

Just as a small advice (not the best as your node should just work fine but.. having a backup plan always helps); you could have a full chain backup in a different storage. Then, if needed, transfer it to your pruned node to quickly recover from any storage corruption
...

💬 1ma commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1852673575)
The precise point at which a fee "perverts" is a red herring, IMO.

If all TXs in the mempool are trying to move sats while economizing the fee as much as possible the free market just finds the % at which transactors are willing to transact, be it 0.8%, or 3% or whatever.

The problem arises when you allow *in the same market* a new kind of TX interested in another use case which doesn't mind straight up paying >95% of fees, because they're doing something else. They quickly displace the in
...

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1852679820)
Thanks for the review vasild!. Will tackle all points in the coming days. I'm currently finishing few bug fixes priorities and will be fully here again.

🤔 sdaftuar reviewed a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-1777611474)
I read the code more carefully (other than the tests, which I just skimmed) -- so far this looks good, just had a few comments. Planning to do some more thorough testing next.

💬 sdaftuar commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1424358331)
Perhaps a comment here would be helpful to explain why this change is needed.