symbol / sdk-typescript

This SDK has been deprecated as of January 2022.
2 stars 1 forks source link

Symbol Message Api? #40

Open fboucquez opened 3 years ago

fboucquez commented 3 years ago

Currently, the old SDKs provide a Message Api to handle different types of messages, raw, plain text, empty, encrypted, and delegate. The first byte will define the type of message and how to read it (or decrypt it). Most apps have used those to create the byte array messages that go in the transfer transactions.

I would suggest that a Message Api/Helper should be provided by the SDKs. Just a simple class that can convert from a message to a byte array depending on the desired message type. I wouldn't use the current Message hierarchy from the old SDKs that I think are a bit too complicated.

I believe that if we leave this out from the SDKs, and apps create their one message formats, apps won't be able to read each other messages correctly. Note that we are still going to support a raw message / byte array for non-text related attachments or custom formats. But in general, apps could use the API for regular messages.

All SDKs should behave similarly when creating and processing messages, I would create vector tests to validate them.

Using https://symbolblog.com/article/aggregate-transactions-in-python-part-2/ as an example, you can see the:

aliceTx = facade.transaction_factory.create_embedded({
    'type': 'transfer',
    'signer_public_key': alicePubkey,
    'recipient_address': bobAddress,
    'mosaics': [(mosaic_id, 2000000)],
    'message': bytes(1) + msgB.encode('utf8') <!--- user needs to remember to use a compatible format
})

I would add a message API that hides the implementation the bytes(1) + msgB.encode('utf8'),

Like:

aliceTx = facade.transaction_factory.create_embedded({
    'type': 'transfer',
    'signer_public_key': alicePubkey,
    'recipient_address': bobAddress,
    'mosaics': [(mosaic_id, 2000000)],
    'message':  messageFacade.plain_text(msgB)
})

Examples:

Naming can be different, but the idea is the same, provide a helper/facade/utility class.

segfaultxavi commented 3 years ago

Just a comment, the current state of things, where you never know if the first byte is part of a raw message or is a "message type" indicator from the SDK is a horrible mistake we should not repeat. Just something to keep in mind :)

fboucquez commented 3 years ago

I do agree is pretty ugly, but we have messages in mainnet with that pattern. Aslo we would need to solve encrypted and delegate messages. This issue is to discuss what to do, keep the existing format, new format or no format at all leaving the responsibility to every app. No silver bullet...

gimre-xymcity commented 3 years ago

Just a comment, the current state of things, where you never know if the first byte is part of a raw message or is a "message type" indicator from the SDK is a horrible mistake we should not repeat. Just something to keep in mind :)

This is not a mistake, this was deliberate decision. In nis1, the byte was meaningful on protocol level.

In catapult-client it is not - the client does not care at all what is inside the message, the message format is an application level thing.

That being said, we should have some NIP or NRC that specifies what markers are used and how they are interpreted. (similar to SLIP-044)

fboucquez commented 2 years ago

This issue also applies to metadata values. How can we provide an API so apps store and loads data using know formats and encoding (plain text, encrypted data, raw/binary data etc).