Add Signet and testnet4 launch shortcuts for Windows

Sjors commented at 3:05 pm on October 18, 2022: member

This makes it easier to launch Signet and testnet4 on Windows. Follows the same pattern as testnet.

Before:

After:

(the testnet4 icon is the same as testnet3, not in the above screenshot)

maflcko added the label GUI on Oct 18, 2022

maflcko added the label Build system on Oct 18, 2022

maflcko added the label DrahtBot Guix build requested on Oct 18, 2022

maflcko commented at 3:15 pm on October 18, 2022: member

DrahtBot may have build results in two days. Though, keep in mind that they are untrusted.

Sjors commented at 3:41 pm on October 18, 2022: member

I’m optimistic that I can install Guix on the new machine faster…

ghost commented at 4:00 pm on October 18, 2022: none

Concept ACK

Tested it on Windows and it works as expected except the color of logo for signet.

0    Directory: C:\Users\Test\AppData\Roaming\Microsoft\Windows\Start Menu\Programs\Bitcoin Core
1
2
3Mode                 LastWriteTime         Length Name
4----                 -------------         ------ ----
5-a----        18-10-2022     15:55            890 Bitcoin Core (64-bit).lnk
6-a----        18-10-2022     15:55           1774 Bitcoin Core (signet, 64-bit).lnk
7-a----        18-10-2022     15:55           1776 Bitcoin Core (testnet, 64-bit).lnk
8-a----        18-10-2022     15:55            885 Uninstall Bitcoin Core (64-bit).lnk

Sjors commented at 4:16 pm on October 18, 2022: member

Looks like I need to set the correct yellow color.

jarolrod commented at 7:03 am on October 19, 2022: member

we have actual ico files under res for the main and testnet icons, you would need to introduce a bitcoin_signet.ico with the correct yellow hue applied under src/qt/res in order to introduce the correct icon.

Looking ahead, I’d like to move away from having these different launchers to being able to select the chain from within the gui.

Sjors commented at 10:56 am on October 20, 2022: member

Looking ahead, I’d like to move away from having these different launchers to being able to select the chain from within the gui

That might run into objections, see discussion in #signet

DrahtBot commented at 2:36 pm on October 22, 2022: contributor

Guix builds

File	commit 2ac71d20b2ebbfea76ee7369fddffbd78bd2c010(master)	commit a0ff23b4284259c8f1b8203d084c75f944b4b1c7(master and this pull)
SHA256SUMS.part	`9e44d66451404ba8...`	`b9c6099ffb8e3e27...`
*-aarch64-linux-gnu-debug.tar.gz	`07f096e7263c307b...`	`49aa481224b69d62...`
*-aarch64-linux-gnu.tar.gz	`9bb0bfc76278a674...`	`68aee27bd71d0537...`
*-arm-linux-gnueabihf-debug.tar.gz	`3c6c97da010bab38...`	`c88e624338856e5c...`
*-arm-linux-gnueabihf.tar.gz	`3fcabcd4b9ebb78e...`	`9eb267a65cbcaf82...`
*-arm64-apple-darwin-unsigned.dmg	`d31f3ee8c98fd78d...`	`d34a25a8063dd823...`
*-arm64-apple-darwin-unsigned.tar.gz	`e806a68ec499b75a...`	`5672fe3c6ee7c680...`
*-arm64-apple-darwin.tar.gz	`2f2ef9bcc90c1fb4...`	`3bdd72523333a129...`
*-powerpc64-linux-gnu-debug.tar.gz	`bc8b64002089116f...`	`76584b96c60e13b0...`
*-powerpc64-linux-gnu.tar.gz	`50ea580389bb7a1a...`	`f5a5acd1b18bf6f3...`
*-powerpc64le-linux-gnu-debug.tar.gz	`ba4228326796eed5...`	`2502f53061c331ce...`
*-powerpc64le-linux-gnu.tar.gz	`6656ca671314f515...`	`d16f3857ef02f2a7...`
*-riscv64-linux-gnu-debug.tar.gz	`53bf2d3002ebca0a...`	`825145de581faa7a...`
*-riscv64-linux-gnu.tar.gz	`5c9d22789581f003...`	`181146582be9ae53...`
*-win64-debug.zip	`697769f0e7b5a249...`	`d6a77eb017d99c77...`
*-win64-setup-unsigned.exe	`d4bc1571db907f36...`	`b78af568c81d770d...`
*-win64-unsigned.tar.gz	`ac4d4af7d79a937f...`	`9364f9f7e348be70...`
*-win64.zip	`402d8caf85519a1e...`	`c9a9d32ff70c0e05...`
*-x86_64-apple-darwin-unsigned.dmg	`f48fb842d677937f...`	`bf6cb43f3dd8ebe9...`
*-x86_64-apple-darwin-unsigned.tar.gz	`cb615dea9c35d6b3...`	`d271ca645e6799c5...`
*-x86_64-apple-darwin.tar.gz	`1791957f096bb6a0...`	`9af37ba6f95785d5...`
*-x86_64-linux-gnu-debug.tar.gz	`c9783c8790820f66...`	`b9fb78bae6a039da...`
*-x86_64-linux-gnu.tar.gz	`4658237de6ca792c...`	`bb46dd3bf2367563...`
*.tar.gz	`d854813212c1ae1b...`	`2f6e8196805eb8b5...`
guix_build.log	`1152087b4e6db1e7...`	`85bbeb69c3f671d8...`
guix_build.log.diff		`d124db1ae8b8747b...`

