Skip to content

Commit

Permalink
fix problem of activation of code that used to be dead
Browse files Browse the repository at this point in the history 
The dead code in question was setting RC pool fill value to max (and new account tokens to zero).
It was dead in mainnet and testnet except for unit tests, that is why some unit tests needed to have
RC cost related values corrected.
  • Loading branch information
@vogel76
ABW authored and vogel76 committed Aug 10, 2023
1 parent 3091bbc commit db338d3
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 94 deletions.
108 changes: 23 additions & 85 deletions libraries/chain/rc/rc_utility.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -761,10 +761,7 @@ struct pre_apply_operation_visitor
_db.rc.regenerate_rc_mana( account, _current_time );
}

void operator()( const account_create_with_delegation_operation& op )const
{
regenerate( op.creator );
}
//void operator()( const account_create_with_delegation_operation& op )const //not needed, RC not active when this op was active

void operator()( const transfer_to_vesting_operation& op )const
{
Expand All @@ -777,35 +774,24 @@ struct pre_apply_operation_visitor
regenerate( op.account );
}

void operator()( const set_withdraw_vesting_route_operation& op )const
{
regenerate( op.from_account );
}
//void operator()( const set_withdraw_vesting_route_operation& op )const //not needed - there is no change in VESTS

void operator()( const delegate_vesting_shares_operation& op )const
{
regenerate( op.delegator );
regenerate( op.delegatee );
}

void operator()( const author_reward_operation& op )const
{
regenerate( op.author );
}
//void operator()( const author_reward_operation& op )const //not needed - since HF17 rewards need to be claimed

void operator()( const curation_reward_operation& op )const
{
regenerate( op.curator );
}
//void operator()( const curation_reward_operation& op )const //not needed - since HF17 rewards need to be claimed

// Is this one actually necessary?
void operator()( const comment_reward_operation& op )const
{
regenerate( op.author );
}
//void operator()( const comment_reward_operation& op )const //it was never needed

void operator()( const fill_vesting_withdraw_operation& op )const
{
//TODO: when that code is moved to direct handling of the operation remember to fix workaround for
//implied route in case all the power down is distributed to other defined routes (also in AH and possibly HAF)
regenerate( op.from_account );
regenerate( op.to_account );
}
Expand All @@ -822,20 +808,11 @@ struct pre_apply_operation_visitor
}
#endif

void operator()( const hardfork_operation& op )const
{
if( op.hardfork_id == HIVE_HARDFORK_0_1 )
{
const auto& idx = _db.get_index< account_index >().indices().get< by_id >();
for( auto it = idx.begin(); it != idx.end(); ++it )
{
regenerate( it->get_name() );
}
}
}
//void operator()( const hardfork_operation& op )const //the code that was here never worked (called before RC was active)

void operator()( const hardfork_hive_operation& op )const
{
//see comment in pre_apply_operation
regenerate( op.account );
for( auto& account : op.other_affected_accounts )
regenerate( account );
Expand All @@ -846,10 +823,7 @@ struct pre_apply_operation_visitor
regenerate( op.account );
}

void operator()( const comment_benefactor_reward_operation& op )const
{
regenerate( op.benefactor );
}
//void operator()( const comment_benefactor_reward_operation& op )const //not needed - since HF17 rewards need to be claimed

void operator()( const producer_reward_operation& op )const
{
Expand All @@ -858,6 +832,7 @@ struct pre_apply_operation_visitor

void operator()( const clear_null_account_balance_operation& op )const
{
//we could just always set RC value on 'null' to 0
regenerate( HIVE_NULL_ACCOUNT );
}

Expand Down Expand Up @@ -918,14 +893,12 @@ struct post_apply_operation_visitor
_db.rc.update_account_after_vest_change( account, _current_time, _fill_new_mana,
_check_for_rc_delegation_overflow );
}
void operator()( const account_create_with_delegation_operation& op )const
{
update_after_vest_change( op.creator );
}

//void operator()( const pow_operation& op )const //not needed, RC not active when pow was active
//void operator()( const account_create_with_delegation_operation& op )const //not needed, RC not active when this op was active

//void operator()( const pow2_operation& op )const //not needed, RC not active when pow2 was active
//void operator()( const pow_operation& op )const //not needed, RC not active when this op was active

//void operator()( const pow2_operation& op )const //not needed, RC not active when this op was active

void operator()( const transfer_to_vesting_operation& op )
{
Expand All @@ -944,20 +917,13 @@ struct post_apply_operation_visitor
update_after_vest_change( op.delegatee, true, true );
}

void operator()( const author_reward_operation& op )const
{
update_after_vest_change( op.author );
//not needed, since HF17 rewards need to be claimed before they affect vest balance
}
//void operator()( const author_reward_operation& op )const //not needed - since HF17 rewards need to be claimed

void operator()( const curation_reward_operation& op )const
{
update_after_vest_change( op.curator );
//not needed, since HF17 rewards need to be claimed before they affect vest balance
}
//void operator()( const curation_reward_operation& op )const //not needed - since HF17 rewards need to be claimed

