Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🤔 ajtowns reviewed a pull request: "signet: fixing mining for OP_TRUE challenge"
(https://github.com/bitcoin/bitcoin/pull/29032#pullrequestreview-2299146330)
ACK ecee62d2ab63685455b428db4e832827b6ce36f1 with nits
💬 ajtowns commented on pull request "signet: fixing mining for OP_TRUE challenge":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r1756114962)
Should keep the `, *,` to ensure `blocktime` and `poolid` are keyword-only args.
💬 maflcko commented on pull request "doc: replaced --enable-debug with -DCMAKE_BUILD_TYPE=Debug in developer-notes":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2345289075)
It would be nice to fix all remaining instances in one go. See also https://github.com/bitcoin/bitcoin/pull/30664#issuecomment-2320892444 and https://github.com/bitcoin/bitcoin/pull/30664#issuecomment-2321610186
💬 kravens commented on issue "GUI event loop should be block free":
(https://github.com/bitcoin/bitcoin/issues/17145#issuecomment-2345407161)
> have you tested this? https://github.com/bitcoin-core/gui-qml

Not yet, but I will try it!
💬 naumenkogs commented on pull request "test: addrman: tried 3 times and never a success so `isTerrible=true`":
(https://github.com/bitcoin/bitcoin/pull/30445#discussion_r1756250067)
Could you expand the comment similar to what's above? "The time is set, but after 3 attempts...". Otherwise ACK :)
👍 naumenkogs approved a pull request: "refactor: move m_is_inbound out of CNodeState"
(https://github.com/bitcoin/bitcoin/pull/30233#pullrequestreview-2299394122)
ACK 129f73cc6e2ed4147f57c3d1dfe2841b8a46e9fe
🤔 bainpa reviewed a pull request: "26.2 final changes"
(https://github.com/bitcoin/bitcoin/pull/30376#pullrequestreview-2299511514)
MAZENTERPRISE
💬 hodlinator commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756344649)
<details>
<summary>New <code>git diff -w</code></summary>

```diff
diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp
index c6ee1ffed9..3fed361e7e 100644
--- a/src/test/util_string_tests.cpp
+++ b/src/test/util_string_tests.cpp
@@ -10,62 +10,140 @@ using namespace util;

BOOST_AUTO_TEST_SUITE(util_string_tests)

-BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec)
+// These are counted correctly, since tinyformat will require the provided number of args.
...
💬 l0rinc commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1756353021)
I did modify `HaveCoin` in this commit - but since you think it's noise, I've reverted
💬 l0rinc commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1756356272)
I don't think there's any difference (except for having to use `ret->second.coin.DynamicMemoryUsage` and `ret->second.coin.IsSpent()` instead), but I don't mind moving it up.
💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf, LogConnectFailure":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756387736)
I pushed a test-only change to reduce the verbosity of the test and also drop the accidental duplicate line.

The tests now look like `PassFmt<0>("");` for the passing case and `FailFmtWithError<0>("%s", check_num_spec);` for the failing case.

I hope this change is acceptable for reviewers to review. Otherwise, I'll revert back to the previous commit hash.
🚀 fanquake merged a pull request: "build: Fix `ENABLE_WALLET` option"
(https://github.com/bitcoin/bitcoin/pull/30867)
💬 Sjors commented on pull request "signet: fixing mining for OP_TRUE challenge":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2345614402)
Taken both suggestions.

I modified the test to actually check the signet commitment absence.
👍 l0rinc approved a pull request: "ci: Print inner env, Make ccache config more flexible"
(https://github.com/bitcoin/bitcoin/pull/30869#pullrequestreview-2298725366)
utACK facb8f16389d1fbf27ee70f887de9cb082028374
💬 l0rinc commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756384586)
the dot is dropped since it's not a [home dir](https://ccache.dev/manual/latest.html#config_cache_dir), right?
💬 l0rinc commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756381105)
This is sourced usually through `test_run_all.sh`, right?
What was the reason for such low values before?
💬 l0rinc commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1756386761)
nit:
> the build path**s** are constant
💬 l0rinc commented on pull request "ci: Print inner env, Make ccache config more flexible":
(https://github.com/bitcoin/bitcoin/pull/30869#discussion_r1755666556)
nit: are you using the two separate variable reference styles because the second one doesn't have any characters after it? To be fair, the first also ends with a `,` so I think we can simplify this:
```suggestion
CI_CCACHE_MOUNT="type=bind,src=$CCACHE_DIR,dst=$CCACHE_DIR"
```
📝 fanquake locked a pull request: "26.2 final changes"
(https://github.com/bitcoin/bitcoin/pull/30376)
bins were uploaded 2 weeks ago on June 18
website PR: https://github.com/bitcoin-core/bitcoincore.org/pull/1039
🚀 fanquake merged a pull request: "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"
(https://github.com/bitcoin/bitcoin/pull/30842)