diff --git a/libraries/chain/db_block.cpp b/libraries/chain/db_block.cpp index 6084bbf2..2ae442c0 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -439,8 +439,8 @@ struct signature_check_visitor { typedef void result_type; - const signed_transaction& trx; - signature_check_visitor( const signed_transaction& t ):trx(t){} + flat_map& sigs; + signature_check_visitor( flat_map& s ):sigs(s){} template result_type operator()( const T& o )const{} @@ -448,7 +448,11 @@ struct signature_check_visitor result_type operator()( const balance_claim_operation& o )const { for( auto& owner : o.owners ) - FC_ASSERT( trx.extra_signatures.find(owner) != trx.extra_signatures.end() ); + { + auto itr = sigs.find(owner); + FC_ASSERT( itr != sigs.end() ); + itr->second = true; + } } }; @@ -468,25 +472,12 @@ processed_transaction database::_apply_transaction( const signed_transaction& tr //This check is used only if this transaction has an absolute expiration time. if( !(skip & skip_transaction_signatures) && trx.relative_expiration == 0 ) { - for( const auto& sig : trx.signatures ) - { - FC_ASSERT( sig.first(*this).key_address() == fc::ecc::public_key( sig.second, trx.digest() ), "", - ("trx",trx) - ("digest",trx.digest()) - ("sig.first",sig.first) - ("key_address",sig.first(*this).key_address()) - ("addr", address(fc::ecc::public_key( sig.second, trx.digest() ))) ); - } + eval_state._sigs.reserve( trx.signatures.size() ); - for( const auto& sig : trx.extra_signatures ) - { - FC_ASSERT( sig.first == address(fc::ecc::public_key( sig.second, trx.digest() )), "", - ("trx",trx) - ("digest",trx.digest()) - ("sig.first",sig.first) - ("addr", address(fc::ecc::public_key( sig.second, trx.digest() ))) ); - } - trx.visit( signature_check_visitor(trx) ); + for( const auto& sig : trx.signatures ) + FC_ASSERT( eval_state._sigs.insert( std::make_pair( address(fc::ecc::public_key( sig, trx.digest() )), false) ).second, "Multiple signatures by same key detected" ) ; + + trx.visit( signature_check_visitor(eval_state._sigs) ); } //If we're skipping tapos check, but not dupe check, assume all transactions have maximum expiration time. @@ -509,15 +500,12 @@ processed_transaction database::_apply_transaction( const signed_transaction& tr //This is the signature check for transactions with relative expiration. if( !(skip & skip_transaction_signatures) ) { + eval_state._sigs.reserve( trx.signatures.size() ); + for( const auto& sig : trx.signatures ) - { - address trx_addr = fc::ecc::public_key(sig.second, trx.digest(tapos_block_summary.block_id)); - FC_ASSERT(sig.first(*this).key_address() == trx_addr, - "", - ("sig.first",sig.first) - ("key_address",sig.first(*this).key_address()) - ("addr", trx_addr)); - } + FC_ASSERT( eval_state._sigs.insert( std::make_pair(address(fc::ecc::public_key( sig, trx.digest(tapos_block_summary.block_id) )),false) ).second, "Multiple signatures by same key detected" ) ; + + trx.visit( signature_check_visitor(eval_state._sigs) ); } //Verify TaPoS block summary has correct ID prefix, and that this block's time is not past the expiration @@ -560,11 +548,17 @@ processed_transaction database::_apply_transaction( const signed_transaction& tr auto range = index.equal_range(GRAPHENE_TEMP_ACCOUNT); std::for_each(range.first, range.second, [](const account_balance_object& b) { FC_ASSERT(b.balance == 0); }); + if( !(skip & (skip_transaction_signatures|skip_authority_check)) ) + { + for( const auto& item : eval_state._sigs ) + FC_ASSERT( item.second, "All signatures must be used", ("item",item) ); + } + return ptrx; } FC_CAPTURE_AND_RETHROW( (trx) ) } operation_result database::apply_operation(transaction_evaluation_state& eval_state, const operation& op) -{ +{ try { int i_which = op.which(); uint64_t u_which = uint64_t( i_which ); if( i_which < 0 ) @@ -578,7 +572,7 @@ operation_result database::apply_operation(transaction_evaluation_state& eval_st auto result = eval->evaluate( eval_state, op, true ); set_applied_operation_result( op_id, result ); return result; -} +} FC_CAPTURE_AND_RETHROW( (eval_state._sigs) ) } const witness_object& database::validate_block_header( uint32_t skip, const signed_block& next_block )const { diff --git a/libraries/chain/evaluator.cpp b/libraries/chain/evaluator.cpp index 2cda4751..b1852370 100644 --- a/libraries/chain/evaluator.cpp +++ b/libraries/chain/evaluator.cpp @@ -30,14 +30,14 @@ namespace graphene { namespace chain { database& generic_evaluator::db()const { return trx_state->db(); } operation_result generic_evaluator::start_evaluate( transaction_evaluation_state& eval_state, const operation& op, bool apply ) - { + { try { trx_state = &eval_state; check_required_authorities(op); auto result = evaluate( op ); if( apply ) result = this->apply( op ); return result; - } + } FC_CAPTURE_AND_RETHROW() } void generic_evaluator::prepare_fee(account_id_type account_id, asset fee) { @@ -75,11 +75,12 @@ namespace graphene { namespace chain { } FC_CAPTURE_AND_RETHROW() } bool generic_evaluator::verify_authority( const account_object& a, authority::classification c ) - { + { try { return trx_state->check_authority( a, c ); - } + } FC_CAPTURE_AND_RETHROW( (a)(c) ) } + void generic_evaluator::check_required_authorities(const operation& op) - { + { try { flat_set active_auths; flat_set owner_auths; op.visit(operation_get_required_auths(active_auths, owner_auths)); @@ -93,7 +94,7 @@ namespace graphene { namespace chain { { FC_ASSERT(verify_authority(id(db()), authority::owner), "", ("id", id)); } - } + } FC_CAPTURE_AND_RETHROW( (op) ) } object_id_type generic_evaluator::get_relative_id( object_id_type rel_id )const { diff --git a/libraries/chain/include/graphene/chain/transaction.hpp b/libraries/chain/include/graphene/chain/transaction.hpp index bc3504f1..5bc96a66 100644 --- a/libraries/chain/include/graphene/chain/transaction.hpp +++ b/libraries/chain/include/graphene/chain/transaction.hpp @@ -147,19 +147,22 @@ namespace graphene { namespace chain { signed_transaction( const transaction& trx = transaction() ) : transaction(trx){} - void sign( key_id_type id, const private_key_type& key ); - void sign( const address& addr, const private_key_type& key ); + /** deprecated, TODO: remove when all references are gone */ + void sign( key_id_type id, const private_key_type& key ) { sign(key); } + void sign( const private_key_type& key ); - flat_map signatures; + vector signatures; + +// flat_map signatures; /** some operations may depend only upon a signature and not * require account approval. This allows those extra signatures * to be added to the transaction. */ - flat_map extra_signatures; +// flat_map extra_signatures; /// Removes all operations and signatures - void clear() { operations.clear(); signatures.clear(); extra_signatures.clear(); } + void clear() { operations.clear(); signatures.clear(); /*extra_signatures.clear();*/ } }; /** @@ -191,5 +194,5 @@ namespace graphene { namespace chain { } } FC_REFLECT( graphene::chain::transaction, (ref_block_num)(ref_block_prefix)(relative_expiration)(operations) ) -FC_REFLECT_DERIVED( graphene::chain::signed_transaction, (graphene::chain::transaction), (signatures)(extra_signatures) ) +FC_REFLECT_DERIVED( graphene::chain::signed_transaction, (graphene::chain::transaction), (signatures) ) FC_REFLECT_DERIVED( graphene::chain::processed_transaction, (graphene::chain::signed_transaction), (operation_results) ) diff --git a/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp b/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp index e520e4c8..32f27d0b 100644 --- a/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp +++ b/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp @@ -40,7 +40,7 @@ namespace graphene { namespace chain { database& db()const { FC_ASSERT( _db ); return *_db; } - bool signed_by( key_id_type id )const; + bool signed_by( key_id_type id ); /** derived from signatures on transaction flat_set
signed_by; @@ -53,8 +53,12 @@ namespace graphene { namespace chain { */ vector operation_results; - const signed_transaction* _trx = nullptr; - database* _db = nullptr; - bool _is_proposed_trx = false; + /** When an address is referenced via check authority it is flagged as being used, + * all addresses must be flagged as being used or the transaction will fail. + */ + flat_map _sigs; + const signed_transaction* _trx = nullptr; + database* _db = nullptr; + bool _is_proposed_trx = false; }; } } // namespace graphene::chain diff --git a/libraries/chain/proposal_object.cpp b/libraries/chain/proposal_object.cpp index 2ab63c68..28d80b70 100644 --- a/libraries/chain/proposal_object.cpp +++ b/libraries/chain/proposal_object.cpp @@ -18,6 +18,7 @@ #include #include #include +#include namespace graphene { namespace chain { @@ -37,7 +38,7 @@ bool proposal_object::is_authorized_to_execute(database& db) const signed_transaction tmp; dry_run_eval._trx = &tmp; for( auto key_id : available_key_approvals ) - tmp.signatures[key_id] = fc::ecc::compact_signature(); + dry_run_eval._sigs.insert( std::make_pair(key_id(db).key_address(),true) ); //insert into dry_run_eval->_trx.signatures //dry_run_eval.signed_by.insert(available_key_approvals.begin(), available_key_approvals.end()); diff --git a/libraries/chain/transaction.cpp b/libraries/chain/transaction.cpp index f47b8e55..6ac810f1 100644 --- a/libraries/chain/transaction.cpp +++ b/libraries/chain/transaction.cpp @@ -60,26 +60,15 @@ graphene::chain::transaction_id_type graphene::chain::transaction::id() const memcpy(result._hash, hash._hash, std::min(sizeof(result), sizeof(hash))); return result; } -void graphene::chain::signed_transaction::sign( key_id_type id, const private_key_type& key ) +void graphene::chain::signed_transaction::sign( const private_key_type& key ) { if( relative_expiration != 0 ) { if( !block_id_cache.valid() ) edump((*this)); assert(block_id_cache.valid()); - signatures[id] = key.sign_compact( digest(*block_id_cache) ); + signatures.push_back( key.sign_compact( digest(*block_id_cache) ) ); } else { - signatures[id] = key.sign_compact( digest() ); - } -} -void graphene::chain::signed_transaction::sign( const address& addr, const private_key_type& key ) -{ - if( relative_expiration != 0 ) - { - if( !block_id_cache.valid() ) edump((*this)); - assert(block_id_cache.valid()); - extra_signatures[addr] = key.sign_compact( digest(*block_id_cache) ); - } else { - extra_signatures[addr] = key.sign_compact( digest() ); + signatures.push_back( key.sign_compact( digest() ) ); } } diff --git a/libraries/chain/transaction_evaluation_state.cpp b/libraries/chain/transaction_evaluation_state.cpp index f273123b..4a11c1f9 100644 --- a/libraries/chain/transaction_evaluation_state.cpp +++ b/libraries/chain/transaction_evaluation_state.cpp @@ -17,6 +17,7 @@ */ #include #include +#include #include #include #include @@ -25,7 +26,9 @@ namespace graphene { namespace chain { bool transaction_evaluation_state::check_authority( const account_object& account, authority::classification auth_class, int depth ) { - if( (!_is_proposed_trx) && (_db->get_node_properties().skip_flags & database::skip_authority_check) ) + if( (!_is_proposed_trx) && (_db->get_node_properties().skip_flags & database::skip_authority_check) ) + return true; + if( (!_is_proposed_trx) && (_db->get_node_properties().skip_flags & database::skip_transaction_signatures) ) return true; if( account.get_id() == GRAPHENE_TEMP_ACCOUNT || approved_by.find(make_pair(account.id, auth_class)) != approved_by.end() ) @@ -91,10 +94,15 @@ namespace graphene { namespace chain { } return false; } - bool transaction_evaluation_state::signed_by( key_id_type id )const + bool transaction_evaluation_state::signed_by( key_id_type id ) { assert(_trx); - return _trx->signatures.find(id) != _trx->signatures.end(); + assert(_db); + //wdump((_sigs)(id(*_db).key_address())(*_trx) ); + auto itr = _sigs.find( id(*_db).key_address() ); + if( itr != _sigs.end() ) + return itr->second = true; + return false; } } } // namespace graphene::chain diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 1643df1c..b558b7b1 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -2168,7 +2168,7 @@ signed_transaction wallet_api::import_balance( string name_or_id, const vector_remote_net->broadcast_transaction(tx); diff --git a/tests/common/database_fixture.cpp b/tests/common/database_fixture.cpp index 93e92530..d6d0e78d 100644 --- a/tests/common/database_fixture.cpp +++ b/tests/common/database_fixture.cpp @@ -89,7 +89,7 @@ database_fixture::database_fixture() } database_fixture::~database_fixture() -{ +{ try { // If we're unwinding due to an exception, don't do any more checks. // This way, boost test's last checkpoint tells us approximately where the error was. if( !std::uncaught_exception() ) @@ -102,7 +102,7 @@ database_fixture::~database_fixture() if( data_dir ) db.close(); return; -} +} FC_CAPTURE_AND_RETHROW() } fc::ecc::private_key database_fixture::generate_private_key(string seed) { diff --git a/tests/tests/authority_tests.cpp b/tests/tests/authority_tests.cpp index ea3a0b2c..65b59b00 100644 --- a/tests/tests/authority_tests.cpp +++ b/tests/tests/authority_tests.cpp @@ -125,9 +125,11 @@ BOOST_AUTO_TEST_CASE( recursive_accounts ) const key_object& key2 = register_key(parent2_key.get_public_key()); const auto& core = asset_id_type()(db); + BOOST_TEST_MESSAGE( "Creating parent1 and parent2 accounts" ); const account_object& parent1 = create_account("parent1", key1.id); const account_object& parent2 = create_account("parent2", key2.id); + BOOST_TEST_MESSAGE( "Creating child account that requires both parent1 and parent2 to approve" ); { auto make_child_op = make_account("child"); make_child_op.owner = authority(2, account_id_type(parent1.id), 1, account_id_type(parent2.id), 1); @@ -140,23 +142,28 @@ BOOST_AUTO_TEST_CASE( recursive_accounts ) const account_object& child = get_account("child"); auto old_balance = fund(child); + BOOST_TEST_MESSAGE( "Attempting to transfer with no signatures, should fail" ); transfer_operation op = {asset(), child.id, account_id_type(), core.amount(500)}; trx.operations.push_back(op); BOOST_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception); - sign(trx, key1.id,parent1_key); - sign(trx, key1.id,parent1_key); - sign(trx, key1.id,parent1_key); + + BOOST_TEST_MESSAGE( "Attempting to transfer with parent1 signature, should fail" ); sign(trx, key1.id,parent1_key); BOOST_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception); trx.signatures.clear(); + + BOOST_TEST_MESSAGE( "Attempting to transfer with parent2 signature, should fail" ); sign(trx, key2.id,parent2_key); BOOST_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception); + + BOOST_TEST_MESSAGE( "Attempting to transfer with parent1 and parent2 signature, should succeed" ); sign(trx, key1.id,parent1_key); PUSH_TX( db, trx, database::skip_transaction_dupe_check ); BOOST_CHECK_EQUAL(get_balance(child, core), old_balance - 500); trx.operations.clear(); trx.signatures.clear(); + BOOST_TEST_MESSAGE( "Adding a key for the child that can override parents" ); fc::ecc::private_key child_key = fc::ecc::private_key::generate(); const key_object& child_key_obj = register_key(child_key.get_public_key()); { @@ -175,22 +182,30 @@ BOOST_AUTO_TEST_CASE( recursive_accounts ) op = {asset(),child.id, account_id_type(), core.amount(500)}; trx.operations.push_back(op); + BOOST_TEST_MESSAGE( "Attempting transfer with no signatures, should fail" ); BOOST_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception); + BOOST_TEST_MESSAGE( "Attempting transfer just parent1, should fail" ); sign(trx, key1.id,parent1_key); BOOST_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception); trx.signatures.clear(); + BOOST_TEST_MESSAGE( "Attempting transfer just parent2, should fail" ); sign(trx, key2.id,parent2_key); BOOST_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception); + + BOOST_TEST_MESSAGE( "Attempting transfer both parents, should succeed" ); sign(trx, key1.id, parent1_key); PUSH_TX( db, trx, database::skip_transaction_dupe_check ); BOOST_CHECK_EQUAL(get_balance(child, core), old_balance - 1000); trx.signatures.clear(); + + BOOST_TEST_MESSAGE( "Attempting transfer with just child key, should succeed" ); sign(trx, child_key_obj.id, child_key); PUSH_TX( db, trx, database::skip_transaction_dupe_check ); BOOST_CHECK_EQUAL(get_balance(child, core), old_balance - 1500); trx.operations.clear(); trx.signatures.clear(); + BOOST_TEST_MESSAGE( "Creating grandparent account, parent1 now requires authority of grandparent" ); auto grandparent = create_account("grandparent"); fc::ecc::private_key grandparent_key = fc::ecc::private_key::generate(); const key_object& grandparent_key_obj = register_key(grandparent_key.get_public_key()); @@ -209,16 +224,21 @@ BOOST_AUTO_TEST_CASE( recursive_accounts ) trx.signatures.clear(); } + BOOST_TEST_MESSAGE( "Attempt to transfer using old parent keys, should fail" ); trx.operations.push_back(op); sign(trx, key1.id, parent1_key); sign(trx, key2.id, parent2_key); BOOST_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception); - sign(trx, grandparent_key_obj.id, grandparent_key); + trx.signatures.clear(); + trx.sign( parent2_key ); + trx.sign( grandparent_key ); + + BOOST_TEST_MESSAGE( "Attempt to transfer using parent2_key and grandparent_key" ); PUSH_TX( db, trx, database::skip_transaction_dupe_check ); BOOST_CHECK_EQUAL(get_balance(child, core), old_balance - 2000); - trx.operations.clear(); - trx.signatures.clear(); + trx.clear(); + BOOST_TEST_MESSAGE( "Update grandparent account authority to be genesis account" ); { account_update_operation op; op.account = grandparent.id; @@ -230,18 +250,22 @@ BOOST_AUTO_TEST_CASE( recursive_accounts ) trx.signatures.clear(); } + BOOST_TEST_MESSAGE( "Create recursion depth failure" ); trx.operations.push_back(op); sign(trx, key2.id,parent2_key); sign(trx, grandparent_key_obj.id,grandparent_key); sign(trx, key_id_type(), delegate_priv_key); //Fails due to recursion depth. BOOST_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception); + BOOST_TEST_MESSAGE( "verify child key can override recursion checks" ); + trx.signatures.clear(); sign(trx, child_key_obj.id, child_key); PUSH_TX( db, trx, database::skip_transaction_dupe_check ); BOOST_CHECK_EQUAL(get_balance(child, core), old_balance - 2500); trx.operations.clear(); trx.signatures.clear(); + BOOST_TEST_MESSAGE( "Verify a cycle fails" ); { account_update_operation op; op.account = parent1.id; @@ -401,12 +425,14 @@ BOOST_AUTO_TEST_CASE( genesis_authority ) uop.key_approvals_to_add.emplace(5); uop.key_approvals_to_add.emplace(6); trx.operations.push_back(uop); - trx.sign(key_id_type(1), genesis_key); + trx.sign(genesis_key); + /* trx.signatures[key_id_type(2)] = trx.signatures[key_id_type(1)]; trx.signatures[key_id_type(3)] = trx.signatures[key_id_type(1)]; trx.signatures[key_id_type(4)] = trx.signatures[key_id_type(1)]; trx.signatures[key_id_type(5)] = trx.signatures[key_id_type(1)]; trx.signatures[key_id_type(6)] = trx.signatures[key_id_type(1)]; + */ db.push_transaction(trx); BOOST_CHECK_EQUAL(get_balance(nathan, asset_id_type()(db)), 0); BOOST_CHECK(db.get(prop.id).is_authorized_to_execute(db)); @@ -473,16 +499,7 @@ BOOST_FIXTURE_TEST_CASE( fired_delegates, database_fixture ) uop.key_approvals_to_add.emplace(8); uop.key_approvals_to_add.emplace(9); trx.operations.back() = uop; - trx.sign(key_id_type(1), genesis_key); - trx.signatures[key_id_type(2)] = trx.signatures[key_id_type(1)]; - trx.signatures[key_id_type(3)] = trx.signatures[key_id_type(1)]; - trx.signatures[key_id_type(4)] = trx.signatures[key_id_type(1)]; - trx.signatures[key_id_type(5)] = trx.signatures[key_id_type(1)]; - trx.signatures[key_id_type(6)] = trx.signatures[key_id_type(1)]; - trx.signatures[key_id_type(7)] = trx.signatures[key_id_type(1)]; - trx.signatures[key_id_type(8)] = trx.signatures[key_id_type(1)]; - trx.signatures[key_id_type(9)] = trx.signatures[key_id_type(1)]; - trx.sign(key_id_type(), genesis_key); + trx.sign(genesis_key); PUSH_TX( db, trx ); BOOST_CHECK(pid(db).is_authorized_to_execute(db)); @@ -968,6 +985,8 @@ BOOST_FIXTURE_TEST_CASE( bogus_signature, database_fixture ) trx.clear(); trx.operations.push_back( xfer_op ); + + BOOST_TEST_MESSAGE( "Transfer signed by alice" ); trx.sign( alice_key_id, alice_key ); flat_set active_set, owner_set; @@ -977,33 +996,20 @@ BOOST_FIXTURE_TEST_CASE( bogus_signature, database_fixture ) PUSH_TX( db, trx, skip ); trx.operations.push_back( xfer_op ); + BOOST_TEST_MESSAGE( "Invalidating Alices Signature" ); // Alice's signature is now invalid BOOST_REQUIRE_THROW( PUSH_TX( db, trx, skip ), fc::exception ); // Re-sign, now OK (sig is replaced) - trx.sign( alice_key_id, alice_key ); + BOOST_TEST_MESSAGE( "Resign with Alice's Signature" ); + trx.signatures.clear(); + trx.sign( alice_key ); PUSH_TX( db, trx, skip ); - trx.signatures.clear(); - trx.sign( charlie_key_id, alice_key ); - // Sign with Alice's key (valid) claiming to be Charlie - BOOST_REQUIRE_THROW( PUSH_TX( db, trx, skip ), fc::exception ); - // and with Charlie's key (invalid) claiming to be Alice - trx.sign( charlie_key_id, alice_key ); - BOOST_REQUIRE_THROW( PUSH_TX( db, trx, skip ), fc::exception ); - trx.signatures.clear(); - // okay, now sign ONLY with Charlie's key claiming to be Alice - trx.sign( charlie_key_id, alice_key ); - BOOST_REQUIRE_THROW( PUSH_TX( db, trx, skip ), fc::exception ); - trx.signatures.clear(); trx.operations.pop_back(); - trx.sign( alice_key_id, alice_key ); - trx.sign( charlie_key_id, charlie_key ); + trx.sign( alice_key ); + trx.sign( charlie_key ); // Signed by third-party Charlie (irrelevant key, not in authority) - PUSH_TX( db, trx, skip ); - trx.operations.push_back( xfer_op ); - trx.sign( alice_key_id, alice_key ); - // Alice's sig is valid but Charlie's is invalid BOOST_REQUIRE_THROW( PUSH_TX( db, trx, skip ), fc::exception ); } FC_LOG_AND_RETHROW() @@ -1032,7 +1038,7 @@ BOOST_FIXTURE_TEST_CASE( voting_account, database_fixture ) op.new_options->votes = flat_set{nathan_delegate(db).vote_id}; op.new_options->num_committee = 1; trx.operations.push_back(op); - trx.sign(nathan_key_id, nathan_private_key); + trx.sign(nathan_private_key); PUSH_TX( db, trx ); trx.clear(); } @@ -1043,12 +1049,13 @@ BOOST_FIXTURE_TEST_CASE( voting_account, database_fixture ) op.new_options->votes.insert(vikram_delegate(db).vote_id); op.new_options->num_committee = 11; trx.operations.push_back(op); - trx.sign(vikram_key_id, vikram_private_key); + trx.sign(vikram_private_key); // Fails because num_committee is larger than the cardinality of committee members being voted for BOOST_CHECK_THROW(PUSH_TX( db, trx ), fc::exception); op.new_options->num_committee = 3; trx.operations = {op}; - trx.sign(vikram_key_id, vikram_private_key); + trx.signatures.clear(); + trx.sign(vikram_private_key); PUSH_TX( db, trx ); trx.clear(); } diff --git a/tests/tests/block_tests.cpp b/tests/tests/block_tests.cpp index 1aeedb6a..a9dbf581 100644 --- a/tests/tests/block_tests.cpp +++ b/tests/tests/block_tests.cpp @@ -297,7 +297,7 @@ BOOST_AUTO_TEST_CASE( undo_pending ) cop.name = "nathan"; cop.owner = authority(1, key_id_type(), 1); trx.operations.push_back(cop); - trx.sign( key_id_type(), delegate_priv_key ); + //trx.sign( delegate_priv_key ); PUSH_TX( db, trx ); now += db.block_interval(); @@ -576,6 +576,43 @@ BOOST_FIXTURE_TEST_CASE( limit_order_expiration, database_fixture ) BOOST_CHECK_EQUAL( get_balance(*nathan, *core), 50000 ); } FC_LOG_AND_RETHROW() } +BOOST_FIXTURE_TEST_CASE( double_sign_check, database_fixture ) +{ try { + generate_block(); + const auto& from = account_id_type()(db); + ACTOR(to); + asset amount(1000); + + trx.set_expiration(db.head_block_time() + fc::minutes(1)); + trx.operations.push_back(transfer_operation({ asset(), from.id, to.id, amount, memo_data() })); + for( auto& op : trx.operations ) op.visit( operation_set_fee( db.current_fee_schedule() ) ); + trx.validate(); + + db.push_transaction(trx, ~0); + + trx.operations.clear(); + trx.operations.push_back(transfer_operation({ asset(), to.id, from.id, amount, memo_data() })); + for( auto& op : trx.operations ) op.visit( operation_set_fee( db.current_fee_schedule() ) ); + trx.validate(); + + BOOST_TEST_MESSAGE( "Verify that not-signing causes an exception" ); + BOOST_REQUIRE_THROW( db.push_transaction(trx, 0 ), fc::exception ); + trx.sign( to_private_key ); + + BOOST_TEST_MESSAGE( "Verify that double-signing causes an exception" ); + BOOST_REQUIRE_THROW( db.push_transaction(trx, 0 ), fc::exception ); + + BOOST_TEST_MESSAGE( "Verify that signing with an extra, unused key fails" ); + trx.signatures.pop_back(); + trx.sign( generate_private_key("bogus") ); + BOOST_REQUIRE_THROW( db.push_transaction(trx, 0 ), fc::exception ); + + BOOST_TEST_MESSAGE( "Verify that signing once with the proper key passes" ); + db.push_transaction(trx, 0 ); + trx.sign( to_private_key ); + +} FC_LOG_AND_RETHROW() } + BOOST_FIXTURE_TEST_CASE( change_block_interval, database_fixture ) { try { generate_block(); @@ -603,6 +640,7 @@ BOOST_FIXTURE_TEST_CASE( change_block_interval, database_fixture ) get_account("init6").get_id(), get_account("init7").get_id()}; trx.operations.push_back(uop); trx.sign(get_account("init0").active.get_keys().front(),delegate_priv_key); + /* trx.sign(get_account("init1").active.get_keys().front(),delegate_priv_key); trx.sign(get_account("init2").active.get_keys().front(),delegate_priv_key); trx.sign(get_account("init3").active.get_keys().front(),delegate_priv_key); @@ -610,6 +648,7 @@ BOOST_FIXTURE_TEST_CASE( change_block_interval, database_fixture ) trx.sign(get_account("init5").active.get_keys().front(),delegate_priv_key); trx.sign(get_account("init6").active.get_keys().front(),delegate_priv_key); trx.sign(get_account("init7").active.get_keys().front(),delegate_priv_key); + */ db.push_transaction(trx); BOOST_CHECK(proposal_id_type()(db).is_authorized_to_execute(db)); } diff --git a/tests/tests/operation_tests2.cpp b/tests/tests/operation_tests2.cpp index 66e2ce57..aaf9b935 100644 --- a/tests/tests/operation_tests2.cpp +++ b/tests/tests/operation_tests2.cpp @@ -962,15 +962,15 @@ BOOST_AUTO_TEST_CASE( balance_object_test ) op.total_claimed = asset(1); op.owners.insert(genesis_state.initial_balances.back().owner); trx.operations = {op}; - trx.sign(*op.owners.begin(), generate_private_key("n")); - trx.sign(op.deposit_to_account(db).active.get_keys().front(), generate_private_key("n")); + trx.sign(generate_private_key("n")); + //trx.sign(generate_private_key("n")); // Fail because I'm claiming the wrong address BOOST_CHECK_THROW(db.push_transaction(trx), fc::exception); trx.clear(); op.owners = {genesis_state.initial_balances.front().owner}; trx.operations = {op}; - trx.sign(*op.owners.begin(), generate_private_key("n")); - trx.sign(op.deposit_to_account(db).active.get_keys().front(), generate_private_key("n")); + trx.sign(generate_private_key("n")); + //trx.sign(generate_private_key("n")); db.push_transaction(trx); // Not using fixture's get_balance() here because it uses fixture's db, not my override