DrahtBot removed the label DrahtBot Guix build requested on Oct 22, 2022

luke-jr commented at 1:15 am on October 27, 2022: member

Most users won’t know what “signet” means, and it isn’t self-explanatory like “testnet”…

If you want a shortcut, please figure out a way to resolve this.

NACK as-is.

fanquake commented at 2:46 pm on February 6, 2023: member

What is the status of this? Seems very low priority, and not currently mergable.

DrahtBot commented at 2:46 pm on February 6, 2023: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	achow101, laanwj, TheCharlatan
Concept NACK	luke-jr, ghost
Concept ACK	michaelfolkson, BrandonOdiwuor

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31042 (build: Rename PACKAGE_* variables to CLIENT_* by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Sjors commented at 4:46 pm on February 6, 2023: member

On my list to rebase, fix the icon color, and reduce potential confusion. I have a Windows setup now.

Sjors force-pushed on Feb 21, 2023

Sjors commented at 11:27 am on February 21, 2023: member

I added the word “test” to the application name.

Since Bitcoin Inquisition uses the default Signet to deploy several proposed soft forks, I think it’s useful to offer easier access (which requires the Inquisition folks to rebase after this is merged).

See #8285 for additional context.

I generated the icon from the mainnet icon by applying the same hue and saturation adjustments:

https://github.com/bitcoin/bitcoin/blob/94070029fb6b783833973f9fe08a3a871994492f/src/qt/networkstyle.cpp#L22

Though the -modulate command uses percentage instead of degrees, so it was a bit confusing to convert:

0convert  src/qt/res/icons/bitcoin.ico -modulate 100,87,119.4 src/qt/res/icons/bitcoin_signet.ico

I added a screenshot in the PR description.

The big icon looks correct, but the mini version in the search result does not. I probably need to change the ImageMagick command somehow. I could also try using the Testnet icon as the basis (and rotate the hue in the other direction).

Sjors force-pushed on Feb 21, 2023

Sjors marked this as ready for review on Feb 21, 2023

Sjors commented at 1:23 pm on February 21, 2023: member

I think I found the right incantation, using the high res PNG as input:

0convert src/qt/res/icons/bitcoin.png -modulate 100,87,119.4 -define icon:auto-resize="256,48,32,16" src/qt/res/icons/bitcoin_signet.ico

I updated the PR description screenshot.

I noticed that testnet has 4 icon resolutions, which I’ve used for signet too. But mainnet has 9. I could regenerate that one, but it would only save 11kb.

Sjors force-pushed on Feb 21, 2023

in share/setup.nsi.in:145 in c80e55caee outdated

141@@ -141,6 +142,7 @@ Section -un.post UNSEC0001
142     Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\Uninstall $(^Name).lnk"
143     Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\$(^Name).lnk"
144     Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (testnet, 64-bit).lnk"
145+    Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (signet, 64-bit).lnk"

Sjors commented at 1:37 pm on February 21, 2023:

Forgot to add test here, pushing fix.

Sjors force-pushed on Feb 21, 2023

ghost commented at 4:55 pm on February 21, 2023: none

Most users won’t know what “signet” means, and it isn’t self-explanatory like “testnet”…

Most users wont use it and it just helps devs and power users on windows. They know what signet means. Everyone else clicking on it is same as downloading a malware instead of bitcoin core which is not something this repo needs to solve. First match in search is always “bitcoin core (mainnet)” and even that would open other network if bitcoin.conf has other network set.

So this NACK makes no sense for windows users.

I added the word “test” to the application name.

Since Bitcoin Inquisition uses the default Signet to deploy several proposed soft forks, I think it’s useful to offer easier access (which requires the Inquisition folks to rebase after this is merged).

See #8285 for additional context.

I generated the icon from the mainnet icon by applying the same hue and saturation adjustments:

https://github.com/bitcoin/bitcoin/blob/94070029fb6b783833973f9fe08a3a871994492f/src/qt/networkstyle.cpp#L22

Though the -modulate command uses percentage instead of degrees, so it was a bit confusing to convert:
0convert  src/qt/res/icons/bitcoin.ico -modulate 100,87,119.4 src/qt/res/icons/bitcoin_signet.ico
I added a screenshot in the PR description.

The big icon looks correct, but the mini version in the search result does not. I probably need to change the ImageMagick command somehow. I could also try using the Testnet icon as the basis (and rotate the hue in the other direction).

NACK for “test signet”

signet already helps to know its NOT mainnet

Sjors commented at 5:47 pm on February 21, 2023: member

I don’t see any harm in adding the word “test”. A user who searches “bitcoin” will see the “signet” entry in their search results and wonder what it is, perhaps even worry about it. Without the word “test” the only two ways to figure out are to google “what is signet” or to launch the application.

I don’t expect most users to know what “mainnet” is. They obviously know what Bitcoin is, but they may not know that one or more test networks exist.

ghost commented at 5:59 pm on February 21, 2023: none

I don’t expect most users to know what “mainnet” is. They obviously know what Bitcoin is, but they may not know that one or more test networks exist.

If someone doesnt know what is mainnet they wont use bitcoin core as a wallet.

TheCharlatan commented at 6:15 pm on April 25, 2023: contributor

Concept ACK

michaelfolkson commented at 7:31 pm on April 25, 2023: contributor

Concept ACK

Perhaps the Bitcoin Design community will have a view on the wording. I’d go with standalone “signet” but I haven’t done any user research on whether this isn’t sufficiently informative.

DrahtBot added the label CI failed on May 29, 2023

DrahtBot removed the label CI failed on May 31, 2023

in share/setup.nsi.in:145 in d4336ff18c outdated

141@@ -141,6 +142,7 @@ Section -un.post UNSEC0001
142     Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\Uninstall $(^Name).lnk"
143     Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\$(^Name).lnk"
144     Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (testnet, 64-bit).lnk"
145+    Delete /REBOOTOK "$SMPROGRAMS\$StartMenuGroup\@PACKAGE_NAME@ (test signet, 64-bit).lnk"

maflcko commented at 9:34 am on August 8, 2023:

Can remove the 64-bit here and above?

Sjors commented at 9:26 am on August 14, 2023:

Done for testnet and signet. I didn’t touch Name "@PACKAGE_NAME@ (64-bit)" though, not sure if that’s safe.

in src/Makefile.qt.include:179 in d4336ff18c outdated

175@@ -176,6 +176,7 @@ QT_RES_ICONS = \
176   qt/res/icons/address-book.png \
177   qt/res/icons/bitcoin.ico \
178   qt/res/icons/bitcoin_testnet.ico \
179+  qt/res/icons/bitcoin_signet.ico \

maflcko commented at 9:36 am on August 8, 2023:

Can just reuse the testnet icon to save 43.4kB in the git repo? Doesn’t seem worth it to burden every clone forever just so that a color in a test-only picture is of a slightly different tone.

Sjors commented at 9:19 am on August 14, 2023:

Or we could just drop testnet3 :-P

I find the difference in color useful enough.

maflcko commented at 9:22 am on August 14, 2023:

I find the difference in color useful enough.

Then, it would be better to have a tiny script in the build process to create the .ico, no? This would make it reproducible, documented and easily modifiable, without on each modification bloating the git repo by 50kb

Sjors commented at 9:30 am on August 14, 2023:

The incantation is in the commit message. Adding a script makes sense, but I’m not familiar enough with the (native) build tools to know where to add it. This particular command would also add a dependency to imagemagick (if we don’t use it already), though I’m sure the operation can be done with other tools if needed.

Seems better to do in a followup and generate the testnet icon that way too.

maflcko approved

maflcko commented at 9:36 am on August 8, 2023: member

Seems fine. Left comments

Sjors force-pushed on Aug 14, 2023

DrahtBot added the label CI failed on Jan 13, 2024

Sjors force-pushed on Feb 13, 2024

maflcko commented at 1:02 pm on February 13, 2024: member

On a second thought, I wonder what the target audience is. Someone aware of a test network, and wanting to use it, should be able to create a shortcut themselves? Creating the shortcut should be the easiest of their problems to get started with a test network.

Sjors commented at 1:20 pm on February 13, 2024: member

Creating a shortcut is easy, but creating an icon with a non-confusing color is not.

Sjors commented at 1:24 pm on February 13, 2024: member

I would also expect not-super-tech-savy people to be testing applications on custom signets, e.g. testing a vault GUI on Inquisition. I would hope they don’t use Windows of course :-)

