💬 maflcko commented on issue "Not able to build and compile on macos":
(https://github.com/bitcoin/bitcoin/issues/29083#issuecomment-1856618608)
Could this be due to an env var? Maybe check `env | grep 'HOST='`?

💬 brunoerg commented on pull request "fuzz: coinselection, improve `min_viable_change`/`change_output_size`":
(https://github.com/bitcoin/bitcoin/pull/28372#issuecomment-1856620276)
> So, instead of passing coin_params.m_cost_of_change and 0, we should be passing coin_params.min_viable_change and coin_params.m_change_fee. Just fixing one or the other was insufficient, but when I replace both all my fuzz crashes pass.

Cool, I think the 0 came from the way we call `ComputeAndSetWaste` internally in `SelectCoinsBnB`.

💬 brunoerg commented on pull request "fuzz: coinselection, improve `min_viable_change`/`change_output_size`":
(https://github.com/bitcoin/bitcoin/pull/28372#issuecomment-1856623275)
Thanks, @furszy and @murchandamus. I will address the suggestions and leave it running for some time before pushing.

💬 sarthak13gupta commented on issue "Not able to build and compile on macos":
(https://github.com/bitcoin/bitcoin/issues/29083#issuecomment-1856629535)
> You said you're running on arm, but configure says otherwise:
>
> > checking build system type... x86_64-apple-darwin22.1.0
>
> Is your terminal running under Rosetta or something.

Yes, now that I checked because it gives compatibility issues with dependencies sometimes.

Should I change it and run it without Rosseta?

But I don't know why this would happen , also when brew is located at `/opt/homebrew/bin/brew`
and I'm installing dependencies using
`arch -arm64 brew install
...

💬 amitiuttarwar commented on pull request "p2p: Increase tx relay rate":
(https://github.com/bitcoin/bitcoin/pull/28592#issuecomment-1856639108)
posted a topic to [delving bitcoin](https://delvingbitcoin.org/t/warnet-increase-tx-relay-rate/294) to brainstorm using warnet to evaluate the impacts of this change

🚀 achow101 merged a pull request: "wallet: birth time update during tx scanning"
(https://github.com/bitcoin/bitcoin/pull/28920)

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

Needs rebase due to a silent conflict with #27581 (`feature_asmap.py`).

💬 mzumsande commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1427230831)
nit: code style, `relay_for_tests`

💬 achow101 commented on pull request "refactor: Remove pre-C++20 code, fs::path cleanup":
(https://github.com/bitcoin/bitcoin/pull/29040#issuecomment-1856669579)
ACK 66667130416b86208e01a0eb5541a15ea805ac26

💬 mzumsande commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1427322222)
Are you suggesting to have to move other existing test-only options (`-acceptstalefeeestimates` or `-fastprune` come to mind) under this command?

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1856680153)
> I think that it should be possible to resolve the problem in a simpler way and without touching unrelated stuff (like when to start "extra block-relay-only connections")

Please look https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1296355296.
The convo started from a different angle but I think the outcome addresses your concern.
We ended up agreeing that keeping the extra block relay connection logic as is in the current change set actually improves certain scenarios. For instan
...

💬 furszy commented on pull request "fuzz: coinselection, improve `min_viable_change`/`change_output_size`":
(https://github.com/bitcoin/bitcoin/pull/28372#issuecomment-1856697460)
> Just an observation on @furszy's suggestion - I think it will cause `stack-buffer-overflow`:
>
> ```diff
> - const auto dust{GetDustThreshold(change_prototype_txout, coin_params.m_discard_feerate)};
> + // We only care about the change output size
> + CScript change_out_script;
> + std::fill_n(change_out_script.begin(), coin_params.change_output_size, OP_TRUE);
> + const auto dust{GetDustThreshold(CTxOut{/*nValueIn=*/0, change_out_script}, coin_params.m_discard_feerate)}
...

🚀 achow101 merged a pull request: "refactor: Remove pre-C++20 code, fs::path cleanup"
(https://github.com/bitcoin/bitcoin/pull/29040)

💬 maflcko commented on pull request "wallet: batch all individual spkms setup db writes in a single db txn":
(https://github.com/bitcoin/bitcoin/pull/28894#discussion_r1427344014)
nit: `u8string` looks wrong here, according to the `fs.h` documentation.

💬 maflcko commented on pull request "test: create deterministic addrman in the functional tests":
(https://github.com/bitcoin/bitcoin/pull/29007#discussion_r1427346763)
Yes. (Not here, obviously, but in a follow-up possibly)

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1427351949)
this looks like a good test to add to tease out the differences in code paths

💬 maflcko commented on pull request "refactor: Remove c_str from util/check":
(https://github.com/bitcoin/bitcoin/pull/26960#issuecomment-1856761834)
> If so, nit: worth adding a little `// TODO` comment to document that with C++20 we can use `consteval` and `string_view`? Personally, I find those quite helpful.

I tried this and it seems to compile, but I don't think it is worth it:

```diff
diff --git a/src/util/check.h b/src/util/check.h
index a02a1de8dc..f15117daef 100644
--- a/src/util/check.h
+++ b/src/util/check.h
@@ -21,9 +21,11 @@ public:
NonFatalCheckError(std::string_view msg, std::string_view file, int line, std::st
...

💬 amitiuttarwar commented on pull request "p2p: attempt to fill full outbound connection slots with peers that support tx relay":
(https://github.com/bitcoin/bitcoin/pull/28538#issuecomment-1856825484)
ACK ad48667ec8e8d563550df768d0b45abf800662d9

📝 fjahr opened a pull request: "refactor: C++20: Use lambda implicit capture and std::rotl"
(https://github.com/bitcoin/bitcoin/pull/29085)
While exploring some C++20 changes and checking against our code I found these two potential improvements:

1. We can replace our custom implementation of `rotl32` in crypto/chacha20 with `std::rotl` from the [new `bit` header](https://en.cppreference.com/w/cpp/header/bit).
2. Implicit capture of `this` in lambdas [is deprecated](https://en.cppreference.com/w/cpp/language/lambda#Lambda_capture). The capture will need to be made explicit in the future, so change this now.

💬 kashifs commented on pull request "Add multiplication operator to CFeeRate":
(https://github.com/bitcoin/bitcoin/pull/29037#issuecomment-1856889095)
Okay, great! I'm slowly working my way through #27877