Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update prompt not shown on Windows #2729

Closed
devinbileck opened this issue Apr 16, 2019 · 21 comments · Fixed by #2737
Closed

Update prompt not shown on Windows #2729

devinbileck opened this issue Apr 16, 2019 · 21 comments · Fixed by #2737

Comments

@devinbileck
Copy link
Member

There are multiple reports of Windows 10 users not receiving the prompt to update to v1.0. Whereas Linux and macOS users are receiving the prompt.

@devinbileck
Copy link
Member Author

I can reproduce the issue on Windows 10 when running from the binary for a previous version, but cannot reproduce when debugging from master (after adjusting VERSION to attempt to trigger the popup). Also, I cannot reproduce the issue when triggering the update mechanism on regtest (using Ctrl+M).

@ghost
Copy link

ghost commented Apr 17, 2019

I won't be surprised if there is some simple string comparison "0.9.x" < "1.0.y" which is working wrong, due to the used library.
Could the knowledgeable devs please point the concerned code piece ?
(In the mean time, maybe an appli message to the users could help ?)

@izzytdi
Copy link

izzytdi commented Apr 17, 2019

Can confirm on windows 10, also not getting 1.0.0 -> 1.0.1 prompt

@devinbileck
Copy link
Member Author

I spotted this in the log, not sure if it is related yet:
Apr-17 01:14:44.718 [JavaFX Application Thread] WARN b.c.a.AlertManager: verifySignature failed

@ripcurlx
Copy link
Contributor

I spotted this in the log, not sure if it is related yet:
Apr-17 01:14:44.718 [JavaFX Application Thread] WARN b.c.a.AlertManager: verifySignature failed

This would explain why the update window is not shown. Somehow the verification of the signature of the update message fails on Windows.

@ripcurlx
Copy link
Contributor

@oscarguindzberg Can it be that this is related to our bitcoinJ updates in the past where we changed some Windows related code? For signature validation we use classes from the bitcoinJ library and this would explain why it started to occur only recently and only on Windows.

@ripcurlx
Copy link
Contributor

ripcurlx commented Apr 17, 2019

I can reproduce the issue on Windows 10 when running from the binary for a previous version, but cannot reproduce when debugging from master (after adjusting VERSION to attempt to trigger the popup). Also, I cannot reproduce the issue when triggering the update mechanism on regtest (using Ctrl+M).

@devinbileck What are your values for AppOptionKeys.USE_DEV_PRIVILEGE_KEYS and AppOptionKeys.IGNORE_DEV_MSG_KEY? This is relevant for your Regtest message filtering.

@oscarguindzberg
Copy link
Contributor

@ripcurlx We did a change on bitcoinj to fix "restore from seed fails on windows" bug, but in fact, we changed the code for every platform.
On top of that, the change on bitcoinj was totally unrelated to signature verification.

I am happy to help if you narrow down this notification problem to a bitcoinj issue.

@ManfredKarrer
Copy link
Contributor

Yes it cannot be related the BitcoinJ part. Most likely its that users have the USE_DEV_PRIVILEGE_KEYS prog arg set. But then they cannot trade as well as the arbitrators are not valid.

@devinbileck
Copy link
Member Author

What are your values for AppOptionKeys.USE_DEV_PRIVILEGE_KEYS and AppOptionKeys.IGNORE_DEV_MSG_KEY? This is relevant for your Regtest message filtering.

While testing on regtest, USE_DEV_PRIVILEGE_KEYS is true and IGNORE_DEV_MSG_KEY is false.

Most likely its that users have the USE_DEV_PRIVILEGE_KEYS prog arg set. But then they cannot trade as well as the arbitrators are not valid.

While running from the binary on mainnet, USE_DEV_PRIVILEGE_KEYS is false and still encounter this issue (I explicitly passed --useDevPrivilegeKeys=false to ensure).

I agree that it does not appear to be related to BitcoinJ, as running from source I get the update prompt. It appears to be specific to running from the binary.

@ripcurlx has anything changed recently with how you generate the Windows binary?

@ManfredKarrer
Copy link
Contributor

Tested with 0.9.4 binary and there is also AlertManager: verifySignature failed
Same signature validation is used for Filter objects and there is no issue.

@ManfredKarrer
Copy link
Contributor

It is some stupid encoding issue.
Bisq’s causes the problem, I tested on DAO_REGTEST and when i remove the it works. I think its the missing UTF8 argiument in getBytes. I will add a PR. In the meantime better not use any characters in the text which might be non OS standard.

@devinbileck
Copy link
Member Author

devinbileck commented Apr 18, 2019

Yep.
I built my own binary from the v1.0.0 tag and still encountered the issue.
I then added utf-8 charset encoding to the verifySignature method and re-built the binary and it fixed it!
Just not sure why it is only an issue with the binary.

@ManfredKarrer
Copy link
Contributor

The character I thought was not the reason, still have not found which causes it...
The reason why it was only in binary is probably because of jdk differences. @devinbileck used Oracle JDk for running from source and in binary we have openJDK.

@devinbileck
Copy link
Member Author

Correction, the binary uses OracleJDK since OpenJDK does not include the javapackager util. I normally run from source using OpenJDK. However, I just tried running from source with OracleJDK and I did not encounter the issue. So perhaps that is not the cause.

@ManfredKarrer
Copy link
Contributor

Ah ok. Hm would be still good to find why the binary behaves differently. Maybe the way how the standard encoding is applied is different.

@ManfredKarrer
Copy link
Contributor

ManfredKarrer commented Apr 18, 2019

They should have made UTF8 standard from day one... A shame that we still suffer from such old issues....

@devinbileck
Copy link
Member Author

Agreed. I will do a little more testing to try to narrow it down.

ManfredKarrer added a commit to ManfredKarrer/bisq that referenced this issue Apr 18, 2019
@devinbileck
Copy link
Member Author

Looks like you are right about the offending character:
image

@ManfredKarrer
Copy link
Contributor

Ah Windows speak Chinese when it's not asked for ;-)

@ripcurlx
Copy link
Contributor

@ManfredKarrer Great that you've found the issue 👍 The update rate of Windows users is already increasing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants