From 040218fdf18f616973c2a93146e777a4ad7baa96 Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Thu, 10 Aug 2017 15:19:10 -0500 Subject: [PATCH 1/2] Resolve #152: Disable auth updates in pending transactions Affected contract methods: - newaccount - updateauth - deleteauth - linkauth - unlinkauth Also, blockchain initialization (the genesis block) behaves like it is applying a block, even though the genesis block isn't really a block. --- libraries/chain/chain_controller.cpp | 7 +++ .../include/eos/chain/chain_controller.hpp | 7 ++- libraries/native_contract/eos_contract.cpp | 54 ++++++++++--------- tests/tests/native_contract_tests.cpp | 24 +++++++++ 4 files changed, 67 insertions(+), 25 deletions(-) diff --git a/libraries/chain/chain_controller.cpp b/libraries/chain/chain_controller.cpp index 3b5adeae55b..ad103acd6e4 100644 --- a/libraries/chain/chain_controller.cpp +++ b/libraries/chain/chain_controller.cpp @@ -432,6 +432,9 @@ void chain_controller::apply_block(const signed_block& next_block, uint32_t skip void chain_controller::_apply_block(const signed_block& next_block) { try { + auto UnsetApplyingBlock = fc::make_scoped_exit([this] { _currently_applying_block = false; }); + _currently_applying_block = true; + uint32_t next_block_num = next_block.block_num(); uint32_t skip = _skip_flags; @@ -836,6 +839,10 @@ void chain_controller::initialize_indexes() { void chain_controller::initialize_chain(chain_initializer_interface& starter) { try { + // Behave as though we are applying a block during chain initialization (it's the genesis block!) + auto UnsetApplyingBlock = fc::make_scoped_exit([this] { _currently_applying_block = false; }); + _currently_applying_block = true; + if (!_db.find()) { _db.with_write_lock([this, &starter] { auto initial_timestamp = starter.get_chain_start_time(); diff --git a/libraries/chain/include/eos/chain/chain_controller.hpp b/libraries/chain/include/eos/chain/chain_controller.hpp index 6f34d98343e..21c37e235bc 100644 --- a/libraries/chain/include/eos/chain/chain_controller.hpp +++ b/libraries/chain/include/eos/chain/chain_controller.hpp @@ -73,6 +73,11 @@ namespace eos { namespace chain { */ signal on_pending_transaction; + /** + * @brief Check whether the controller is currently applying a block or not + * @return True if the controller is now applying a block; false otherwise + */ + bool is_applying_block()const { return _currently_applying_block; } /** * The controller can override any script endpoint with native code. @@ -321,7 +326,7 @@ namespace eos { namespace chain { optional _pending_tx_session; deque _pending_transactions; - bool _pushing = false; + bool _currently_applying_block = false; uint64_t _skip_flags = 0; flat_map _checkpoints; diff --git a/libraries/native_contract/eos_contract.cpp b/libraries/native_contract/eos_contract.cpp index bd551a0d813..8a39ff9cb27 100644 --- a/libraries/native_contract/eos_contract.cpp +++ b/libraries/native_contract/eos_contract.cpp @@ -1,5 +1,6 @@ #include +#include #include #include #include @@ -56,19 +57,20 @@ void apply_eos_newaccount(apply_context& context) { a.name = create.name; a.creation_date = db.get(dynamic_global_property_object::id_type()).time; }); - const auto& owner_permission = db.create([&create, &new_account](permission_object& p) { - p.name = "owner"; - p.parent = 0; - p.owner = new_account.name; - p.auth = std::move(create.owner); - }); - db.create([&create, &owner_permission](permission_object& p) { - p.name = "active"; - p.parent = owner_permission.id; - p.owner = owner_permission.owner; - p.auth = std::move(create.active); - }); - + if (context.controller.is_applying_block()) { + const auto& owner_permission = db.create([&create, &new_account](permission_object& p) { + p.name = "owner"; + p.parent = 0; + p.owner = new_account.name; + p.auth = std::move(create.owner); + }); + db.create([&create, &owner_permission](permission_object& p) { + p.name = "active"; + p.parent = owner_permission.id; + p.owner = owner_permission.owner; + p.auth = std::move(create.active); + }); + } const auto& creatorBalance = context.mutable_db.get(create.creator); @@ -388,11 +390,12 @@ void apply_eos_updateauth(apply_context& context) { if (permission) { EOS_ASSERT(parent.id == permission->parent, message_precondition_exception, "Changing parent authority is not currently supported"); - db.modify(*permission, [&update, parent = parent.id](permission_object& po) { - po.auth = update.authority; - po.parent = parent; - }); - } else { + if (context.controller.is_applying_block()) + db.modify(*permission, [&update, parent = parent.id](permission_object& po) { + po.auth = update.authority; + po.parent = parent; + }); + } else if (context.controller.is_applying_block()) { db.create([&update, parent = parent.id](permission_object& po) { po.name = update.permission; po.owner = update.account; @@ -425,7 +428,8 @@ void apply_eos_deleteauth(apply_context& context) { "Cannot delete a linked authority. Unlink the authority first"); } - db.remove(permission); + if (context.controller.is_applying_block()) + db.remove(permission); } void apply_eos_linkauth(apply_context& context) { @@ -445,10 +449,11 @@ void apply_eos_linkauth(apply_context& context) { if (link) { EOS_ASSERT(link->required_permission != requirement.requirement, message_precondition_exception, "Attempting to update required authority, but new requirement is same as old"); - db.modify(*link, [requirement = requirement.requirement](permission_link_object& link) { - link.required_permission = requirement; - }); - } else { + if (context.controller.is_applying_block()) + db.modify(*link, [requirement = requirement.requirement](permission_link_object& link) { + link.required_permission = requirement; + }); + } else if (context.controller.is_applying_block()) { db.create([&requirement](permission_link_object& link) { link.account = requirement.account; link.code = requirement.code; @@ -467,7 +472,8 @@ void apply_eos_unlinkauth(apply_context& context) { auto linkKey = boost::make_tuple(unlink.account, unlink.code, unlink.type); auto link = db.find(linkKey); EOS_ASSERT(link != nullptr, message_precondition_exception, "Attempting to unlink authority, but no link found"); - db.remove(*link); + if (context.controller.is_applying_block()) + db.remove(*link); } } // namespace eos diff --git a/tests/tests/native_contract_tests.cpp b/tests/tests/native_contract_tests.cpp index 67acc728617..90067a06c8b 100644 --- a/tests/tests/native_contract_tests.cpp +++ b/tests/tests/native_contract_tests.cpp @@ -400,6 +400,10 @@ BOOST_FIXTURE_TEST_CASE(auth_tests, testing_fixture) { Make_Key(k2); Set_Authority(chain, alice, "spending", "active", Key_Authority(k1_public_key)); + // Ensure authority doesn't appear until next block + BOOST_CHECK_EQUAL((chain_db.find(boost::make_tuple("alice", "spending"))), nullptr); + chain.produce_blocks(); + { auto obj = chain_db.find(boost::make_tuple("alice", "spending")); BOOST_CHECK_NE(obj, nullptr); @@ -414,6 +418,8 @@ BOOST_FIXTURE_TEST_CASE(auth_tests, testing_fixture) { BOOST_CHECK_THROW(Set_Authority(chain, alice, "spending", "owner", Key_Authority(k1_public_key)), message_precondition_exception); Delete_Authority(chain, alice, "spending"); + BOOST_CHECK_NE((chain_db.find(boost::make_tuple("alice", "spending"))), nullptr); + chain.produce_blocks(); { auto obj = chain_db.find(boost::make_tuple("alice", "spending")); @@ -423,7 +429,11 @@ BOOST_FIXTURE_TEST_CASE(auth_tests, testing_fixture) { chain.produce_blocks(); Set_Authority(chain, alice, "trading", "active", Key_Authority(k1_public_key)); + BOOST_CHECK_EQUAL((chain_db.find(boost::make_tuple("alice", "trading"))), nullptr); + chain.produce_blocks(); Set_Authority(chain, alice, "spending", "trading", Key_Authority(k2_public_key)); + BOOST_CHECK_EQUAL((chain_db.find(boost::make_tuple("alice", "spending"))), nullptr); + chain.produce_blocks(); { auto trading = chain_db.find(boost::make_tuple("alice", "trading")); @@ -456,7 +466,11 @@ BOOST_FIXTURE_TEST_CASE(auth_tests, testing_fixture) { BOOST_CHECK_THROW(Set_Authority(chain, alice, "spending", "active", Key_Authority(k1_public_key)), message_precondition_exception); Delete_Authority(chain, alice, "spending"); + BOOST_CHECK_NE((chain_db.find(boost::make_tuple("alice", "spending"))), nullptr); + chain.produce_blocks(); Delete_Authority(chain, alice, "trading"); + BOOST_CHECK_NE((chain_db.find(boost::make_tuple("alice", "trading"))), nullptr); + chain.produce_blocks(); { auto trading = chain_db.find(boost::make_tuple("alice", "trading")); @@ -475,8 +489,14 @@ BOOST_FIXTURE_TEST_CASE(auth_links, testing_fixture) { try { Make_Key(scud); Set_Authority(chain, alice, "spending", "active", Key_Authority(spending_public_key)); + chain.produce_blocks(); Set_Authority(chain, alice, "scud", "spending", Key_Authority(scud_public_key)); + chain.produce_blocks(); Link_Authority(chain, alice, "spending", eos, "transfer"); + BOOST_CHECK_EQUAL( + (chain_db.find(boost::make_tuple("alice", "eos", "transfer"))), + nullptr); + chain.produce_blocks(); { auto obj = chain_db.find(boost::make_tuple("alice", "eos", "transfer")); @@ -487,6 +507,10 @@ BOOST_FIXTURE_TEST_CASE(auth_links, testing_fixture) { try { } Unlink_Authority(chain, alice, eos, "transfer"); + BOOST_CHECK_NE( + (chain_db.find(boost::make_tuple("alice", "eos", "transfer"))), + nullptr); + chain.produce_blocks(); { auto obj = chain_db.find(boost::make_tuple("alice", "eos", "transfer")); From 4b9752a3a59f1f74c557c947b311f38188d966c3 Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Thu, 10 Aug 2017 15:40:03 -0500 Subject: [PATCH 2/2] Ref #152: Add with_applying_block --- libraries/chain/chain_controller.cpp | 21 +++++++++++-------- .../include/eos/chain/chain_controller.hpp | 9 ++++++++ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/libraries/chain/chain_controller.cpp b/libraries/chain/chain_controller.cpp index ad103acd6e4..25b0d6daa41 100644 --- a/libraries/chain/chain_controller.cpp +++ b/libraries/chain/chain_controller.cpp @@ -427,14 +427,16 @@ void chain_controller::apply_block(const signed_block& next_block, uint32_t skip if (_checkpoints.rbegin()->first >= block_num) skip = ~0;// WE CAN SKIP ALMOST EVERYTHING } - with_skip_flags(skip, [&](){ _apply_block(next_block); }); + + with_applying_block([&] { + with_skip_flags(skip, [&] { + _apply_block(next_block); + }); + }); } void chain_controller::_apply_block(const signed_block& next_block) { try { - auto UnsetApplyingBlock = fc::make_scoped_exit([this] { _currently_applying_block = false; }); - _currently_applying_block = true; - uint32_t next_block_num = next_block.block_num(); uint32_t skip = _skip_flags; @@ -839,10 +841,6 @@ void chain_controller::initialize_indexes() { void chain_controller::initialize_chain(chain_initializer_interface& starter) { try { - // Behave as though we are applying a block during chain initialization (it's the genesis block!) - auto UnsetApplyingBlock = fc::make_scoped_exit([this] { _currently_applying_block = false; }); - _currently_applying_block = true; - if (!_db.find()) { _db.with_write_lock([this, &starter] { auto initial_timestamp = starter.get_chain_start_time(); @@ -883,7 +881,12 @@ chain_controller::chain_controller(database& database, fork_database& fork_db, b initialize_indexes(); starter.register_types(*this, _db); - initialize_chain(starter); + + // Behave as though we are applying a block during chain initialization (it's the genesis block!) + with_applying_block([&] { + initialize_chain(starter); + }); + spinup_db(); spinup_fork_db(); diff --git a/libraries/chain/include/eos/chain/chain_controller.hpp b/libraries/chain/include/eos/chain/chain_controller.hpp index 21c37e235bc..d56df89baa8 100644 --- a/libraries/chain/include/eos/chain/chain_controller.hpp +++ b/libraries/chain/include/eos/chain/chain_controller.hpp @@ -260,6 +260,15 @@ namespace eos { namespace chain { void apply_block(const signed_block& next_block, uint32_t skip = skip_nothing); void _apply_block(const signed_block& next_block); + template + auto with_applying_block(Function&& f) -> decltype((*((Function*)nullptr))()) { + auto on_exit = fc::make_scoped_exit([this](){ + _currently_applying_block = false; + }); + _currently_applying_block = true; + return f(); + } + void check_transaction_authorization(const SignedTransaction& trx)const; ProcessedTransaction apply_transaction(const SignedTransaction& trx, uint32_t skip = skip_nothing);