💬 maflcko commented on pull request "checkqueue: implement a new scriptcheck worker pool with atomic variables":
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3009103057)
> is not exposed.

it is: See " View more details on Cirrus CI " on the checks page

💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#discussion_r2169456219)
accidental comment?

💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#discussion_r2169473815)
this changes behavior a bit but should give the coverage we want still, as all we care about is getting 1p2c into mempool with TRUC

👍 instagibbs approved a pull request: "test: Fix reorg patterns in tests to use proper fork-based approach"
(https://github.com/bitcoin/bitcoin/pull/32587#pullrequestreview-2962737235)
ACK 0feecd78e52aede26c07391b9f361410e6a8a4ad

though I think the last two commits can be squashed

💬 mzumsande commented on issue "`dumptxoutset` rollback feature does not take forks into account":
(https://github.com/bitcoin/bitcoin/issues/32817#issuecomment-3009115241)
We could also automate it: Iterate over the block index to also find all fork blocks at the target height and invalidate those, only then do the invalidation on the main chain / do the dump, and finally reconsider everything. Though I'm not sure if that would be overkill.

💬 pinheadmz commented on issue "b-msghand invoked oom-killer in Debug build: Master (v28.99) crashing during IBD":
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-3009115501)
I'm going to debug-build and restart an IBD on another 4CPU 8GB VM from recent master commit c43cc48aaa with `txindex=1` and I'll keep an eye on it -- if it syncs without OOM I'll just close this issue

💬 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?