DrahtBot removed the label CI failed on Feb 13, 2024

achow101 commented at 4:00 pm on April 9, 2024: member

The PR didn’t seem to attract much attention in the past. Also, the issue seems not important enough right now to keep it sitting around idle in the list of open PRs.

Closing due to lack of interest.

achow101 closed this on Apr 9, 2024

Sjors commented at 5:28 pm on April 9, 2024: member

cc @laanwj

Regarding the word “test”: there are scammers selling signet coins, so it seems important to emphasize

achow101 reopened this on Apr 9, 2024

laanwj requested review from laanwj on Apr 14, 2024

laanwj commented at 9:28 am on April 14, 2024: member

Concept ACK, i think it’s important that signet gets its own launch icon if testnet has, it’s more useful for testing than testnet. The other option would be to keep only the mainnet icon. But i think there’s value in making windows users aware of these kind of things (so they don’t reinvent wheels, or, test on mainnet 😅 ).

BrandonOdiwuor commented at 4:18 pm on April 15, 2024: contributor

Concept ACK

DrahtBot added the label Needs rebase on Sep 2, 2024

Add Signet launch shortcut for Windows

cd src/qt/res/icons
convert bitcoin.png -modulate 100,87,119.4 -define icon:auto-resize="256,48,32,16" bitcoin_signet.ico