void operator()( const fill_vesting_withdraw_operation& op )const
{
//see comment in pre_apply_operation
update_after_vest_change( op.from_account, true, true );
update_after_vest_change( op.to_account );
}
Expand All @@ -974,33 +940,12 @@ struct post_apply_operation_visitor
}
#endif

void operator()( const hardfork_operation& op )const
{
if( op.hardfork_id == HIVE_HARDFORK_0_1 )
{
const auto& idx = _db.get_index< account_index, by_id >();
for( auto it = idx.begin(); it != idx.end(); ++it )
update_after_vest_change( it->get_name() );
}

if( op.hardfork_id == HIVE_HARDFORK_0_20 )
{
const auto& params = _db.get< rc_resource_param_object, by_id >( rc_resource_param_id_type() );

_db.modify( _db.get< rc_pool_object, by_id >( rc_pool_id_type() ), [&]( rc_pool_object& p )
{
for( size_t i = 0; i < HIVE_RC_NUM_RESOURCE_TYPES; ++i )
{
p.set_pool( i, int64_t( params.resource_param_array[i].resource_dynamics_params.max_pool_size ) );
}

p.set_pool( resource_new_accounts, 0 );
} );
}
}
//void operator()( const hardfork_operation& op )const //the code that was here never worked (called before RC was active)

void operator()( const hardfork_hive_operation& op )const
{
//TODO: when moving this code to actual operation make sure the problem in AH is fixed
//(final values of funds taken never showing up in history - always with zeros)
update_after_vest_change( op.account, true, true );
for( auto& account : op.other_affected_accounts )
update_after_vest_change( account, true, true );
Expand All @@ -1011,11 +956,7 @@ struct post_apply_operation_visitor
update_after_vest_change( op.account );
}

void operator()( const comment_benefactor_reward_operation& op )const
{
update_after_vest_change( op.benefactor );
//not needed, since HF17 rewards need to be claimed before they affect vest balance
}
//void operator()( const comment_benefactor_reward_operation& op )const //not needed - since HF17 rewards need to be claimed

void operator()( const producer_reward_operation& op )const
{
Expand Down Expand Up @@ -1043,10 +984,7 @@ struct post_apply_operation_visitor
//void operator()( const remove_proposal_operation& op )const

template< typename Op >
void operator()( const Op& op )const
{
// ilog( "handling post-apply operation default" );
}
void operator()( const Op& op )const {}
};

void resource_credits::on_post_apply_operation_impl( const hive::protocol::operation& op ) const
Expand Down
18 changes: 10 additions & 8 deletions tests/unit/tests/direct_rc_delegation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ BOOST_AUTO_TEST_CASE( update_outdel_overflow )
BOOST_REQUIRE( alice_rc_before.get_delegated_rc() == uint64_t(vesting_amount) );
BOOST_REQUIRE( alice_rc_before.get_received_rc() == 0 );
BOOST_REQUIRE( alice_rc_before.last_max_rc == creation_rc );
BOOST_REQUIRE( alice_rc_before.rc_manabar.current_mana == creation_rc - ( 28756 + 28757 ) ); // cost of the two delegate rc ops (the one to dave costs more because more data is in the op)
BOOST_REQUIRE( alice_rc_before.rc_manabar.current_mana == creation_rc - ( 57288 + 57290 ) ); // cost of the two delegate rc ops (the one to dave costs more because more data is in the op)

BOOST_REQUIRE( bob_rc_account_before.rc_manabar.current_mana == creation_rc + 10 );
BOOST_REQUIRE( bob_rc_account_before.last_max_rc == creation_rc + 10 );
Expand All @@ -587,7 +587,7 @@ BOOST_AUTO_TEST_CASE( update_outdel_overflow )
BOOST_REQUIRE( dave_rc_account_before.last_max_rc == creation_rc + vesting_amount - 10 );
BOOST_REQUIRE( dave_rc_account_before.get_received_rc() == uint64_t(vesting_amount - 10) );

generate_blocks(10); //couple blocks to let rc mana regenerate
generate_blocks(20); //couple blocks to let rc mana regenerate

// we delegate and it affects one delegation
// Delegate 5 vests out, alice has 5 remaining rc, it's lower than the rc_adjustment which is 10
Expand All @@ -605,7 +605,7 @@ BOOST_AUTO_TEST_CASE( update_outdel_overflow )
BOOST_REQUIRE( alice_rc_after.get_delegated_rc() == uint64_t(vesting_amount) - 5 );
BOOST_REQUIRE( alice_rc_after.get_received_rc() == 0 );
BOOST_REQUIRE( alice_rc_after.last_max_rc == creation_rc );
BOOST_REQUIRE( alice_rc_after.rc_manabar.current_mana == creation_rc - 28943 );
BOOST_REQUIRE( alice_rc_after.rc_manabar.current_mana == creation_rc - 58047 );

