💬 maflcko commented on pull request "refactor: Remove pre-C++20 code, fs::path cleanup":
(https://github.com/bitcoin/bitcoin/pull/29040#discussion_r1422829381)
> What would be an example in C++20 where it is broken by calling `string()` (1.) and ok by converting `wchar_t`->`char8_t`->`char` (2.2)?

This has nothing to do with C++20. If you call the wrong function `string()` vs `u8string()`, you will also get the wrong result with C++17.

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1422836196)
While I like the idea, I don't think it will work as you expect. We are passing `CFeeRate(0)` to `add_coin()`, which generates coins with fee=0, which at the "spending now vs spending in the future" calculation point means a negative waste (because of `waste = fee_to_spend_now - fee_to_spend_in_the_future` --> `waste = 0 - something`). And this negative number increases with the number of selected coins. So, at least in a first glance, this means that the process will always prefer the solution
...

💬 maflcko commented on pull request "build: Enable -Wunreachable-code":
(https://github.com/bitcoin/bitcoin/pull/28999#issuecomment-1850520824)
> we could wrap the macros in a function

I don't think this works, because it will replace `__FILE__, __LINE__, __func__` with a meaningless string literal constant.

💬 eragmus commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1850530167)
> The shipcoiners have truly arrived, as evidenced by this thread. Thanks, bitcoin core. Unbelievable.

Ad hominem (and certainly not applicable to all commenters), the second lowest level of disagreement. Better to level up & address the actual arguments being made.

💬 theuni commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#discussion_r1422852478)
I think I'd prefer to leave this with its comment. Note that we don't use `std::counl_zero` directly, we want kinda the opposite: `(sizeof(x) * 8 )- std::countl_zero(x);`

👋 josibake's pull request is ready for review: "wallet, rpc: `FundTransaction` refactor"
(https://github.com/bitcoin/bitcoin/pull/28560)

💬 maflcko commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#discussion_r1422858528)
Can remove this from configure.ac now?

💬 josibake commented on pull request "wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction":
(https://github.com/bitcoin/bitcoin/pull/25273#issuecomment-1850549633)
For the reviewers of this PR, https://github.com/bitcoin/bitcoin/pull/28560 is a follow-up that further refactors `FundTransaction` by cleaning up the SFFO parsing and removing the SFFO logic from `FundTransaction`. It does this by passing a `CRecipient` vector to `FundTransaction`, which also sets us up to be able to pass silent payment addresses to `FundTransaction` in a later PR.

💬 theuni commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#discussion_r1422867877)
I worked on a branch last week that left me very confused: https://github.com/theuni/bitcoin/commits/pr28674-rebase2/

It's the _last_ commit (which I would expect to be a no-op) which causes the slowdown. For whatever reason, the built-in functions like `be64toh` are much faster than my hand-written internal ones which implement _the same code that glibc does_ in its headers! I'm still scratching my head trying to work out what's happening.

But for now, now that we have c++20, I'll go ahe
...

💬 theuni commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#discussion_r1422871347)
Looks like it. Will do in the updated PR.

💬 maflcko commented on pull request "[POC][WIP] C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-1850577489)
Needs rebase

💬 maflcko commented on pull request "[POC][WIP] C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-1850578430)
> WIP: our `prevector` implementation requires modification for `std::ranges::views::reverse` to work. More specifically: it needs to implement the [`bidirectional_range`](https://en.cppreference.com/w/cpp/ranges/bidirectional_range) concept, which currently it seems not to (see `static_assert(std::ranges::bidirectional_range<prevector>)` output below) as we're not satisfying [`range`](https://en.cppreference.com/w/cpp/ranges/range). Once that's done, we can update the last usage of `reverse_ite
...

💬 Retropex commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1850614834)
If you can't see yourself that the thousands of tokens created with the inscriptions are scams I really can't do anything for you, for the images it's the same, inscribe ten times the same things with a slight declination and then move on to something else when people are no longer interested. This cycle continues indefinitely.

💬 ryanofsky commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-1850626464)
Added 1 commits b23409474f6fe874a25374bb978db9e5682954e6 -> 6d8cb4d4cd6062ad06cd351439e9139e028995d3 ([`pr/rmutil.5`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.5) -> [`pr/rmutil.6`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.6), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/rmutil.5...pr/rmutil.6)) updating libraries.md to describe differences between util and common libraries better.

re: https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-184902640
...

💬 kashifs commented on pull request "Make bitcoin-tx replaceable value optional":
(https://github.com/bitcoin/bitcoin/pull/29022#discussion_r1422933362)
Yes, that's correct. I've updated the text to read as you suggested. Thanks!

💬 eragmus commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1850654602)
> If you can't see yourself that the thousands of tokens created with the inscriptions are scams I really can't do anything for you, for the images it's the same, inscribe ten times the same things with a slight declination and then move on to something else when people are no longer interested. This cycle continues indefinitely.

People lose their money by voluntarily choosing to gamble in the casino (the house always wins), so do we ban casinos? We don't, not in the US at least. Inscriptions a
...

💬 Retropex commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1850681975)
> People lose their money by voluntarily choosing to gamble in the casino

It's true, but going to a casino does not oblige the entire planet to have an indelible mark of fact that you went to a casino. Inscriptions do.

> People also use their bitcoin for other distasteful things

Yes and it doesn't bother me because they are financial txs.


> Bitcoin is a decentralized permissionless network

Indeed, therefore, nodes runners have the right to have options to protect themselves from
...

💬 stickies-v commented on pull request "[WIP] C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-1850688351)
> Needs rebase

Thx, rebased to include #28349 & updated PR description

💬 brunoerg commented on pull request "doc/reduce-traffic: update/clarify max outbound connection count":
(https://github.com/bitcoin/bitcoin/pull/29052#discussion_r1422994881)
nit:
```suggestion
block-relay-only ones and occasionally 1 short-lived feeler or an extra block-relay-only connection.
```

💬 MarnixCroes commented on pull request "doc/reduce-traffic: update/clarify max outbound connection count":
(https://github.com/bitcoin/bitcoin/pull/29052#discussion_r1423007653)
my bad,
done