From 84b75188b32a6ba5b4d0f2f60f8d1839b52a5fd8 Mon Sep 17 00:00:00 2001 From: Daniel Larimer Date: Mon, 10 Jul 2017 16:08:12 -0400 Subject: [PATCH] Fix Unit Tests with Scope Requirements fix Name() conversion names now support any characters and those outside the 32 defined are treated the same as '.'. --- libraries/chain/chain_controller.cpp | 11 ++- .../include/eos/chain/chain_controller.hpp | 1 + .../chain/include/eos/chain/exceptions.hpp | 2 + .../eos/chain/message_handling_contexts.hpp | 2 + .../include/eos/chain/wasm_interface.hpp | 1 - libraries/chain/message_handling_contexts.cpp | 20 ++++ libraries/chain/wasm_interface.cpp | 20 +--- libraries/native_contract/eos_contract.cpp | 10 +- libraries/types/include/eos/types/native.hpp | 5 +- tests/common/macro_support.hpp | 10 ++ tests/tests/block_tests.cpp | 97 ++++++++++++++++++- tests/tests/native_contract_tests.cpp | 3 +- 12 files changed, 153 insertions(+), 29 deletions(-) diff --git a/libraries/chain/chain_controller.cpp b/libraries/chain/chain_controller.cpp index 280859ef2..30c0d26ce 100644 --- a/libraries/chain/chain_controller.cpp +++ b/libraries/chain/chain_controller.cpp @@ -507,12 +507,13 @@ void chain_controller::validate_transaction(const SignedTransaction& trx)const { try { EOS_ASSERT(trx.messages.size() > 0, transaction_exception, "A transaction must have at least one message"); + validate_scope(trx); + validate_expiration(trx); validate_uniqueness(trx); validate_tapos(trx); validate_referenced_accounts(trx); - validate_expiration(trx); - for (const auto& tm : trx.messages) { /// TODO: this loop can be processed in parallel + for (const auto& tm : trx.messages) { const Message* m = reinterpret_cast(&tm); // m(tm); m->for_each_handler( [&]( const AccountName& a ) { @@ -536,6 +537,12 @@ try { } FC_CAPTURE_AND_RETHROW( (trx) ) } +void chain_controller::validate_scope( const SignedTransaction& trx )const { + EOS_ASSERT(trx.scope.size() > 0, transaction_exception, "No scope specified by transaction" ); + for( uint32_t i = 1; i < trx.scope.size(); ++i ) + EOS_ASSERT( trx.scope[i-1] < trx.scope[i], transaction_exception, "Scopes must be sorted and unique" ); +} + void chain_controller::validate_uniqueness( const SignedTransaction& trx )const { if( !should_check_for_duplicate_transactions() ) return; diff --git a/libraries/chain/include/eos/chain/chain_controller.hpp b/libraries/chain/include/eos/chain/chain_controller.hpp index 72eac03df..c60f0ef13 100644 --- a/libraries/chain/include/eos/chain/chain_controller.hpp +++ b/libraries/chain/include/eos/chain/chain_controller.hpp @@ -244,6 +244,7 @@ namespace eos { namespace chain { void validate_tapos(const SignedTransaction& trx)const; void validate_referenced_accounts(const SignedTransaction& trx)const; void validate_expiration(const SignedTransaction& trx) const; + void validate_scope(const SignedTransaction& trx) const; /// @} void validate_message_precondition(precondition_validate_context& c)const; diff --git a/libraries/chain/include/eos/chain/exceptions.hpp b/libraries/chain/include/eos/chain/exceptions.hpp index 22e341d13..96889e637 100644 --- a/libraries/chain/include/eos/chain/exceptions.hpp +++ b/libraries/chain/include/eos/chain/exceptions.hpp @@ -47,6 +47,8 @@ namespace eos { namespace chain { FC_DECLARE_DERIVED_EXCEPTION( tx_duplicate_sig, eos::chain::transaction_exception, 3030005, "duplicate signature included" ) FC_DECLARE_DERIVED_EXCEPTION( invalid_committee_approval, eos::chain::transaction_exception, 3030006, "committee account cannot directly approve transaction" ) FC_DECLARE_DERIVED_EXCEPTION( insufficient_fee, eos::chain::transaction_exception, 3030007, "insufficient fee" ) + FC_DECLARE_DERIVED_EXCEPTION( tx_missing_scope, eos::chain::transaction_exception, 3030008, "missing required scope" ) + FC_DECLARE_DERIVED_EXCEPTION( tx_missing_recipient, eos::chain::transaction_exception, 3030009, "missing required recipient" ) FC_DECLARE_DERIVED_EXCEPTION( invalid_pts_address, eos::chain::utility_exception, 3060001, "invalid pts address" ) FC_DECLARE_DERIVED_EXCEPTION( insufficient_feeds, eos::chain::chain_exception, 37006, "insufficient feeds" ) diff --git a/libraries/chain/include/eos/chain/message_handling_contexts.hpp b/libraries/chain/include/eos/chain/message_handling_contexts.hpp index b44252367..87eca7eeb 100644 --- a/libraries/chain/include/eos/chain/message_handling_contexts.hpp +++ b/libraries/chain/include/eos/chain/message_handling_contexts.hpp @@ -26,6 +26,8 @@ public: * @throws tx_missing_auth If no sufficient permission was found */ void require_authorization(const types::AccountName& account); + void require_scope(const types::AccountName& account)const; + void require_recipient(const types::AccountName& account)const; bool all_authorizations_used() const; const chainbase::database& db; ///< database where state is stored diff --git a/libraries/chain/include/eos/chain/wasm_interface.hpp b/libraries/chain/include/eos/chain/wasm_interface.hpp index d0f0dd311..7d152354b 100644 --- a/libraries/chain/include/eos/chain/wasm_interface.hpp +++ b/libraries/chain/include/eos/chain/wasm_interface.hpp @@ -42,7 +42,6 @@ class wasm_interface { Runtime::ModuleInstance* current_module = nullptr; ModuleState* current_state = nullptr; - void requireScope( AccountName scope )const; private: void load( const AccountName& name, const chainbase::database& db ); diff --git a/libraries/chain/message_handling_contexts.cpp b/libraries/chain/message_handling_contexts.cpp index fc996ddf5..d8843eb6b 100644 --- a/libraries/chain/message_handling_contexts.cpp +++ b/libraries/chain/message_handling_contexts.cpp @@ -8,15 +8,35 @@ namespace eos { namespace chain { void message_validate_context::require_authorization(const types::AccountName& account) { + auto itr = boost::find_if(msg.authorization, [&account](const types::AccountPermission& ap) { return ap.account == account; }); + EOS_ASSERT(itr != msg.authorization.end(), tx_missing_auth, "Required authorization ${auth} not found", ("auth", account)); used_authorizations[itr - msg.authorization.begin()] = true; } +void message_validate_context::require_scope(const types::AccountName& account)const { + auto itr = boost::find_if(trx.scope, [&account](const auto& scope) { + return scope == account; + }); + + EOS_ASSERT( itr != trx.scope.end(), tx_missing_scope, + "Required scope ${scope} not declared by transaction", ("scope",account) ); +} + +void message_validate_context::require_recipient(const types::AccountName& account)const { + auto itr = boost::find_if(msg.recipients, [&account](const auto& recipient) { + return recipient == account; + }); + + EOS_ASSERT( itr != msg.recipients.end(), tx_missing_recipient, + "Required recipient ${recipient} not declared by message", ("recipient",account)("recipients",msg.recipients) ); +} + bool message_validate_context::all_authorizations_used() const { return boost::algorithm::all_of_equal(used_authorizations, true); } diff --git a/libraries/chain/wasm_interface.cpp b/libraries/chain/wasm_interface.cpp index 487f4b464..efbf955a0 100644 --- a/libraries/chain/wasm_interface.cpp +++ b/libraries/chain/wasm_interface.cpp @@ -31,16 +31,11 @@ DEFINE_INTRINSIC_FUNCTION1(env,requireAuth,requireAuth,none,i64,account) { } DEFINE_INTRINSIC_FUNCTION1(env,requireNotice,requireNotice,none,i64,account) { - auto& wasm = wasm_interface::get(); - const auto& msg = wasm.current_validate_context->msg; - for( const auto& r : msg.recipients ) - if( r == Name(account) ) return; - FC_ASSERT( !"missing expected recipient", "expected ${auth} to be notified", ("auth",Name(account)) ); + wasm_interface::get().current_validate_context->require_recipient( account ); } DEFINE_INTRINSIC_FUNCTION1(env,requireScope,requireScope,none,i64,scope) { - auto& wasm = wasm_interface::get(); - wasm.requireScope( scope ); + wasm_interface::get().current_validate_context->require_scope( scope ); } DEFINE_INTRINSIC_FUNCTION3(env,remove_i64,remove_i64,i32,i64,scope,i64,table,i64,key) @@ -53,7 +48,7 @@ DEFINE_INTRINSIC_FUNCTION5(env,store_i64,store_i64,i32,i64,scope,i64,table,i64,k FC_ASSERT( valuelen >= 0 ); auto& wasm = wasm_interface::get(); - wasm.requireScope( scope ); + wasm.current_validate_context->require_scope( scope ); FC_ASSERT( wasm.current_apply_context, "no apply context found" ); @@ -495,13 +490,4 @@ DEFINE_INTRINSIC_FUNCTION1(env,toUpper,toUpper,none,i32,charptr) { current_state = &state; } - - void wasm_interface::requireScope( AccountName scope )const { try { - const auto& trx = current_validate_context->trx; - for( const auto& s : trx.scope ) - if( s == scope ) return; - FC_ASSERT( !"missing expected scope", "expected ${scope} to be in transaction scope", ("scope",Name(scope)) ); - } FC_CAPTURE_AND_RETHROW( (Name(scope)) ) } - - } } diff --git a/libraries/native_contract/eos_contract.cpp b/libraries/native_contract/eos_contract.cpp index 6aec49c0f..a28b747e7 100644 --- a/libraries/native_contract/eos_contract.cpp +++ b/libraries/native_contract/eos_contract.cpp @@ -77,7 +77,11 @@ void validate_eos_transfer(message_validate_context& context) { auto transfer = context.msg.as(); try { EOS_ASSERT(transfer.amount > Asset(0), message_validate_exception, "Must transfer a positive amount"); - EOS_ASSERT(context.msg.has_notify(transfer.to), message_validate_exception, "Must notify recipient of transfer"); + context.require_recipient(transfer.to); + context.require_recipient(transfer.from); + + context.require_scope(transfer.to); + context.require_scope(transfer.from); } FC_CAPTURE_AND_RETHROW((transfer)) } @@ -128,6 +132,10 @@ void validate_eos_lock(message_validate_context& context) { message_validate_exception, "Recipient account must be notified"); EOS_ASSERT(context.msg.has_notify(config::StakedBalanceContractName), message_validate_exception, "Staked Balance Contract (${name}) must be notified", ("name", config::StakedBalanceContractName)); + context.require_recipient(lock.to); + context.require_scope(lock.to); + context.require_recipient(lock.from); + context.require_scope(lock.from); } /** diff --git a/libraries/types/include/eos/types/native.hpp b/libraries/types/include/eos/types/native.hpp index 92dd4559c..6668cabee 100644 --- a/libraries/types/include/eos/types/native.hpp +++ b/libraries/types/include/eos/types/native.hpp @@ -66,6 +66,7 @@ namespace eos { namespace types { void set( const char* str ) { try { + value = 0; const auto len = strnlen(str,14); FC_ASSERT( len <= 13 ); for( uint32_t i = 0; i <= 12 && i < len; ++i ) { @@ -83,8 +84,8 @@ namespace eos { namespace types { if( c >= 'a' && c <= 'z' ) return (c - 'a') + 1; if( c >= '1' && c <= '5' ) - return (c - '1') + 26; - FC_ASSERT( c == '.', "invalid character '${c}' (${i}) in Name string", ("c", String(&c,1))("i",int(c)) ); + return (c - '1') + 27; + //FC_ASSERT( c == '.', "invalid character '${c}' (${i}) in Name string", ("c", String(&c,1))("i",int(c)) ); return 0; } diff --git a/tests/common/macro_support.hpp b/tests/common/macro_support.hpp index 81235fbc3..2590f7837 100644 --- a/tests/common/macro_support.hpp +++ b/tests/common/macro_support.hpp @@ -23,9 +23,15 @@ #define MKNET2_MACRO(x, name, chain) name.connect_blockchain(chain); #define MKNET2(name, ...) MKNET1(name) BOOST_PP_SEQ_FOR_EACH(MKNET2_MACRO, name, __VA_ARGS__) +inline std::vector sort_names( std::vector&& names ) { + std::sort( names.begin(), names.end() ); + return names; +} + #define MKACCT_IMPL(chain, name, creator, active, owner, recovery, deposit) \ { \ eos::chain::SignedTransaction trx; \ + trx.scope = sort_names({ #creator, "system" }); \ trx.emplaceMessage(config::SystemContractName, \ vector{#creator, config::StakedBalanceContractName, config::EosContractName}, \ vector{}, \ @@ -59,6 +65,7 @@ #define XFER5(chain, sender, recipient, amount, memo) \ { \ eos::chain::SignedTransaction trx; \ + trx.scope = sort_names({#sender,#recipient}); \ trx.emplaceMessage(config::EosContractName, vector{#sender, #recipient}, \ vector{}, \ "transfer", types::transfer{#sender, #recipient, amount, memo}); \ @@ -113,6 +120,7 @@ #define MKPDCR4(chain, owner, key, cfg) \ { \ eos::chain::SignedTransaction trx; \ + trx.scope = sort_names( {#owner, "staked"} ); \ trx.emplaceMessage(config::StakedBalanceContractName, vector{#owner}, \ vector{}, \ "setproducer", types::setproducer{#owner, key, cfg}); \ @@ -129,6 +137,7 @@ #define APPDCR4(chain, voter, producer, approved) \ { \ eos::chain::SignedTransaction trx; \ + trx.scope = sort_names( {#voter, "staked"} ); \ trx.emplaceMessage(config::StakedBalanceContractName, vector{#voter, #producer}, \ vector{}, \ "okproducer", types::okproducer{0, 1, approved? 1 : 0}); \ @@ -142,6 +151,7 @@ #define UPPDCR4(chain, owner, key, cfg) \ { \ eos::chain::SignedTransaction trx; \ + trx.scope = sort_names( {#owner, "staked"} ); \ trx.emplaceMessage(config::StakedBalanceContractName, vector{owner}, \ vector{}, \ "setproducer", types::setproducer{owner, key, cfg}); \ diff --git a/tests/tests/block_tests.cpp b/tests/tests/block_tests.cpp index e68690b06..d3f3462a1 100644 --- a/tests/tests/block_tests.cpp +++ b/tests/tests/block_tests.cpp @@ -46,6 +46,95 @@ using namespace eos; using namespace chain; + +/* + * This code is saved because it provides an alternative implementation of string to int conversion. + struct Tame { + uint64_t value = 0; + bool valid()const { return 0 == (value >> 60); } + bool empty()const { return 0 == value; } + bool good()const { return !empty() && valid(); } + + Tame( const char* str ) { set(str); } + Tame( const String& str ) { set( str.c_str() ); } + + void set( const char* str ) { + try { + idump((std::string(str))); + const auto len = strnlen(str,14); + value = 0; + FC_ASSERT( len <= 13 ); + for( uint32_t i = 0; i < len && i < 12; ++i ) + value |= uint64_t(char_to_symbol( str[i] )) << (59-(i*5)); + if( len == 13 ) + value |= (0x0f & char_to_symbol( str[ 12 ] )); + }FC_CAPTURE_AND_RETHROW( (str) ) } + + + uint8_t char_to_symbol( char c ) const { + static const char* charmap = ".abcdefghijklmnopqrstuvwxyz12345"; + if( c >= 'a' && c <= 'z' ) { + idump((int(((c-'a') + 1)))(string()+c)); + // FC_ASSERT( charmap[((c-'a') + 1)] == c ); + return uint8_t(c - 'a') + 1; + } + if( c >= '1' && c <= '5' ) { + idump((int(((c-'1') + 26)))(string()+c)); + // FC_ASSERT( charmap[((c-'1') + 26)] == c ); + return uint8_t(c - '1') + 27; + } + FC_ASSERT( c == '.', "invalid character '${c}' (${i}) in Tame string", ("c", String(&c,1))("i",int(c)) ); + return 0; + } + + Tame( uint64_t v = 0 ):value(v){ + // FC_ASSERT( !(v>>(5*12)), "invalid name id" ); + } + + explicit operator String()const { + static const char* charmap = ".abcdefghijklmnopqrstuvwxyz12345"; + String str; + uint64_t tmp = value; + for( uint32_t i = 0; i < 12; ++i ) { + str += charmap[ 0x1f & (tmp >> (59-i*5)) ]; + } + str += charmap[0x0f & tmp]; + boost::algorithm::trim_right_if( str, []( char c ){ return c == '.'; } ); + return str; + } + + String toString() const { return String(*this); } + + Tame& operator=( uint64_t v ) { + value = v; + return *this; + } + + Tame& operator=( const String& n ) { + value = Tame(n).value; + return *this; + } + Tame& operator=( const char* n ) { + value = Tame(n).value; + return *this; + } + + template + friend Stream& operator << ( Stream& out, const Tame& n ) { + return out << String(n); + } + + friend bool operator < ( const Tame& a, const Tame& b ) { return a.value < b.value; } + friend bool operator > ( const Tame& a, const Tame& b ) { return a.value > b.value; } + friend bool operator == ( const Tame& a, const Tame& b ) { return a.value == b.value; } + friend bool operator != ( const Tame& a, const Tame& b ) { return a.value != b.value; } + + operator bool()const { return value; } + operator uint64_t()const { return value; } + }; +*/ + + BOOST_AUTO_TEST_SUITE(block_tests) BOOST_AUTO_TEST_CASE(name_test) { @@ -55,11 +144,9 @@ BOOST_AUTO_TEST_CASE(name_test) { BOOST_CHECK_EQUAL( String("temp"), String(temp) ); BOOST_CHECK_EQUAL( String("temp.temp"), String(Name("temp.temp")) ); BOOST_CHECK_EQUAL( String(""), String(Name()) ); - BOOST_CHECK_EQUAL( String("hello"), String(Name("hello")) ); - BOOST_REQUIRE_THROW( Name(-1), fc::exception ); // only lower 60 bits are valid - BOOST_REQUIRE_THROW( Name("Hello"), fc::exception ); // capital invalid - BOOST_REQUIRE_THROW( Name("9ello"), fc::exception ); // number 9 invalid - BOOST_REQUIRE_THROW( Name("6ello"), fc::exception ); // number 6 invalid + BOOST_REQUIRE_EQUAL( String("hello"), String(Name("hello")) ); + BOOST_REQUIRE_EQUAL( Name(-1), Name(String(Name(-1))) ); + BOOST_REQUIRE_EQUAL( String(Name(-1)), String(Name(String(Name(-1)))) ); } // Simple test of block production and head_block_num tracking diff --git a/tests/tests/native_contract_tests.cpp b/tests/tests/native_contract_tests.cpp index dda6d2919..cffa9d63e 100644 --- a/tests/tests/native_contract_tests.cpp +++ b/tests/tests/native_contract_tests.cpp @@ -90,6 +90,7 @@ BOOST_FIXTURE_TEST_CASE(transfer, testing_fixture) trx.messages.resize(1); trx.set_reference_block(chain.head_block_id()); trx.expiration = chain.head_block_time() + 100; + trx.scope = sort_names( {"inita", "initb"} ); trx.messages[0].recipients = {"inita", config::EosContractName}; types::transfer trans = { "inita", "initb", Asset(100), "transfer 100" }; @@ -104,7 +105,7 @@ BOOST_FIXTURE_TEST_CASE(transfer, testing_fixture) auto unpack_trans = trx.messageAs(0); - BOOST_REQUIRE_THROW(chain.push_transaction(trx), message_validate_exception); // "fail to notify receiver, initb" + BOOST_REQUIRE_THROW(chain.push_transaction(trx), message_validate_exception); // "fail to notify receiver, initb and sender, inita" trx.messages[0].recipients = {"inita", "initb"}; trx.setMessage(0, "transfer", trans); chain.push_transaction(trx); -- GitLab