Fix all outstanding unexpected test failures

This commit is contained in:
Nathan Hourt 2015-07-01 13:53:04 -04:00
parent e336691e59
commit f29eaa92e2
4 changed files with 56 additions and 45 deletions

View file

@ -425,17 +425,17 @@ void database::notify_changed_objects()
changed_objects(changed_ids); changed_objects(changed_ids);
} }
processed_transaction database::apply_transaction( const signed_transaction& trx, uint32_t skip ) processed_transaction database::apply_transaction(const signed_transaction& trx, uint32_t skip)
{ {
processed_transaction result; processed_transaction result;
with_skip_flags( skip, [&]() with_skip_flags(skip, [&]()
{ {
result = _apply_transaction( trx ); result = _apply_transaction(trx);
} ); });
return result; return result;
} }
processed_transaction database::_apply_transaction( const signed_transaction& trx ) processed_transaction database::_apply_transaction(const signed_transaction& trx)
{ try { { try {
uint32_t skip = get_node_properties().skip_flags; uint32_t skip = get_node_properties().skip_flags;
trx.validate(); trx.validate();
@ -450,13 +450,14 @@ processed_transaction database::_apply_transaction( const signed_transaction& tr
//This check is used only if this transaction has an absolute expiration time. //This check is used only if this transaction has an absolute expiration time.
if( !(skip & skip_transaction_signatures) && trx.relative_expiration == 0 ) if( !(skip & skip_transaction_signatures) && trx.relative_expiration == 0 )
{ {
eval_state._sigs.reserve( trx.signatures.size() ); eval_state._sigs.reserve(trx.signatures.size());
for( const auto& sig : trx.signatures ) for( const auto& sig : trx.signatures )
{ {
FC_ASSERT( eval_state._sigs.insert( std::make_pair( public_key_type(fc::ecc::public_key( sig, trx.digest() )), false) ).second, "Multiple signatures by same key detected" ) ; FC_ASSERT( eval_state._sigs.insert(std::make_pair(public_key_type(fc::ecc::public_key(sig, trx.digest())),
false)).second,
"Multiple signatures by same key detected" );
} }
} }
//If we're skipping tapos check, but not dupe check, assume all transactions have maximum expiration time. //If we're skipping tapos check, but not dupe check, assume all transactions have maximum expiration time.
@ -522,8 +523,8 @@ processed_transaction database::_apply_transaction( const signed_transaction& tr
ref_block_height = uint32_t( x0 ); ref_block_height = uint32_t( x0 );
} }
const block_summary_object& tapos_block_summary const block_summary_object& tapos_block_summary =
= static_cast<const block_summary_object&>( static_cast<const block_summary_object&>(
get_index<block_summary_object>() get_index<block_summary_object>()
.get(block_summary_id_type(ref_block_height)) .get(block_summary_id_type(ref_block_height))
); );
@ -531,13 +532,16 @@ processed_transaction database::_apply_transaction( const signed_transaction& tr
//This is the signature check for transactions with relative expiration. //This is the signature check for transactions with relative expiration.
if( !(skip & skip_transaction_signatures) ) if( !(skip & skip_transaction_signatures) )
{ {
eval_state._sigs.reserve( trx.signatures.size() ); eval_state._sigs.reserve(trx.signatures.size());
for( const auto& sig : trx.signatures ) for( const auto& sig : trx.signatures )
{ {
FC_ASSERT(eval_state._sigs.insert( FC_ASSERT(eval_state._sigs.insert(std::make_pair(
std::make_pair(public_key_type(fc::ecc::public_key(sig, trx.digest(tapos_block_summary.block_id) )), public_key_type(
false)).second, "Multiple signatures by same key detected"); fc::ecc::public_key(sig,
trx.digest(tapos_block_summary.block_id))),
false)).second,
"Multiple signatures by same key detected");
} }
} }
@ -564,7 +568,7 @@ processed_transaction database::_apply_transaction( const signed_transaction& tr
}); });
} }
eval_state.operation_results.reserve( trx.operations.size() ); eval_state.operation_results.reserve(trx.operations.size());
//Finally process the operations //Finally process the operations
processed_transaction ptrx(trx); processed_transaction ptrx(trx);
@ -574,14 +578,15 @@ processed_transaction database::_apply_transaction( const signed_transaction& tr
eval_state.operation_results.emplace_back(apply_operation(eval_state, op)); eval_state.operation_results.emplace_back(apply_operation(eval_state, op));
++_current_op_in_trx; ++_current_op_in_trx;
} }
ptrx.operation_results = std::move( eval_state.operation_results ); ptrx.operation_results = std::move(eval_state.operation_results);
//Make sure the temp account has no non-zero balances //Make sure the temp account has no non-zero balances
const auto& index = get_index_type<account_balance_index>().indices().get<by_account>(); const auto& index = get_index_type<account_balance_index>().indices().get<by_account>();
auto range = index.equal_range(GRAPHENE_TEMP_ACCOUNT); 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); }); 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)) ) //Make sure all signatures were needed to validate the transaction
if( !(skip & (skip_transaction_signatures|skip_authority_check)) )
{ {
for( const auto& item : eval_state._sigs ) for( const auto& item : eval_state._sigs )
{ {

View file

@ -36,7 +36,6 @@ digest_type processed_transaction::merkle_digest()const
digest_type transaction::digest()const digest_type transaction::digest()const
{ {
//Only use this digest() for transactions with absolute expiration times. //Only use this digest() for transactions with absolute expiration times.
if( relative_expiration != 0 ) edump((*this));
assert(relative_expiration == 0); assert(relative_expiration == 0);
digest_type::encoder enc; digest_type::encoder enc;
fc::raw::pack( enc, *this ); fc::raw::pack( enc, *this );
@ -60,15 +59,15 @@ graphene::chain::transaction_id_type graphene::chain::transaction::id() const
memcpy(result._hash, hash._hash, std::min(sizeof(result), sizeof(hash))); memcpy(result._hash, hash._hash, std::min(sizeof(result), sizeof(hash)));
return result; return result;
} }
void graphene::chain::signed_transaction::sign( const private_key_type& key ) void graphene::chain::signed_transaction::sign(const private_key_type& key)
{ {
if( relative_expiration != 0 ) if( relative_expiration != 0 )
{ {
if( !block_id_cache.valid() ) edump((*this)); // Relative expiration is set, meaning we must include the block ID in the signature
assert(block_id_cache.valid()); assert(block_id_cache.valid());
signatures.push_back( key.sign_compact( digest(*block_id_cache) ) ); signatures.push_back(key.sign_compact(digest(*block_id_cache)));
} else { } else {
signatures.push_back( key.sign_compact( digest() ) ); signatures.push_back(key.sign_compact(digest()));
} }
} }

View file

@ -587,31 +587,33 @@ BOOST_FIXTURE_TEST_CASE( double_sign_check, database_fixture )
trx.set_expiration(db.head_block_time() + fc::minutes(1)); trx.set_expiration(db.head_block_time() + fc::minutes(1));
trx.operations.push_back(transfer_operation({ asset(), from.id, to.id, amount, memo_data() })); 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() ) ); for( auto& op : trx.operations ) op.visit(operation_set_fee(db.current_fee_schedule()));
trx.validate(); trx.validate();
db.push_transaction(trx, ~0); db.push_transaction(trx, ~0);
trx.operations.clear(); trx.operations.clear();
trx.operations.push_back(transfer_operation({ asset(), to.id, from.id, amount, memo_data() })); 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() ) ); for( auto& op : trx.operations ) op.visit(operation_set_fee(db.current_fee_schedule()));
trx.validate(); trx.validate();
BOOST_TEST_MESSAGE( "Verify that not-signing causes an exception" ); BOOST_TEST_MESSAGE( "Verify that not-signing causes an exception" );
BOOST_REQUIRE_THROW( db.push_transaction(trx, 0 ), fc::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_TEST_MESSAGE( "Verify that double-signing causes an exception" );
BOOST_REQUIRE_THROW( db.push_transaction(trx, 0 ), fc::exception ); trx.sign(to_private_key);
trx.sign(to_private_key);
BOOST_REQUIRE_THROW( db.push_transaction(trx, 0), fc::exception );
BOOST_TEST_MESSAGE( "Verify that signing with an extra, unused key fails" ); BOOST_TEST_MESSAGE( "Verify that signing with an extra, unused key fails" );
trx.signatures.pop_back(); trx.signatures.pop_back();
trx.sign( generate_private_key("bogus") ); trx.sign(generate_private_key("bogus"));
BOOST_REQUIRE_THROW( db.push_transaction(trx, 0 ), fc::exception ); BOOST_REQUIRE_THROW( db.push_transaction(trx, 0), fc::exception );
BOOST_TEST_MESSAGE( "Verify that signing once with the proper key passes" ); BOOST_TEST_MESSAGE( "Verify that signing once with the proper key passes" );
db.push_transaction(trx, 0 ); trx.signatures.pop_back();
trx.sign( to_private_key ); db.push_transaction(trx, 0);
trx.sign(to_private_key);
} FC_LOG_AND_RETHROW() } } FC_LOG_AND_RETHROW() }

