From b16f21c310e6c922cb407ffe2f0af547ae0df5f9 Mon Sep 17 00:00:00 2001 From: Benjamin Kolb Date: Tue, 27 Nov 2018 21:01:51 +0100 Subject: [PATCH 1/3] fix attestation and revocation --- runtime/src/attestation.rs | 167 +++++++++++++++++++++++++++++++++---- src/main.rs | 2 +- 2 files changed, 151 insertions(+), 18 deletions(-) diff --git a/runtime/src/attestation.rs b/runtime/src/attestation.rs index 05f9b9b25c..80d271949e 100644 --- a/runtime/src/attestation.rs +++ b/runtime/src/attestation.rs @@ -10,8 +10,6 @@ pub trait Trait: balances::Trait { type Signature: Verify + Member + Codec + Default; } -pub type Revoked = bool; - decl_module! { pub struct Module for enum Call where origin: T::Origin { @@ -22,9 +20,9 @@ decl_module! { } let mut existing_attestations_for_claim = >::get(claim_hash.clone()); - let mut last_attested : Option<(T::AccountId,T::Signature,Revoked)> = None; + let mut last_attested : Option<(T::Hash,T::AccountId,T::Signature,bool)> = None; for v in existing_attestations_for_claim.clone() { - if v.0.eq(&sender) { + if v.1.eq(&sender) { last_attested = Some(v); break; } @@ -32,7 +30,8 @@ decl_module! { match last_attested { Some(_v) => return Err("already attested"), None => { - existing_attestations_for_claim.push((sender.clone(), signature.clone(), false)); + existing_attestations_for_claim.push((claim_hash.clone(), sender.clone(), signature.clone(), false)); + >::insert(claim_hash.clone(), existing_attestations_for_claim); Ok(()) }, } @@ -44,19 +43,20 @@ decl_module! { return Err("bad signature") } - let existing_attestations_for_claim = >::get(claim_hash.clone()); - let mut last_attested : Option<(T::AccountId,T::Signature,Revoked)> = None; - for v in existing_attestations_for_claim.clone() { - if v.0.eq(&sender) { - last_attested = Some(v); + let mut last_attested : bool = false; + let mut existing_attestations_for_claim = >::get(claim_hash.clone()); + for v in existing_attestations_for_claim.iter_mut() { + if v.1.eq(&sender) && !v.3 { + last_attested = true; + v.3 = true; } } - match last_attested { - None => return Err("not attested"), - Some(mut v) => { - v.2 = false; - Ok(()) - }, + if last_attested { + let copy_of_attestations = (&mut existing_attestations_for_claim).clone(); + >::insert(claim_hash.clone(), copy_of_attestations); + Ok(()) + } else { + Err("not attested") } } } @@ -64,6 +64,139 @@ decl_module! { decl_storage! { trait Store for Module as Attestation { - Attestations get(attestations): map T::Hash => Vec<(T::AccountId,T::Signature,Revoked)>; + Attestations get(attestations): map T::Hash => Vec<(T::Hash,T::AccountId,T::Signature,bool)>; + } +} + + +#[cfg(test)] +mod tests { + use super::*; + use parity_codec::alloc::vec::Vec; + use system; + use runtime_io::with_externalities; + use primitives::{H256, H512, Blake2Hasher}; + use runtime_primitives::Ed25519Signature; + use primitives::*; + use parity_codec::Encode; + + use sr_primitives::{ + BuildStorage, traits::{BlakeTwo256}, testing::{Digest, DigestItem, Header} + }; + + impl_outer_origin! { + pub enum Origin for Test {} + } + + #[derive(Clone, Eq, PartialEq)] + pub struct Test; + impl system::Trait for Test { + type Origin = Origin; + type Index = u64; + type BlockNumber = u64; + type Hash = H256; + type Hashing = BlakeTwo256; + type Digest = Digest; + type AccountId = H256; + type Header = Header; + type Event = (); + type Log = DigestItem; + } + impl balances::Trait for Test { + type Balance = u64; + type AccountIndex = u64; + type OnFreeBalanceZero = (); + type EnsureAccountLiquid = (); + type Event = (); + } + + impl Trait for Test { + type Signature = Ed25519Signature; + } + type Attestation = Module; + + // This function basically just builds a genesis storage key/value store according to + // our desired mockup. + fn new_test_ext() -> sr_io::TestExternalities { + let mut t = system::GenesisConfig::::default().build_storage().unwrap().0; + // We use default for brevity, but you can configure as desired if needed. + t.extend(balances::GenesisConfig::::default().build_storage().unwrap().0); + t.into() + } + + fn hash_to_u8 (hash : T) -> Vec{ + return hash.encode(); + } + + #[test] + fn check_bad_signature() { + with_externalities(&mut new_test_ext(), || { + assert_err!(Attestation::add(Origin::signed(H256::from(1)), H256::from(2), Ed25519Signature::from(H512::from(3))), "bad signature"); + assert_err!(Attestation::revoke(Origin::signed(H256::from(1)), H256::from(2), Ed25519Signature::from(H512::from(3))), "bad signature"); + }); + } + + #[test] + fn check_add_attestation() { + with_externalities(&mut new_test_ext(), || { + let pair = ed25519::Pair::from_seed(b"Alice "); + let hash = H256::from(1); + let bytes = hash_to_u8(hash); + let signed = Ed25519Signature::from(pair.sign(&bytes)); + let account_hash = H256::from(pair.public().0); + assert_ok!(Attestation::add(Origin::signed(account_hash.clone()), hash.clone(), signed.clone())); + let existing_attestations_for_claim = Attestation::attestations(hash.clone()); + assert_eq!(existing_attestations_for_claim.len(), 1); + assert_eq!(existing_attestations_for_claim[0].0, hash.clone()); + assert_eq!(existing_attestations_for_claim[0].1, account_hash.clone()); + assert_eq!(existing_attestations_for_claim[0].2, signed.clone()); + assert_eq!(existing_attestations_for_claim[0].3, false); + }); + } + + #[test] + fn check_revoke_attestation() { + with_externalities(&mut new_test_ext(), || { + let pair = ed25519::Pair::from_seed(b"Alice "); + let hash = H256::from(1); + let bytes = hash_to_u8(hash); + let signed = Ed25519Signature::from(pair.sign(&bytes)); + let account_hash = H256::from(pair.public().0); + assert_ok!(Attestation::add(Origin::signed(account_hash.clone()), hash.clone(), signed.clone())); + assert_ok!(Attestation::revoke(Origin::signed(account_hash.clone()), hash.clone(), signed.clone())); + let existing_attestations_for_claim = Attestation::attestations(hash.clone()); + assert_eq!(existing_attestations_for_claim.len(), 1); + assert_eq!(existing_attestations_for_claim[0].0, hash.clone()); + assert_eq!(existing_attestations_for_claim[0].1, account_hash.clone()); + assert_eq!(existing_attestations_for_claim[0].2, signed.clone()); + assert_eq!(existing_attestations_for_claim[0].3, true); + }); + } + + #[test] + fn check_double_attestation() { + with_externalities(&mut new_test_ext(), || { + let pair = ed25519::Pair::from_seed(b"Alice "); + let hash = H256::from(1); + let bytes = hash_to_u8(hash); + let signed = Ed25519Signature::from(pair.sign(&bytes)); + let account_hash = H256::from(pair.public().0); + assert_ok!(Attestation::add(Origin::signed(account_hash.clone()), hash.clone(), signed.clone())); + assert_err!(Attestation::add(Origin::signed(account_hash.clone()), hash.clone(), signed.clone()), "already attested"); + }); + } + + #[test] + fn check_double_revoke_attestation() { + with_externalities(&mut new_test_ext(), || { + let pair = ed25519::Pair::from_seed(b"Alice "); + let hash = H256::from(1); + let bytes = hash_to_u8(hash); + let signed = Ed25519Signature::from(pair.sign(&bytes)); + let account_hash = H256::from(pair.public().0); + assert_ok!(Attestation::add(Origin::signed(account_hash.clone()), hash.clone(), signed.clone())); + assert_ok!(Attestation::revoke(Origin::signed(account_hash.clone()), hash.clone(), signed.clone())); + assert_err!(Attestation::revoke(Origin::signed(account_hash.clone()), hash.clone(), signed.clone()), "not attested"); + }); } } diff --git a/src/main.rs b/src/main.rs index 64b8b6bb7b..915bb82074 100644 --- a/src/main.rs +++ b/src/main.rs @@ -42,7 +42,7 @@ fn run() -> cli::error::Result<()> { version: env!("CARGO_PKG_VERSION"), executable_name: "node", author: "Anonymous ", - description: "Substrate Node Template", + description: "KILT Node", }; cli::run(::std::env::args(), Exit, version) } From dde072d0dab5f919142b6f53cb1d9223dd20c05d Mon Sep 17 00:00:00 2001 From: Benjamin Kolb Date: Wed, 28 Nov 2018 08:49:11 +0100 Subject: [PATCH 2/3] avoid unnecessary clone --- runtime/src/attestation.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/runtime/src/attestation.rs b/runtime/src/attestation.rs index 80d271949e..423a49b9ed 100644 --- a/runtime/src/attestation.rs +++ b/runtime/src/attestation.rs @@ -52,8 +52,7 @@ decl_module! { } } if last_attested { - let copy_of_attestations = (&mut existing_attestations_for_claim).clone(); - >::insert(claim_hash.clone(), copy_of_attestations); + >::insert(claim_hash.clone(), existing_attestations_for_claim.clone()); Ok(()) } else { Err("not attested") From 4441009538a0f0e7c7df558369eb5ecf1088fc3f Mon Sep 17 00:00:00 2001 From: Benjamin Kolb Date: Wed, 28 Nov 2018 11:48:17 +0100 Subject: [PATCH 3/3] rename message --- runtime/src/attestation.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/src/attestation.rs b/runtime/src/attestation.rs index 423a49b9ed..445fc868e1 100644 --- a/runtime/src/attestation.rs +++ b/runtime/src/attestation.rs @@ -55,7 +55,7 @@ decl_module! { >::insert(claim_hash.clone(), existing_attestations_for_claim.clone()); Ok(()) } else { - Err("not attested") + Err("no valid attestation found") } } } @@ -195,7 +195,7 @@ mod tests { let account_hash = H256::from(pair.public().0); assert_ok!(Attestation::add(Origin::signed(account_hash.clone()), hash.clone(), signed.clone())); assert_ok!(Attestation::revoke(Origin::signed(account_hash.clone()), hash.clone(), signed.clone())); - assert_err!(Attestation::revoke(Origin::signed(account_hash.clone()), hash.clone(), signed.clone()), "not attested"); + assert_err!(Attestation::revoke(Origin::signed(account_hash.clone()), hash.clone(), signed.clone()), "no valid attestation found"); }); } }