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

🚀 ryanofsky merged a pull request: "util: Use consteval checked format string in FatalErrorf, LogConnectFailure"
(https://github.com/bitcoin/bitcoin/pull/30546)

📝 fjahr opened a pull request: "scripted-diff: Modernize nLocalServices to m_local_services"
(https://github.com/bitcoin/bitcoin/pull/30885)
The type of the `nLocalServices` variable was changed to `std::atomic<ServiceFlags>` in #30807 and I suggested the variable name to get updated with a scripted diff along with it. It wasn't included in the PR but I am still suggesting to do it as a follow-up since I had already prepared the commit.

💬 maflcko commented on issue "Nuke format linter":
(https://github.com/bitcoin/bitcoin/issues/30530#issuecomment-2346873781)
<!-- https://github.com/bitcoin/bitcoin/pull/30386#discussion_r1684380835

📝 instagibbs opened a pull request: "rpc: Add support to populate PSBT input utxos via rpc"
(https://github.com/bitcoin/bitcoin/pull/30886)
Resolves https://github.com/bitcoin/bitcoin/issues/30873

Question(s):
1) Was this functionality already exposed somewhere I missed?
2) Should "non-witness utxos" always be the type added when updating the PSBT? Looks like we fill in non-witness wherever we can, and witness whenever scraping from utxo set

⚠️ katesalazar opened an issue: "Closing a wallet using the fa46088440 28.x QT client segfaults"
(https://github.com/bitcoin/bitcoin/issues/30887)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Closing a descriptor wallet using the fa46088440 28.x QT client segfaults.
Closing a descriptor wallet using the fa46088440 28.x console client works fine.


### Expected behaviour

Not a crash.


### Steps to reproduce

--enable-wallet --disable-shared --with-pic --no-create --no-recursion

### Relevant log output

standard segfault line


### How did you obtain Bitcoin Core

Compiled
...

👍 itornaza approved a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440#pullrequestreview-2301029767)
Concept ACK 23374a22f442939802c0296d6447e0db02d513da

Do you consider updating for cmake? It would be helpful for me to build and run tests locally.