From ae4e4c7f533e3eb7c31e4370e37a5030a5ba97dc Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Wed, 9 Aug 2017 15:33:24 -0500 Subject: [PATCH 1/7] [FC] Add cancel() to scoped_exit Allow scoped_exit objects to be canceled so they do not call their callbacks when destroyed. --- libraries/fc/include/fc/scoped_exit.hpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libraries/fc/include/fc/scoped_exit.hpp b/libraries/fc/include/fc/scoped_exit.hpp index 98ec16dfa5b..5890e9d310c 100644 --- a/libraries/fc/include/fc/scoped_exit.hpp +++ b/libraries/fc/include/fc/scoped_exit.hpp @@ -9,12 +9,15 @@ namespace fc { scoped_exit( C&& c ):callback( std::forward(c) ){} scoped_exit( scoped_exit&& mv ):callback( std::move( mv.callback ) ){} + void cancel() { canceled = true; } + ~scoped_exit() { - try { callback(); } catch( ... ) {} + if (!canceled) + try { callback(); } catch( ... ) {} } scoped_exit& operator = ( scoped_exit&& mv ) { - callback = std::move(mv); + callback = std::move(mv.callback); return *this; } private: @@ -22,6 +25,7 @@ namespace fc { scoped_exit& operator=( const scoped_exit& ); Callback callback; + bool canceled = false; }; template From 1f4753d06fba3ca87aaf26c03dff73f54cac4b03 Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Wed, 9 Aug 2017 15:38:40 -0500 Subject: [PATCH 2/7] Add utilities/parallel_markers.hpp Move logic from apply_context::unused_authorizations out to a header so other code can use it. The logic in question takes an array of data, and a parallel array of "markers" (booleans, in this instance), and returns a range containing only the data elements whose corresponding marker matches a provided value (i.e. the boolean was true). Stay tuned to see how else I use this code! ;) --- libraries/chain/message_handling_contexts.cpp | 15 ++----- .../eos/utilities/parallel_markers.hpp | 39 +++++++++++++++++++ 2 files changed, 42 insertions(+), 12 deletions(-) create mode 100644 libraries/utilities/include/eos/utilities/parallel_markers.hpp diff --git a/libraries/chain/message_handling_contexts.cpp b/libraries/chain/message_handling_contexts.cpp index 098872c5cd7..9501b1e126b 100644 --- a/libraries/chain/message_handling_contexts.cpp +++ b/libraries/chain/message_handling_contexts.cpp @@ -4,11 +4,10 @@ #include #include +#include + #include #include -#include -#include -#include namespace eos { namespace chain { @@ -48,15 +47,7 @@ bool apply_context::all_authorizations_used() const { } vector apply_context::unused_authorizations() const { - auto RemoveUsed = boost::adaptors::filtered([](const auto& tuple) { - return !boost::get<0>(tuple); - }); - auto ToPermission = boost::adaptors::transformed([](const auto& tuple) { - return boost::get<1>(tuple); - }); - - // zip the parallel arrays, filter out the used authorizations, and return just the permissions that are left - auto range = boost::combine(used_authorizations, msg.authorization) | RemoveUsed | ToPermission; + auto range = utilities::FilterDataByMarker(msg.authorization, used_authorizations, false); return {range.begin(), range.end()}; } diff --git a/libraries/utilities/include/eos/utilities/parallel_markers.hpp b/libraries/utilities/include/eos/utilities/parallel_markers.hpp new file mode 100644 index 00000000000..e59b5ba6708 --- /dev/null +++ b/libraries/utilities/include/eos/utilities/parallel_markers.hpp @@ -0,0 +1,39 @@ +#pragma once + +#include +#include +#include + +namespace eos { namespace utilities { + +/** + * @brief Return values in DataRange corresponding to matching Markers + * + * Takes two parallel ranges, a Data range containing data values, and a Marker range containing markers on the + * corresponding data values. Returns a new Data range containing only the values corresponding to markers which match + * markerValue + * + * For example: + * @code{.cpp} + * vector data = {'A', 'B', 'C'}; + * vector markers = {true, false, true}; + * auto markedData = FilterDataByMarker(data, markers, true); + * // markedData contains {'A', 'C'} + * @endcode + */ +template +DataRange FilterDataByMarker(DataRange data, MarkerRange markers, const Marker& markerValue) { + auto RemoveMismatchedMarkers = boost::adaptors::filtered([&markerValue](const auto& tuple) { + return boost::get<0>(tuple) == markerValue; + }); + auto ToData = boost::adaptors::transformed([](const auto& tuple) { + return boost::get<1>(tuple); + }); + + // Zip the ranges together, filter out data with markers that don't match, and return the data without the markers + auto range = boost::combine(markers, data) | RemoveMismatchedMarkers | ToData; + return {range.begin(), range.end()}; +} + +}} // namespace eos::utilities + From 16306e9c03df786c1d604c3886d4094a715bca0d Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Wed, 9 Aug 2017 15:54:25 -0500 Subject: [PATCH 3/7] Ref #7: Upgrade AuthorityChecker I want two things from AuthorityChecker that it wasn't doing yet: 1. I want it to track which of the provided keys were used, so I can reject transactions which bear more signatures than are necessary 2. To sign a transaction with no unnecessary keys, I want it to support taking a list of available public keys and an authority, then telling me which of those keys I should use to fully satisfy the authority, without having any unnecessary keys As an added bonus, having both of these operations handled by AuthorityChecker means that determining the set of keys needed to sign a transaction, and determining whether a transaction is properly signed, use the same code. :D --- libraries/chain/chain_controller.cpp | 4 + .../chain/include/eos/chain/authority.hpp | 51 +------ .../include/eos/chain/authority_checker.hpp | 127 +++++++++++++++++ tests/tests/misc_tests.cpp | 128 ++++++++++++++++-- tests/tests/native_contract_tests.cpp | 1 + 5 files changed, 252 insertions(+), 59 deletions(-) create mode 100644 libraries/chain/include/eos/chain/authority_checker.hpp diff --git a/libraries/chain/chain_controller.cpp b/libraries/chain/chain_controller.cpp index 2f68c5f34fb..b1a5d8897d7 100644 --- a/libraries/chain/chain_controller.cpp +++ b/libraries/chain/chain_controller.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include @@ -505,6 +506,9 @@ void chain_controller::check_transaction_authorization(const SignedTransaction& ("auth", declaredAuthority)); } } + + EOS_ASSERT(checker.all_keys_used(), tx_irrelevant_sig, + "Transaction bears irrelevant signatures from these keys: ${keys}", ("keys", checker.unused_keys())); } ProcessedTransaction chain_controller::apply_transaction(const SignedTransaction& trx, uint32_t skip) diff --git a/libraries/chain/include/eos/chain/authority.hpp b/libraries/chain/include/eos/chain/authority.hpp index 3b68ab211a6..13a218ecdc3 100644 --- a/libraries/chain/include/eos/chain/authority.hpp +++ b/libraries/chain/include/eos/chain/authority.hpp @@ -30,55 +30,8 @@ struct shared_authority { shared_vector keys; }; -/** - * @brief This class determines whether a set of signing keys are sufficient to satisfy an authority or not - * - * To determine whether an authority is satisfied or not, we first determine which keys have approved of a message, and - * then determine whether that list of keys is sufficient to satisfy the authority. This class takes a list of keys and - * provides the @ref satisfied method to determine whether that list of keys satisfies a provided authority. - * - * @tparam F A callable which takes a single argument of type @ref AccountPermission and returns the corresponding - * authority - */ -template -class AuthorityChecker { - F PermissionToAuthority; - flat_set signingKeys; - -public: - AuthorityChecker(F PermissionToAuthority, const flat_set& signingKeys) - : PermissionToAuthority(PermissionToAuthority), signingKeys(signingKeys) {} - - bool satisfied(const types::AccountPermission& permission) const { - return satisfied(PermissionToAuthority(permission)); - } - template - bool satisfied(const AuthorityType& authority) const { - UInt32 weight = 0; - for (const auto& kpw : authority.keys) { - if (signingKeys.count(kpw.key)) { - weight += kpw.weight; - if (weight >= authority.threshold) - return true; - } - } - for (const auto& apw : authority.accounts) -//#warning TODO: Recursion limit? Yes: implement as producer-configurable parameter - if (satisfied(apw.permission)) { - weight += apw.weight; - if (weight >= authority.threshold) - return true; - } - return false; - } -}; - -inline bool operator < ( const types::AccountPermission& a, const types::AccountPermission& b ) { - return std::tie( a.account, a.permission ) < std::tie( b.account, b.permission ); -} -template -AuthorityChecker MakeAuthorityChecker(F&& pta, const flat_set& signingKeys) { - return AuthorityChecker(std::forward(pta), signingKeys); +inline bool operator< (const types::AccountPermission& a, const types::AccountPermission& b) { + return std::tie(a.account, a.permission) < std::tie(b.account, b.permission); } /** diff --git a/libraries/chain/include/eos/chain/authority_checker.hpp b/libraries/chain/include/eos/chain/authority_checker.hpp new file mode 100644 index 00000000000..dca8b39a1d3 --- /dev/null +++ b/libraries/chain/include/eos/chain/authority_checker.hpp @@ -0,0 +1,127 @@ +#pragma once + +#include +#include + +#include + +#include + +#include +#include + +namespace eos { namespace chain { + +namespace detail { +using MetaPermission = static_variant; + +struct GetWeightVisitor { + using result_type = UInt32; + + template + UInt32 operator()(const Permission& permission) { return permission.weight; } +}; + +// Orders permissions descending by weight, and breaks ties with Key permissions being less than Account permissions +struct MetaPermissionComparator { + bool operator()(const MetaPermission& a, const MetaPermission& b) const { + GetWeightVisitor scale; + if (a.visit(scale) > b.visit(scale)) return true; + return a.contains() && !b.contains(); + } +}; + +using MetaPermissionSet = boost::container::flat_multiset; +} + +/** + * @brief This class determines whether a set of signing keys are sufficient to satisfy an authority or not + * + * To determine whether an authority is satisfied or not, we first determine which keys have approved of a message, and + * then determine whether that list of keys is sufficient to satisfy the authority. This class takes a list of keys and + * provides the @ref satisfied method to determine whether that list of keys satisfies a provided authority. + * + * @tparam F A callable which takes a single argument of type @ref AccountPermission and returns the corresponding + * authority + */ +template +class AuthorityChecker { + F PermissionToAuthority; + vector signingKeys; + vector usedKeys; + + struct WeightTallyVisitor { + using result_type = UInt32; + + AuthorityChecker& checker; + UInt32 totalWeight = 0; + + WeightTallyVisitor(AuthorityChecker& checker) + : checker(checker) {} + + UInt32 operator()(const types::KeyPermissionWeight& permission) { + auto itr = boost::find(checker.signingKeys, permission.key); + if (itr != checker.signingKeys.end()) { + checker.usedKeys[itr - checker.signingKeys.begin()] = true; + totalWeight += permission.weight; + } + return totalWeight; + } + UInt32 operator()(const types::AccountPermissionWeight& permission) { + //TODO: Recursion limit? Yes: implement as producer-configurable parameter + if (checker.satisfied(permission.permission)) + totalWeight += permission.weight; + return totalWeight; + } + }; + +public: + AuthorityChecker(F PermissionToAuthority, const flat_set& signingKeys) + : PermissionToAuthority(PermissionToAuthority), + signingKeys(signingKeys.begin(), signingKeys.end()), + usedKeys(signingKeys.size(), false) + {} + + bool satisfied(const types::AccountPermission& permission) { + return satisfied(PermissionToAuthority(permission)); + } + template + bool satisfied(const AuthorityType& authority) { + // Save the current used keys; if we do not satisfy this authority, the newly used keys aren't actually used + auto KeyReverter = fc::make_scoped_exit([this, keys = usedKeys] () mutable { + usedKeys = keys; + }); + + // Sort key permissions and account permissions together into a single set of MetaPermissions + detail::MetaPermissionSet permissions; + permissions.insert(authority.keys.begin(), authority.keys.end()); + permissions.insert(authority.accounts.begin(), authority.accounts.end()); + + // Check all permissions, from highest weight to lowest, seeing if signingKeys satisfies them or not + WeightTallyVisitor visitor(*this); + for (const auto& permission : permissions) + // If we've got enough weight, to satisfy the authority, return! + if (permission.visit(visitor) >= authority.threshold) { + KeyReverter.cancel(); + return true; + } + return false; + } + + bool all_keys_used() const { return boost::algorithm::all_of_equal(usedKeys, true); } + flat_set used_keys() const { + auto range = utilities::FilterDataByMarker(signingKeys, usedKeys, true); + return {range.begin(), range.end()}; + } + flat_set unused_keys() const { + auto range = utilities::FilterDataByMarker(signingKeys, usedKeys, false); + return {range.begin(), range.end()}; + } +}; + +template +AuthorityChecker MakeAuthorityChecker(F&& pta, const flat_set& signingKeys) { + return AuthorityChecker(std::forward(pta), signingKeys); +} + +}} // namespace eos::chain diff --git a/tests/tests/misc_tests.cpp b/tests/tests/misc_tests.cpp index e197926d399..a3e5b6d458a 100644 --- a/tests/tests/misc_tests.cpp +++ b/tests/tests/misc_tests.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include #include @@ -88,13 +88,39 @@ BOOST_AUTO_TEST_CASE(authority_checker) Make_Key(c); auto& c = c_public_key; - auto GetNullAuthority = [](auto){return Authority();}; + auto GetNullAuthority = [](auto){abort(); return Authority();}; auto A = Complex_Authority(2, ((a,1))((b,1)),); - BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {a, b}).satisfied(A)); - BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {a, b, c}).satisfied(A)); - BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {a, c}).satisfied(A)); - BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {b, c}).satisfied(A)); + { + auto checker = MakeAuthorityChecker(GetNullAuthority, {a, b}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 0); + } + { + auto checker = MakeAuthorityChecker(GetNullAuthority, {a, c}); + BOOST_CHECK(!checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 0); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 2); + } + { + auto checker = MakeAuthorityChecker(GetNullAuthority, {a, b, c}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.used_keys().count(a), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(b), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().count(c), 1); + } + { + auto checker = MakeAuthorityChecker(GetNullAuthority, {b, c}); + BOOST_CHECK(!checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 0); + } A = Complex_Authority(3, ((a,1))((b,1))((c,1)),); BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {c, b, a}).satisfied(A)); @@ -115,10 +141,44 @@ BOOST_AUTO_TEST_CASE(authority_checker) auto GetCAuthority = [c_public_key](auto){return Complex_Authority(1, ((c, 1)),);}; A = Complex_Authority(2, ((a, 2))((b, 1)), (("hello", "world", 1))); - BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {a}).satisfied(A)); - BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {b}).satisfied(A)); - BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {c}).satisfied(A)); - BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {b,c}).satisfied(A)); + { + auto checker = MakeAuthorityChecker(GetCAuthority, {a}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(checker.all_keys_used()); + } + { + auto checker = MakeAuthorityChecker(GetCAuthority, {b}); + BOOST_CHECK(!checker.satisfied(A)); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 0); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().count(b), 1); + } + { + auto checker = MakeAuthorityChecker(GetCAuthority, {c}); + BOOST_CHECK(!checker.satisfied(A)); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 0); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().count(c), 1); + } + { + auto checker = MakeAuthorityChecker(GetCAuthority, {b,c}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 0); + BOOST_CHECK_EQUAL(checker.used_keys().count(b), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(c), 1); + } + { + auto checker = MakeAuthorityChecker(GetCAuthority, {b,c,a}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(a), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.unused_keys().count(b), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().count(c), 1); + } A = Complex_Authority(2, ((a, 1))((b, 1)), (("hello", "world", 1))); BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {a}).satisfied(A)); @@ -127,12 +187,60 @@ BOOST_AUTO_TEST_CASE(authority_checker) BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {a,b}).satisfied(A)); BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {b,c}).satisfied(A)); BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {a,c}).satisfied(A)); + { + auto checker = MakeAuthorityChecker(GetCAuthority, {a,b,c}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().count(c), 1); + } A = Complex_Authority(2, ((a, 1))((b, 1)), (("hello", "world", 2))); BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {a,b}).satisfied(A)); BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {c}).satisfied(A)); BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {a}).satisfied(A)); BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {b}).satisfied(A)); + { + auto checker = MakeAuthorityChecker(GetCAuthority, {a,b,c}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.used_keys().count(c), 1); + } + + Make_Key(d); + auto& d = d_public_key; + Make_Key(e); + auto& e = e_public_key; + + auto GetAuthority = [d_public_key, e] (const types::AccountPermission& perm) { + if (perm.account == "top") + return Complex_Authority(2, ((d, 1)), (("bottom", "bottom", 1))); + return Key_Authority(e); + }; + + A = Complex_Authority(5, ((a, 2))((b, 2))((c, 2)), (("top", "top", 5))); + { + auto checker = MakeAuthorityChecker(GetAuthority, {a,b,c,d,e}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 3); + BOOST_CHECK_EQUAL(checker.used_keys().count(d), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(e), 1); + } + { + auto checker = MakeAuthorityChecker(GetAuthority, {a,b,c,e}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 3); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(a), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(b), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(c), 1); + } } FC_LOG_AND_RETHROW() } /// Test creating the wallet diff --git a/tests/tests/native_contract_tests.cpp b/tests/tests/native_contract_tests.cpp index 4ccf99f080a..09d07443b25 100644 --- a/tests/tests/native_contract_tests.cpp +++ b/tests/tests/native_contract_tests.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include From 7eb2013d7be2fbf761c052157962ced4d72c4402 Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Wed, 9 Aug 2017 16:31:45 -0500 Subject: [PATCH 4/7] Add support for auto-signing testing transactions Implement automatic signing of transactions in the test framework. Currently disabled, as it's not yet working --- tests/common/database_fixture.cpp | 37 +++++++++++++++++++++++++++++++ tests/common/database_fixture.hpp | 20 +++++++++++++++++ tests/common/macro_support.hpp | 24 ++++++++++---------- 3 files changed, 69 insertions(+), 12 deletions(-) diff --git a/tests/common/database_fixture.cpp b/tests/common/database_fixture.cpp index e04ae6f037d..a1e25be44f3 100644 --- a/tests/common/database_fixture.cpp +++ b/tests/common/database_fixture.cpp @@ -27,6 +27,7 @@ #include #include +#include #include @@ -37,6 +38,8 @@ #include #include +#include + #include #include #include @@ -94,9 +97,15 @@ private_key_type testing_fixture::get_private_key(const public_key_type& public_ return itr->second; } +flat_set testing_fixture::available_keys() const { + auto range = key_ring | boost::adaptors::map_keys; + return {range.begin(), range.end()}; +} + testing_blockchain::testing_blockchain(chainbase::database& db, fork_database& fork_db, block_log& blocklog, chain_initializer_interface& initializer, testing_fixture& fixture) : chain_controller(db, fork_db, blocklog, initializer, native_contract::make_administrator()), + db(db), fixture(fixture) {} void testing_blockchain::produce_blocks(uint32_t count, uint32_t blocks_to_miss) { @@ -157,6 +166,34 @@ types::PublicKey testing_blockchain::get_block_signing_key(const types::AccountN return get_database().get(producerName).signing_key; } +void testing_blockchain::sign_transaction(SignedTransaction& trx) { + auto GetAuthority = [this](const types::AccountPermission& permission) { + auto key = boost::make_tuple(permission.account, permission.permission); + return db.get(key).auth; + }; + auto checker = MakeAuthorityChecker(GetAuthority, fixture.available_keys()); + + for (const auto& message : trx.messages) + for (const auto& authorization : message.authorization) + if (!checker.satisfied(authorization)) + elog("Attempting to automatically sign transaction, but testing_fixture doesn't have the keys!"); + + for (const auto& key : checker.used_keys()) + // TODO: Use a real chain_id here + trx.sign(fixture.get_private_key(key), chain_id_type{}); +} + +ProcessedTransaction testing_blockchain::push_transaction(SignedTransaction trx, uint32_t skip_flags) { + if (skip_trx_sigs) + skip_flags |= chain_controller::skip_transaction_signatures; + + if (auto_sign_trxs) { + sign_transaction(trx); + } + + return chain_controller::push_transaction(trx, skip_flags); +} + void testing_network::connect_blockchain(testing_blockchain& new_database) { if (blockchains.count(&new_database)) return; diff --git a/tests/common/database_fixture.hpp b/tests/common/database_fixture.hpp index b655700cd88..fd6d95603cb 100644 --- a/tests/common/database_fixture.hpp +++ b/tests/common/database_fixture.hpp @@ -119,6 +119,7 @@ class testing_fixture { void store_private_key(const private_key_type& key); private_key_type get_private_key(const public_key_type& public_key) const; + flat_set available_keys() const; protected: std::vector anonymous_temp_dirs; @@ -178,8 +179,27 @@ class testing_blockchain : public chain_controller { /// @brief Get the specified block producer's signing key PublicKey get_block_signing_key(const AccountName& producerName); + /// @brief Attempt to sign the provided transaction using the keys available to the testing_fixture + void sign_transaction(SignedTransaction& trx); + + /// @brief Override push_transaction to apply testing policies + ProcessedTransaction push_transaction(SignedTransaction trx, uint32_t skip_flags = 0); + + /// @brief Set whether testing_blockchain::push_transaction checks signatures by default + /// @param skip_sigs If true, push_transaction will skip signature checks; otherwise, no changes will be made + void set_skip_transaction_signature_checking(bool skip_sigs) { + skip_trx_sigs = skip_sigs; + } + /// @brief Set whether testing_blockchain::push_transaction attempts to sign transactions or not + void set_auto_sign_transactions(bool auto_sign) { + auto_sign_trxs = auto_sign; + } + protected: + chainbase::database& db; testing_fixture& fixture; + bool skip_trx_sigs = true; + bool auto_sign_trxs = false; }; using boost::signals2::scoped_connection; diff --git a/tests/common/macro_support.hpp b/tests/common/macro_support.hpp index ee9df2096c7..bfec2829676 100644 --- a/tests/common/macro_support.hpp +++ b/tests/common/macro_support.hpp @@ -51,7 +51,7 @@ inline std::vector sort_names( std::vector&& names ) { "newaccount", types::newaccount{#creator, #name, owner, active, recovery, deposit}); \ trx.expiration = chain.head_block_time() + 100; \ trx.set_reference_block(chain.head_block_id()); \ - chain.push_transaction(trx, chain_controller::skip_transaction_signatures); \ + chain.push_transaction(trx); \ BOOST_TEST_CHECKPOINT("Created account " << #name); \ } #define MKACCT2(chain, name) \ @@ -83,7 +83,7 @@ inline std::vector sort_names( std::vector&& names ) { "updateauth", types::updateauth{#account, authname, parentname, auth}); \ trx.expiration = chain.head_block_time() + 100; \ trx.set_reference_block(chain.head_block_id()); \ - chain.push_transaction(trx, chain_controller::skip_transaction_signatures); \ + chain.push_transaction(trx); \ BOOST_TEST_CHECKPOINT("Set " << #account << "'s " << authname << " authority."); \ } @@ -96,7 +96,7 @@ inline std::vector sort_names( std::vector&& names ) { "deleteauth", types::deleteauth{#account, authname}); \ trx.expiration = chain.head_block_time() + 100; \ trx.set_reference_block(chain.head_block_id()); \ - chain.push_transaction(trx, chain_controller::skip_transaction_signatures); \ + chain.push_transaction(trx); \ BOOST_TEST_CHECKPOINT("Deleted " << #account << "'s " << authname << " authority."); \ } @@ -109,7 +109,7 @@ inline std::vector sort_names( std::vector&& names ) { "linkauth", types::linkauth{#account, #codeacct, messagetype, authname}); \ trx.expiration = chain.head_block_time() + 100; \ trx.set_reference_block(chain.head_block_id()); \ - chain.push_transaction(trx, chain_controller::skip_transaction_signatures); \ + chain.push_transaction(trx); \ BOOST_TEST_CHECKPOINT("Link " << #codeacct << "::" << messagetype << " to " << #account \ << "'s " << authname << " authority."); \ } @@ -124,7 +124,7 @@ inline std::vector sort_names( std::vector&& names ) { "unlinkauth", types::unlinkauth{#account, #codeacct, messagetype}); \ trx.expiration = chain.head_block_time() + 100; \ trx.set_reference_block(chain.head_block_id()); \ - chain.push_transaction(trx, chain_controller::skip_transaction_signatures); \ + chain.push_transaction(trx); \ BOOST_TEST_CHECKPOINT("Unlink " << #codeacct << "::" << messagetype << " from " << #account); \ } #define LINKAUTH3(chain, account, codeacct) LINKAUTH5(chain, account, codeacct, "") @@ -138,7 +138,7 @@ inline std::vector sort_names( std::vector&& names ) { "transfer", types::transfer{#sender, #recipient, Amount.amount}); \ trx.expiration = chain.head_block_time() + 100; \ trx.set_reference_block(chain.head_block_id()); \ - chain.push_transaction(trx, chain_controller::skip_transaction_signatures); \ + chain.push_transaction(trx); \ BOOST_TEST_CHECKPOINT("Transfered " << Amount << " from " << #sender << " to " << #recipient); \ } #define XFER4(chain, sender, recipient, amount) XFER5(chain, sender, recipient, amount, "") @@ -151,7 +151,7 @@ inline std::vector sort_names( std::vector&& names ) { "lock", types::lock{#sender, #recipient, amount}); \ trx.expiration = chain.head_block_time() + 100; \ trx.set_reference_block(chain.head_block_id()); \ - chain.push_transaction(trx, chain_controller::skip_transaction_signatures); \ + chain.push_transaction(trx); \ BOOST_TEST_CHECKPOINT("Staked " << amount << " to " << #recipient); \ } #define STAKE3(chain, account, amount) STAKE4(chain, account, account, amount) @@ -165,7 +165,7 @@ inline std::vector sort_names( std::vector&& names ) { "unlock", types::unlock{#account, amount}); \ trx.expiration = chain.head_block_time() + 100; \ trx.set_reference_block(chain.head_block_id()); \ - chain.push_transaction(trx, chain_controller::skip_transaction_signatures); \ + chain.push_transaction(trx); \ BOOST_TEST_CHECKPOINT("Begin unstake " << amount << " to " << #account); \ } @@ -177,7 +177,7 @@ inline std::vector sort_names( std::vector&& names ) { "claim", types::claim{#account, amount}); \ trx.expiration = chain.head_block_time() + 100; \ trx.set_reference_block(chain.head_block_id()); \ - chain.push_transaction(trx, chain_controller::skip_transaction_signatures); \ + chain.push_transaction(trx); \ BOOST_TEST_CHECKPOINT("Finish unstake " << amount << " to " << #account); \ } @@ -190,7 +190,7 @@ inline std::vector sort_names( std::vector&& names ) { "setproducer", types::setproducer{#owner, key, cfg}); \ trx.expiration = chain.head_block_time() + 100; \ trx.set_reference_block(chain.head_block_id()); \ - chain.push_transaction(trx, chain_controller::skip_transaction_signatures); \ + chain.push_transaction(trx); \ BOOST_TEST_CHECKPOINT("Create producer " << #owner); \ } #define MKPDCR3(chain, owner, key) MKPDCR4(chain, owner, key, BlockchainConfiguration{}); @@ -207,7 +207,7 @@ inline std::vector sort_names( std::vector&& names ) { "okproducer", types::okproducer{#voter, #producer, approved? 1 : 0}); \ trx.expiration = chain.head_block_time() + 100; \ trx.set_reference_block(chain.head_block_id()); \ - chain.push_transaction(trx, chain_controller::skip_transaction_signatures); \ + chain.push_transaction(trx); \ BOOST_TEST_CHECKPOINT("Set producer approval from " << #voter << " for " << #producer << " to " << approved); \ } @@ -220,7 +220,7 @@ inline std::vector sort_names( std::vector&& names ) { "setproducer", types::setproducer{owner, key, cfg}); \ trx.expiration = chain.head_block_time() + 100; \ trx.set_reference_block(chain.head_block_id()); \ - chain.push_transaction(trx, chain_controller::skip_transaction_signatures); \ + chain.push_transaction(trx); \ BOOST_TEST_CHECKPOINT("Update producer " << owner); \ } #define UPPDCR3(chain, owner, key) UPPDCR4(chain, owner, key, chain.get_producer(owner).configuration) From 4bb63e76ecaab3b2609e7fc258a14b522a7597fb Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Wed, 9 Aug 2017 18:00:14 -0500 Subject: [PATCH 5/7] Move validate_referenced_accounts call to block/push level Previously, validate_referenced_accounts was called during transaction application; however, account creation is one of those changes that doesn't really take effect until the end of the block, so we want to validate the referenced accounts for all transactions at the beginning of the block so that if trx A creates account joe, then trx B references joe, that reference will fail if A and B are in the same block. We also call it during push_transaction, as we do want to check it for pending transactions that aren't in a block yet. --- libraries/chain/chain_controller.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libraries/chain/chain_controller.cpp b/libraries/chain/chain_controller.cpp index b1a5d8897d7..ffff7d970b3 100644 --- a/libraries/chain/chain_controller.cpp +++ b/libraries/chain/chain_controller.cpp @@ -246,6 +246,7 @@ ProcessedTransaction chain_controller::_push_transaction(const SignedTransaction _pending_tx_session = _db.start_undo_session(true); auto temp_session = _db.start_undo_session(true); + validate_referenced_accounts(trx); check_transaction_authorization(trx); auto pt = _apply_transaction(trx); _pending_transactions.push_back(trx); @@ -331,6 +332,7 @@ signed_block chain_controller::_generate_block( try { auto temp_session = _db.start_undo_session(true); + validate_referenced_accounts(tx); check_transaction_authorization(tx); _apply_transaction(tx); temp_session.squash(); @@ -441,8 +443,10 @@ void chain_controller::_apply_block(const signed_block& next_block) for (const auto& cycle : next_block.cycles) for (const auto& thread : cycle) - for (const auto& trx : thread.user_input) + for (const auto& trx : thread.user_input) { + validate_referenced_accounts(trx); check_transaction_authorization(trx); + } /* We do not need to push the undo state for each transaction * because they either all apply and are valid or the @@ -524,7 +528,6 @@ try { validate_expiration(trx); validate_uniqueness(trx); validate_tapos(trx); - validate_referenced_accounts(trx); } FC_CAPTURE_AND_RETHROW( (trx) ) } From 2b30499fdb479c61f9cf7c0f9a42ec3e50807feb Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Wed, 9 Aug 2017 18:03:24 -0500 Subject: [PATCH 6/7] Fix test broken by last commit --- tests/tests/native_contract_tests.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/tests/native_contract_tests.cpp b/tests/tests/native_contract_tests.cpp index 09d07443b25..fda3b9585d4 100644 --- a/tests/tests/native_contract_tests.cpp +++ b/tests/tests/native_contract_tests.cpp @@ -34,6 +34,7 @@ BOOST_FIXTURE_TEST_CASE(create_account, testing_fixture) BOOST_CHECK_EQUAL(chain.get_liquid_balance("inita"), Asset(100000)); Make_Account(chain, joe, inita, Asset(1000)); + chain.produce_blocks(); Transfer_Asset(chain, inita, joe, Asset(1000)); { // test in the pending state From afa2fe2d755cdfa42d11196ef454b670ce09791b Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Thu, 10 Aug 2017 09:32:43 -0500 Subject: [PATCH 7/7] validate(Authority&) -> validate(const Authority&) --- libraries/chain/include/eos/chain/authority.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/include/eos/chain/authority.hpp b/libraries/chain/include/eos/chain/authority.hpp index 13a218ecdc3..95845f6a486 100644 --- a/libraries/chain/include/eos/chain/authority.hpp +++ b/libraries/chain/include/eos/chain/authority.hpp @@ -38,7 +38,7 @@ inline bool operator< (const types::AccountPermission& a, const types::AccountPe * Makes sure all keys are unique and sorted and all account permissions are unique and sorted and that authority can * be satisfied */ -inline bool validate( types::Authority& auth ) { +inline bool validate(const types::Authority& auth) { const types::KeyPermissionWeight* prev = nullptr; decltype(auth.threshold) totalWeight = 0;