Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Abstract FRAME's Account System #7792

Open
danforbes opened this issue Dec 26, 2020 · 7 comments
Open

Abstract FRAME's Account System #7792

danforbes opened this issue Dec 26, 2020 · 7 comments
Labels
J0-enhancement An additional feature request. J2-unconfirmed Issue might be valid, but it’s not yet known.

Comments

@danforbes
Copy link
Contributor

danforbes commented Dec 26, 2020

I am going to kick things off by mostly paraphrasing/quoting a discussion with @thiolliere & @shawntabrizi related to FRAME's account management capabilities, specifically the management of account data. I am labeling this Issue as unconfirmed for now, because I would like someone like Guillaume and/or Shawn to chime in with more details about the why these capabilities exist and the needs that should be taken into consideration when designing improvements.

I began by asking why it seems that most FRAME runtimes use account data exclusively for the storage of Balances data (e.g. Node Template, Polkadot). Guillaume stated the following, and also mentioned that this is something Shawn had thought about:

Yes you are right, API is not handy to stored multiple pallet information. Though it might be doable by handimplementing some traits.

Shawn followed up with these details:

I think it can be abstracted to be more ergonomic to allow people to define whatever account structure they want
and their own apis around those accounts
to do this, we must extract the assumptions about the account type from frame_system, and instead have it be its own crate/pallet
then you can imagine users could replace the frame_accounts pallet with their own version
which is still compatible with frame_system, and this I think is the solution to full flexibility of accounts in substrate, something I am personally interested in
as is, we kind of force users to fork frame_system to do fancy things, which is obv the exact opposite of what we want
the traits should be super minimal and only what frame system would need to know or modify about an account. for example, a function to get and increment the nonce, to get an increment reference counters, etc
but beyond those fundamental functions and information, a developer could design the account however they want
even could have each piece of data be its own storage map if they wanted to be really inefficient, but again we leave that entirely to the developer

Shawn also provided a snippet of (pseudo)code:

trait FrameAccount<AccountIndex, RefFormat> {

    fn get_nonce() -> AccountIndex;

    fn set_nonce();

    fn get_ref_count() -> RefFormat;

    fn set_ref_count();
}

I am interested in the handling of account data because I believe it is related to providing friendly, flexible support for identity data.

#7363 is an existing PR (Issue #7343) that seems related to this.

@danforbes danforbes added J0-enhancement An additional feature request. J2-unconfirmed Issue might be valid, but it’s not yet known. labels Dec 26, 2020
@burdges
Copy link

burdges commented Dec 27, 2020

I'd think Namecoin-like functionality gives a classical example accounts doing something else. It fits that interface fine I think.

I suppose Namecoin gets complicated since you need BLS signatures from BEEFY along with our fancy validator roll up designs to succinctly prove name ownership, but still..

As a rule, one should avoid personal details on public blockchains, preferably not even behind commitments or hashes. There are mechanisms for producing unique information, like proof-of-personhood parties, or maybe signing a fixed message with the RSA key in an epassport, assuming the epassports use roughly RSA-FDH and not RSA-PSS or similar.

@thiolliere
Copy link
Contributor

TLDR: abstracting system account and allowing to store more information in T::AccountData is 2 different problem to me, though they can be done together for better API maybe.

I agree having frame-system as minimal as possible is generally a plus, and abstracting account system can potentially make sense for some usage. But I don't see why abstracting account system is needed to have more complex AccountData stored alongside FRAME account info (nonce and ref count).

If we extract account info into its own pallet, or use a trait instead, anyway we need to provide new types/functions to be able to handle multiple value stored alongside the account info. (like for now polkadot have balances stored in account info, similar trait as StoredMap will still be needed even if account is in its own pallet).

If we just want to have multiple pallets storing their information alongside FRAME account info (nonce and refcount) then the current abstraction is enough. AccountData is abstract and can hold multiple pallet information already, this is what I meant by "API is not handy to stored multiple pallet information. Though it might be doable by handimplementing some traits.".

That said, I agree it is better if ppl have types provided to implement multiple pallet information in AccountData easily.

