Concordium / concordium-reference-wallet-android

Reference Android wallet for the Concordium blockchain
Apache License 2.0
12 stars 2 forks source link

Information in memo-raw files is shown as hex value #33

Closed concordium-cl closed 3 years ago

concordium-cl commented 3 years ago

Bug Description For a memo.txt with content "this is a memo text in a file", the wallet prints the hex value of the memo.

IMG-4027 (1)

Steps to Reproduce -do a scheduled transfer with --memo-raw memo.txt

Expected Result The text should be printed as in the text file.

Actual Result See above.

Versions

abizjak commented 3 years ago

That is not correct, or at least is not how it is designed.

The --memo-raw with concordium-client means it will include the contents of the file literally. The tools are meant to expect valid CBOR data, which that file is not. So the wallet fails to decode it, and falls back to the default behaviour, which is showing the raw data encoded in hex (which is in general the best it can do).

So the mobile wallet is behaving as intended here and you are breaking the system assumptions by using --memo-raw with invalid data. Part of the design was of the memos was that the tools will ensure the correct encoding, but the chain will not check it. We added memo-raw to the concordium-client, which is meant for advanced uses where the file is produced in some other way.

Users should never use --memo-raw unless they really understand what they are doing. They should either use --memo (in most cases) or --memo-json.

So if anything, your objection is to the design, and this is not a bug in the app.

concordium-cl commented 3 years ago

Okay, then I've misunderstood. I thought the content is meant to be included as is, also if it's not valid CBOR.

concordium-cl commented 3 years ago

Hex value of memo in case of invalid CBOR.

abizjak commented 3 years ago

Okay, then I've misunderstood. I thought the content is meant to be included as is, also if it's not valid CBOR.

I think you understood correctly, the contents of the file is included as is in the memo.

But the key thing is that this content is not valid CBOR and we have agreed that tools will expect valid CBOR, and fall back to displaying the raw data*.

*Displaying the raw data is a bit tricky. In your case we could display the actual string that you put in the file, since the file has only printable ASCII characters. But in general the memo will contain arbitrary bytes, some of which are nice printable characters, and some of which are not. To always be able to display that the contents is display in hex instead.

Does that make more sense?