search

status-im / status-protocol-go

Status Protocol implementation in Go
Mozilla Public License 2.0
0 stars 1 forks source link

Move accounts to status-go-protocol #11

Closed cammellos closed 5 years ago

cammellos commented 5 years ago

As a developer I want to have accounts stored in status-go-protocol So I don't have to re-implement it myself

Description

This task involves replacing the persistence layer from status-react to this library.

It needs to fullfil the current API that realm offers https://github.com/status-im/status-react/blob/develop/src/status_im/data_store/contacts.cljs , or equivalent.

It does not involve porting any logic unless necessary, the focus is on making sure an eventual removal of realm will not break data compatibility on future versions.

Depends on #2

Notes

It can be argued that accounts do not belong here and belongs maybe in status-go, we can have a discussion on where best to place it.

Accounts is different from other data as needs to be stored in a separate database as not user-specific.

Acceptance criteria

dshulyak commented 5 years ago

@cammellos I was going to work on this, but in status-go.

It can be argued that accounts do not belong here and belongs maybe in status-go, we can have a discussion on where best to place it.

so what can be an argument for not keeping them in status-go? as i understand this code is used during login and node startup, to fetch user configuration, and it also requires native bindings, cause node is not yet started when list of accounts need to be made available

cammellos commented 5 years ago

I guess the argument would be that if it s code used by any client, then it would be probably more useful to keep it here, as new clients are not expected to use status-go directly (such as the console client, which will have to re implement the account management logic)

Having said that, given that this has not been integrated yet, the fact that I don't have a strong opinion, I would lean towards having it in status-go for now, it s going to be much faster and we can consider moving it here at a later stage. @adamb what do you think?

StatusWrike commented 5 years ago

➤ Wrike Bot commented:

Dmitry Shulyak All the necessary task dependencies have been completed, so you can start working on it right away.

cammellos commented 5 years ago

Closing as it's not going to be implemented here.