A simple first thought I have, is just to have AccountData as a tuple of the wanted types, and then give types which implements StoredMap for each element of the tuple. Instead of having pallet_balances::AccountStore being System, it could be SystemTupleElement1.
The code would look a bit like:

/// A tuple of at least 1 element.
trait TupleWithAtLeast1Element {
	type Element1: Default;
}

impl<A: Default> TupleWithAtLeast1Element for (A,) {
	type Element1 = A;
}
impl<A: Default, B: Default> TupleWithAtLeast1Element for (A, B) {
	type Element1 = A;
}

/// A type which implements StoredMap of the first element of a tuple account data.
// (name is bad though)
struct SystemTupleElement1For<T>(core::marker::PhantomData<T>);

impl<T: Config> StoredMap<T::AccountId, <T::AccountData as TupleWithAtLeast1Element>::Element1>
for SystemTupleElement1For<T>
where T::AccountData: TupleWithAtLeast1Element
{
	fn get(k: &T::AccountId) -> <T::AccountData as TupleWithAtLeast1Element>::Element1 {
		let _tuple = Account::<T>::get(k).data;
		// I guess all previous element of the tuple needs to implement Decode,
		// or otherwise instead of a tuple we can encode to a more complex structure which counts
		// how much byte are used by each element, for example (but we can probably do better):
		// `len_of_element_1 ++ element_1_encoded ++ len_of_element_2 ++ element_2_encoded ...`
		todo!("extract the element 1 from data")
	}

	fn try_mutate_exists<R, E: From<StoredMapError>>(
		_k: &T::AccountId,
		_f: impl FnOnce(&mut Option<<T::AccountData as TupleWithAtLeast1Element>::Element1>) -> Result<R, E>,
	) -> Result<R, E> {
		todo!()
	}
}

// Do a macro to generate those codes for various tuple size

// Then in runtime it would look like this:
impl system::Config for Runtime {
...
type AccountData = (Balance, MyPalletBInfo);
}
impl balances::Config for Runtime {
type AccountStore = SystemTupleElement1;
}

But we can improve stuff differently, like AccountData instead of being the type, it could be a tuple of pallets, and each pallets would provide the additional AccountData by implementing a new trait PalletAccountData.

@shawntabrizi
Copy link
Contributor

@thiolliere my general thought is that the entire API around handling and managing account data right now is defined by frame_system. There are tricks we can do within frame_system to try and make the APIs better and more flexible, and we should do that, but end of the day, users may have their own idea of how they want to handle user storage.

I would want to refactor accounts such that another user could build their own accounts module and replace frame_accounts, and thus build an entire blockchain using their own accounts primitives and APIs.

@kianenigma
Copy link
Contributor

@shawntabrizi putting your last comment next the code snipped that you supposedly sent:

trait FrameAccount<AccountIndex, RefFormat> {
    fn get_nonce() -> AccountIndex;
    fn set_nonce();
    fn get_ref_count() -> RefFormat;
    fn set_ref_count();
}

While I like the notion of a configurable frame_accounts crate that can be replaced with anything else, I don't see how this brings further flexibility than the current AccountData, given that the trait is still enforcing a notion of ref_count and nonce, plus some additional stuff. Current AccountData can also support this by having various combinations of data in its .data field (which goes in the direction of @thiolliere's comment).

So all in all, not having read the offline conversation that happened, by reading this issue I can't figure out if this is about:

  1. a revamp of account structure?
  2. making AccountData.data more flexible to make it easily shared between multiple pallets?

@thiolliere
Copy link
Contributor

At least the current structure constrain you to have everything store in a single storage. From other ppl comments it seems it can be useful to be able to organize in other way.

@shawntabrizi
Copy link
Contributor

Yeah, @thiolliere is exactly right. For example, with things in a single storage, you may run into issues if you are wrapping multiple mutates within one another.

@kianenigma
Copy link
Contributor

I see, thanks for explaining!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J0-enhancement An additional feature request. J2-unconfirmed Issue might be valid, but it’s not yet known.
Projects
None yet
Development

No branches or pull requests

5 participants