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: readable linking challenge #483

Merged
merged 7 commits into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 4 additions & 5 deletions pallets/pallet-did-lookup/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ test-log = "0.2.11"

[dependencies]
# External dependencies
base58.workspace = true
blake2 = {version = "0.10.6", default-features = false}
codec = {package = "parity-scale-codec", workspace = true, features = ["derive"]}
hex.workspace = true
libsecp256k1 = {workspace = true, features = ["hmac"]}
Expand Down Expand Up @@ -59,6 +61,7 @@ runtime-benchmarks = [
"sp-runtime/runtime-benchmarks",
]
std = [
"blake2/std",
"codec/std",
"frame-benchmarking?/std",
"frame-support/std",
Expand All @@ -75,8 +78,4 @@ std = [
"sp-runtime/std",
"sp-std/std",
]
try-runtime = [
"frame-support/try-runtime",
"frame-system/try-runtime",
"kilt-support/try-runtime",
]
try-runtime = ["frame-support/try-runtime", "frame-system/try-runtime", "kilt-support/try-runtime"]
82 changes: 74 additions & 8 deletions pallets/pallet-did-lookup/src/associate_account_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,31 @@ use crate::{
signature::get_wrapped_payload,
};

use base58::ToBase58;
use blake2::{Blake2b512, Digest};
use codec::{Decode, Encode, MaxEncodedLen};
use scale_info::TypeInfo;
use scale_info::{
prelude::{format, string::String},
TypeInfo,
};
use sp_runtime::{traits::Verify, AccountId32, MultiSignature};
use sp_std::{fmt::Debug, vec, vec::Vec};

#[derive(Clone, Debug, Eq, PartialEq, Encode, Decode, MaxEncodedLen, TypeInfo)]
pub enum AssociateAccountRequest {
Dotsama(AccountId32, MultiSignature),
Polkadot(AccountId32, MultiSignature),
Ethereum(AccountId20, EthereumSignature),
}

impl AssociateAccountRequest {
pub fn verify<T: crate::Config>(
pub fn verify<DidIdentifier: AsRef<[u8]>, BlockNumber: Debug>(
&self,
did_identifier: <T as crate::Config>::DidIdentifier,
expiration: <T as frame_system::Config>::BlockNumber,
did_identifier: &DidIdentifier,
expiration: BlockNumber,
) -> bool {
let encoded_payload = (&did_identifier, expiration).encode();
let encoded_payload = get_challenge(did_identifier, expiration).into_bytes();
match self {
AssociateAccountRequest::Dotsama(acc, proof) => proof.verify(
AssociateAccountRequest::Polkadot(acc, proof) => proof.verify(
&get_wrapped_payload(&encoded_payload[..], crate::signature::WrapType::Substrate)[..],
acc,
),
Expand All @@ -53,8 +59,68 @@ impl AssociateAccountRequest {

pub fn get_linkable_account(&self) -> LinkableAccountId {
match self {
AssociateAccountRequest::Dotsama(acc, _) => LinkableAccountId::AccountId32(acc.clone()),
AssociateAccountRequest::Polkadot(acc, _) => LinkableAccountId::AccountId32(acc.clone()),
AssociateAccountRequest::Ethereum(acc, _) => LinkableAccountId::AccountId20(*acc),
}
}
}

/// Build the challenge that must be signed to prove the consent to of an
weichweich marked this conversation as resolved.
Show resolved Hide resolved
/// account to be linked to a DID.
pub fn get_challenge<DidIdentifier: AsRef<[u8]>, BlockNumber: Debug>(
ntn-x2 marked this conversation as resolved.
Show resolved Hide resolved
did_identifier: &DidIdentifier,
expiration: BlockNumber,
) -> String {
format!(
"Publicly link the signing address to did:kilt:{} before block number {:?}",
to_ss58(did_identifier.as_ref(), 38),
expiration
)
}

// Copied from /~https://github.com/paritytech/substrate/blob/ad5399644aebc54e32a107ac37ae08e6cd1f0cfb/primitives/core/src/crypto.rs#L324
// Copyright (C) Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: Apache-2.0
fn to_ss58(public_key: &[u8], prefix: u16) -> String {
// We mask out the upper two bits of the ident - SS58 Prefix currently only
// supports 14-bits
let ident: u16 = prefix & 0b0011_1111_1111_1111;
let mut v = match ident {
0..=63 => vec![ident as u8],
64..=16_383 => {
// upper six bits of the lower byte(!)
let first = ((ident & 0b0000_0000_1111_1100) as u8) >> 2;
// lower two bits of the lower byte in the high pos,
// lower bits of the upper byte in the low pos
let second = ((ident >> 8) as u8) | ((ident & 0b0000_0000_0000_0011) as u8) << 6;
vec![first | 0b01000000, second]
}
_ => unreachable!("masked out the upper two bits; qed"),
};
v.extend(public_key);
let r = ss58hash(&v);
v.extend(&r[0..2]);
v.to_base58()
}

const PREFIX: &[u8] = b"SS58PRE";

fn ss58hash(data: &[u8]) -> Vec<u8> {
let mut ctx = Blake2b512::new();
ctx.update(PREFIX);
ctx.update(data);
ctx.finalize().to_vec()
}

#[cfg(test)]
mod tests {
use super::get_challenge;

#[test]
fn test_get_challenge() {
assert_eq!(
get_challenge(&[1u8; 32], 5),
"Publicly link the signing address to did:kilt:4nwPAmtsK5toZfBM9WvmAe4Fa3LyZ3X3JHt7EUFfrcPPAZAm before block number 5"
);
}
}
6 changes: 3 additions & 3 deletions pallets/pallet-did-lookup/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ benchmarks! {
assert!(ConnectedAccounts::<T>::get(&previous_did, linkable_id.clone()).is_some());
let origin = T::EnsureOrigin::generate_origin(caller, did.clone());
let id_arg = linkable_id.clone();
let req = AssociateAccountRequest::Dotsama(connected_acc_id.into(), sig.into());
let req = AssociateAccountRequest::Polkadot(connected_acc_id.into(), sig.into());
}: associate_account<T::RuntimeOrigin>(origin, req, bn)
verify {
assert!(ConnectedDids::<T>::get(linkable_id.clone()).is_some());
Expand Down Expand Up @@ -122,7 +122,7 @@ benchmarks! {
assert!(ConnectedAccounts::<T>::get(&previous_did, linkable_id.clone()).is_some());
let origin = T::EnsureOrigin::generate_origin(caller, did.clone());
let id_arg = linkable_id.clone();
let req = AssociateAccountRequest::Dotsama(connected_acc_id.into(), sig.into());
let req = AssociateAccountRequest::Polkadot(connected_acc_id.into(), sig.into());
}: associate_account<T::RuntimeOrigin>(origin, req, bn)
verify {
assert!(ConnectedDids::<T>::get(linkable_id.clone()).is_some());
Expand Down Expand Up @@ -155,7 +155,7 @@ benchmarks! {
assert!(ConnectedAccounts::<T>::get(&previous_did, linkable_id.clone()).is_some());
let origin = T::EnsureOrigin::generate_origin(caller, did.clone());
let id_arg = linkable_id.clone();
let req = AssociateAccountRequest::Dotsama(connected_acc_id, sig.into());
let req = AssociateAccountRequest::Polkadot(connected_acc_id, sig.into());
}: associate_account<T::RuntimeOrigin>(origin, req, bn)
verify {
assert!(ConnectedDids::<T>::get(linkable_id.clone()).is_some());
Expand Down
8 changes: 4 additions & 4 deletions pallets/pallet-did-lookup/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ mod connection_record;
mod migration_state;
mod signature;

#[cfg(test)]
#[cfg(all(test, feature = "std"))]
Copy link
Member

Choose a reason for hiding this comment

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

I have never seen it used anywhere else. Are you sure there's no other way to do this? Perhaps feature-gating the specific tests or the mock itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We always run tests with at least the std feature. I also want to test get_challenge in a no_std environment since debug might be implemented differently there (e.g. for accounts it simply prints nothing).

To be able to test this without std I need to disable all tests that require std.

mod tests;

#[cfg(test)]
#[cfg(all(test, feature = "std"))]
mod mock;

#[cfg(feature = "runtime-benchmarks")]
Expand Down Expand Up @@ -95,7 +95,7 @@ pub mod pallet {
type OriginSuccess: CallSources<AccountIdOf<Self>, DidIdentifierOf<Self>>;

/// The identifier to which accounts can get associated.
type DidIdentifier: Parameter + MaxEncodedLen + MaybeSerializeDeserialize;
type DidIdentifier: Parameter + AsRef<[u8]> + MaxEncodedLen + MaybeSerializeDeserialize;

/// The currency that is used to reserve funds for each did.
type Currency: ReservableCurrency<AccountIdOf<Self>>;
Expand Down Expand Up @@ -252,7 +252,7 @@ pub mod pallet {
);

ensure!(
req.verify::<T>(did_identifier.clone(), expiration),
req.verify::<T::DidIdentifier, T::BlockNumber>(&did_identifier, expiration),
Error::<T>::NotAuthorized
);

Expand Down
2 changes: 1 addition & 1 deletion pallets/pallet-did-lookup/src/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ pub(crate) fn add_legacy_association<T: Config>(
ConnectedAccounts::<T>::insert(&did_identifier, &account, ());
}

#[cfg(test)]
#[cfg(all(test, feature = "std"))]
mod tests {
use frame_support::assert_ok;
use kilt_support::deposit::Deposit;
Expand Down
20 changes: 10 additions & 10 deletions pallets/pallet-did-lookup/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use sp_runtime::{

use crate::{
account::{AccountId20, EthereumSignature},
associate_account_request::AssociateAccountRequest,
associate_account_request::{get_challenge, AssociateAccountRequest},
linkable_account::LinkableAccountId,
migration_state::MigrationState,
migrations::{add_legacy_association, get_mixed_storage_iterator, MixedStorageKey},
Expand Down Expand Up @@ -98,16 +98,16 @@ fn test_add_association_account() {
let expire_at: BlockNumber = 500;
let account_hash_alice = MultiSigner::from(pair_alice.public()).into_account();
let sig_alice_0 = MultiSignature::from(
pair_alice.sign(&[b"<Bytes>", &Encode::encode(&(&DID_00, expire_at))[..], b"</Bytes>"].concat()[..]),
pair_alice.sign(&[b"<Bytes>", get_challenge(&DID_00, expire_at).as_bytes(), b"</Bytes>"].concat()[..]),
);
let sig_alice_1 = MultiSignature::from(
pair_alice.sign(&[b"<Bytes>", &Encode::encode(&(&DID_01, expire_at))[..], b"</Bytes>"].concat()[..]),
pair_alice.sign(&[b"<Bytes>", get_challenge(&DID_01, expire_at).as_bytes(), b"</Bytes>"].concat()[..]),
);

// new association. No overwrite
assert!(DidLookup::associate_account(
mock_origin::DoubleOrigin(ACCOUNT_00, DID_00).into(),
AssociateAccountRequest::Dotsama(account_hash_alice.clone(), sig_alice_0),
AssociateAccountRequest::Polkadot(account_hash_alice.clone(), sig_alice_0),
expire_at,
)
.is_ok());
Expand All @@ -132,7 +132,7 @@ fn test_add_association_account() {
// overwrite existing association
let res = DidLookup::associate_account(
mock_origin::DoubleOrigin(ACCOUNT_00, DID_01).into(),
AssociateAccountRequest::Dotsama(account_hash_alice.clone(), sig_alice_1.clone()),
AssociateAccountRequest::Polkadot(account_hash_alice.clone(), sig_alice_1.clone()),
expire_at,
);
if let Err(err) = res {
Expand Down Expand Up @@ -163,7 +163,7 @@ fn test_add_association_account() {
// overwrite existing deposit
assert!(DidLookup::associate_account(
mock_origin::DoubleOrigin(ACCOUNT_01, DID_01).into(),
AssociateAccountRequest::Dotsama(account_hash_alice.clone(), sig_alice_1),
AssociateAccountRequest::Polkadot(account_hash_alice.clone(), sig_alice_1),
expire_at,
)
.is_ok());
Expand Down Expand Up @@ -203,7 +203,7 @@ fn test_add_eth_association() {
let eth_account = AccountId20(eth_pair.public().to_eth_address().unwrap());

let wrapped_payload = get_wrapped_payload(
&Encode::encode(&(&DID_00, expire_at))[..],
get_challenge(&DID_00, expire_at).as_bytes(),
crate::signature::WrapType::Ethereum,
);

Expand All @@ -215,7 +215,7 @@ fn test_add_eth_association() {
AssociateAccountRequest::Ethereum(eth_account, EthereumSignature::from(sig)),
expire_at,
);
assert!(res.is_ok());
assert_ok!(res);
assert_eq!(
ConnectedDids::<Test>::get(LinkableAccountId::from(eth_account)),
Some(ConnectionRecord {
Expand Down Expand Up @@ -252,7 +252,7 @@ fn test_add_association_account_invalid_signature() {
assert_noop!(
DidLookup::associate_account(
mock_origin::DoubleOrigin(ACCOUNT_00, DID_01).into(),
AssociateAccountRequest::Dotsama(account_hash_alice, sig_alice_0),
AssociateAccountRequest::Polkadot(account_hash_alice, sig_alice_0),
expire_at,
),
Error::<Test>::NotAuthorized
Expand Down Expand Up @@ -280,7 +280,7 @@ fn test_add_association_account_expired() {
assert_noop!(
DidLookup::associate_account(
mock_origin::DoubleOrigin(ACCOUNT_00, DID_01).into(),
AssociateAccountRequest::Dotsama(account_hash_alice, sig_alice_0),
AssociateAccountRequest::Polkadot(account_hash_alice, sig_alice_0),
expire_at,
),
Error::<Test>::OutdatedProof
Expand Down
6 changes: 3 additions & 3 deletions runtimes/peregrine/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ fn test_derive_did_key_web3name() {
fn test_derive_did_key_lookup() {
assert_eq!(
RuntimeCall::DidLookup(pallet_did_lookup::Call::associate_account {
req: AssociateAccountRequest::Dotsama(
req: AssociateAccountRequest::Polkadot(
AccountId::new([1u8; 32]),
sp_runtime::MultiSignature::from(sp_core::ed25519::Signature([0; 64]))
),
Expand Down Expand Up @@ -335,7 +335,7 @@ fn test_migration_filter_migrating() {
// This should not work during migration:
assert!(!MigrationFilter::contains(&RuntimeCall::DidLookup(
pallet_did_lookup::Call::associate_account {
req: pallet_did_lookup::associate_account_request::AssociateAccountRequest::Dotsama(
req: pallet_did_lookup::associate_account_request::AssociateAccountRequest::Polkadot(
AccountId32::from([0u8; 32]),
sp_runtime::MultiSignature::Ecdsa(Signature([0u8; 65]))
),
Expand Down Expand Up @@ -379,7 +379,7 @@ fn test_migration_filter_done() {
// This should only work after the migration:
assert!(MigrationFilter::contains(&RuntimeCall::DidLookup(
pallet_did_lookup::Call::associate_account {
req: pallet_did_lookup::associate_account_request::AssociateAccountRequest::Dotsama(
req: pallet_did_lookup::associate_account_request::AssociateAccountRequest::Polkadot(
AccountId32::from([0u8; 32]),
sp_runtime::MultiSignature::Ecdsa(Signature([0u8; 65]))
),
Expand Down
6 changes: 3 additions & 3 deletions runtimes/spiritnet/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ fn test_derive_did_key_web3name() {
fn test_derive_did_key_lookup() {
assert_eq!(
RuntimeCall::DidLookup(pallet_did_lookup::Call::associate_account {
req: AssociateAccountRequest::Dotsama(
req: AssociateAccountRequest::Polkadot(
AccountId::new([1u8; 32]),
sp_runtime::MultiSignature::from(sp_core::ed25519::Signature([0; 64]))
),
Expand Down Expand Up @@ -335,7 +335,7 @@ fn test_migration_filter_migrating() {
// This should not work during migration:
assert!(!MigrationFilter::contains(&RuntimeCall::DidLookup(
pallet_did_lookup::Call::associate_account {
req: pallet_did_lookup::associate_account_request::AssociateAccountRequest::Dotsama(
req: pallet_did_lookup::associate_account_request::AssociateAccountRequest::Polkadot(
AccountId32::from([0u8; 32]),
sp_runtime::MultiSignature::Ecdsa(Signature([0u8; 65]))
),
Expand Down Expand Up @@ -379,7 +379,7 @@ fn test_migration_filter_done() {
// This should only work after the migration:
assert!(MigrationFilter::contains(&RuntimeCall::DidLookup(
pallet_did_lookup::Call::associate_account {
req: pallet_did_lookup::associate_account_request::AssociateAccountRequest::Dotsama(
req: pallet_did_lookup::associate_account_request::AssociateAccountRequest::Polkadot(
AccountId32::from([0u8; 32]),
sp_runtime::MultiSignature::Ecdsa(Signature([0u8; 65]))
),
Expand Down
6 changes: 6 additions & 0 deletions support/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,9 @@ impl From<sr25519::Public> for SubjectId {
SubjectId(acc.into())
}
}

impl AsRef<[u8]> for SubjectId {
fn as_ref(&self) -> &[u8] {
self.0.as_ref()
}
}