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

Extend Utility pallet with multisig and pseudonyms #4462

Merged
merged 26 commits into from Dec 22, 2019
Merged

Conversation

@gavofyork
Copy link
Member

gavofyork commented Dec 19, 2019

Adds three new new pieces of functionality to the Utility pallet:

  • fn as_sub, allowing any account to change sender to some derivative account ID. The set of derivative addresses are formed as the Blake2 hash of the owner address and index, prefixed with a context string.
  • fn as_multi, fn approve_as_multi, fn cancel_as_multi, allowing any set of accounts to proxy a message from a well known account ID, once a threshold of approvals is met.
  • Alters the existing batch functionality so that it works with any origin.

This also cleans up the weights.

TODO:

  • Tests for Timepoint stuff
  • Weights for everything
  • Docs (including complexity)
  • Module-level docs
  • Maybe refactor into structs
  • Batch should work with signed origins
  • Consider splitting deposit into base and threshold multiplier

Closes #4442

gavofyork and others added 16 commits Dec 19, 2019
…ultisig-utility
@gavofyork gavofyork requested a review from shawntabrizi Dec 22, 2019
gavofyork added 3 commits Dec 22, 2019
/// 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 {
let entropy = (b"modlpy/utilisuba", who, index).using_encoded(blake2_256);
T::AccountId::decode(&mut &entropy[..]).unwrap_or_default()

This comment has been minimized.

Copy link
@pepyakin

pepyakin Dec 22, 2019

Contributor

This assumes serialized format of T::AccountId, i.e. that it is a plain [u8; 32].

This comment has been minimized.

Copy link
@gavofyork

gavofyork Dec 22, 2019

Author Member

it assumes that it's no larger than a hash, but doesn't need to be as big, as the tests show. i think that this is fine. other parts of the codebase also assume that AccountIds can be decoded from unformatted data.

This comment has been minimized.

Copy link
@pepyakin

pepyakin Dec 22, 2019

Contributor

True. I was more concerned about that T::AccountId can be anything (theoretically) and it might be that not all variations of entropy are suitable for decoding.

If the other parts do assume that AccountId can be decoded from unformatted data then we might want to embrace that and document it explicitly. (Not to suggest doing this in this PR though).

frame/utility/src/lib.rs Outdated Show resolved Hide resolved
frame/utility/src/lib.rs Outdated Show resolved Hide resolved
frame/utility/src/lib.rs Outdated Show resolved Hide resolved
frame/utility/src/lib.rs Show resolved Hide resolved
frame/utility/src/lib.rs Outdated Show resolved Hide resolved
gavofyork added 2 commits Dec 22, 2019
@gavofyork gavofyork requested review from kianenigma and thiolliere as code owners Dec 22, 2019
@gavofyork

This comment has been minimized.

Copy link
Member Author

gavofyork commented Dec 22, 2019

@pepyakin other points should be addressed in the latest changes.

frame/utility/src/lib.rs Outdated Show resolved Hide resolved
frame/utility/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

pepyakin left a comment

Looks good

frame/utility/src/lib.rs Show resolved Hide resolved
bin/node/runtime/src/lib.rs Outdated Show resolved Hide resolved
gavofyork and others added 3 commits Dec 22, 2019
Co-Authored-By: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-Authored-By: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-Authored-By: Sergei Pepyakin <sergei@parity.io>
@gavofyork gavofyork merged commit bbb363f into master Dec 22, 2019
@gavofyork gavofyork added this to Done in Runtime via automation Dec 22, 2019
@gavofyork gavofyork deleted the gav-multisig-utility branch Dec 22, 2019
Copy link
Member

shawntabrizi left a comment

Looked good to me too

bkchr added a commit that referenced this pull request Dec 30, 2019
Changes in #4462 required a
metadata version increment that was forgotten.
gavofyork added a commit that referenced this pull request Dec 30, 2019
Changes in #4462 required a
metadata version increment that was forgotten.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Runtime
  
Done
3 participants
You can’t perform that action at this time.