Add support for multiple accounts on the same connection

[adriacidre]

This is basically adding a new layer between connection and channel, so we can support multiple accounts over the same connection like:

sdk := sdk.New(rpc)

john := sdk.SignupAndLogin("randomPwd")
johnsChannel, _ := john.JoinPublicChannel("randomchannel")
johnsChannel.Subscribe(func(msg *sdk.Msg){
  println(msg.Text)
})

louis := sdk.SignupAndLogin("randomPwd")
louisChannel, _ := john.JoinPublicChannel("randomchannel")
louisChannel.Publish("Hello john!")

[gravityblast]

nice feature @adriacidre !

I noticed that when we call status_login, in status-go we inject the current account keys in whisper, so I'm not sure if it can be used by multiple accounts.

For sure it works because when we use shh_post we need to specify the keys to use for the message so the message sending works, but it might be a security issue?

[adriacidre]

I see your point, so, every time you log in, you change your SelectedAccount.

On the other hand SelectedAccount is not used on any ipc call from the SDK, and same security risks could be achieved with the current approach with:

john := sdk.SignupAndLogin("randomPwd")
ch, _ := john.JoinPublicChannel("randomchannel")
ch.Subscribe(func(msg *sdk.Msg){
  println(msg.Text)
})

louis := sdk.SignupAndLogin("randomPwd")
ch, _ := louis.JoinPublicChannel("randomchannel")
ch.Publish("Hello john!")

I would say this is a concern to be tracked on the status-go side (as is a current existing problem).

[gravityblast]

@adriacidre we actually call SelectAccount in the status service, and it's called from the sdk:

https://github.com/status-im/status-go/blob/develop/services/status/api.go#L40

I don't think it's a current existing problem because the node is used by just one user at a time, so maybe we should do the same in the sdk

[adriacidre]

Yes, what i meant is this problem is already happening with the currently implemented SDK approach.

Even if we want to avoid multiple account management or add support for it, its something that has to happen on status-go side, and not in here.

[gravityblast]

Yeah I know, you are right. That's why maybe it's better not to allow users to have multiple accounts in the sdk since it won't be implemented in status-go

[adriacidre]

@pilu It's actually something required by Push Notifications V2

[gravityblast]

@adriacidre I understand, shall we then change the status service not to inject the whisper identity when used as an external API?

[adriacidre]

A proper research / development should be done on the push notifications side, i'll expose the problem to @pombeirp on our weekly call today :)

[gravityblast]

@adriacidre nice! I'm just worried about security, but the code here makes sense!

[adriacidre]

Sure, i'll merge it if you don't mind. As this really improves code readability.

BTW I'll keep you informed about this as soon as i have some more info :)

[gravityblast]

👍