💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-3647697081)
> `PR` performs better with `100 mb` dbcache than `master` with `7 GB`;

wat

💬 pablomartin4btc commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r2615258470)
tiny nit (not a blocker):
```suggestion
getaddrmaninfo RPC internally. Users querying older, unmaintained node versions would need to use an older bitcoin-cli version. (#26988)
```

🤔 pablomartin4btc reviewed a pull request: "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency"
(https://github.com/bitcoin/bitcoin/pull/26988#pullrequestreview-3573191027)
ACK 325f98bca07c3a0c48a46dc18f67376d44c5341d

_(Left a small [suggestion](https://github.com/bitcoin/bitcoin/pull/26988/files#r2615258470) since the older node may be operated by someone other than the CLI user, and upgrading it might not be possible or appropriate when this change is the only reason. I think this would also align with @jonatack’s earlier [note](https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-3559469161).)_

💬 Raimo33 commented on pull request "streams: replace `std::find` with `memchr` (5x improvement)":
(https://github.com/bitcoin/bitcoin/pull/34044#issuecomment-3647753086)
> I would be curious whether a `-reindex -stopafterblockimport` speedup is measurable - since my understanding is that the magic is usually at the beginning anyway, so it's also possible this ends up slowing down the average case.

I've ran a reindex benchmark, see updated PR description. I'm not sure if ~1.2% is an irrelevant gain, but I argue a 5-6x improvement on the `FindByte` method is significant and should be considered for merging regardless, even for possible future use of this method
...

💬 pablomartin4btc commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2615354482)
nit (if you have to retouch and agree):
```suggestion
#define detail_LogIfCategoryAcceptsLevel(category, level, ...) \
```
or `detail_LogIfCategoryIsAcceptedAtLevel`... perhaps it's clearer and more consistent with existing internal call API `LogAcceptCategory()`.

🤔 pablomartin4btc reviewed a pull request: "log: Remove brittle and confusing LogPrintLevel"
(https://github.com/bitcoin/bitcoin/pull/34051#pullrequestreview-3573326964)
ACK fa33a9f5be1beabfee6c217d133cc51dea3b9a63

I agree with the benefits mentioned in the PR description.

⚠️ husstege-e opened an issue: "Wallet: gettransaction(txid).details changes after confirmation of RBF replacement"
(https://github.com/bitcoin/bitcoin/issues/34064)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

For a replaced transaction (txA), the output of gettransaction(txA, true).details
changes after the replacement transaction (txB) is confirmed.

Specifically, a 'receive' entry present after bumpfee disappears once txB is
confirmed, even though the queried txid remains txA.

After confirmation, repeated calls are stable.


### Expected behaviour

For a given txid, gettransaction(txid).deta
...

🤔 pablomartin4btc reviewed a pull request: "lint: Remove confusing, redundant, and brittle lint-spelling"
(https://github.com/bitcoin/bitcoin/pull/34053#pullrequestreview-3573513114)
ACK fa904fc683c0892eb800ddb986fdc0c646721077

💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2615504333)
Yes, that's also my understanding. I'm working on a PR that will resolve this.

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2615528552)
If I understand it correctly it's just wasted work in rare cases - in which case I agree we should tackle it in a follow-up.

🤔 pinheadmz reviewed a pull request: "Broadcast own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-3573153184)
code-review ACK up to a57ef7ddbacebf23c4c50c3fc58615a58339dcc2 (about 2/3rds through). Also built on macos/arm64 and ran unit and functional tests.

Added a few non blocking nits.

Will finish review next week, re-test in warnet, and play with the feature in the GUI ;-)

💬 pinheadmz commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2615489907)
a57ef7ddbacebf23c4c50c3fc58615a58339dcc2

nit: suggestion
`- Mark that given recipient node has confirmed receipt of a transaction`

💬 pinheadmz commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2615289286)
4c8dfd4f7d60c9fb01d7613945f46d453c808237

If we get to this point, the log message will be re-printed fairly rapidly until an OK address is found and resets the counter?

💬 pinheadmz commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2615351368)
...although, we check `IsRoutable()` when adding to AddrMan so I think it's more likely this code never executes. (Uncovered by tests, too: https://corecheck.dev/bitcoin/bitcoin/pulls/29415)

💬 pinheadmz commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2615234534)
4c8dfd4f7d60c9fb01d7613945f46d453c808237

nit: should this be `[in,out]`?

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2615548852)
This is already mentioned in a comment about a different part, but applies here as well. A variation of this comment could be added here.

https://github.com/bitcoin/bitcoin/pull/29415/files#diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3R1644-R1645

🤔 instagibbs reviewed a pull request: "Replace cluster linearization algorithm with SFL"
(https://github.com/bitcoin/bitcoin/pull/32545#pullrequestreview-3564457615)
reviewed through ab1416bb471c9326bb77ee464be6aeebf663e024

continuing on with optimizations next

💬 instagibbs commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2611075318)
1c008c063df383aff64c564b116802b427a228ce

```Suggestion
void TestOptimalLinearization(const std::vector<uint8_t>& enc, const std::vector<FeeFrac>& optimal_diagram)
```

💬 instagibbs commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2608193810)
8f0828ea5144f261ff5e0cc39ee5eed17d0eec57

why this reduction? commit message commenting on it could be helpful

💬 instagibbs commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2611100880)
1c008c063df383aff64c564b116802b427a228ce

Please leave a note that 1M iterations is enough for all given dependency graphs but often a lot less than MaxOptimalLinearizationIters

💬 instagibbs commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2611720199)
c2eaf22503f7af057599f02ecb5f24a267ea33e2

Exposing my ignorance here, but why reserve this value? Deserves a comment.