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

Unify models with aleph-message #2

Open
hoh opened this issue Oct 28, 2021 · 7 comments
Open

Unify models with aleph-message #2

hoh opened this issue Oct 28, 2021 · 7 comments

Comments

@hoh
Copy link
Member

hoh commented Oct 28, 2021

Context

The repository aleph-message contains a definition of the Aleph messages defined in Python/pydantic. These definitions are very similar to those in ./src/messages/messages.ts.

Problem

Message model definitions differ between messages.js and aleph-message. It would be useful for users to find the same model definitions in both languages.

When a model definition is modified, it has to be modified in both repositories.

Solutions

The main approach that comes to my mind would be to move the TS model definitions in aleph-message and make aleph-sdk-ts depend on it.
Automated tests in aleph-message could then check that model definitions in all languages match each other.

@Rgascoin
Copy link
Member

Rgascoin commented Nov 4, 2021

Hello,

I agree that having a single model definition easily synchronizable between different languages will be better.

Moving the TS model definitions in aleph-message seems fine to me.

To get message.ts from aleph-message, I am thinking about using the git submodule feature.
Do you know a better way to achieve it, or does it seems fine to you?

@hoh
Copy link
Member Author

hoh commented Nov 4, 2021

I was thinking about moving message.ts in this repo and npm install aleph-message from the other repository.

@Rgascoin
Copy link
Member

Rgascoin commented Nov 5, 2021

I am ok with this, do you want me to first update message.ts to match your model definition?

@hoh
Copy link
Member Author

hoh commented Nov 9, 2021

Yes please 👍

@Rgascoin
Copy link
Member

Rgascoin commented Nov 17, 2021

After tests, I ended up with this first version,

Some definition differs:

  • Chain: I only add currently supported Chain
  • MessageType: same as Chain
  • BaseMessage: got the same fields but not with the same Optional params to avoid too many changes in the structure.
  • MessagesResponse: As the API doesn't send the same responses type for post/aggregate/store, I don't yet find a way to end up with a single definition. Don't a [messageType]Response with their definition will be better?

Open to your feedback!

File Content:
/**
 * Chain defines which account was used to publish a message.
 * It is automatically provided when publishing messages.
 */

export enum Chain {
    DOT = "DOT",
    ETH = "ETH",
    SOL = "SOL",
}

/**
 * Supported hash functions
 */
export enum HashType {
    sha256 = "sha256",
}

/**
 * Message types supported by Aleph
 */
export enum MessageType {
    post = "POST",
    aggregate = "AGGREGATE",
    store = "STORE",
}

export enum ItemType {
    inline = "inline",
    storage = "storage",
    ipfs = "ipfs",
}

type MongoDBID = {
    $oid: string;
};

/**
 * Some POST messages have a 'ref' field referencing other content
 */
export type ChainRef = {
    chain: string;
    channel?: string;
    item_content: string;
    item_hash: string;
    item_type: string;
    sender: string;
    signature: string;
    time: number;
    type: MessageType.post;
};

type MessageConfirmationHash = {
    binary: string;
    type: string;
};

/**
 * Format of the result when a message has been confirmed on a blockchain
 */
type MessageConfirmation = {
    chain: string;
    height: number;
    hash: string | MessageConfirmationHash;
};

export type BaseContent = {
    address: string;
    time: number;
};

export type AggregateContentKey = {
    name: string;
};

export type AggregateContent<T> = BaseContent & {
    key: string | AggregateContentKey;
    content: T;
};
export type PostContent<T> = BaseContent & {
    content?: T;
    type: string;
    ref?: string | ChainRef;
};

export type StoreContent = BaseContent & {
    item_type: string;
    item_hash?: string;
    size?: number;
    content_type?: string;
    ref?: string;
};

export type BaseMessage = {
    _id?: MongoDBID;
    chain: Chain;
    sender: string;
    type: MessageType;
    channel: string;
    confirmations?: MessageConfirmation[];
    confirmed: boolean;
    signature: string;
    size: number;
    time: number;
    item_type: ItemType;
    item_content: string;
    hash_type?: string;
    item_hash: string;
    content?: BaseContent;
};

export type AggregateMessage<T> = BaseMessage & {
    content: AggregateContent<T>;
    type: MessageType.aggregate;
};

export type PostMessage<T> = BaseMessage & {
    content: PostContent<T>;
    type: MessageType.post;
};

export type StoreMessage = BaseMessage & {
    content: StoreContent;
    type: MessageType.store;
};

@hoh
Copy link
Member Author

hoh commented Nov 19, 2021

Thanks !

Chain: I only add currently supported Chain

I think some historical messages used other chains such as NULS2. All historical messages should either be supported by our models or be specifically listed as unsupported.

This script can download many or all the messages from Aleph, and then this test can check if they all match the specification.

MessageType: same as Chain

PROGRAM and FORGET are two new message types I created, and must be supported by all our tools.

BaseMessage: got the same fields but not with the same Optional params to avoid too many changes in the structure.

I believe many fields were mentioned as Optional in order to support historical messages. In some cases, the model for new messages should be more strict than the model for past messages.

MessagesResponse: As the API doesn't send the same responses type for post/aggregate/store, I don't yet find a way to end up with a single definition. Don't a [messageType]Response with their definition will be better?

This model was designed quickly with /api/v0/messages.json in mind. Proposals to improve this are welcome.

@BjrInt
Copy link
Member

BjrInt commented Sep 8, 2022

Possible solution : aleph-im/aleph-message#13

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

No branches or pull requests

3 participants