BOOST_REQUIRE( bob_rc_account_after.rc_manabar.current_mana == creation_rc + 5 );
BOOST_REQUIRE( bob_rc_account_after.last_max_rc == creation_rc + 5 );
Expand All @@ -631,7 +631,7 @@ BOOST_AUTO_TEST_CASE( update_outdel_overflow )
BOOST_REQUIRE( alice_rc_after_two.get_delegated_rc() == uint64_t(vesting_amount) - 11 );
BOOST_REQUIRE( alice_rc_after_two.get_received_rc() == 0 );
BOOST_REQUIRE( alice_rc_after_two.last_max_rc == creation_rc );
BOOST_REQUIRE( alice_rc_after_two.rc_manabar.current_mana == creation_rc - ( 28943 + 28944 ) );
BOOST_REQUIRE( alice_rc_after_two.rc_manabar.current_mana == creation_rc - ( 58047 + 58048 ) );

BOOST_REQUIRE( bob_rc_account_after_two.rc_manabar.current_mana == creation_rc );
BOOST_REQUIRE( bob_rc_account_after_two.last_max_rc == creation_rc );
Expand Down Expand Up @@ -1341,7 +1341,7 @@ BOOST_AUTO_TEST_CASE( rc_negative_regeneration_bug )
generate_block();

//wait a bit so the RC used for comment is restored
generate_blocks( db->head_block_time() + fc::seconds( 60 ) );
generate_blocks( db->head_block_time() + fc::seconds( 150 ) );

BOOST_TEST_MESSAGE( "All delegators make RC delegations to their delegatees with the same power" );
auto rc_delegate = [&]( const account_name_type& from, const account_name_type& to, int64_t amount, const fc::ecc::private_key& key )
Expand Down Expand Up @@ -1387,7 +1387,9 @@ BOOST_AUTO_TEST_CASE( rc_negative_regeneration_bug )
int64_t undelegated = db->get_account( "delegator1" ).get_next_vesting_withdrawal().value;
rc_delegate( "delegator3", "pattern3", full_vest - undelegated, delegator3_private_key );
generate_block();
//pattern2 RC regeneration is triggered by author_reward_operation, but it doesn't modify RC because rewards go to separate balance until claimed
//pattern2 RC regeneration used to be triggered by author_reward_operation, but since it doesn't modify RC, that was removed
//we need to trigger regeneration manually
db->rc.regenerate_rc_mana( pattern2_rc, db->head_block_time().sec_since_epoch() );
BOOST_REQUIRE_EQUAL( delegatee_rc.rc_manabar.current_mana, pattern2_rc.rc_manabar.current_mana - undelegated );
//pattern3 undelegated exactly the same amount of RC as was dropped from delegatee by delegator1 powering down
BOOST_REQUIRE_EQUAL( delegatee_rc.rc_manabar.current_mana, pattern3_rc.rc_manabar.current_mana );
Expand All @@ -1402,7 +1404,7 @@ BOOST_AUTO_TEST_CASE( update_outdel_overflow_delegatee )
{
try
{
BOOST_TEST_MESSAGE( "Testing: update_outdel_overflow with a vesting delegation" );
BOOST_TEST_MESSAGE( "Testing: update_outdel_overflow with a vesting delegation" );
generate_block();
db_plugin->debug_update( [=]( database& db )
{
Expand Down Expand Up @@ -1462,7 +1464,7 @@ BOOST_AUTO_TEST_CASE( update_outdel_overflow_delegatee )
BOOST_REQUIRE( bob_rc_account_before.get_delegated_rc() == uint64_t(vesting_amount) );
BOOST_REQUIRE( bob_rc_account_before.last_max_rc == creation_rc );
BOOST_REQUIRE( bob_rc_account_before.get_received_rc() == 0 );
BOOST_REQUIRE( bob_rc_account_before.rc_manabar.current_mana == creation_rc - ( 28755 + 28756 ) ); // cost of the two delegate rc ops (the one to dave costs more because more data is in the op)
BOOST_REQUIRE( bob_rc_account_before.rc_manabar.current_mana == creation_rc - ( 57287 + 57289 ) ); // cost of the two delegate rc ops (the one to dave costs more because more data is in the op)

BOOST_REQUIRE( eve_rc_account_before.rc_manabar.current_mana == creation_rc + vesting_amount - 10 );
BOOST_REQUIRE( eve_rc_account_before.last_max_rc == creation_rc + vesting_amount - 10 );
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/tests/rc_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ BOOST_AUTO_TEST_CASE( rc_tx_order_bug )

ACTORS( (alice)(bob) )
generate_block();
vest( "initminer", "bob", ASSET( "70000.000 TESTS" ) ); //<- change that amount to tune RC cost
vest( "initminer", "bob", ASSET( "35000.000 TESTS" ) ); //<- change that amount to tune RC cost
fund( "alice", ASSET( "1000.000 TESTS" ) );

const auto& alice_rc = db->get_account( "alice" );
Expand Down

0 comments on commit db338d3

Please sign in to comment.