This commit also removes the 64-bit mention from testnet.

77b2923f87

Add Testnet4 launch shortcut for Windows cfd03de965

Sjors force-pushed on Sep 3, 2024

Sjors commented at 8:34 am on September 3, 2024: member

Rebased for CMake (dropping src/Makefile.qt.include) and added a testnet4 shortcut.

Sjors renamed this:
~~Add Signet launch shortcut for Windows~~
Add Signet and testnet4 launch shortcuts for Windows
on Sep 3, 2024

DrahtBot removed the label Needs rebase on Sep 3, 2024

achow101 commented at 10:43 pm on September 3, 2024: member

ACK cfd03de965a081facbd72316c76603dd7aa511bd

Did not test, but the code matches the pattern used for the other networks.

DrahtBot requested review from TheCharlatan on Sep 3, 2024

DrahtBot requested review from BrandonOdiwuor on Sep 3, 2024

achow101 removed review request from BrandonOdiwuor on Oct 15, 2024

achow101 requested review from hebasto on Oct 15, 2024

laanwj commented at 8:42 am on October 16, 2024: member

\

DrahtBot requested review from BrandonOdiwuor on Oct 16, 2024

laanwj approved

laanwj commented at 8:42 am on October 16, 2024: member

ACK cfd03de965a081facbd72316c76603dd7aa511bd

TheCharlatan approved

TheCharlatan commented at 1:48 pm on October 21, 2024: contributor

ACK cfd03de965a081facbd72316c76603dd7aa511bd

fanquake merged this on Oct 21, 2024

fanquake closed this on Oct 21, 2024

bitcoin deleted a comment on Oct 21, 2024

hebasto commented at 2:41 pm on October 22, 2024: member

Post-merge ACK cfd03de965a081facbd72316c76603dd7aa511bd.

Sjors deleted the branch on Oct 24, 2024

Add Signet and testnet4 launch shortcuts for Windows #26334

Guix builds

Code Coverage

Reviews

Conflicts