View file

@ -124,16 +124,19 @@ BOOST_AUTO_TEST_CASE( withdraw_permission_test )
REQUIRE_THROW_WITH_VALUE(op, amount_to_withdraw, asset(6)); REQUIRE_THROW_WITH_VALUE(op, amount_to_withdraw, asset(6));
trx.clear(); trx.clear();
trx.operations.push_back(op); trx.operations.push_back(op);
trx.sign(dan_key_id, dan_private_key); trx.sign(dan_private_key);
PUSH_TX( db, trx ); PUSH_TX( db, trx );
// would be legal on its own, but doesn't work because trx already withdrew // would be legal on its own, but doesn't work because trx already withdrew
REQUIRE_THROW_WITH_VALUE(op, amount_to_withdraw, asset(5)); REQUIRE_THROW_WITH_VALUE(op, amount_to_withdraw, asset(5));
// Make sure we can withdraw again this period, as long as we're not exceeding the periodic limit // Make sure we can withdraw again this period, as long as we're not exceeding the periodic limit
trx.operations.back() = op; // withdraw 1 trx.clear();
trx.ref_block_prefix++; // make it different from previous trx so it's non-duplicate // withdraw 1
trx.sign(dan_key_id, dan_private_key); trx.operations = {op};
// make it different from previous trx so it's non-duplicate
trx.ref_block_prefix++;
trx.sign(dan_private_key);
PUSH_TX( db, trx ); PUSH_TX( db, trx );
trx.clear(); trx.clear();
} }
@ -149,10 +152,10 @@ BOOST_AUTO_TEST_CASE( withdraw_permission_test )
BOOST_CHECK(permit_object.withdrawal_limit == asset(5)); BOOST_CHECK(permit_object.withdrawal_limit == asset(5));
BOOST_CHECK(permit_object.withdrawal_period_sec == fc::hours(1).to_seconds()); BOOST_CHECK(permit_object.withdrawal_period_sec == fc::hours(1).to_seconds());
BOOST_CHECK_EQUAL(permit_object.claimed_this_period.value, 2 ); BOOST_CHECK_EQUAL(permit_object.claimed_this_period.value, 2 );
BOOST_CHECK(permit_object.expiration == first_start_time + 5*permit_object.withdrawal_period_sec ); BOOST_CHECK(permit_object.expiration == first_start_time + 5*permit_object.withdrawal_period_sec);
generate_blocks(first_start_time + permit_object.withdrawal_period_sec); generate_blocks(first_start_time + permit_object.withdrawal_period_sec);
// lazy update: verify period_start_time isn't updated until new trx occurs // lazy update: verify period_start_time isn't updated until new trx occurs
BOOST_CHECK(permit_object.period_start_time == first_start_time ); BOOST_CHECK(permit_object.period_start_time == first_start_time);
} }
{ {
@ -163,11 +166,12 @@ BOOST_AUTO_TEST_CASE( withdraw_permission_test )
op.withdraw_to_account = dan_id; op.withdraw_to_account = dan_id;
op.amount_to_withdraw = asset(5); op.amount_to_withdraw = asset(5);
trx.operations.push_back(op); trx.operations.push_back(op);
trx.sign(dan_key_id, dan_private_key); trx.sign(dan_private_key);
//Throws because nathan doesn't have the money //Throws because nathan doesn't have the money
BOOST_CHECK_THROW(PUSH_TX( db, trx ), fc::exception); BOOST_CHECK_THROW(PUSH_TX( db, trx ), fc::exception);
op.amount_to_withdraw = asset(1); op.amount_to_withdraw = asset(1);
trx.operations.back() = op; trx.clear();
trx.operations = {op};
trx.sign(dan_key_id, dan_private_key); trx.sign(dan_key_id, dan_private_key);
PUSH_TX( db, trx ); PUSH_TX( db, trx );
} }
@ -181,14 +185,14 @@ BOOST_AUTO_TEST_CASE( withdraw_permission_test )
const withdraw_permission_object& permit_object = permit(db); const withdraw_permission_object& permit_object = permit(db);
BOOST_CHECK(permit_object.authorized_account == dan_id); BOOST_CHECK(permit_object.authorized_account == dan_id);
BOOST_CHECK(permit_object.withdraw_from_account == nathan_id); BOOST_CHECK(permit_object.withdraw_from_account == nathan_id);
BOOST_CHECK(permit_object.period_start_time == first_start_time + permit_object.withdrawal_period_sec ); BOOST_CHECK(permit_object.period_start_time == first_start_time + permit_object.withdrawal_period_sec);
BOOST_CHECK(permit_object.expiration == first_start_time + 5*permit_object.withdrawal_period_sec ); BOOST_CHECK(permit_object.expiration == first_start_time + 5*permit_object.withdrawal_period_sec);
BOOST_CHECK(permit_object.withdrawal_limit == asset(5)); BOOST_CHECK(permit_object.withdrawal_limit == asset(5));
BOOST_CHECK(permit_object.withdrawal_period_sec == fc::hours(1).to_seconds()); BOOST_CHECK(permit_object.withdrawal_period_sec == fc::hours(1).to_seconds());
generate_blocks(permit_object.expiration); generate_blocks(permit_object.expiration);
} }
// Ensure the permit object has been garbage collected // Ensure the permit object has been garbage collected
BOOST_CHECK(db.find_object( permit ) == nullptr ); BOOST_CHECK(db.find_object(permit) == nullptr);
{ {
withdraw_permission_claim_operation op; withdraw_permission_claim_operation op;
@ -197,7 +201,7 @@ BOOST_AUTO_TEST_CASE( withdraw_permission_test )
op.withdraw_to_account = dan_id; op.withdraw_to_account = dan_id;
op.amount_to_withdraw = asset(5); op.amount_to_withdraw = asset(5);
trx.operations.push_back(op); trx.operations.push_back(op);
trx.sign(dan_key_id, dan_private_key); trx.sign(dan_private_key);
//Throws because the permission has expired //Throws because the permission has expired
BOOST_CHECK_THROW(PUSH_TX( db, trx ), fc::exception); BOOST_CHECK_THROW(PUSH_TX( db, trx ), fc::exception);
} }
@ -388,7 +392,7 @@ BOOST_AUTO_TEST_CASE( feed_limit_test )
op.asset_to_update = bit_usd.get_id(); op.asset_to_update = bit_usd.get_id();
op.issuer = bit_usd.issuer; op.issuer = bit_usd.issuer;
trx.operations = {op}; trx.operations = {op};
trx.sign(nathan_key_id, nathan_private_key); trx.sign(nathan_private_key);
db.push_transaction(trx); db.push_transaction(trx);
BOOST_TEST_MESSAGE("Checking current_feed is null"); BOOST_TEST_MESSAGE("Checking current_feed is null");
@ -396,8 +400,9 @@ BOOST_AUTO_TEST_CASE( feed_limit_test )
BOOST_TEST_MESSAGE("Setting minimum feeds to 3"); BOOST_TEST_MESSAGE("Setting minimum feeds to 3");
op.new_options.minimum_feeds = 3; op.new_options.minimum_feeds = 3;
trx.clear();
trx.operations = {op}; trx.operations = {op};
trx.sign(nathan_key_id, nathan_private_key); trx.sign(nathan_private_key);
db.push_transaction(trx); db.push_transaction(trx);
BOOST_TEST_MESSAGE("Checking current_feed is not null"); BOOST_TEST_MESSAGE("Checking current_feed is not null");