From b75983c587bb9a6cd38c56517c45d82df3aa29b3 Mon Sep 17 00:00:00 2001 From: Bart Wyatt Date: Thu, 7 Sep 2017 18:08:20 -0400 Subject: [PATCH] test for a deferred transaction via proxy account exposed many bugs in the contract code and producer fixed what I found ref EOSIO/eos#175 --- contracts/CMakeLists.txt | 1 + contracts/eoslib/token.hpp | 8 +- contracts/eoslib/transaction.h | 2 +- contracts/eoslib/transaction.hpp | 20 +++-- contracts/proxy/CMakeLists.txt | 2 + contracts/proxy/proxy.cpp | 66 +++++++++++++++++ contracts/proxy/proxy.hpp | 13 ++++ libraries/chain/block.cpp | 10 +-- libraries/chain/chain_controller.cpp | 30 ++++---- libraries/chain/wasm_interface.cpp | 17 ++++- tests/slow_tests/deferred_tests.cpp | 105 +++++++++++++++++++++++++++ 11 files changed, 241 insertions(+), 33 deletions(-) create mode 100644 contracts/proxy/CMakeLists.txt create mode 100644 contracts/proxy/proxy.cpp create mode 100644 contracts/proxy/proxy.hpp create mode 100644 tests/slow_tests/deferred_tests.cpp diff --git a/contracts/CMakeLists.txt b/contracts/CMakeLists.txt index ddc318089b7..62adf0c349f 100644 --- a/contracts/CMakeLists.txt +++ b/contracts/CMakeLists.txt @@ -1,6 +1,7 @@ add_subdirectory(currency) add_subdirectory(exchange) add_subdirectory(infinite) +add_subdirectory(proxy) add_subdirectory(test_api) add_subdirectory(simpledb) #add_subdirectory(social) diff --git a/contracts/eoslib/token.hpp b/contracts/eoslib/token.hpp index 44ffc7ae006..fe4cbdac548 100644 --- a/contracts/eoslib/token.hpp +++ b/contracts/eoslib/token.hpp @@ -381,7 +381,7 @@ namespace eos { * MeToYou.quantity = Tokens(100); * @endcode */ - struct Transfer { + struct PACKED(Transfer) { /** * Defines transfer action type * @brief Defines transfer action type @@ -402,6 +402,12 @@ namespace eos { * @brief Quantity of token to be transferred */ Tokens quantity; + + /** + * Length of the memo field, included for binary compatibility + * @brief Length of the memo field + */ + const uint8_t memo_length = 0; }; /// @} tokenhppapi } // namespace eos diff --git a/contracts/eoslib/transaction.h b/contracts/eoslib/transaction.h index bf223b2d955..645bf0fc6d4 100644 --- a/contracts/eoslib/transaction.h +++ b/contracts/eoslib/transaction.h @@ -136,7 +136,7 @@ extern "C" { * @param data - the payload data for this message * @param size - the size of `data` */ - MessageHandle messageCreate(AccountName code, FuncName type, void* data, int size); + MessageHandle messageCreate(AccountName code, FuncName type, void const* data, int size); /** * @brief require a permission for the pending message diff --git a/contracts/eoslib/transaction.hpp b/contracts/eoslib/transaction.hpp index 1de441ed532..ff77edb7449 100644 --- a/contracts/eoslib/transaction.hpp +++ b/contracts/eoslib/transaction.hpp @@ -17,13 +17,24 @@ namespace eos { class Transaction; class Message { public: - template - Message(const AccountName& code, const FuncName& type, const Message& message, Permissions... permissions ) - : handle(messageCreate(code, type, &message, sizeof(Message))) + template + Message(const AccountName& code, const FuncName& type, const Payload& payload, Permissions... permissions ) + : handle(messageCreate(code, type, &payload, sizeof(Payload))) { addPermissions(permissions...); } + template + Message(const AccountName& code, const FuncName& type, const Payload& payload ) + : handle(messageCreate(code, type, &payload, sizeof(Payload))) + { + } + + Message(const AccountName& code, const FuncName& type) + : handle(messageCreate(code, type, nullptr, 0)) + { + } + // no copy construtor due to opaque handle Message( const Message& ) = delete; @@ -87,12 +98,11 @@ namespace eos { } } - void addScope(AccountName scope, bool readOnly) { + void addScope(AccountName scope, bool readOnly = false) { assertValidHandle(); transactionRequireScope(handle, scope, readOnly ? 1 : 0); } - template void addMessage(Message &message) { assertValidHandle(); message.assertValidHandle(); diff --git a/contracts/proxy/CMakeLists.txt b/contracts/proxy/CMakeLists.txt new file mode 100644 index 00000000000..b2606ed5cc8 --- /dev/null +++ b/contracts/proxy/CMakeLists.txt @@ -0,0 +1,2 @@ +file(GLOB SOURCE_FILES "*.cpp") +add_wast_target(proxy "${SOURCE_FILES}" "${CMAKE_SOURCE_DIR}/contracts" ${CMAKE_CURRENT_SOURCE_DIR}) diff --git a/contracts/proxy/proxy.cpp b/contracts/proxy/proxy.cpp new file mode 100644 index 00000000000..c6183894a3d --- /dev/null +++ b/contracts/proxy/proxy.cpp @@ -0,0 +1,66 @@ +#include +#include + +namespace proxy { + using namespace eos; + + template + void apply_transfer(AccountName code, const T& transfer) { + const auto self = currentCode(); + Config config; + assert(Configs::get(config, self), "Attempting to use unconfigured proxy"); + if (transfer.from == self) { + assert(transfer.to == config.owner, "proxy may only pay its owner" ); + } else { + assert(transfer.to == self, "proxy is not involved in this transfer"); + T newTransfer = T(transfer); + newTransfer.from = self; + newTransfer.to = config.owner; + + auto outMsg = Message(code, N(transfer), newTransfer, self, N(code)); + Transaction out; + out.addMessage(outMsg); + out.addScope(self); + out.addScope(config.owner); + out.send(); + } + } + + void apply_setowner(AccountName owner) { + const auto self = currentCode(); + Config config; + bool configured = Configs::get(config, self); + config.owner = owner; + if (configured) { + Configs::update(config, self); + } else { + Configs::store(config, self); + } + } + +} + +using namespace proxy; +using namespace eos; + +extern "C" { + void init() { + } + + /// The apply method implements the dispatch of events to this contract + void apply( uint64_t code, uint64_t action ) { + if( code == N(currency) ) { + if( action == N(transfer) ) { + apply_transfer(code, currentMessage()); + } + } else if ( code == N(eos) ) { + if( action == N(transfer) ) { + apply_transfer(code, currentMessage()); + } + } else if (code == N(proxy) ) { + if ( action == N(setowner)) { + apply_setowner(currentMessage()); + } + } + } +} diff --git a/contracts/proxy/proxy.hpp b/contracts/proxy/proxy.hpp new file mode 100644 index 00000000000..76aad80ebc5 --- /dev/null +++ b/contracts/proxy/proxy.hpp @@ -0,0 +1,13 @@ +#include +#include + +namespace proxy { + struct PACKED( Config ) { + Config( AccountName o = AccountName() ):owner(o){} + const uint64_t key = N(config); + AccountName owner; + }; + + using Configs = Table; + +} /// namespace proxy \ No newline at end of file diff --git a/libraries/chain/block.cpp b/libraries/chain/block.cpp index 27a4e352087..1e761f0a1c9 100644 --- a/libraries/chain/block.cpp +++ b/libraries/chain/block.cpp @@ -104,14 +104,8 @@ namespace eos { namespace chain { for( const auto& trx : user_input ) ids.push_back( transaction_digest(trx) ); - /** - * When generating the merkle hash of an output transaction we hash it - * a second time. This is because the transaction has not been confirmed as - * "valid and applied" just "produced". Later, when this transaction is included - * as part of "generated input" its ID will be used without the extra hash. - for( const auto& trx : output_transactions ) - ids.push_back( digest_type::hash(trx.merkle_digest()) ); - */ + for( const auto& trx : generated_input ) + ids.push_back( trx.id ); return merkle(ids); diff --git a/libraries/chain/chain_controller.cpp b/libraries/chain/chain_controller.cpp index 2a5ee342750..11391397717 100644 --- a/libraries/chain/chain_controller.cpp +++ b/libraries/chain/chain_controller.cpp @@ -321,6 +321,20 @@ signed_block chain_controller::_generate_block( FC_ASSERT( producer_obj.signing_key == block_signing_private_key.get_public_key() ); + // + // The following code throws away existing pending_tx_session and + // rebuilds it by re-applying pending transactions. + // + // This rebuild is necessary because pending transactions' validity + // and semantics may have changed since they were received, because + // time-based semantics are evaluated based on the current block + // time. These changes can only be reflected in the database when + // the value of the "when" variable is known, which means we need to + // re-apply pending transactions in this method. + // + _pending_tx_session.reset(); + _pending_tx_session = _db.start_undo_session(true); + const auto& generated = _db.get_index().equal_range(generated_transaction_object::PENDING); vector pending; @@ -337,20 +351,6 @@ signed_block chain_controller::_generate_block( auto schedule = scheduler(pending, get_global_properties()); - // - // The following code throws away existing pending_tx_session and - // rebuilds it by re-applying pending transactions. - // - // This rebuild is necessary because pending transactions' validity - // and semantics may have changed since they were received, because - // time-based semantics are evaluated based on the current block - // time. These changes can only be reflected in the database when - // the value of the "when" variable is known, which means we need to - // re-apply pending transactions in this method. - // - _pending_tx_session.reset(); - _pending_tx_session = _db.start_undo_session(true); - signed_block pending_block; pending_block.cycles.reserve(schedule.cycles.size()); @@ -413,7 +413,7 @@ signed_block chain_controller::_generate_block( } } - size_t postponed_tx_count = _pending_transactions.size() - valid_transaction_count - invalid_transaction_count; + size_t postponed_tx_count = pending.size() - valid_transaction_count - invalid_transaction_count; if( postponed_tx_count > 0 ) { wlog( "Postponed ${n} transactions due to block size limit", ("n", postponed_tx_count) ); diff --git a/libraries/chain/wasm_interface.cpp b/libraries/chain/wasm_interface.cpp index 27870547207..012480026ba 100644 --- a/libraries/chain/wasm_interface.cpp +++ b/libraries/chain/wasm_interface.cpp @@ -251,12 +251,19 @@ DEFINE_INTRINSIC_FUNCTION0(env,transactionCreate,transactionCreate,i32) { return ptrx.handle; } +static void emplace_scope(const Name& scope, std::vector& scopes) { + auto i = std::upper_bound( scopes.begin(), scopes.end(), scope); + if (i == scopes.begin() || *(i - 1) != scope ) { + scopes.insert(i, scope); + } +} + DEFINE_INTRINSIC_FUNCTION3(env,transactionRequireScope,transactionRequireScope,none,i32,handle,i64,scope,i32,readOnly) { auto& ptrx = wasm_interface::get().current_apply_context->get_pending_transaction(handle); if(readOnly == 0) { - ptrx.scope.emplace_back(scope); + emplace_scope(scope, ptrx.scope); } else { - ptrx.readscope.emplace_back(scope); + emplace_scope(scope, ptrx.readscope); } ptrx.check_size(); @@ -312,7 +319,11 @@ DEFINE_INTRINSIC_FUNCTION4(env,messageCreate,messageCreate,i32,i64,code,i64,type DEFINE_INTRINSIC_FUNCTION3(env,messageRequirePermission,messageRequirePermission,none,i32,handle,i64,account,i64,permission) { auto apply_context = wasm_interface::get().current_apply_context; - apply_context->require_authorization(Name(account), Name(permission)); + // if this is not sent from the code account with the permission of "code" then we must + // presently have the permission to add it, otherwise its a failure + if (!(account == apply_context->code.value && Name(permission) == Name("code"))) { + apply_context->require_authorization(Name(account), Name(permission)); + } auto& pmsg = apply_context->get_pending_message(handle); pmsg.authorization.emplace_back(Name(account), Name(permission)); } diff --git a/tests/slow_tests/deferred_tests.cpp b/tests/slow_tests/deferred_tests.cpp new file mode 100644 index 00000000000..09f8a8fba31 --- /dev/null +++ b/tests/slow_tests/deferred_tests.cpp @@ -0,0 +1,105 @@ +/* + * Copyright (c) 2017, Respective Authors. + * + * The MIT License + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include + +#include +#include +#include +#include +#include + +#include + +#include + +#include "../common/database_fixture.hpp" + +#include +#include +#include +#include +#include +#include + +#include + +using namespace eos; +using namespace chain; + + +void Set_Proxy_Owner( testing_blockchain& chain, AccountName proxy, AccountName owner ) { + eos::chain::SignedTransaction trx; + trx.scope = sort_names({proxy,owner}); + transaction_emplace_message(trx, "proxy", + vector({ }), + "setowner", owner); + + trx.expiration = chain.head_block_time() + 100; + transaction_set_reference_block(trx, chain.head_block_id()); + idump((trx)); + chain.push_transaction(trx); +} + +// declared in slow_test.cpp +namespace slow_tests { + void SetCode( testing_blockchain& chain, AccountName account, const char* wast ); +} + +BOOST_AUTO_TEST_SUITE(deferred_tests) + +BOOST_FIXTURE_TEST_CASE(opaque_proxy, testing_fixture) +{ try { + Make_Blockchain(chain); + chain.produce_blocks(1); + Make_Account(chain, newguy); + Make_Account(chain, proxy); + chain.produce_blocks(1); + + slow_tests::SetCode(chain, "proxy", proxy_wast); + //chain.produce_blocks(1); + + Set_Proxy_Owner(chain, "proxy", "newguy"); + chain.produce_blocks(7); + + Transfer_Asset(chain, inita, proxy, Asset(100)); + chain.produce_blocks(1); + BOOST_CHECK_EQUAL(chain.get_liquid_balance("newguy"), Asset(0)); + BOOST_CHECK_EQUAL(chain.get_liquid_balance("inita"), Asset(100000-300)); + BOOST_CHECK_EQUAL(chain.get_liquid_balance("proxy"), Asset(100)); + + + chain.produce_blocks(1); + BOOST_CHECK_EQUAL(chain.get_liquid_balance("newguy"), Asset(100)); + BOOST_CHECK_EQUAL(chain.get_liquid_balance("inita"), Asset(100000-300)); + BOOST_CHECK_EQUAL(chain.get_liquid_balance("proxy"), Asset(0)); + + BOOST_CHECK_EQUAL(chain.head_block_num(), 12); + BOOST_CHECK(chain.fetch_block_by_number(12).valid()); + BOOST_CHECK(!chain.fetch_block_by_number(12)->cycles.empty()); + BOOST_CHECK(!chain.fetch_block_by_number(12)->cycles.front().empty()); + BOOST_CHECK_EQUAL(chain.fetch_block_by_number(12)->cycles.front().front().generated_input.size(), 1); +} FC_LOG_AND_RETHROW() } + +BOOST_AUTO_TEST_SUITE_END()