From e80bcbbbb2c426f231a6d12cb7dea17c94797588 Mon Sep 17 00:00:00 2001 From: Anton Perkov Date: Thu, 1 Feb 2018 13:47:28 -0500 Subject: [PATCH] bugfixes in singleton, bugfixes in identity contract, more tests for identity contracts (one fails because of bug in next_primary) --- contracts/eosiolib/singleton.hpp | 27 ++-- contracts/identity/identity.hpp | 20 ++- .../include/eosio/chain/apply_context.hpp | 4 +- tests/wasm_tests/identity_tests.cpp | 139 +++++++++++++++++- 4 files changed, 164 insertions(+), 26 deletions(-) diff --git a/contracts/eosiolib/singleton.hpp b/contracts/eosiolib/singleton.hpp index 01e9528d4..8ae8e6a07 100644 --- a/contracts/eosiolib/singleton.hpp +++ b/contracts/eosiolib/singleton.hpp @@ -27,12 +27,18 @@ namespace eosio { char temp[1024+8]; *reinterpret_cast(temp) = SingletonName; auto read = load_i64( scope, Code, SingletonName, temp, sizeof(temp) ); - assert( read > 0, "singleton does not exist" ); - datastream ds(temp + sizeof(SingletonName), read); + assert( read < 0, "singleton does not exist" ); + return unpack( temp + sizeof(SingletonName), read ); + } - T result; - unpack( ds, result ); - return result; + static T get_or_default( scope_name scope = Code, const T& def = T() ) { + char temp[1024+8]; + *reinterpret_cast(temp) = SingletonName; + auto read = load_i64( scope, Code, SingletonName, temp, sizeof(temp) ); + if ( read < 0 ) { + return def; + } + return unpack( temp + sizeof(SingletonName), read ); } static T get_or_create( scope_name scope = Code, const T& def = T() ) { @@ -45,11 +51,7 @@ namespace eosio { set( def, scope ); return def; } - - datastream ds(temp + sizeof(SingletonName), read); - T result; - ds >> result; - return result; + return unpack( temp + sizeof(SingletonName), read ); } static void set( const T& value = T(), scope_name scope = Code ) { @@ -64,6 +66,11 @@ namespace eosio { store_i64( scope, SingletonName, buf, sizeof(buf) ); } + + static void remove( scope_name scope = Code ) { + uint64_t key = SingletonName; + remove_i64( scope, SingletonName, &key ); + } }; } /// namespace eosio diff --git a/contracts/identity/identity.hpp b/contracts/identity/identity.hpp index dac7741cb..647772381 100644 --- a/contracts/identity/identity.hpp +++ b/contracts/identity/identity.hpp @@ -238,9 +238,11 @@ namespace identity { if (sizeof(account_name) == row.data.size()) { account_name account = *reinterpret_cast(row.data.data()); if (ident == get_claimed_identity(account) && is_trusted(row.certifier)) { - // the certifier became trusted, need to set the flag - row.trusted = 1; - certs.store( row, 0 ); //assuming 0 means bill to the same account + if (DeployToAccount == current_receiver()) { + // the certifier became trusted and we have permission to update the flag + row.trusted = 1; + certs.store( row, 0 ); //assuming 0 means bill to the same account + } return *reinterpret_cast(row.data.data()); } } else { @@ -263,7 +265,7 @@ namespace identity { trustrow def; def.trusted = 0; trustrow row = trust_table::get_or_default( trusted, by, def ); - return def.trusted; + return row.trusted; } static bool is_trusted( account_name acnt ) { @@ -332,17 +334,21 @@ namespace identity { if (value.property == N(owner)) { assert(sizeof(account_name) == value.data.size(), "data size doesn't match account_name size"); account_name acnt = *reinterpret_cast(value.data.data()); - accounts_table::set( cert.identity, acnt ); + if (cert.certifier == acnt) { //only self-certitication affects accounts_table + accounts_table::set( cert.identity, acnt ); + } } } else { - //remove both tursted and untrusted because we cannot now if it was trusted back at creation time + //remove both tursted and untrusted because we cannot know if it was trusted back at creation time certs.remove(value.property, 0, cert.certifier); certs.remove(value.property, 1, cert.certifier); //special handling for owner if (value.property == N(owner)) { assert(sizeof(account_name) == value.data.size(), "data size doesn't match account_name size"); account_name acnt = *reinterpret_cast(value.data.data()); - accounts_table::remove( acnt ); + if (cert.certifier == acnt) { //only self-certitication affects accounts_table + accounts_table::remove( acnt ); + } } } } diff --git a/libraries/chain/include/eosio/chain/apply_context.hpp b/libraries/chain/include/eosio/chain/apply_context.hpp index 40ff52787..e594bd6a1 100644 --- a/libraries/chain/include/eosio/chain/apply_context.hpp +++ b/libraries/chain/include/eosio/chain/apply_context.hpp @@ -346,7 +346,7 @@ using apply_handler = std::function; template< typename KeyType, int NullKeyCount, typename Scope, typename ... Args > struct lower_bound_tuple_impl { static auto get(const contracts::table_id_object& tid, const KeyType* keys, Args... args) { - return lower_bound_tuple_impl::get(tid, keys, KeyType(0), args...); + return lower_bound_tuple_impl::get(tid, keys, raw_key_value(keys, scope_to_key_index_v + NullKeyCount), args...); } }; @@ -363,7 +363,7 @@ using apply_handler = std::function; template< typename KeyType, int NullKeyCount, typename Scope, typename ... Args > struct upper_bound_tuple_impl { static auto get(const contracts::table_id_object& tid, const KeyType* keys, Args... args) { - return upper_bound_tuple_impl::get(tid, keys, KeyType(-1), args...); + return upper_bound_tuple_impl::get(tid, keys, raw_key_value(keys, scope_to_key_index_v + NullKeyCount), args...); } }; diff --git a/tests/wasm_tests/identity_tests.cpp b/tests/wasm_tests/identity_tests.cpp index 49defd4c5..86ca564af 100644 --- a/tests/wasm_tests/identity_tests.cpp +++ b/tests/wasm_tests/identity_tests.cpp @@ -410,7 +410,21 @@ BOOST_FIXTURE_TEST_CASE( trust_untrust, identity_tester ) try { BOOST_FIXTURE_TEST_CASE( certify_decertify_owner, identity_tester ) try { BOOST_REQUIRE_EQUAL(success(), create_identity("alice", identity_val)); - // certify owner, should populate "account" singleton + //bob certifies ownership for alice + BOOST_REQUIRE_EQUAL(success(), certify("bob", identity_val, vector{ mutable_variant_object() + ("property", "owner") + ("type", "account") + ("data", to_uint8_vector(N(alice))) + ("memo", "claiming onwership") + ("confidence", 100) + })); + BOOST_REQUIRE_EQUAL( true, get_certrow(identity_val, "owner", 0, "bob").is_object() ); + //it should not affect "account" singleton in alice's scope since it's not self-certification + BOOST_REQUIRE_EQUAL( true, get_accountrow("alice").is_null() ); + //it also shouldn't affect "account" singleton in bob's scope since he certified not himself + BOOST_REQUIRE_EQUAL( true, get_accountrow("bob").is_null() ); + + // alice certifies her ownership, should populate "account" singleton BOOST_REQUIRE_EQUAL(success(), certify("alice", identity_val, vector{ mutable_variant_object() ("property", "owner") ("type", "account") @@ -427,7 +441,7 @@ BOOST_FIXTURE_TEST_CASE( certify_decertify_owner, identity_tester ) try { BOOST_REQUIRE_EQUAL( "account", certrow["type"].as_string() ); BOOST_REQUIRE_EQUAL( N(alice), to_uint64(certrow["data"]) ); - //check that singleton "account" in the scope of identity contains the owner + //check that singleton "account" in the alice's scope contains the identity fc::variant acntrow = get_accountrow("alice"); BOOST_REQUIRE_EQUAL( true, certrow.is_object() ); BOOST_REQUIRE_EQUAL( identity_val, acntrow["identity"].as_uint64() ); @@ -435,6 +449,19 @@ BOOST_FIXTURE_TEST_CASE( certify_decertify_owner, identity_tester ) try { // ownership was certified by alice, but not by a block producer or someone trusted by a block producer BOOST_REQUIRE_EQUAL(0, get_owner_for_identity(identity_val)); + //remove bob's certification + BOOST_REQUIRE_EQUAL(success(), certify("bob", identity_val, vector{ mutable_variant_object() + ("property", "owner") + ("type", "account") + ("data", to_uint8_vector(N(alice))) + ("memo", "claiming onwership") + ("confidence", 0) + })); + //singleton "account" in the alice's scope still should contain the identity + acntrow = get_accountrow("alice"); + BOOST_REQUIRE_EQUAL( true, certrow.is_object() ); + BOOST_REQUIRE_EQUAL( identity_val, acntrow["identity"].as_uint64() ); + //remove owner certification BOOST_REQUIRE_EQUAL(success(), certify("alice", identity_val, vector{ mutable_variant_object() ("property", "owner") @@ -446,13 +473,13 @@ BOOST_FIXTURE_TEST_CASE( certify_decertify_owner, identity_tester ) try { certrow = get_certrow(identity_val, "owner", 0, "alice"); BOOST_REQUIRE_EQUAL(true, certrow.is_null()); - //check that singleton "account" in the scope of identity contains the owner + //check that singleton "account" in the alice's scope doesn't contain the identity acntrow = get_accountrow("alice"); BOOST_REQUIRE_EQUAL(true, certrow.is_null()); } FC_LOG_AND_RETHROW() //certify_decertify_owner -BOOST_FIXTURE_TEST_CASE( trusted_owner, identity_tester ) try { +BOOST_FIXTURE_TEST_CASE( owner_certified_by_producer, identity_tester ) try { BOOST_REQUIRE_EQUAL(success(), create_identity("alice", identity_val)); // certify owner by a block producer, should result in trusted certification @@ -476,7 +503,38 @@ BOOST_FIXTURE_TEST_CASE( trusted_owner, identity_tester ) try { //alice still has not claimed the identity - she is not the official owner yet BOOST_REQUIRE_EQUAL(0, get_owner_for_identity(identity_val)); - //now alice claims it + //alice claims it + BOOST_REQUIRE_EQUAL(success(), certify("alice", identity_val, vector{ mutable_variant_object() + ("property", "owner") + ("type", "account") + ("data", to_uint8_vector(N(alice))) + ("memo", "claiming onwership") + ("confidence", 100) + })); + BOOST_REQUIRE_EQUAL( true, get_certrow(identity_val, "owner", 0, "alice").is_object()); + + //now alice should be the official owner + BOOST_REQUIRE_EQUAL(N(alice), get_owner_for_identity(identity_val)); + + //block producer decertifies ownership + BOOST_REQUIRE_EQUAL(success(), certify("inita", identity_val, vector{ mutable_variant_object() + ("property", "owner") + ("type", "account") + ("data", to_uint8_vector(N(alice))) + ("memo", "") + ("confidence", 0) + })); + //self-certification made by alice still exists + BOOST_REQUIRE_EQUAL( true, get_certrow(identity_val, "owner", 0, "alice").is_object()); + //but now she is not official owner + BOOST_REQUIRE_EQUAL(0, get_owner_for_identity(identity_val)); + +} FC_LOG_AND_RETHROW() //owner_certified_by_producer + +BOOST_FIXTURE_TEST_CASE( owner_certified_by_trusted_account, identity_tester ) try { + BOOST_REQUIRE_EQUAL(success(), create_identity("bob", identity_val)); + + //alice claims the identity created by bob BOOST_REQUIRE_EQUAL(success(), certify("alice", identity_val, vector{ mutable_variant_object() ("property", "owner") ("type", "account") @@ -484,12 +542,79 @@ BOOST_FIXTURE_TEST_CASE( trusted_owner, identity_tester ) try { ("memo", "claiming onwership") ("confidence", 100) })); - std::cout << "Identity val: " << identity_val << std::endl; BOOST_REQUIRE_EQUAL( true, get_certrow(identity_val, "owner", 0, "alice").is_object()); + //alice claimed the identity, but it hasn't been certified yet - she is not the official owner + BOOST_REQUIRE_EQUAL(0, get_owner_for_identity(identity_val)); + + //block producer trusts bob + BOOST_REQUIRE_EQUAL(success(), settrust("initb", "bob", 1)); + BOOST_REQUIRE_EQUAL(true, get_trustrow("initb", "bob").is_object()); + + // bob (trusted account) certifies alice's ownership, it should result in trusted certification + BOOST_REQUIRE_EQUAL(success(), certify("bob", identity_val, vector{ mutable_variant_object() + ("property", "owner") + ("type", "account") + ("data", to_uint8_vector(N(alice))) + ("memo", "") + ("confidence", 100) + })); + BOOST_REQUIRE_EQUAL( true, get_certrow(identity_val, "owner", 1, "bob").is_object() ); + //no untrusted row should exist + BOOST_REQUIRE_EQUAL( true, get_certrow(identity_val, "owner", 0, "bob").is_null() ); + //now alice should be the official owner BOOST_REQUIRE_EQUAL(N(alice), get_owner_for_identity(identity_val)); -} FC_LOG_AND_RETHROW() //trusted_owner + //block producer stops trusting bob + BOOST_REQUIRE_EQUAL(success(), settrust("initb", "bob", 0)); + BOOST_REQUIRE_EQUAL(true, get_trustrow("initb", "bob").is_null()); + + //certification made by bob is still flaged as trusted + BOOST_REQUIRE_EQUAL( true, get_certrow(identity_val, "owner", 1, "bob").is_object() ); + + //but now alice shouldn't be the official owner + BOOST_REQUIRE_EQUAL(0, get_owner_for_identity(identity_val)); + +} FC_LOG_AND_RETHROW() //owner_certified_by_trusted_account + +BOOST_FIXTURE_TEST_CASE( owner_certification_becomes_trusted, identity_tester ) try { + BOOST_REQUIRE_EQUAL(success(), create_identity("bob", identity_val)); + + // bob (not trusted so far) certifies alice's ownership, it should result in untrusted certification + BOOST_REQUIRE_EQUAL(success(), certify("bob", identity_val, vector{ mutable_variant_object() + ("property", "owner") + ("type", "account") + ("data", to_uint8_vector(N(alice))) + ("memo", "") + ("confidence", 100) + })); + BOOST_REQUIRE_EQUAL( true, get_certrow(identity_val, "owner", 0, "bob").is_object() ); + //no trusted row should exist + BOOST_REQUIRE_EQUAL( true, get_certrow(identity_val, "owner", 1, "bob").is_null() ); + + //alice claims the identity created by bob + BOOST_REQUIRE_EQUAL(success(), certify("alice", identity_val, vector{ mutable_variant_object() + ("property", "owner") + ("type", "account") + ("data", to_uint8_vector(N(alice))) + ("memo", "claiming onwership") + ("confidence", 100) + })); + BOOST_REQUIRE_EQUAL( true, get_certrow(identity_val, "owner", 0, "alice").is_object()); + //alice claimed the identity, but it is certified by untrusted accounts only - she is not the official owner + BOOST_REQUIRE_EQUAL(0, get_owner_for_identity(identity_val)); + + //block producer trusts bob + BOOST_REQUIRE_EQUAL(success(), settrust("initb", "bob", 1)); + BOOST_REQUIRE_EQUAL(true, get_trustrow("initb", "bob").is_object()); + + //old certification made by bob still shouldn't be flaged as trusted + BOOST_REQUIRE_EQUAL( true, get_certrow(identity_val, "owner", 0, "bob").is_object() ); + + //but effectively bob's certification should became trusted + BOOST_REQUIRE_EQUAL(N(alice), get_owner_for_identity(identity_val)); + + } FC_LOG_AND_RETHROW() //owner_certification_becomes_trusted BOOST_AUTO_TEST_SUITE_END() #endif -- GitLab