Refactor as_sub to make things clearer. (#6503)

* Refactor as_sub to make things clearer.

- `as_sub` becomes `as_alternative`
- `as_sub_limited` becomes `as_derivative`
- `as_alternative` and `as_derivative` generate a mutually exclusive set of accounts.

* Test fix

* Add test

* Fix test

* Remove `as_alternative`.

* Docs.
This commit is contained in:
Gavin Wood
2020-06-26 11:03:02 +02:00
committed by GitHub
parent 39c3d1c026
commit 0532db6b86
4 changed files with 27 additions and 59 deletions
+3 -11
View File
@@ -201,19 +201,11 @@ fn filtering_works() {
assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone()));
expect_event(RawEvent::ProxyExecuted(Err(DispatchError::BadOrigin)));
let sub_id = Utility::sub_account_id(1, 0);
Balances::mutate_account(&sub_id, |a| a.free = 1000);
let derivative_id = Utility::derivative_account_id(1, 0);
Balances::mutate_account(&derivative_id, |a| a.free = 1000);
let inner = Box::new(Call::Balances(BalancesCall::transfer(6, 1)));
let call = Box::new(Call::Utility(UtilityCall::as_sub(0, inner.clone())));
assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()));
expect_event(RawEvent::ProxyExecuted(Ok(())));
assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()));
expect_event(RawEvent::ProxyExecuted(Err(DispatchError::BadOrigin)));
assert_ok!(Proxy::proxy(Origin::signed(4), 1, None, call.clone()));
expect_event(RawEvent::ProxyExecuted(Ok(())));
let call = Box::new(Call::Utility(UtilityCall::as_limited_sub(0, inner.clone())));
let call = Box::new(Call::Utility(UtilityCall::as_derivative(0, inner.clone())));
assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()));
expect_event(RawEvent::ProxyExecuted(Ok(())));
assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()));
+2 -9
View File
@@ -38,13 +38,7 @@ benchmarks! {
let caller = account("caller", 0, SEED);
}: _(RawOrigin::Signed(caller), calls)
as_sub {
let u in 0 .. 1000;
let caller = account("caller", u, SEED);
let call = Box::new(frame_system::Call::remark(vec![]).into());
}: _(RawOrigin::Signed(caller), u as u16, call)
as_limited_sub {
as_derivative {
let u in 0 .. 1000;
let caller = account("caller", u, SEED);
let call = Box::new(frame_system::Call::remark(vec![]).into());
@@ -61,8 +55,7 @@ mod tests {
fn test_benchmarks() {
new_test_ext().execute_with(|| {
assert_ok!(test_benchmark_batch::<Test>());
assert_ok!(test_benchmark_as_sub::<Test>());
assert_ok!(test_benchmark_as_limited_sub::<Test>());
assert_ok!(test_benchmark_as_derivative::<Test>());
});
}
}
+16 -33
View File
@@ -16,7 +16,7 @@
// limitations under the License.
//! # Utility Module
//! A stateless module with helpers for dispatch management.
//! A stateless module with helpers for dispatch management which does no re-authentication.
//!
//! - [`utility::Trait`](./trait.Trait.html)
//! - [`Call`](./enum.Call.html)
@@ -29,10 +29,15 @@
//! corresponding `set_storage`s, for efficient multiple payouts with just a single signature
//! verify, or in combination with one of the other two dispatch functionality.
//! - Pseudonymal dispatch: A stateless operation, allowing a signed origin to execute a call from
//! an alternative signed origin. Each account has 2**16 possible "pseudonyms" (alternative
//! an alternative signed origin. Each account has 2 * 2**16 possible "pseudonyms" (alternative
//! account IDs) and these can be stacked. This can be useful as a key management tool, where you
//! need multiple distinct accounts (e.g. as controllers for many staking accounts), but where
//! it's perfectly fine to have each of them controlled by the same underlying keypair.
//! Derivative accounts are, for the purposes of proxy filtering considered exactly the same as
//! the oigin and are thus hampered with the origin's filters.
//!
//! Since proxy filters are respected in all dispatches of this module, it should never need to be
//! filtered by any proxy.
//!
//! ## Interface
//!
@@ -42,7 +47,7 @@
//! * `batch` - Dispatch multiple calls from the sender's origin.
//!
//! #### For pseudonymal dispatch
//! * `as_sub` - Dispatch a call from a secondary ("sub") signed origin.
//! * `as_derivative` - Dispatch a call from a derivative signed origin.
//!
//! [`Call`]: ./enum.Call.html
//! [`Trait`]: ./trait.Trait.html
@@ -155,31 +160,6 @@ decl_module! {
Self::deposit_event(Event::BatchCompleted);
}
/// Send a call through an indexed pseudonym of the sender.
///
/// NOTE: If you need to ensure that any account-based filtering is honored (i.e. because
/// you expect `proxy` to have been used prior in the call stack and you want it to apply to
/// any sub-accounts), then use `as_limited_sub` instead.
///
/// The dispatch origin for this call must be _Signed_.
///
/// # <weight>
/// - Base weight: 2.861 µs
/// - Plus the weight of the `call`
/// # </weight>
#[weight = (
call.get_dispatch_info().weight.saturating_add(3_000_000),
call.get_dispatch_info().class,
)]
fn as_sub(origin, index: u16, call: Box<<T as Trait>::Call>) -> DispatchResult {
let who = ensure_signed(origin)?;
// This is a freshly authenticated new account, the origin restrictions doesn't apply.
let pseudonym = Self::sub_account_id(who, index);
call.dispatch(frame_system::RawOrigin::Signed(pseudonym).into())
.map(|_| ()).map_err(|e| e.error)
}
/// Send a call through an indexed pseudonym of the sender.
///
/// Filter from origin are passed along. The call will be dispatched with an origin which
@@ -187,7 +167,10 @@ decl_module! {
///
/// NOTE: If you need to ensure that any account-based filtering is not honored (i.e.
/// because you expect `proxy` to have been used prior in the call stack and you do not want
/// the call restrictions to apply to any sub-accounts), then use `as_sub` instead.
/// the call restrictions to apply to any sub-accounts), then use `as_multi_threshold_1`
/// in the Multisig pallet instead.
///
/// NOTE: Prior to version *12, this was called `as_limited_sub`.
///
/// The dispatch origin for this call must be _Signed_.
///
@@ -199,10 +182,10 @@ decl_module! {
call.get_dispatch_info().weight.saturating_add(3_000_000),
call.get_dispatch_info().class,
)]
fn as_limited_sub(origin, index: u16, call: Box<<T as Trait>::Call>) -> DispatchResult {
fn as_derivative(origin, index: u16, call: Box<<T as Trait>::Call>) -> DispatchResult {
let mut origin = origin;
let who = ensure_signed(origin.clone())?;
let pseudonym = Self::sub_account_id(who, index);
let pseudonym = Self::derivative_account_id(who, index);
origin.set_caller_from(frame_system::RawOrigin::Signed(pseudonym));
call.dispatch(origin).map(|_| ()).map_err(|e| e.error)
}
@@ -210,8 +193,8 @@ decl_module! {
}
impl<T: Trait> Module<T> {
/// Derive a sub-account ID from the owner account and the sub-account index.
pub fn sub_account_id(who: T::AccountId, index: u16) -> T::AccountId {
/// Derive a derivative account ID from the owner account and the sub-account index.
pub fn derivative_account_id(who: T::AccountId, index: u16) -> T::AccountId {
let entropy = (b"modlpy/utilisuba", who, index).using_encoded(blake2_256);
T::AccountId::decode(&mut &entropy[..]).unwrap_or_default()
}
+6 -6
View File
@@ -138,16 +138,16 @@ fn expect_event<E: Into<TestEvent>>(e: E) {
}
#[test]
fn as_sub_works() {
fn as_derivative_works() {
new_test_ext().execute_with(|| {
let sub_1_0 = Utility::sub_account_id(1, 0);
let sub_1_0 = Utility::derivative_account_id(1, 0);
assert_ok!(Balances::transfer(Origin::signed(1), sub_1_0, 5));
assert_noop!(Utility::as_sub(
assert_noop!(Utility::as_derivative(
Origin::signed(1),
1,
Box::new(Call::Balances(BalancesCall::transfer(6, 3))),
), BalancesError::<Test, _>::InsufficientBalance);
assert_ok!(Utility::as_sub(
assert_ok!(Utility::as_derivative(
Origin::signed(1),
0,
Box::new(Call::Balances(BalancesCall::transfer(2, 3))),
@@ -158,9 +158,9 @@ fn as_sub_works() {
}
#[test]
fn as_sub_filters() {
fn as_derivative_filters() {
new_test_ext().execute_with(|| {
assert_noop!(Utility::as_sub(
assert_noop!(Utility::as_derivative(
Origin::signed(1),
1,
Box::new(Call::System(frame_system::Call::remark(vec![]))),