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

Remove the automatic registration of emoji reactions #145

Closed
RiccardoM opened this issue Apr 27, 2020 · 4 comments · Fixed by #150
Closed

Remove the automatic registration of emoji reactions #145

RiccardoM opened this issue Apr 27, 2020 · 4 comments · Fixed by #150
Assignees
Labels
kind/enhancement Enhance an already existing feature; no "New feature" to add
Milestone

Comments

@RiccardoM
Copy link
Contributor

Context

Currently, when an emoji is used a reaction to a post, a new reaction is registered inside the state to make sure that when it's stored, no errors are returned. While this methods works great and ensures that each reaction is registered before being added to a post, it also has one big downside: duplication.

When we register reactions, each one of them is associated to a subspace which identifies a client application. This means that if we consider having 10 different client applications, and inside each one of them the ":smile:" reaction is used, we will have 10 different registered reaction with the same shortcode and the same value, but a different subspace each time. Also, if there's one emoji which has multiple shortcodes, this will be registered multiple times for the same client.

While this might seem like a small thing, considering that there are currently more than 3,000 emojis this lead in the future to an unnecessary increased size in the store.

Solution

A possible solution to this problem is changing how reactions are currently handled inside Desmos. This could be done as follows.

Once a user adds a reaction to a post, the following operations will take place:

  1. If the reaction value is a shortcode but does not represent an emoji, check if the shortcode is registered.
    If not throw an error.
    Otherwise, store inside the state the shortcode associated to the reaction.
  2. If the reaction is an emoji, get the shortcode associated to it (if there are multiple, pick the first) and store inside the state the associated shortcode.
  3. In both cases, emit both the reaction value and shortcode.

Once a user removes a reaction to a post, the following operations will take place:

  1. If the reaction value is a shortcode:
    1. If the shortcode represents a registered reaction, search a reaction with the same code.
    2. If the shortcode represents an emoji, get the emoji with that shortocde and search all the reactions with either one of the associated shortcodes.
  2. If the reaction value is an emoji, get the emoji with that shortcode and remove all the reactions having either one of the shortcodes as value.

This way we will be able to have a store inside which the reactions are all represented by their shortcodes which will allow us in a later time to get both the value and shortocode when returning them to the clients that asks them.

Notes

Note that emoji-based reactions are not checked for registration, thus reducing the size of the used disk space as well as making the whole process a lot more faster.

Conclusion

I think this system will perform better in a long run, but I want to hear if you have suggestions about it @kwunyeung @bragaz

@RiccardoM RiccardoM added the kind/enhancement Enhance an already existing feature; no "New feature" to add label Apr 27, 2020
@leobragaz
Copy link
Contributor

So if I understood this well, only shortcodes will be registered, emoji reaction will not right?
So basically all the emojis could be used without being registered to the store and the store will have inside it only custom reactions from users, correct?

@RiccardoM
Copy link
Contributor Author

So if I understood this well, only shortcodes will be registered, emoji reaction will not right?
So basically all the emojis could be used without being registered to the store and the store will have inside it only custom reactions from users, correct?

Exactly

@leobragaz leobragaz self-assigned this Apr 30, 2020
@kwunyeung
Copy link
Contributor

kwunyeung commented May 1, 2020

How about we have a global predefined list of registered emoji? e.g., register a list of Github supported emoji shortcodes by default. It's similar to how the Cosmos SDK handle errors.

@RiccardoM
Copy link
Contributor Author

@kwunyeung I've thought about this as well, but there are two issues with this approach:

  1. The registration of emojis should be done for all subspace, which we do not know in advance.
    If we register them only for a single subspace, in all the other ones will not work. If we register them globally we go against the design of per-subspace reactions that has been discussed inside Post reaction registration and limiting #94.
  2. Each time a unicode update is released, we should stop the chain, create the list of emojis and start the chain again with the updated list. This will require a lot of maintainance and cause a waste of time, which is not desirable.

If we just check the code that the user sends against the current list of known emoji shortcodes, it will make sure that future updates do not require a chain upgrade (the validator nodes can just do a software upgrade and everything will be fine) and also we do not pollute the state with useless lists that are already present somewhere else (i.e. inside the emoji library).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Enhance an already existing feature; no "New feature" to add
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants