Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048602251)
The linked article is long and convoluted and here I want to highlight only a short part of it that is relevant. Also having the text here is helpful if the web page disappears in the future.
💬 l0rinc commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#issuecomment-2812335126)
utACK 27f11217ca63e0f8f78f14db139150052dcd9962
💬 hebasto commented on pull request "build: Restore cross-compilation for Android":
(https://github.com/bitcoin/bitcoin/pull/32262#issuecomment-2812338837)
> I'd say a CI task config should exist. Otherwise, there is no way to test the build easily for non-android people. No opinion on adding it to the live CI matrix here in this repo.

I meant this ^ without adding to the live CI matrix in this repo.
💬 fanquake commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048607821)
Then I'd suggest atleast trying to consolidate what is here, given you already link to the same place above, with just "GetThreadTimes():" as a comment (that seems unecessary).
💬 hebasto commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048612501)
But this introduces another inconsistency between Windows and other platforms. Initially, `run: ./bin/bench_bitcoin.exe -sanity-check -priority-level=high` was written to mimic the `bench_sanity_check_high_priority` test task behaviour.
💬 fanquake commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048614265)
Can you explain your issue with running more tests in this CI?
💬 hebasto commented on pull request "doc: Add note for building on macOS (Intel) with CMake ≥ 4.0":
(https://github.com/bitcoin/bitcoin/pull/32289#issuecomment-2812355350)
> ... that should probably be fixed _properly_, in some way.

I haven't found a way to do it that covers all use cases, is user-friendly, and doesn't introduce excessive complexity.
💬 hebasto commented on pull request "doc: Add note for building on macOS (Intel) with CMake ≥ 4.0":
(https://github.com/bitcoin/bitcoin/pull/32289#discussion_r2048621583)
> This says it applies to users with Homebrew installed, when _not_ building with homebrew-provided tools? What are the "tools" here?

For example, LLVM toolchain.
💬 ismaelsadeeq commented on issue "RPC: `getblockstats` might not return the *effective* percentile fee rate":
(https://github.com/bitcoin/bitcoin/issues/31140#issuecomment-2812361361)
> Because of this sorting, "effective" feerate isn't really meaningful.

When a child transaction (fee rate 100) is paying for parent transactions (fee rates 4 and 5), they should be considered as a single chunk with a combined fee rate (e.g., ~40).
So instead of sorting as [100, 5, 4, 3, 2, 2, 1, 1, 1], a more accurate representation would be [40, 3, 2, 2, 1, 1]. This will provide a more accurate view (different percentile fee rate when they are sorted individually) that represents what fee rat
...
💬 hebasto commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048628558)
If it only takes a couple of extra seconds, why run a different set of tests for each platform?

Windows should, to the best of our ability, be treated the same as any other platform.
💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#issuecomment-2812395790)
`ee16345af3...19c8336d97`: address suggestions
💬 fanquake commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048646279)
> If it only takes a couple of extra seconds,

I haven't measured, for all users, developers and CIs. The point of this change is to immediately start catching regressions in at least one CI.
💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048646883)
Reduced the text and removed the link to the doc since it is already a few lines earlier.
💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2048648039)
Removed and added a note to the PR description about the alternatives.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2812407333)
Rebased, now that https://github.com/bitcoin/bitcoin/pull/31551 was merged - will redo the IBD benchmarks (since we have bigger obfuscatable chunks now) to see if any of the commit messages or descriptions need changing.
The PR is otherwise ready for review again!
👍 hebasto approved a pull request: "ci: drop -priority-level from bench in win cross CI"
(https://github.com/bitcoin/bitcoin/pull/32288#pullrequestreview-2775259752)
ACK 27f11217ca63e0f8f78f14db139150052dcd9962.
💬 hebasto commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048658794)
> The point of this change is to immediately start catching regressions in at least one CI.

Ok. Resolved.
💬 l0rinc commented on pull request "[IBD] flush UTXOs in bigger batches based on dbcache size":
(https://github.com/bitcoin/bitcoin/pull/31645#discussion_r2048662881)
Thanks, ended up with:
```C++
argsman.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: calculated from the provided `-dbcache` value or %u if none are provided)", node::GetDbBatchSize(DEFAULT_DB_CACHE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
```
which prints:
```bash
./build/bin/bitcoind -dbcache=1000 -help-debug | grep -A2 dbbatchsize
-dbbatchsize
Maximum database write batch size in bytes (default: ca
...
💬 maflcko commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#discussion_r2048663455)
The suggestion sounds reasonable. I'd be happy to review a pull doing it.
maflcko closed an issue: "estimateSmartFee error: "Insufficient data or no feerate found"
(https://github.com/bitcoin/bitcoin/issues/32178)
💬 maflcko commented on issue "estimateSmartFee error: "Insufficient data or no feerate found":
(https://github.com/bitcoin/bitcoin/issues/32178#issuecomment-2812429621)
This works as expected, as far as I can see, so closing for now.