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

feat(fc-pallet-pass): handle provisioning of system accounts for session keys #28

Merged
merged 5 commits into from
Nov 22, 2024

Conversation

pandres95
Copy link
Member

@pandres95 pandres95 commented Nov 19, 2024

To reduce the usability friction in pallet-pass we created the concept of Sessions, which are analogous to pure proxy controllers, for which an arbitrary account, represented by a public key (i.e. the session key), previously attached to a pass Account is allowed to execute calls on behalf of such account.

This implies that to correctly pass the transaction validity step before a session key account can dispatch a call on behalf of the Pass Account, the account needs to exist in the system (a.k.a. be provisioned).

This Pull Request resolves this by handling the provider references such account needs in a manner that works both for accounts that already exist in the chain as well as accounts that do not yet exist.

Related to #27: Should we be using signed origins as session keys in the future? Or should we find a way to extend the validity of some pass credentials over time, in such a way we handle these future validations via the pass Account only?

This new extension skips certain checks (like `CheckNonce`) if the caller is a session key.

This extension should be deprecated in the future in favour of a couple of `TransactionExtension`s that: 1) issues an origin for the account, and 2) after checks like Nonce, replaces this custom origin with `frame_system::Origin::signed(account_key)`, where `account_key` is the `AccountId` of the account being authorized by `pallet-pass`.
@pandres95 pandres95 force-pushed the feat/pass-extension-skip-check-if-pass-account branch from 9ad3f1d to d6b77ae Compare November 19, 2024 02:50
@pandres95 pandres95 changed the title feat(fc-pallet-pass): add SkipCheckIfPassAccount feat(fc-pallet-pass): handle provisioning of system accounts for session keys Nov 20, 2024
Copy link
Contributor

@jgutierrezre jgutierrezre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Member

@olanod olanod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use the scheduler to auto remove sessions based on their expiration block and extend the removal schedule when necessary.

Side note based on our talk, in a different PR let's improve our account generation in account_id_for which currently trusts user provided data when it shouldn't. I propose something like

// we know the length of HashedUserId
let mut input = [0u8; TWICE_THE_LEN_OF_HASHED_USER_ID];
input[..HASHED_USER_ID_LEN] = input.copy_from_slice(&user);
let account = hash_input_and_magically_convert_to_account(&input);

pallets/pass/src/lib.rs Outdated Show resolved Hide resolved
pallets/pass/src/lib.rs Outdated Show resolved Hide resolved
…e deleting session provider to be infallible
@pandres95 pandres95 requested a review from olanod November 22, 2024 05:59
Copy link
Member

@olanod olanod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@olanod olanod merged commit ab7ec10 into main Nov 22, 2024
6 checks passed
@olanod olanod deleted the feat/pass-extension-skip-check-if-pass-account branch November 22, 2024 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants