BirthdayResearch / defichain-app

DeFi Blockchain desktop app for Windows, Linux and Mac.
https://defichain.com
MIT License
156 stars 57 forks source link

Sign a message is not working when it includes empty spaces #763

Open AltCoiFish opened 3 years ago

AltCoiFish commented 3 years ago

What happened:

Using the version v.2.3.3 and the CLI within the DFI-Wallet:

signmessage "8xxxxx" "dfip-1 no"

its not working and it falls back to the man help page of signmessage.

but

when I try signmessage "8xxxxx" "dfip-1-no"

its working fine

same problem also for verifying any messages , if it includes a space its not working. I also testes single-quotes 'it's also not working.

Based on some users on the MN Telegram group, the signing is working when using the dfi-cli from ain

defichain-bot commented 3 years ago

@AltCoiFish: Thanks for opening an issue, it is currently awaiting triage.

The triage/accepted label can be added by foundation members by writing /triage accepted in a comment.

In the meantime, you can:

  1. Checkout DeFiChain’s Github issue page to see if your issue has already been reported
  2. Submit any logs if you have them, this will greatly expedite the process for us.
  3. You can also join our Telegram or Reddit community channels.
Details I am a bot created to help the [DeFiCh](https://github.com/DeFiCh) developers manage community feedback and contributions. You can check out my [manifest file](https://github.com/DeFiCh/app/blob/master/.github/governance.yml) to understand my behavior and what I can do. If you want to use this for your project, you can check out the [DeFiCh/oss-governance-bot](https://github.com/DeFiCh/oss-governance-bot) repository.
izzycsy commented 3 years ago

Hi @AltCoiFish , you can't sign using the app, signing is only supported through the defi cli.

AltCoiFish commented 3 years ago

Hi @AltCoiFish , you can't sign using the app, signing is only supported through the defi cli.

Not sure if this is what it should be, as the full command --help suite is shown on the Wallet CLI and it might confuse users what they can and can not use.

We and I am / are a bit confused, as we all store our collateral in the wallet and not the ain therefore the basic features should be implemented in the wallet or disable the CLI completely on the wallet

Also for inexperienced users, running an additional CLI tool does not really help getting people into running master nodes and therefore make DFI more decentralized.

Can you please explain, why the signing was and will not be implemented in the current wallet GUI CLI ?

I know you still struggling with all the changes in the new protocol and also I do understand that 98% of all master nodes (link will open telegram message) are not run by users like us, therefore we can not build any priory towards these features to be implemented.

To give you an idea of master node distribution: (since a few weeks ago)

image

Signing messages is an essential and useful part of the decentralized way of interacting and building trust without trusting a central authorities and should be implemented in any client.

izzycsy commented 3 years ago

Hi @AltCoiFish , yes I understand that there are non-technical users and we're trying to improve the UI/UX of the app. On the other hand, there are technical users such as yourself, we're trying our best to find the sweet spot. If you don't mind, you can copy & paste your enquiry and publish it on DeFiChain Discussions, I'll inform the team to answer your queries when they're available. Kindly understand that GitHub Issues are for bug related issues, and will become stale over time. Thank you for your understanding.

AltCoiFish commented 3 years ago

Hi @AltCoiFish , yes I understand that there are non-technical users and we're trying to improve the UI/UX of the app. On the other hand, there are technical users such as yourself, we're trying our best to find the sweet spot. If you don't mind, you can copy & paste your enquiry and publish it on DeFiChain Discussions, I'll inform the team to answer your queries when they're available. Kindly understand that GitHub Issues are for bug related issues, and will become stale over time. Thank you for your understanding.

Thank you , I did open a discussion here https://github.com/DeFiCh/app/discussions/776

Kindly understand that GitHub Issues are for bug related issues, and will become stale over time. Thank you for your understanding.

Signing was partially possible using the Wallet , but not messages which contains spaces this is why I considered it a bug. Perhaps for the future, please clarify what you consider a bug?

izzycsy commented 3 years ago

Hi @AltCoiFish , the team has been notified on this issue, please give us time to investigate. Thank you.

izzycsy commented 3 years ago

Hi @AltCoiFish v2.4.0 is out, this is a mandatory update. Please read through the release notes before updating. This comment is to notify you about the update.

AltCoiFish commented 3 years ago

Hi @AltCoiFish v2.4.0 is out, this is a mandatory update. Please read through the release notes before updating. This comment is to notify you about the update.

thank you for the update, but can you also update on the signing problem, does https://github.com/DeFiCh/app/releases/tag/v2.4.0`solve this problem describe in this issue?

Stonygan commented 3 years ago

The Problem ist not solved, signing is only possible without spaces in version 2.4.0. Therefore, it is mandatory that the next "Votingtemplate" is without spaces. @uzyn can you please take note of that.

izzycsy commented 3 years ago

Hi @AltCoiFish @Stonygan , v2.4.2 is out, this is a mandatory update. Please read through the release notes before updating. This comment is to notify you about the update.

AltCoiFish commented 3 years ago

Hi @AltCoiFish @Stonygan , v2.4.2 is out, this is a mandatory update. Please read through the release notes before updating. This comment is to notify you about the update.

thank you for the update, but can you also update on the signing problem, does https://github.com/DeFiCh/app/releases/tag/v2.4.2`solve this problem described in this issue?

uzyn commented 3 years ago

The new DFIPs no longer use space. So you should be able to sign them easily on the app itself.

See: https://github.com/DeFiCh/dfips/issues/31#issuecomment-853593413

DFIP this round (June 2021) uses the following format:

Voting

Refer to announcement for June voting dates, and to README for voting instructions.

Vote options

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

AltCoiFish commented 3 years ago

Not sure if we should close this issue, as the signing only works without spaces. Currently signing is only working for phrases like I-am-signing-this-at-03-07-2021.

izzycsy commented 3 years ago

Hi @AltCoiFish @Stonygan , update v2.6.0 is out, this is a mandatory update. 
Please read through the release notes before updating. This comment is to notify you about the update.

izzycsy commented 3 years ago

Hi @AltCoiFish , I think your feedback has been answered, I'll proceed to close this issue. Thank you for your patience!

AltCoiFish commented 3 years ago

The new DFIPs no longer use space. So you should be able to sign them easily on the app itself.

See: DeFiCh/dfips#31 (comment)

DFIP this round (June 2021) uses the following format:

Voting

Refer to announcement for June voting dates, and to README for voting instructions.

Vote options

* Yes, I agree. Sign: `dfip-2106-XX-yes`

* No, I do not agree. Sign: `dfip-2106-XX-no`

* Neutral. Sign: `dfip-2106-XX-neutral`.

I did see the changes on signing messages for voting. We are not addressing this issue anymore, for any other purposes ?

From the top of my mind I would come up with a few other examples, why we could need spaces in a signed message. Not sure how everyone else feels about the lack of spaces in signed messages?

izzycsy commented 3 years ago

Hi @AltCoiFish , update v2.6.1 is out. 
Please read through the release notes before updating. This comment is to notify you about the update.

izzycsy commented 3 years ago

Hi @AltCoiFish , update v2.6.2 is out. 
Please read through the release notes before updating. This comment is to notify you about the update.

izzycsy commented 3 years ago

Hi @AltCoiFish , update v2.6.3 is out. 
Please read through the release notes before updating. This comment is to notify you about the update.

izzycsy commented 2 years ago

Hi @AltCoiFish , update v2.7.0 is out, this is a mandatory update. Kindly backup wallet before updating. 
Please read through the release notes before updating. This comment is to notify you about the update.

izzycsy commented 2 years ago

Hi @AltCoiFish , update v2.7.1 is out, this is a mandatory update. Kindly backup wallet before updating.
 Please read through the release notes before updating. This comment is to notify you about the update.

AltCoiFish commented 2 years ago

Issue is still open, just a to not forget about it.

AltCoiFish commented 2 years ago

Any progress here ?

John-Gee commented 2 years ago

I just tried it still fails, I'm guessing the wrapper does not propagate the quotes, but not even \" work.

AltCoiFish commented 1 year ago

Trying to keep this issue open until solved. Nearly one year and no updates here