-
Notifications
You must be signed in to change notification settings - Fork 51
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Refactoring + accusation payout toggle
When someone double bakes or endorses, anyone can submit an on-chain proof of equivocation. The bakers that includes the proof gets rewarded with half of the security deposit of the faulty baker. It is important to note that equivocation has been very rare on Tezos so this PR does not change TRD's behavior in the vast majority of cases. Out of the discussions leading to PR #415 being merged, it has emerged that TRD pays out accusation rewards inconsistently: RPC reward model will rely on the on-chain calculation of rewards, which includes accusation rewards. Tzstats does not include accusation rewards. Tzkt used to, but #470 changed the tzkt code to not do it. We agreed to make it configurable. This PR does it. It has turned into a major refactoring: the logic to calculate rewards used to be isolated per provider: tzkt, tzstats, rpc would make their own calculations and return the reward amount via the rewards model. It makes more sense to have this calculation done at payment producer level. To make this possible, I extended the model. While it was previously retuning only one value `total_reward_amount`, it now returns several additional values such as `equivocation_losses`, `offline_losses`, `rewards_and_fees`. The provider code populates these fields but no longer perform calculation of the reward amount to be split. Instead, this calculation is performed by the below core logic which is provider-independent: ``` if rewards_type.isEstimated(): logger.info("Using estimated rewards for payouts calculations") total_estimated_block_reward = reward_model.num_baking_rights * block_reward total_estimated_endorsement_reward = reward_model.num_endorsing_rights * endorsement_reward total_reward_amount = total_estimated_block_reward + total_estimated_endorsement_reward elif rewards_type.isActual(): logger.info("Using actual rewards for payouts calculations") if self.pay_denunciation_rewards: total_reward_amount = reward_model.total_reward_amount else: # omit denunciation rewards total_reward_amount = reward_model.rewards_and_fees - reward_model.equivocation_losses elif rewards_type.isIdeal(): logger.info("Using ideal rewards for payouts calculations") if self.pay_denunciation_rewards: total_reward_amount = reward_model.total_reward_amount + reward_model.offline_losses else: # omit denunciation rewards and double baking loss total_reward_amount = reward_model.rewards_and_fees + reward_model.offline_losses ``` payout_accusation_rewards is the new configuration option. It defaults to false, but you must set it to true if you use RPC provider otherwise you get an error: RPC can't distinguish between an accusation reward and a baking reward unless you parse every block (which is the job of an indexer). In other terms, RPC provider will populate `total_reward_amount` but not `rewards_and_fees` or `accusation_rewards` - it will leave these fields to "None". Extending the model required several changes to tests as well. Specifically tzkt tests continue to be a pain. In PR #501 we had hardcoded the accusation rewards. Now, with our extended model, we can compare apples to apples again, and I removed these hardcoded values. Configuration ------------- I modified configure.py to set the new flag. All flags are currently mandatory, so I did not change that, but anyone upgrading TRD will have to modify their config, which is not great. What does the team think? Should we default to false when the config flag is absent? Testing ------- I did some manual testing with the following baker: ``` baking_address: tz1WnfXMPaNTBmH7DBPwqCWs9cPDJdkGBTZ8 ``` On cycle 78. This baker was rewarded for denunciating a double baking on this cycle. I observed that the rewards were higher when the new setting was set to "True". I also observed that the rewards value was consistent between tzkt and tzstats.
- Loading branch information
1 parent
c11cf96
commit 9d49713
Showing
30 changed files
with
334 additions
and
187 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,24 @@ | ||
class RewardProviderModel: | ||
def __init__(self, delegate_staking_balance, total_reward_amount, delegator_balance_dict) -> None: | ||
def __init__(self, delegate_staking_balance, num_baking_rights, num_endorsing_rights, | ||
total_reward_amount, rewards_and_fees, equivocation_losses, denunciation_rewards, offline_losses, delegator_balance_dict) -> None: | ||
super().__init__() | ||
self.delegator_balance_dict = delegator_balance_dict | ||
self.total_reward_amount = total_reward_amount | ||
self.delegate_staking_balance = delegate_staking_balance | ||
self.num_baking_rights = num_baking_rights | ||
self.num_endorsing_rights = num_endorsing_rights | ||
# rewards that should have been earned, had the baker been online | ||
self.offline_losses = offline_losses | ||
|
||
# total reward as recorded in-protocol | ||
self.total_reward_amount = total_reward_amount | ||
|
||
# When using indexers, the total amount above can be itemized as follows: | ||
|
||
# * baking rewards, fees, revelation rewards | ||
self.rewards_and_fees = rewards_and_fees | ||
|
||
# * losses from double baking/endorsing | ||
self.equivocation_losses = equivocation_losses | ||
|
||
# * rewards from denunciating other people's double baking/endorsing | ||
self.denunciation_rewards = denunciation_rewards |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.