-
Notifications
You must be signed in to change notification settings - Fork 19
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
OmniAccount derivation #3126
OmniAccount derivation #3126
Conversation
who: Identity, | ||
member_account: MemberAccount, | ||
maybe_account_store_hash: Option<H256>, | ||
who: Identity, // `Identity` used to derive OmniAccount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that we keep a similar concept of PrimeIdentity
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question - on the contrary, actuall y I don't want to introduce the concept of prime identity.
who
should have been of T::AccountId
type (which is the type of OmniAccount), but we need to create the AccountStore if not present. For that we do need an Identity
type.
I feel this is not very consistent, any better ideas? Or we introduce an explicit create_account_store
extrinsic, which is called when the user logs in for the first time? I'm just a bit concerned by the UX (user needs ti sign the create_account_store
request)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should have an extrinsic to create the omni account, which internally creates the account store, adding the first account. It would make it easier to understand for the users.
It made sense for the "link_identity" concept not to have a "create_id_graph", as users would just link another identity(s) to it's main (prime) identity, but for "add_account", we need to answer the question "add account to what?", so it feels natural to first create the omni account. Similar to a Multisig, you first create the Multisig, then you can add/remove signatories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic makes sense.
I just want minimal steps from UX perspective. Does create_account_store
require user signature, or do we create it for users on the background as soon as user logs into our webapp? If the latter, is it then a public call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also cc @jonalvarezz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the worst case scenario for new users, the UX for calling create_account_store
, should be the same as calling add_account
for the first time. I think we should require the signature, but this could be part of the "login" mechanism
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of having matching api signatures as Francisco suggests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add an create_account_store
interface, however, it should only be called when we really need to create a store (e.g. when adding accounts), not upon every login.
When to call which will be worker's logic then
member_account_hash: H256, | ||
public_identity: MemberIdentity, | ||
) -> DispatchResult { | ||
pub fn publicize_account(origin: OriginFor<T>, member_account: Identity) -> DispatchResult { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hash()
in MemberAccount
is quite useful 👍🏼
Context
As topic - it uses consistent method to derive an
OmniAccount
from a givenIdentity
, so that the derived OmniAccount has no private key even for substrate/evm users.To make the difference, the mapping from substrate/evm accounts is called
to_native_account()
.It also simplifies some extrinsic parameter: e.g.
add_acount
doesn't require AccountStoreHash anymore.