💬 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

💬 ismaelsadeeq commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1427410801)
Package are always topologically sorted, so am assuming this will always be best case no?

💬 ismaelsadeeq commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1427403296)
`IsConsistentPackage` in param is Package?
```suggestion
/** Construct a map from the txid of a transaction to the txids of its in-package ancestor set,
* including itself. Package must be IsConsistentPackage, otherwise this
* this returns an empty map. Package */
```

💬 ismaelsadeeq commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1427408824)
`if (curr_result_iter == ancestor_set_map.end()) return;` is redundant the map has already been populated with all package transactions, the statement will always be false.
So assert for that.
And return early when the transaction in-package ancestor set is not empty, that means its has been visited
```suggestion
auto curr_result_iter = ancestor_set_map.find(curr_txid);
// All transactions were populated to ancestor_set_map this should never happen
assert(curr_result_iter != an
...

💬 The-Sycorax commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1857098045)
The sheer amount of spam caused by inscriptions on the Bitcoin blockchain is actually quite alarming, it should be addressed comprehensively, and in a way that dose not harm the BRC-20 market or people's livelihoods.

Some individuals are not considering the nature of the vulnerability, it has nothing to do with getting rid of inscriptions. It has to do with the network congestion caused by these types transactions. It's not just a matter of higher fees or slower processing times; this situat
...