OriginProtocol / origin

Monorepo for our developer tools and decentralized marketplace application
https://www.originprotocol.com/developers
MIT License
652 stars 196 forks source link

A lookup table can be used instead of adding a new column for every service to the identity table #2077

Open shahthepro opened 5 years ago

shahthepro commented 5 years ago

At present the identity table schema contains a column for every supported attestation type (like Google, Facebook, etc..).

This won't scale well in the long run. It'd be better if we can add a lookup table and store the verified identities in that table.

shahthepro commented 5 years ago

@franckc Any thoughts on this?

I'm thinking of adding a verified_accounts table with columns: eth_address(foreign key), provider(enum of all providers) and verified (boolean). And updated_at and created_at if necessary.

If bridge and identity packages share a database in all environments, We can just check if an entry exists in attestation table in bridge package.

franckc commented 5 years ago

What are exactly the "scaling" concerns with having a flat identity table ? Is it DB read/write performance ? Something else ?

A few things:

Overall, I'm not super concerned about having a flat identity table with a dozen or so attestation columns. Adding kakao, wechat, linkedin, Instagram, Youtube should be totally find. Now, if we have good reasons to think we'll need to store lot more than that then would be worth considering doing this data migration.

shahthepro commented 5 years ago

@franckc

What are exactly the "scaling" concerns with having a flat identity table ? Is it DB read/write performance ? Something else ?

AFAIK, Adding more number of columns can affect the read performance, unless we are using a columnar database. Also, if we normalize the table, we may be able to index and space-partition it, so that we can get better read performance.

Also, there is no direct way to delete a specific migration. So if we decide to remove an attestation type in future, we may be running into a few issues.

However, as you said, with the number of the columns we have right now, it may be unnecessary to worry about all these right now. 😄

Now, if we have good reasons to think we'll need to store lot more than that then would be worth considering doing this data migration.

@micahalcorn may be able to tell us if we plan on adding more attestations in future.

franckc commented 5 years ago

Re read performance I think that would be premature optimization, especially given our scale (both in terms of read volume and number of rows) is minuscule :)

Re ability to remove an attestation type in the future, we should be able to write a migration script that just deletes the associated column.