Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 Sjors commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-2045743484)
cc @laanwj
📝 achow101 reopened a pull request: "Add Signet launch shortcut for Windows"
(https://github.com/bitcoin/bitcoin/pull/26334)
This makes it easier to launch Signet on Windows. Follows the same pattern as testnet.

Before:

<img width="766" alt="testnet" src="https://user-images.githubusercontent.com/10217/196468934-ee29d129-871b-4612-bde4-842f191403a7.png">

After:
<img width="500" alt="signet1" src="https://user-images.githubusercontent.com/10217/220358057-d9efc532-272c-45e7-81fa-3a52f58a0f29.png">
<img width="527" alt="signet2" src="https://user-images.githubusercontent.com/10217/220358067-62b3b76f-604a-4163-
...
💬 RandyMcMillan commented on pull request "gui: Hide peers details":
(https://github.com/bitcoin-core/gui/pull/505#issuecomment-2045842312)
41a1a86: update per previous suggestions

> tACK [436ac4f](https://github.com/bitcoin-core/gui/commit/436ac4f42b88e8131bf1e02554a20b3c549e7d6f)
>
> UI look & feel.
> **nit**: not a blocker, please consider previous [feedback](https://github.com/bitcoin-core/gui/pull/505#pullrequestreview-1630116141) regarding commit title length and meaningfulness, maybe `gui:` + this PR's title.
👍 pablomartin4btc approved a pull request: "Hide peers details"
(https://github.com/bitcoin-core/gui/pull/505#pullrequestreview-1989980960)
re ACK 41a1a8615dd48fdd9811b9824c49ceb934c6375e
💬 whitslack commented on pull request "contrib/init: (OpenRC) use -daemonwait to wait for startup completion":
(https://github.com/bitcoin/bitcoin/pull/24066#issuecomment-2045899595)
For posterity: note that Gentoo switched to shipping [its own local version](https://github.com/gentoo/gentoo/blob/380aad5fc649a00ae46644c130fb9a3b8970ee09/net-p2p/bitcoin-core/files/bitcoind.openrc) of the OpenRC runscript for Bitcoin Core last year because this PR had stalled.
👍 hernanmarino approved a pull request: "ci: disable `_FORTIFY_SOURCE` with MSAN"
(https://github.com/bitcoin/bitcoin/pull/29837#pullrequestreview-1990006290)
utACK 08ff17d1420a3d1c14c6b1a5436678fbb1dd9cbc . Relevant CI test seems to be working OK.
💬 hernanmarino commented on pull request "refactor, bench, fuzz: Drop unneeded `UCharCast` calls":
(https://github.com/bitcoin/bitcoin/pull/29820#issuecomment-2045952268)
ACK 56e1e5dd10cbe51d3abc3fbf532b6b41bf62a889
💬 hernanmarino commented on pull request "assumeutxo: Fix -reindex before snapshot was validated":
(https://github.com/bitcoin/bitcoin/pull/29726#issuecomment-2045961526)
crACK b7ba60f81a33db876f88b5f9af1e5025d679b5be . Good fix
💬 hernanmarino commented on pull request "lint: scripted-diff verification also requires GNU grep":
(https://github.com/bitcoin/bitcoin/pull/29689#issuecomment-2045970490)
cr ACK 3bf4f8db669e1e274ce2633cf84add2938b9914b
👍 hernanmarino approved a pull request: "test: remove duplicated ban test"
(https://github.com/bitcoin/bitcoin/pull/29688#pullrequestreview-1990065659)
tested ACK e30e8625bbc42045b8b757a8d7e80c20cc61cebf
💬 1440000bytes commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2045979791)
Concept ACK
🚀 fanquake merged a pull request: "refactor, bench, fuzz: Drop unneeded `UCharCast` calls"
(https://github.com/bitcoin/bitcoin/pull/29820)
💬 hernanmarino commented on pull request "Bugfix: bitcoin-cli: Check length of peer.transport_protocol_type":
(https://github.com/bitcoin/bitcoin/pull/29657#issuecomment-2045981680)
utACK c3e632b44153e314ef946f342c68c2758b1cbc4d
💬 sipa commented on pull request "Safegcd-based modular inverses in MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/21590#issuecomment-2046018528)
Rebased, and switched to `std::countr_zero` instead of CTZ builtins.
📝 pablomartin4btc opened a pull request: "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection"
(https://github.com/bitcoin-core/gui/pull/815)
<details>
<summary>Currenlty on <code>master</code>, when the "mask values" checkbox is ticked if the user selects a different wallet, the history action is enable and if the user clicks on it can see all the transactions in the transaction view.</summary>

![Peek 2024-04-09 17-37](https://github.com/bitcoin-core/gui/assets/110166421/d8e2fdd1-aaa6-4506-acde-51fa45a74910)

</details>
<details>
<summary>This PR fixes it.</summary>

![Peek 2024-04-09 17-45](https://github.com/bitcoin-core/
...
🤔 jarolrod reviewed a pull request: "Don't permit port in proxy IP option"
(https://github.com/bitcoin-core/gui/pull/813#pullrequestreview-1990175028)
Just to point out first, this will only work effectively in the case that editing has finished, as in clicking on another item that will take focus. If you only edit the proxy, leave focus on that box and click ok to save the setting, the invalid proxy will be saved.

Will propose a proper patch soon.
💬 0xB10C commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1558318881)
Tried this, but [didn't work for zero arguments tracepoint](https://github.com/bitcoin/bitcoin/actions/runs/8618313790/job/23620364926?pr=26593#step:6:4106) (the newly introduced tests catched this). However, we can use ` __VA_OPT__` from C++ 20.
⚠️ ryanofsky opened an issue: "RFC: "Insufficient review" tag for closed PRs"
(https://github.com/bitcoin/bitcoin/issues/29839)
During PR triage today, it seems like we closed or considered closing a number of PRs purely due to lack of review and interest in reviewing, not due to any problems with the PRs themselves.

I think it's reasonable to close PRs like these, because we have a severe lack of reviewer bandwidth, so having lots of low priority PRs open can make it harder for higher priority PRs to receive the attention they deserve.

However, I think it would be good to distinguish between PRs that are good but
...
🤔 kevkevinpal reviewed a pull request: "Logging cleanup"
(https://github.com/bitcoin/bitcoin/pull/29798#pullrequestreview-1990459252)
Concept ACK

just a question is there any reason to keep `BCLog::NONE` in `logging.h` and not completely delete it I did
`grep --exclude-dir=".deps" -nri "BCLog::NONE" ./src/ --binary-files=without-match`
and
`grep --exclude-dir=".deps" -nri "LogFlags::NONE" ./src/ --binary-files=without-match`
and only see it being used in `./src/logging.{h,cpp}` and `./src//qt/transactiondesc.cpp`
💬 kevkevinpal commented on pull request "Logging cleanup":
(https://github.com/bitcoin/bitcoin/pull/29798#discussion_r1558466877)
I noticed in 9fb321ad2b0176955718317b796715250eac01f4 that we introduce this code, I am not sure if it makes sense to remove this `0` value as the first commit so we're not adding code and removing within two commits