Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 fanquake commented on pull request "build: Revert "Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"":
(https://github.com/bitcoin/bitcoin/pull/30883#issuecomment-2346666009)
Going to merge this, given multiple developers have reported issues.
fanquake closed an issue: "cmake: GenerateHeaderFrom very slow"
(https://github.com/bitcoin/bitcoin/issues/30881)
🚀 fanquake merged a pull request: "build: Revert "Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`""
(https://github.com/bitcoin/bitcoin/pull/30883)
💬 jonatack commented on pull request "memusage: let PoolResource keep track of all allocated/deallocated memory":
(https://github.com/bitcoin/bitcoin/pull/28939#discussion_r1757158064)
@martinus PR needs rebase/update to cmake. This makefile has been removed.
📝 sipa opened a pull request: "streams: cache file position within AutoFile"
(https://github.com/bitcoin/bitcoin/pull/30884)
This is intended to address #30833.
👍 instagibbs approved a pull request: "test: switch MiniWallet padding unit from weight to vsize"
(https://github.com/bitcoin/bitcoin/pull/30718#pullrequestreview-2300766803)
ACK 7d1c8d0aa3ebe30d7633caf9bc4765927e4d6e08

seems straight forward to me, and all usages appear to be concerned with wallet/mempool things, not consensus related activities which may require weight-level precision.

For weight-level consensus precision I suspect wallet-like functionality is less interesting.
💬 instagibbs commented on pull request "test: switch MiniWallet padding unit from weight to vsize":
(https://github.com/bitcoin/bitcoin/pull/30718#discussion_r1757153370)
without this multiplier was there an issue in this PR?
💬 maflcko commented on pull request "wallet: clarify FundTransaction signature":
(https://github.com/bitcoin/bitcoin/pull/29564#issuecomment-2346714791)
@S3RK Are you still working on this?
💬 hebasto commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757184931)
> Also maybe I need to think about it more, but I'm not sure why choosing CMAKE_BINARY_DIR would be better than CMAKE_SOURCE_DIR in either case as long a CMAKE_BINARY_DIR is a subdirectory of CMAKE_SOURCE_DIR. Are we trying to optimize for cases where CMAKE_BINARY_DIR is not a subdirectory?

I don't think it is related to whether `CMAKE_BINARY_DIR` is a subdirectory of `CMAKE_SOURCE_DIR`.

Due to https://github.com/bitcoin/bitcoin/blob/07c7c96022dd325be1cd3b353d575eb6a5593f55/src/CMakeLists.
...
💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2346723974)
reACK a5ea6467b6a694facc1f5efe460bf30539167597

via `git range-diff master 59028de422bbf8ccf53da27f1153e343952e585f a5ea6467b6a694facc1f5efe460bf30539167597`

Just benchmark changes, with examples 11-14 added in 2fcfb4c0392a2f3114195e6e9979c1431798632c and 15-19 in 8fbc6f3c33c6a5b23314279f0856de0625cc6d3f

New benchmarks all run for each commit properly post-introduction.
my benchmark runs for comparison starting from introduction commit to tip: https://gist.github.com/instagibbs/058f5158
...
💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2346729363)
I rebased the PR against latest `master`. Manually resolved a small code conflict in src/chainparams.cpp and a large one in contrib/signet/miner. In new OOP version of contrib/signet/miner my change is much smaller :)

Also I noticed that Makefile based testing was replaced with CMake based approach. I added new C++ file with tests (chainparams_tests.cpp) to src/test/CMakeLists.txt instead of src/Makefile.test.include.
💬 ryanofsky commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757199833)
> I don't think it is related to whether `CMAKE_BINARY_DIR` is a subdirectory of `CMAKE_SOURCE_DIR` or not.

Ccache documentation describes behavior of base_dir: "ccache will rewrite absolute paths into paths relative to the current working directory, but only absolute paths that begin with **base_dir**"

So if CMAKE_BINARY_DIR is a subdirectory of CMAKE_SOURCE_DIR, and base_dir is specified as CMAKE_SOURCE_DIR, then that should be a strict improvement because it will allow cache hits across
...
💬 maflcko commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757200582)
> last commit, which was mostly motivated by [#30811 (comment)](https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2331096969).

Sorry, I was just testing, not trying to imply that this is the common case. Using worktrees (different source dirs) seems more common than different build dirs for the same cmake config.
🤔 murchandamus reviewed a pull request: "RPC: improve SFFO arg parsing, error catching and coverage"
(https://github.com/bitcoin/bitcoin/pull/30844#pullrequestreview-2300840916)
Awesome, thanks for picking this up.

crACK cddcbaf81e8e3d107cd321ee2c9a7fd786a8917e
davidgumberg closed a pull request: "Lint: improve subtree exclusion"
(https://github.com/bitcoin/bitcoin/pull/29965)
💬 davidgumberg commented on pull request "Lint: improve subtree exclusion":
(https://github.com/bitcoin/bitcoin/pull/29965#issuecomment-2346756266)
Closing this for now, it's up for grabs, if I have a chance to work on this again soon I'll split into smaller PR's
💬 hebasto commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757214440)
> So if CMAKE_BINARY_DIR is a subdirectory of CMAKE_SOURCE_DIR, and base_dir is specified as CMAKE_SOURCE_DIR, then that should be a strict improvement because it will allow cache hits across multiple source directories, and multiple build directories, as long as build directories are not outside the source directories.

For the latter case, using different build directories will result in cache misses anyway, right?

Anyway, I agree with @maflcko and @ryanofsky that using git worktrees shou
...
🚀 ryanofsky merged a pull request: "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage"
(https://github.com/bitcoin/bitcoin/pull/30618)
💬 ryanofsky commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1757234183)
> > So if CMAKE_BINARY_DIR is a subdirectory of CMAKE_SOURCE_DIR, and base_dir is specified as CMAKE_SOURCE_DIR, then that should be a strict improvement because it will allow cache hits across multiple source directories, and multiple build directories, as long as build directories are not outside the source directories.
>
> For the latter case, using different build directories will result in cache misses anyway, right?

It seems like that should be true but I haven't experimented or thou
...
💬 monlovesmango commented on issue "28.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/30854#issuecomment-2346844899)
great job putting this guide together rkrux! found it easy to follow, thank you!

### 4. Mempool Conflicting Transactions
couple things about this section.

at the end of the section it says:
>Execute the `gettransaction` RPC for the new transaction and check the `mempoolconflicts` field - it contains the previous transaction!

but the new transaction will only have `walletconflicts` populated and not `mempoolconflicts`. its the old/original transaction that now has both `walletconflicts
...