Reward can be negative when transaction fees exceed 2× the coinbase

[ruuda]

Summary

When the block’s transaction fees approach 2× the block’s coinbase reward, the formulas for computing the rewards break down.

Details

For common class, reward is proportional to effectiveCommonPoolWeighting: https://github.com/jrwashburn/mina-pool-payout/blob/05dcebcc977591a0de7c00f7a46a9b60727e3789/src/core/payoutCalculator/PayoutCalculator.ts#L94-L96

Which is the quotient of the staker’s commonStake and the sum of all staker’s commonStake: https://github.com/jrwashburn/mina-pool-payout/blob/05dcebcc977591a0de7c00f7a46a9b60727e3789/src/core/payoutCalculator/PayoutCalculator.ts#L84-L85 https://github.com/jrwashburn/mina-pool-payout/blob/05dcebcc977591a0de7c00f7a46a9b60727e3789/src/core/payoutCalculator/PayoutCalculator.ts#L66

For non-locked stake, the commonStake is stakingBalance times a factor 2 - superChargedWeightingDiscount: https://github.com/jrwashburn/mina-pool-payout/blob/05dcebcc977591a0de7c00f7a46a9b60727e3789/src/core/payoutCalculator/PayoutCalculator.ts#L60-L62

This means that if superChargedWeightingDiscount > 2, commonStake turns negative.

The superChargedWeightingDiscount is set to the block’s user transaction fees divided by the block’s coinbase: https://github.com/jrwashburn/mina-pool-payout/blob/05dcebcc977591a0de7c00f7a46a9b60727e3789/src/core/payoutCalculator/PayoutCalculator.ts#L51 https://github.com/jrwashburn/mina-pool-payout/blob/05dcebcc977591a0de7c00f7a46a9b60727e3789/src/core/payoutCalculator/PayoutCalculator.ts#L43

usercommandtransactionfees is controlled by user input, it depends on transactions that users submit. If one malicious user is willing to spend twice the coinbase on transaction fees, it would cause this script to compute rewards for common shares as negative. Which rewards turn negative depends on the ratio of locked to non-locked stake:

• If there is sufficient locked stake to make sumEffectiveCommonPoolStakes come out positive, then the quotient effectivePoolStakes[staker.publicKey].commonStake / sumEffectiveCommonPoolStakes is negative for non-locked stakes but positive for locked stakes.
• If there is insufficient locked stake, sumEffectiveCommonPoolStakes will itself be negative. Then the negatives cancel for non-locked stake, and non-locked stakes get a positive reward, but now locked stakes get a negative reward.
• If the ratio between locked and non-locked stake is just right, sumEffectiveCommonPoolStakes could come out to zero, making the entire computation invalid due to division by zero.

One might argue that transaction fees in practice are much lower than the coinbase so this is a hypothetical problem, but to me this demonstrates a deeper problem with the formulas. If the block has a positive coinbase reward, and we have to distribute that among stakers, there should not be any point in the parameter space where a staker gets a negative reward.

[jrwashburn]

Thanks for the very detailed issue!

By default, the payout runs with isolated supercharged pool: https://github.com/jrwashburn/mina-pool-payout/blob/main/src/core/payoutCalculator/PayoutCalculatorIsolateSuperCharge.ts

That said, some of the details of the issue are relevant.

I don't think it is possible in the protocol for transaction fees to exceed the coinbase; instead, an empty block would be created. So I don't think this is a valid issue; however, it would make good sense to add additional guards to fail on the scenarios you note. I think the best behavior would be to fail since I don't understand how this situation could arise, or what the calculation should be if it does.

[garethtdavies]

transaction fees are received by the block producer so are effectively unbounded to the upside. We've already seen massive transaction fee blocks on the KuCoin launch.

Snark fees can't exceed coinbase + tx fees, as those are paid out.

This is how I defined the supercharged weighting. The idea was as tx fees become larger, a larger share should go to non-supercharged holders. https://docs.minaexplorer.com/minaexplorer/calculating-payments#defining-the-supercharged-weighting

[garethtdavies]

Examples where tx fees > 2x coinbase are really hard to find but here's one for a long time ago that could be used to test this https://minaexplorer.com/block/3NLFAaRj1HWSZSWVYqihUiHpkznFzcwh6uLvM2ytxwphuBV9ZWaX

[jrwashburn]

I guess my cold / snark fee obsession has gone to my head. Transaction fees, not snark fees. Sorry. Will correct this when I can - probably week after next. Open for PR's as well obviously.

[ruuda]

Open for PR's as well obviously.

It’s not clear to me what a proper solution would be; I don’t understand why superchargedWeightingDiscount is in there in the first place and why transaction fees should affect how the coinbase is distributed among stakers. The formulas would make more sense to me without superchargedWeightingDiscount, but presumably it was put in place for a reason?

[jrwashburn]

The supercharged weight smooths out the impact of locked vs. unlocked accounts, and is intended to give unlocked accounts a higher proportional share of each block, whether supercharged or not. This was based on Gareth's original design and he referenced above: https://docs.minaexplorer.com/minaexplorer/calculating-payments. It looks like I took a shortcut in implementing it along the way leading to the vulnerability you have pointed out.

The alternative approach, which is the default now, only shares the supercharged portion of any block with unlocked accounts. My reasoning for the alternative implementation has been that for smaller pools, the smoothing hurts locked accounts in epochs where the unlocked accounts are unlucky, and it's cleaner/simpler to just let unlocked accounts share in the supercharged portion exclusively. The default implementation doesn't have any superchargedWeightingDiscount.

I don't know if/how many people use the weighted calculator, but I think the fix for the weighted approach is straightforward and think I'll just implement Gareth's formula for those that want that approach.

[jrwashburn]

Could be closed by PR #107 with a simple check - if transaction fees >= coinbase, discount will be set to 1.

Not sure what to do supercharged blocks that burn the excess 720 ... should it be treated like a regular block or a supercharged block? The difference in the guard would be

transactionFees >= block.coinbase - burnAmount vs. transactionFees >= block.coinbase