💬 mzumsande commented on pull request "test: allow all functional tests to be run or skipped with --usecli":
(https://github.com/bitcoin/bitcoin/pull/32290#discussion_r2162581057)
Makes sense - I renamed it to `_rpc`, added a comment and fixed the additional spots. There is a legit for `rpc` use in setting `ensure_ascii`.

💬 mzumsande commented on pull request "test: allow all functional tests to be run or skipped with --usecli":
(https://github.com/bitcoin/bitcoin/pull/32290#discussion_r2162581154)
Done.

💬 mzumsande commented on pull request "test: allow all functional tests to be run or skipped with --usecli":
(https://github.com/bitcoin/bitcoin/pull/32290#discussion_r2162582593)
yes - I removed this in commit "test: allow all functional tests to be run or skipped with --usecli" which touches this line anyway.

🤔 glozow reviewed a pull request: "doc: archive 28.2 release notes"
(https://github.com/bitcoin/bitcoin/pull/32797#pullrequestreview-2951621314)
ACK 907842363c64621b32acbb0d02e8500058b6ae57

via `git diff HEAD:doc/release-notes/release-notes-28.2.md v28.2:doc/release-notes.md`

🚀 glozow merged a pull request: "doc: archive 28.2 release notes"
(https://github.com/bitcoin/bitcoin/pull/32797)

💬 mzumsande commented on pull request "test: allow all functional tests to be run or skipped with --usecli":
(https://github.com/bitcoin/bitcoin/pull/32290#issuecomment-2998106087)
[60106f3](https://github.com/bitcoin/bitcoin/commit/60106f32852524498d5d6e958a3bcabdfb06ee1a) to [666016e](https://github.com/bitcoin/bitcoin/commit/666016e56b28b77f798dc85c767b95c1ca0abfae):
Addressed feedback by @maflcko and rebased.

🤔 pinheadmz reviewed a pull request: "Introduce SockMan ("lite"): low-level socket handling for HTTP"
(https://github.com/bitcoin/bitcoin/pull/32747#pullrequestreview-2951071498)
Rebase to address review by @vasild THANKS!

💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2162235428)
👍

💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2162325395)
👍

💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2162331716)
👍

💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2162324748)
Good catch, taken. I was hoping to get away with only ever using one client in this test but this makes more sense for coverage anyway ;-)

💬 pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#discussion_r2162288958)
Oh yes thanks, done by inserting a move-only commit

💬 theStack commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2162793069)
micro-optimization nit: as a DoS score of 1 is still okay, could only consider peers which are strictly above?
```suggestion
// Performance optimization: only consider peers with a DoS score > 1.
if (entry.GetDosScore(max_ann, max_mem) >> FeeFrac{1, 1}) {
```

💬 theStack commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2162806157)
to get rid of unrelated fee/size terminology, I wonder if a dedicated dos score type would make sense, like e.g.:
```
// Use FeeFrac class to represent the DoS score as fraction,
// with more generic member methods for (de)nominator access
class DosScore : public FeeFrac {
private:
// prevent direct access of base fraction values
using FeeFrac::fee, FeeFrac::size;
public:
using FeeFrac::FeeFrac; // inherit constructors
int64_t nominator() const { return fee; }
int3
...

💬 theStack commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2162799520)
could add `Assume`s here before the subtractions each (`m_total_usage >= ann.GetUsage()`, `m_count_announcements >= 1`) to ensure they won't underflow

💬 theStack commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2162780473)
should this be
```suggestion
auto it_lower = m_orphans.get<ByWtxid>().lower_bound(ByWtxidView{wtxid, MIN_PEER});
return it_lower != m_orphans.get<ByWtxid>().end() && it_lower->m_tx->GetWitnessHash() == wtxid;
```
instead, if only for consistency? There are two more places I've found where an indexed iterator is compared to a non-indexed `.end()`: `::AddAnnouncer`, `::GetTx`

📝 theStack opened a pull request: "mempool: use `FeeFrac` for ancestor/descendant score comparators"
(https://github.com/bitcoin/bitcoin/pull/32799)
Rather than determining fee-rates for the mempool index scores and comparators manually in a rather tedious way (even involving floating-points), use the `FeeFrac` class [1] to simplify and deduplicate the code. Note that though this is intended to be a refactoring PR, there might be subtle differences in behaviour due to floating-point arithmetic involved in the original code (to avoid overflows at the cost of precision loss), but these shouldn't matter.

[1] introduced in PR #29242, commit c
...

💬 theStack commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#issuecomment-2998558126)
Concept ACK

📝 musaHaruna opened a pull request: "Distinguish between vsize and sigop adjusted mempool vsize"
(https://github.com/bitcoin/bitcoin/pull/32800)
### Motivation and Problem
Fixes [32775](https://github.com/bitcoin/bitcoin/issues/32775)
CTxMemPoolEntry::GetTxSize() returns the larger of two values: the BIP 141 virtual size (vsize) and the "sigop-adjusted size." This sigop-adjusted size is used by mempool validation and mining algorithms as a safeguard to prevent overfilling blocks with transactions that approach both the weight and signature operation (sigop) limits in a way that could exploit block space efficiency.

In the current im
...

💬 Sjors commented on pull request "descriptors: Allow `H` as a hardened indicator":
(https://github.com/bitcoin/bitcoin/pull/32788#issuecomment-2999109559)
> you could encode an xpub into a format like bech32, encoded into QR alphanumeric mode and save ~15% over a base58 encoded xpub in QR binary mode

I think it would be nice in general to have a bech32m encoding for extended keys, because part of the reason why descriptors look weird is the mix of bech32 and base58.