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

The account abstraction pallet #130

Open
wants to merge 26 commits into
base: polkadot-v0.9.39
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
9856d9e
Init pallet-account
akru Feb 7, 2023
8e049e6
Merge remote-tracking branch 'origin/polkadot-v0.9.37' into feature/p…
akru Feb 20, 2023
a909f8e
Added simple salt implementation & mocks
akru Feb 20, 2023
3619ac4
Custom origin driven pallet-account
akru Mar 7, 2023
2cec263
Merge remote-tracking branch 'origin/polkadot-v0.9.37'
akru Mar 28, 2023
d810947
proxy call implementation
akru Mar 28, 2023
6e71c60
Update frame/pallet-account/src/lib.rs
akru Apr 6, 2023
c1a740e
Update frame/pallet-account/src/pallet/mod.rs
akru Apr 6, 2023
6f5aa27
Update frame/pallet-account/src/lib.rs
akru Apr 6, 2023
ee7e727
Update frame/pallet-account/src/lib.rs
akru Apr 6, 2023
0973c5a
Update frame/pallet-account/src/lib.rs
akru Apr 6, 2023
98700fa
Temporary remove meta_call
akru Apr 6, 2023
f8149af
Rewrite origin storage to make it storage info compatible
akru Apr 6, 2023
b0c4bb0
Use workspace authors
akru Apr 7, 2023
11ec601
Added TODO for weights
akru Apr 20, 2023
eb56aa2
Fix weights setting
akru Apr 20, 2023
b494247
Fix tests
akru Apr 20, 2023
0307dcd
Added negative tests
akru Apr 20, 2023
34b2d24
Fix cargo fmt
akru Apr 20, 2023
c829786
Added benchmarking
akru Apr 24, 2023
564a2e9
Fix cargo fmt
akru Apr 24, 2023
83fa23b
Use runtime level compatible hashing
akru Apr 24, 2023
ca3bddc
Merge branch 'polkadot-v0.9.39' into feature/pallet-account
akru Apr 24, 2023
3a1ae8b
Update frame/pallet-account/src/lib.rs
akru May 30, 2023
7aca6a1
Added event checks into tests
akru May 30, 2023
81f533d
Added OnKillAccount handler & creation deposit functionality
akru May 31, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ members = [
"frame/collator-selection",
"frame/custom-signatures",
"frame/dapps-staking",
"frame/pallet-account",
"frame/pallet-xcm",
"frame/pallet-xvm",
"frame/xc-asset-config",
Expand Down
46 changes: 46 additions & 0 deletions frame/pallet-account/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
[package]
name = "pallet-account"
version = "0.1.0"
authors.workspace = true
edition.workspace = true
homepage.workspace = true
repository.workspace = true

[dependencies]
log = { workspace = true }
serde = { workspace = true, optional = true }

# Substrate
parity-scale-codec = { workspace = true }
frame-support = { workspace = true }
frame-system = { workspace = true }
scale-info = { workspace = true }
sp-core = { workspace = true }
sp-runtime = { workspace = true }
sp-std = { workspace = true }

# Benchmarks
frame-benchmarking = { workspace = true, optional = true }

[dev-dependencies]
pallet-balances = { workspace = true, features = ["std"] }
assert_matches = { workspace = true }
hex-literal = { workspace = true }

[features]
default = ["std"]
std = [
"parity-scale-codec/std",
"frame-support/std",
"frame-system/std",
"scale-info/std",
"serde",
"sp-core/std",
"sp-runtime/std",
"sp-std/std",
]

runtime-benchmarks = [
"frame-benchmarking",
]
try-runtime = ["frame-support/try-runtime"]
199 changes: 199 additions & 0 deletions frame/pallet-account/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
// This file is part of Astar.

// Copyright (C) 2019-2023 Stake Technologies Pte.Ltd.
// SPDX-License-Identifier: GPL-3.0-or-later

// Astar is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Astar is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Astar. If not, see <http://www.gnu.org/licenses/>.

//! # Account abstraction pallet
//!
//! ## Overview
//!
//! An accout abstraction pallet makes it possible to derive new blockchain based
//! account for an existing external owned account (seed phrase based). The onchain
//! account could be derived to multiple address spaces: H160 and SS58. For example,
//! it makes possible to predictably interact between substrate native account and
//! EVM smart contracts.
//!
//! ## Interface
//!
//! ### Dispatchable Function
//!
//! * new_origin() - create new origin for account
//! * proxy_call() - make proxy call with derived account as origin
//!

#![cfg_attr(not(feature = "std"), no_std)]

pub mod origins;
pub use origins::*;

pub mod weights;
pub use weights::*;

pub use pallet::*;

#[cfg(test)]
mod mock;
#[cfg(test)]
mod tests;
akru marked this conversation as resolved.
Show resolved Hide resolved

#[frame_support::pallet]
#[allow(clippy::module_inception)]
pub mod pallet {
use crate::*;

use frame_support::pallet_prelude::*;
use frame_support::{
dispatch::{Dispatchable, GetDispatchInfo},
traits::IsSubType,
};
use frame_system::pallet_prelude::*;

/// The current storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);

#[pallet::pallet]
#[pallet::generate_store(pub(super) trait Store)]
#[pallet::storage_version(STORAGE_VERSION)]
pub struct Pallet<T>(PhantomData<T>);

#[pallet::config]
pub trait Config: frame_system::Config {
/// Custom origin type.
type CustomOrigin: Parameter + TryInto<Self::AccountId> + MaxEncodedLen;
/// Parameter that defin different origin options and how to create it.
akru marked this conversation as resolved.
Show resolved Hide resolved
type CustomOriginKind: Parameter + OriginDeriving<Self::AccountId, Self::CustomOrigin>;
/// The runtime origin type.
type RuntimeOrigin: From<Self::CustomOrigin>
+ From<frame_system::RawOrigin<Self::AccountId>>;
/// The overarching call type.
type RuntimeCall: Parameter
+ Dispatchable<RuntimeOrigin = <Self as Config>::RuntimeOrigin>
+ GetDispatchInfo
+ From<frame_system::Call<Self>>
+ IsSubType<Call<Self>>
+ IsType<<Self as frame_system::Config>::RuntimeCall>;
/// General event type.
type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;
}

#[pallet::error]
pub enum Error<T> {
/// Origin with given index not registered.
UnregisteredOrigin,
/// Signature does not match Signer, check nonce, magic and try again.
BadSignature,
}

#[pallet::event]
#[pallet::generate_deposit(pub(crate) fn deposit_event)]
pub enum Event<T: Config> {
NewOrigin {
account: T::AccountId,
origin: T::CustomOrigin,
},
ProxyCall {
origin: T::CustomOrigin,
result: DispatchResult,
},
}

#[pallet::origin]
pub type Origin<T> = <T as Config>::CustomOrigin;

/// Account origins
#[pallet::storage]
pub type AccountOrigin<T: Config> =
StorageDoubleMap<_, Blake2_128Concat, T::AccountId, Twox64Concat, u32, T::CustomOrigin>;

/// Account last origin index
#[pallet::storage]
pub type AccountLastOrigin<T: Config> =
StorageMap<_, Twox64Concat, T::AccountId, u32, ValueQuery>;

#[pallet::call]
impl<T: Config> Pallet<T> {
/// Derive new origin for account.
///
/// The dispatch origin for this call must be _Signed_.
#[pallet::weight(T::WeightInfo::new_origin())]
#[pallet::call_index(0)]
pub fn new_origin(
Copy link
Member

@shaunxw shaunxw May 18, 2023

Choose a reason for hiding this comment

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

As the origin derivation algorithm is deterministic, could we skip the origin registering? The derived origin could be derived just in time in a proxy_call, like:

Account::proxy_call(alice, derive_kind, index, call);

If we do need to cache the derived origin to reduce gas costs, the cache should be cleaned on kill account.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe derivation algorithm is just a sample, it not restricted to be deterministic. Account registration helps to prevent spamming and account address manipulation.

Copy link
Member

Choose a reason for hiding this comment

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

@akru what is the usecase behind allowing single account to have multiple derived address of the same type?

The way I see it, it just adds unnecessary complexity.
If we only allow one account type to be derived from the origin, then it's much simpler.
If an account wants to control multiple different derived accounts, it can use proxy to achieve that.

Copy link
Member

Choose a reason for hiding this comment

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

Shaun also commented above(here) about what happens when account is killed. Without any locked or reserved funds, these storage items will remain in the DB even without an owner.

origin: OriginFor<T>,
origin_kind: T::CustomOriginKind,
) -> DispatchResult {
let who = ensure_signed(origin)?;

let next_index = AccountLastOrigin::<T>::get(&who);
let new_origin = origin_kind.derive(&who, next_index);
AccountOrigin::<T>::insert(&who, next_index, new_origin.clone());
AccountLastOrigin::<T>::insert(&who, next_index + 1);
Comment on lines +170 to +171
Copy link
Member

Choose a reason for hiding this comment

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

This is still free - so my comment from 2 months ago is still valid.
You should charge rent fee for these two items since you're inflating the database.

Check how pallet-assets is doing it, or any other pallet in substrate that creates some storage items (e.g. pallet-identity, pallet-multisig, etc.).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically, you pay transaction fee to cover it (prevent spam), it’s not required at all, just one of ways to do it. I’m not sure it’s required for the prototype.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My personal opinion - better charge fee than just lock token forever. Charged fee much more controllable, it could be burn, redistributed or whatever you want. But locked deposit is just locked.

Copy link
Member

Choose a reason for hiding this comment

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

The way it's implemented now - sure, that's viable option. It still needs to be added though since it's missing now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure something is missing now, what precisely you are talking about?

Copy link
Member

Choose a reason for hiding this comment

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

I'm talking about payment for the created storage.

Technically, you pay transaction fee to cover it (prevent spam), it’s not required at all, just one of ways to do it. I’m not sure it’s required for the prototype.

Given that our handling of fees is copy/paste of Polkadot, it is how we do things, isn't it?
Don't understand why this pallet should be any different.
Are you proposing we mix it up, some pallets charging for rent fee, while some don't? 🤷‍♂️

It's been on review for months, and better to do it right immediately then have someone else fix it later, add storage migration logic, etc. Doesn't matter if it's prototype or not, IMO.


Self::deposit_event(Event::NewOrigin {
account: who,
origin: new_origin,
});

Ok(())
}

/// Dispatch the given `call` from an account that the sender is authorised.
///
/// The dispatch origin for this call must be _Signed_.
///
/// Parameters:
/// - `origin_index`: Account origin index for using.
/// - `call`: The call to be made by the `derived` account.
#[pallet::weight({
let di = call.get_dispatch_info();
(T::WeightInfo::proxy_call()
.saturating_add(T::DbWeight::get().reads_writes(1, 1))
.saturating_add(di.weight),
di.class)
})]
#[pallet::call_index(1)]
pub fn proxy_call(
origin: OriginFor<T>,
#[pallet::compact] origin_index: u32,
call: Box<<T as Config>::RuntimeCall>,
) -> DispatchResult {
let who = ensure_signed(origin)?;
ensure!(
origin_index < AccountLastOrigin::<T>::get(&who),
Error::<T>::UnregisteredOrigin
);

let custom_origin = AccountOrigin::<T>::get(&who, origin_index)
.ok_or(Error::<T>::UnregisteredOrigin)?;

let e = if let Ok(id) = custom_origin.clone().try_into() {
// in case of native dispatch with system signed origin
call.dispatch(frame_system::RawOrigin::Signed(id).into())
} else {
// in other case dispatch with custom origin
call.dispatch(custom_origin.clone().into())
};

Self::deposit_event(Event::ProxyCall {
origin: custom_origin,
result: e.map(|_| ()).map_err(|e| e.error),
});

Ok(())
}
}
}
Loading