💬 HowHsu commented on pull request "checkqueue: implement a new scriptcheck worker pool with atomic variables":
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3009126452)
> > is not exposed.
>
> it is: See " View more details on Cirrus CI " on the checks page

Sorry, there is node logs, I missed them, thanks again.

💬 HowHsu commented on pull request "checkqueue: implement a new scriptcheck worker pool with atomic variables":
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3009142588)
From the CI log, seems too much threads causes failure, I'll temporarily set a smaller number limitation of scriptcheck workers to sort of confirm it.

💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169518099)
Added as suggested. Thanks!

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2169518557)
# Benchmark results

Tested 99b48777ee using `ab -k -c 1 -n 10000` over [block 900005 (having >5000 txs)](https://mempool.space/block/000000000000000000017bfd05b5fa367a424c4a565a4baf7950d9e8605df8ec).

## `-locationsindex=1`

```
Document Path: /rest/txfromblock/000000000000000000017bfd05b5fa367a424c4a565a4baf7950d9e8605df8ec-0.bin
Document Length: 384 bytes
Requests per second: 7387.93 [#/sec] (mean)
Time per request: 0.135 [ms] (mean)

Document Path:
...

💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169521413)
Fixed

💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169523019)
Fixed

💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169524485)
Added as suggested. Thanks!

💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#issuecomment-3009191149)
Thanks @hodlinator and @janb84 for the detailed review. Well Appreciated, Please let me know if there's any comment left unaddressed.

💬 maflcko commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#discussion_r2169541692)
no strong opinion. Generally, nits are left for the author to decide on (take or leave)

💬 sipa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2169647676)
Well if you're going to assume `at_size <= INT32_MAX`, why bother changing the type?

💬 polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2169656553)
Yep, I think I will not :) https://github.com/bitcoin/bitcoin/pull/32750#issuecomment-3009102393:
> In FeeFrac size is defined as an int32_t so using uint32 doesn't give us any "advantage".

🤔 furszy reviewed a pull request: "rpc: use CScheduler for HTTPRPCTimer"
(https://github.com/bitcoin/bitcoin/pull/32796#pullrequestreview-2963076372)
Because the `scheduler` is also used for the validation signals (see [here](https://github.com/bitcoin/bitcoin/blob/c43cc48aaaaa5c1c9fcb1175b415f7bc33e8537f/src/init.cpp#L1382)), using it for RPC as well seems likely to delay block processing — we only allow up to 10 queued tasks at a time before pausing block processing (see [here](https://github.com/bitcoin/bitcoin/blob/c43cc48aaaaa5c1c9fcb1175b415f7bc33e8537f/src/validation.cpp#L3441)).
So, in other words, adding 10 long-lived timers through
...

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3009418385)
> Whiteboarded in person. Hopefully this explanation is clear and explains it in a more permanent location. If this ends up being correct we can adapt the benchmark to be more realistic.

🤦 yes sorry, I was being dumb. I think I'm still missing why peer 0 is separate from the others? These are the numbers I get:

- announcement limit `A=24,000`
- number of peers `P=125`
- number of unique transactions `N` = A / P = 24,000 / 125 = 192
- total memory limit `M` = 404k * P = 50,500,000wu
-
...

💬 luke-jr commented on pull request "rpc, doc: clarify watch-only wallets balances in RPCHelp":
(https://github.com/bitcoin/bitcoin/pull/32761#issuecomment-3009423742)
I don't get it. Watch-only wallets are treated the same as non-watch-only, aren't they?

💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3009427139)
I think the peer that "triggers" to the LimitOrphan eviction should be one that is not by itself violating per-peer dos limits, otherwise it'll be the first one evicted from, immediately halting the eviction process.

💬 hodlinator commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2169685817)
If `Commit()` fails and (while failing) were to close the file internally as suggested, it should probably throw an exception after that so calling code doesn't try to continue using a closed `AutoFile`.

An alternative would be to have a `AutoFile::CommitAndClose()`-function that always closes regardless of errors and clearly signals that calling code does not expect to continue using the `AutoFile`?

💬 luke-jr commented on pull request "wallet: Replace CWalletTx::mapValue and vOrderForm with explicit class members":
(https://github.com/bitcoin/bitcoin/pull/32763#issuecomment-3009454105)
Concept NACK, agree with @ryanofsky's concerns. I don't think there's any benefit to refactoring this either?

⚠️ NowYu opened an issue: "First v"
(https://github.com/bitcoin/bitcoin/issues/32820)
### Motivation

Segrhiij

### Possible solution

_No response_

### Useful Skills

* Compiling Bitcoin Core from source
* Running the C++ unit tests and the Python functional tests
* ...


### Guidance for new contributors

Want to work on this issue?

For guidance on contributing, please read [CONTRIBUTING.md](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md) before opening your pull request.

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3009463187)
> it'll be the first one evicted from, immediately halting the eviction process

Does this require the "aggressive" approach where we trim a peer until it is within limits? Even if peer0 is one of the 50.5MWu ones, we would only need to delete 1 item to make it no longer the most DoSy, and then we round robin through all 125?

✅ willcl-ark closed an issue: "First v"
(https://github.com/bitcoin/bitcoin/issues/32820)