Fix a bug on minimum voting mana requirement

[sydneyitguy]

A new user will have 0 SP and 0 VP, but votings should go through even though it has zero effect on the rewards.

Currently the code change was missing, so new users are getting
{“error”:“server_error”,“error_description”:“voter.voting_manabar.current_mana > 0: Account does not have enough mana to vote.“} when they vote.

Temporary work-around: Delegate 0.001 SP to the new account

[TimCliff]

This will likely need to be done as a hardfork change (with a hardfork condition check before the difference) otherwise it could cause a fork between nodes running the new vs. old code.

[iamsmooth]

Is it even possible for voting mana to be negative? Perhaps a simpler change is to remove the test altogether, which seems more consistent with the current rules about 'dust votes'. Agree this needs to be a hard fork.

[mvandeberg]

Voting mana cannot be negative be the amount consumed is a percentage of the remaining mana. In fact, I think it is impossible for voting mana to actually go to 0 if the user non-zero SP. This change is needed for the corner case of an account with SP, but does need to be gated by a hardfork check.

[mvandeberg]

Needs hardfork check

[youkaicountry]

@sydneyitguy Please make the requested changes from code review, otherwise this PR will be closed in two weeks. We're trying to tidy up the open PRs, thanks for your understanding!

[sydneyitguy]

@youkaicountry

I'm not a blockchain developer, so I don't know how I can do "hardfork check".
However, I think this is a critical issue because new users cannot even vote once at the moment, so someone has to fix..

Can anyone help? (or should I re-create this as an issue ticket?)

[mvandeberg]

A hardfork check is:

if( db.has_hardfork( STEEM_HARDFORK_0_21 ) )
{
   // new logic
}
else
{
   // old logic
}

If you make such a change without this check then an upgraded node might have different acceptance criteria for an operation or might even have a different result in a calculation. In either case, this leads to a blockchain fork between new nodes and old nodes.

You can see this throughout the codebase. Ideally, this would also be filed as a bug report and the hardfork constant would include the issue number as well.

An example:

steem/libraries/chain/steem_evaluator.cpp

Lines 1417 to 1429 in 3730a83

	if( _db.has_hardfork( STEEM_HARDFORK_0_20__1764 ) )
	{
	abs_rshares -= STEEM_VOTE_DUST_THRESHOLD;
	abs_rshares = std::max( int64_t(0), abs_rshares );
	}
	else if( _db.has_hardfork( STEEM_HARDFORK_0_14__259 ) )
	{
	FC_ASSERT( abs_rshares > STEEM_VOTE_DUST_THRESHOLD || o.weight == 0, "Voting weight is too small, please accumulate more voting power or steem power." );
	}
	else if( _db.has_hardfork( STEEM_HARDFORK_0_13__248 ) )
	{
	FC_ASSERT( abs_rshares > STEEM_VOTE_DUST_THRESHOLD || abs_rshares == 1, "Voting weight is too small, please accumulate more voting power or steem power." );
	}

This contains all previous iterations of logic surrounding the STEEM_VOTE_DUST_THRESHOLD over several hardforks (13, 14, and 20). You can go back to the issues (#248, #259, and #1764) to see the motivation behind each change.

[sydneyitguy]

@mvandeberg Thank you for your kind explanation 🙂
I have added a hard-fork check, and just removed the assertion on HF21 as you pointed out (voting mana cannot be negative value anyway).

[mvandeberg]

Related to #3004

[mvandeberg]

Fixed in #3004