Normalized Beaconing Reward Units Capped at 1 instead of 2 (HIP15)

Hi team,

After doing some weekend research and random sample checks of reward unit distributions, @thenorpan and I have noticed that the the reward formula for w>N might have a bug that caps the total reward units at 1 instead of 2.

According to HIP15 (Reward formula) the transmitter's reward units should grow asymptotically towards 2, as shown in yellow bar in the graph. Furthermore, in the the text for w>N it says:

we give a small portion of the witness rewards to the transmitter up to 1 additional unit of rewards

Key here is to understand that it says up to 1 additional unit, on top of the 1 reward unit received for witnessing w=N.

Looking into the code it seems like the normalize_reward_unit function runs after Unit has been calculated. Consequently all reward units to beaconers are capped at 1 instead of 2.

Maybe I'm reading it the wrong way, let me know if I've missed anything :)

blockchain-core/src/transactions/v2/blockchain_txn_rewards_v2.erl

Lines 934 to 943 in dda3843

    
           poc_challengee_reward_unit(WitnessRedundancy, DecayRate, Witnesses) -> 
        
               case {WitnessRedundancy, DecayRate} of 
        
                   {undefined, _} -> {error, witness_redundancy_undefined}; 
        
                   {_, undefined} -> {error, poc_reward_decay_rate_undefined}; 
        
                   {N, R} -> 
        
                       W = length(Witnesses), 
        
                       Unit = poc_reward_tx_unit(R, W, N), 
        
                       {ok, normalize_reward_unit(Unit)} 
        
               end.

blockchain-core/src/transactions/v2/blockchain_txn_rewards_v2.erl

Line 945 in d7abd28

normalize_reward_unit(Unit) when Unit > 1.0 -> 1.0;

Thanks for the report @marcusze.

I believe clamping the normalized unit to 1.0 is by design since when the eventual rewards are being calculated each poc_challengee_reward_unit gets accumulated and scaled according to the TxScale value. Refer here + here and this + this.

Admittedly it's been a while since I've looked at this code, but it appears that the accumulation would imply that the "units" do get added up once the rewards have been processed.

Thanks for getting back @vihu! However I still think the normalisation (line 941) deviates from the intended design in HIP15.

To support my point I'd like to attach some of my calculations on a few samples of POC transactions. Where w>N we can see in the rightmost columns that the beaconer's implied reward units differ from the theoretical. Please have a look at this sheet: Beaconer Reward Unit Samples and let me know what you think.

Unfortunately the fix is more complicated as it will require a backwards compatibility shim via a chain var. Also, should we not be capping it to 2 here?

There's definitely a mismatch here though and we should fix it. Whether it's a cap to 2, I don't know yet, but it would seem that the reward_unit must tend towards 2 as the number of witnesses continues to increase (once above N).

So at the very minimum, we need:

Introduce new chain var "fix_reward_unit" or something, could just be a boolean
Guard new changes/fixes behind that
Test, verify with aux ledger and/or community members who monitor the chain
Release, communicate and activate the new var

I found some similar problems too . in #884
Invalid reward issue: From rewards distributed for a beacon with varying number of witnesses in HIP15, if there are invalid witness, the reward of beacon(TX) will be reduced (TX scale )by invalid. But I found that all the reward of beacon keep the same if valid witness more than 4. And the reward of witness(RX) is not as doucument expressed. the whole rewards distributed need to be correct when invalid witness there.

Beacon reward issue: from rewards distributed for a beacon with varying number of witnesses in HIP15, the reward of TX will increase when the valid witness increase while it will decrease the reward of TX will decreas when the valid witness decrease. but I found that all the reward of TX are same at same block.

This has been deployed https://github.com/helium/miner/blob/master/audit/var-83.md

	poc_challengee_reward_unit(WitnessRedundancy, DecayRate, Witnesses) ->
	case {WitnessRedundancy, DecayRate} of
	{undefined, _} -> {error, witness_redundancy_undefined};
	{_, undefined} -> {error, poc_reward_decay_rate_undefined};
	{N, R} ->
	W = length(Witnesses),
	Unit = poc_reward_tx_unit(R, W, N),
	{ok, normalize_reward_unit(Unit)}
	end.