Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 l0rinc commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3393852130)
code review reACK 023cd5a5469ad61205bf7bb1135895f2b4a20ea9

Thanks for the quick responses.
Build failures seem systemic and unrelated.
💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2423304917)
Here I'm only modifying them because the build was failing otherwise, but I have indeed added tests to https://github.com/bitcoin/bitcoin/pull/32313 which will ideally be merged before this. Either way the other one needs rebasing.
💬 l0rinc commented on pull request "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2423309528)
Different files though, but yeah, will adjust if I need to push again, thanks.
💬 fanquake commented on pull request "[29.x] Backport logging ratelimiting":
(https://github.com/bitcoin/bitcoin/pull/33225#discussion_r2423424121)
@Crypt-iQ It's still fine to open a PR against 29.x to adjust this.
⚠️ fanquake reopened an issue: "ci: remove third-party javascript usage from Windows CI"
(https://github.com/bitcoin/bitcoin/issues/32508)
See:

https://github.com/bitcoin/bitcoin/blob/c779ee3a4044df3a263394bbb8b104aeeaa7c727/.github/workflows/ci.yml#L189-L190

We shouldn't need to use a third-party repo, that runs some Javascript, to configure a command prompt. The comment in our code also doesn't explain why this is necessary. We should be able to drop this dependency.

Also discussed in #32396.
💬 fanquake commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3394088606)
Binaries are available: https://bitcoincore.org/bin/bitcoin-core-30.0/
Website has been updated: https://bitcoincore.org/en/releases/30.0/
💬 maflcko commented on pull request "[29.x] Backport logging ratelimiting":
(https://github.com/bitcoin/bitcoin/pull/33225#discussion_r2423464895)
Could do it for master and backport?
💬 maflcko commented on issue "Intermittent CI network issue downloading assets.json from GitHub":
(https://github.com/bitcoin/bitcoin/issues/33599#issuecomment-3394123866)
I guess the IPs are detected as and blocked as "LLM scrapers". Possible workarounds could be:

* Use a mirror/proxy to download the json file
* Use a git clone (sparse), which may not be rate limited
📝 prusnak opened a pull request: "contrib: add desktop file"
(https://github.com/bitcoin-core/gui/pull/902)
from https://github.com/bitcoin-core/packaging

=> https://github.com/bitcoin-core/packaging/blob/main/debian/bitcoin-qt.desktop

this simplifies packaging, because currently one needs to fetch a desktop file from another repo instead of using just the main source code one
💬 sipa commented on pull request "txgraph: randomize order of same-feerate distinct-cluster transactions":
(https://github.com/bitcoin/bitcoin/pull/33335#issuecomment-3394366181)
Rebased on #33157, as it looks that's close.
👍 TheCharlatan approved a pull request: "miner: empty mempool special case for waitNext()"
(https://github.com/bitcoin/bitcoin/pull/33566#pullrequestreview-3328593103)
ACK 2e8fff3f17366e9c2ba054023ad8bd89ac51d584
👍 TheCharlatan approved a pull request: "[IBD] coins: increase default UTXO flush batch size to 32 MiB"
(https://github.com/bitcoin/bitcoin/pull/31645#pullrequestreview-3328689546)
ACK b6f8c48946cbfceb066de660c485ae1bd2c27cc1

I don't think complicating this with a dynamic calculation makes sense either given the trade offs documented in this discussion.
🤔 hebasto reviewed a pull request: "ci: Use native platform for win-cross task"
(https://github.com/bitcoin/bitcoin/pull/33558#pullrequestreview-3328936219)
> * Unlock the CI task to run on riscv64 at all

Isn't this relevant to other tasks as well, for example, `ci/test/00_setup_env_arm.sh`?
👍 hebasto approved a pull request: "ci: Use native platform for win-cross task"
(https://github.com/bitcoin/bitcoin/pull/33558#pullrequestreview-3329020667)
ACK fa6fd16f36e1240cda58a46e1717b02e8d3172a3, tested on Ubuntu 24.04, RISC-V.
💬 andrewtoth commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2424497697)
We should try and insert a different `coin` at the same `outpoint`, like we do for the `AddCoin` test above. That way the below check `cache.AccessCoin(outpoint) == coin` makes sure the original `coin` emplaced was not overwritten by the second one.
💬 kevkevinpal commented on pull request "refactor: Construct g_verify_flag_names on first use":
(https://github.com/bitcoin/bitcoin/pull/33600#issuecomment-3394837857)
ACK [faa9d10](https://github.com/bitcoin/bitcoin/pull/33600/commits/faa9d10c84bc6b465cbca266468990cc716b4300)

Looks good to me, makes sense to follow the construct on first use idiom.

I also did a quick grep to see if `g_verify_flag_names` was used anywhere else, and it was not
💬 pablomartin4btc commented on pull request "argsman, cli: GNU-style command-line option parsing (allows options after non-option arguments)":
(https://github.com/bitcoin/bitcoin/pull/33540#issuecomment-3394956274)
> * Is there a way to pass `bitcoin-cli` RPC arguments that begin with a `-` character?

At the moment, please correct me if I'm wrong, there's no or I haven't seen an RPC that would receive a valid argument that starts with `-` (but I understand it could be in the future), and there's no test for it otherwise I think the PR would have caught it.

Testing the scenario you mentioned in `master` would work like for example in:

```
./build_master/bin/bitcoin-cli -regtest -datadir=/tmp/b
...
💬 kevkevinpal commented on pull request "doc: archive release notes for v30.0":
(https://github.com/bitcoin/bitcoin/pull/33601#discussion_r2424672309)
you can drop `kevkevin` I think thats a duplicate for me
💬 l0rinc commented on pull request "doc: archive release notes for v30.0":
(https://github.com/bitcoin/bitcoin/pull/33601#discussion_r2424684332)
Like this? :p
```suggestion
- kevin
```