introduce "self-ideal" rewards like Backerei #415
Conversation
I am the maintainer of backerei and in the process of depcrecating it. Backerei payout operations are different than TRD: its default, non-configurable behaviour is to pay rewards assuming perfect behaviour of the baker doing the payouts, while still accounting for imperfect behaviour of other bakers. This acts as a liveness guarantee and ensures maximum payouts for delegators, but without overpaying for other baker's downtime, like the current "ideal/expected" payout does. This PR adds this capability to TRD. Nomenclature ============ The current master branch has "ideal" and "actual" payouts. I initially thought about naming this new payout type "ideal_self" or "self-ideal" but it makes much more sense to do some renaming: | old | new | | ---- | ------ | |ideal |expected| |actual|actual | | |ideal | The new names are more self-explanatory: "pay expected rewards" convey that these rewards are not realized yet, while "pay ideal rewards" includes the fees and extra rewards, which makes sense. A drawback is that this will require a configuration change to current users and will need to be release-noted. But, we expect most users of the old-style "ideal/expected" rewards to be using them to pay out future cycles. This is not possible with the new style "ideal" type, because this reward type expect the cycle to actually have run, so these users will have an error upon upgrade, which will force them to upgrade their configuration from `rewards_type: ideal` to `rewards_type: expected`. I am open to any suggestions about better naming for this new model. Implementation ============== It only works with tzstats and tzkt API; at the moment, the TRD implementation relying on Tezos Node RPC does not implement detailed calculation of rewards, instead relying on unfreezing events to determine the rewards. So, there is no way to add this feature without implementing the detailed calculation itself. As a result, this type of rewards will error out when attempted to run with a Tezos Node RPC backend. Both tzkt and tzstats "rewards" endpoint provide fields for missed baking and endorsing rewards. When rewards_type.isIdeal is true, we add these rewards to the "actual" rewards calculation, and this becomes our total rewards. While doing this, I observed that TRD was never paying out stolen block rewards when used with the tzstats backend. This bug has been fixed. I also observed that tzkt's rewards endpoints (but not tzstats) exposes these fields: * `missedOwnBlockFees` * `missedExtraBlockFees` * `uncoveredOwnBlockFees` * `uncoveredExtraBlockFees` These are the fees that the baker would have earned, had they baked the blocks that they missed. We include them in the calculation because it makes sense to do so. tzstats does not expose these lost fees, so there is an ideal reward discrepancy between tzkt and tzstats backend when the baker fails to bake one or more blocks in the cycle, with tzstats backend reporting ideal payout lower than its true value. But this is an edge case so it makes sense to enable ideal payouts with tzstats backend anyway. We documented it. Testing ======= I tested different cycles for different bakers and verified that the calculations were the same on tzkt/tzstats and that the actual rewards matched what was showing on tzstats.
|
If a payout style cannot be calculated using all three available methods, then it shouldn't be implemented. I'm having difficulty understanding your change in payout calculations. you said:
But that's exactly the definition of ideal rewards: bake every prio 0 block and endorse every right. This is the "ideal" scenario. With ideal, if you miss a bake or an endorsement, you still pay out as if you did submit that bake. Obviously, actual rewards doesn't include those bakes you missed. Can you please rephrase what your new payouts term means? |
|
Hi Matthew, thanks for looking. PR is not ready but description is accurate. Say you baked and endorsed perfectly during one cycle, but due to some other baker being offline, you endorsed a priority 1 block due to no fault of your own. You are not going to earn the full 1.25 tez for this block, but if you pay expected rewards, you will pay out delegators based on the full amount. Same thing if you bake a block that does not contain enough endorsements because other bakers were offline. Because of this, you end up overpaying. Using expected rewards, a large enough baker will almost certainly overpay at every cycle. The expected payouts option is thus not very useful, unless you pay before the cycle actually runs. In this case, you have no choice but to make predictions based on rights. How the new "ideal" payout type is being calculated is: actual rewards + 1.25 per missed endorsement + 40 per missed block. Another way of looking at the proposed "ideal" payout type: it pays the precise amount needed for baking bad to consider your payouts "accurate".
Taking a closer look at the RPC code, it looks like I will be able to compute proposed "ideal" rewards with RPC as well (because it is possible to query the number of bakes and endorses per cycle), so if you agree in the merits of the feature, I'll go ahead and add it to this PR. |
That is not correct. If you bake a prio 1 TRD does not take that into consideration for 'ideal' payouts. Only prio0 bakes and all endorsements are considered for 'ideal'. Under 'actual', yes, you would pay additional rewards because you actually earned extra rewards.
How? If I'm a "large" baker and I bake 300 prio0 blocks in cycle 4 and I endorse 4000 times, both 'ideal' and 'actual' rewards are the same. Any baker, no matter "large" or "small", that has 100% efficiency in a cycle, ideal and actual rewards are the same. We use 'actual' and BB shows accurate payouts for our delegators, so I'm not understanding this reasoning when it's already correct. I'm not trying to be difficult, I'm just not understanding why we need to change something that is not broken. I'm not understanding why you think the current 'actual' and 'ideal' are calculated incorrectly. We have hundreds of bakers that use TRD every cycle and nobody has said anything about the inaccuracy of the calculated payouts. |
Your statement is correct, but it is not what I said. If you endorse a priority 1 block, you will earn 0.833333 tez, see documentation. If you bake a prio 0 block but one of the endorsers of the previous block was offline, you will earn 38.75 tez.
This is not correct for the reason stated above.
Provided you have 100% reliability, "actual" will show accurate payouts on BB. But if you fail to endorse a block or fail to bake at prio 0, "actual" will show as too low on BB. With my new payout model, you will show as accurate and shoulder the cost of your downtime without passing it to your delegators.
Current "actual" and "ideal" payouts are calculated correctly. I am not changing any of the payout model, I am just introducing a third one which maps backerei's default behaviour. Separately, I am proposing a renaming as follows:
|
|
@utdrmac I have now added support for the rpc method as well. Next I will add tests, but what is your sentiment? Do you understand where I am coming from with this new payout type? |
|
This is one of my oldest delegators, https://baking-bad.org/tz1RJTLee1JMDskpixR4mqBsT8iXGdZJJhX4, aside from our server migration in C335, he shows perfect payouts for past 50+ cycles. Surely, somewhere in all that time, we have missed endorsements, or the previous-block had 1 less endorsement. We've (and TRD) always used 'actual' payouts. Just to clarify, you don't see any issue with 'actual' correct? And I think I understand what you're saying: The issue is that current 'ideal' assumes perfect behavior for you and everyone else. Going on your example, if you bake block X prio 0, but block X - 1 only had 31 endorsements, instead of getting 40xtz, you get 38.75. TRD does not account for this. TRD assumes all rewards are earned under 'ideal' and thus you would be paying out an extra 1.25 that cycle because the baker of X-1 messed up, or an endorser didn't submit. Thus, you are being penalized for no fault of your own. After thinking about this, I'm more inclined to merge the behavior you are proposing into 'ideal', rather than introduce a new behavior. I'm curious on @dansan566 and @jdsika opinions here. |
No, I don't see any issue with actual. It looks like I am wrong with baking-bad and it is "forgiving" you for missed endorsements. But the calculation method I am proposing here has been used for calculating Cryptium Labs payouts since cycle 11 (one of the largest bakers until recently). I think it has merit.
The issue is that you can not use proposed "ideal" payouts before the cycle runs ( |
|
Dear all, thank you very much for the discussion and suggestions. I would very much take a "deep dive" into the topic today and discuss it with Daniel. I think we have to formulate some suiting definitions here that satisfy everyone and I am sure this is achievable. "Names" are amoungst the hardest things in programming :) Best regards |
|
I will try to recap to see if I understood it correctly. What does "actual" rewards mean for TRD:
What is "ideal":
-> The naming is derived from the assumption that we will encounter "ideal" network conditions and a 100% participation of all in the lottery drawn validators. What is "ideal" for baeckerei?
@nicolasochem I take from your conversation that you did NOT find any bug in the calculation method with regard to my explanation above? I think the word "actual rewards" does refer correctly to what a baker does "actually" earn from successful validation operations and does share this with the clients. Slashed funds and gains from accusations are a both not accounted for as they are somehow both sides of the same medal. The word "ideal" can imo refer correctly to an ideal network behaviour and at the same time it is also "expected" for people who are rather positive thinkers but I think the expectation is that the network does always have hicups. So I myself expect lost endorsements (we are often losing due to network errors or bugs in the software and not due to our own wrong doing). So for me it IS "ideal" and not "expected".
|
In the tzkt case, client do NOT gain with additional stolen block (prio 1). I fixed it in this PR.
Based on my reading of the code, clients DO gain from slashing other bakers. Please correct me if I'm wrong.
No, "ideal" for backerei does pay for fees, nonce revelations, stolen blocks and equivocation accusation reward. It deviates from the TRD "actual" definition by adding 1.25 tez per missed endorsement and 40 tez per missed bake to the total amount to be distributed.
I did find a bug where tzkt didn't pay for stolen blocks. I also think that TRD distributes the accusation rewards. (why wouldn't it?) |
|
Thinking a bit more about this, maybe "pay estimated rewards" is the better way to put it.
|
|
@utdrmac I merged master and fixed some tests. I also renamed "expected" to "estimated" as I was suggesting earlier. Could you please have a second look at this? You said
If it helps, I can also break this work in 2, do this first, and then debate the merits of a third behavior in a next PR. |
|
Let's try it with a table with definitions taken from BakingBad. An "X" indicates that the delegator participates, "-" not and "O" for optional depending on baker config.
** Assuming you do not compensate for misstakes of others If the code for "Actual" is paying out the accusation rewards it is in fact a bug in my opinion. The rationale is very clear for me: |
|
Your table is both additive and substractive. May I suggest another table with only positive quantities. X means "taken into account for reward calculation" and "-" means "not taken into account".
I am also removing the transfer/allocation fees since I believe it's irrelevant for this stage where we calculate total rewards. I also think the losses due to slashings and revelation failures are irrelevant in the total reward calculation. These losses are most certainly higher than any rewards, so you would not even be running a payout engine in such situation.
If you keep the accusation rewards, you may face pressure to transfer them back to the faulty party. If you distributed them to your delegates, then you can wash your hands of it and be immune from pressure. We can make it configurable, but most people will stick to the default and I think that the default should be to pay them out. But this is a separate discussion and I'll fix the PR to not change today's behavior. |
|
@nicolasochem I don't agree with you on the accusation reward. Often it is a mistake (start backup node but the main node is still running) and therefore human error and not an attack that leads to a double baking event. Often it is even a friendly baker. We would rather send the accusation reward back to the baker instead of distributing it. If this really happens, I think it should always be discussed and distributed (to delegators, slashed baker, keep it) separately. |
|
The best option would be to drop the "categories" of "ideal" etc and just add the configuration table to the config file. |
|
OK, I see the point. I agree that distributing the accusation rewards is controversial and it should be a config option. I don't think everything should be a config option. Nothing else is controversial or ambiguous, and too many knobs is not always good, and "estimated/ideal/actual" is quite self-explanatory. How about a toggle "pay accusation rewards" (default to no) and keep the three categories? |
Thanks @akrykunenko for helping with testing. This issue is present in master as well: It was done like this since the tzkt support was added. We should not fix this within this PR. I think it should be configurable as said above. |
|
@vkresch we had several strange failures for the validation_baker_address test. Is it using an external RPC? What can be the cause? TImeouts? Update: it worked with a re run |
@nicolasochem do I understand that only tzkt has this behavior? In this case I would really prefer to modify the tzkt method so that it does not payout those accusation rewards in order for all methods to behave the same way and THEN make an issue to make this configurable in the future. I would say that this is my last note for this PR before I want to merge it. @dansan566 takes care about the descrepancies between tzkt and tzstats in a conversation with their devs. We will leave a note in the release notes about our observation. |
|
@nicolasochem I merged #470 and resolved the merge conflict of this branch. Please verify the comment I made for "ideal" and that I added the slashing rewards correctly in ideal. |
|
looks like tzkt ideal is still taking accusation rewards into account. |
|
@dansan566 yes, I understood from Nicolas that it should be part of ideal with every backend. But if you tell me it is now just part of tzkt I will remove it there again. |
It was just part of tzkt. I pushed a fix so now accusation rewards are not part of any calculation for any reward type and any data source. |
|
I did some final tests. All look good from my side. PR can be merged IMO. Now, I also know why tzstats and tzkt numbers can differ: When an endorsement is included in a block N, the baker actually validates block N-1, as per Tezos docs. Efforts: 7h |
As I was working on some refactoring, I took a look at the tzkt integration tests. It appears that the tzkt tests are "cached" because they take one hour to run. Having discovered that, I then re-ran the tests. To do that, I deleted the cached data in tests/integration/tzkt_data/* I discovered that I actually broke the tests in #415 Previously, tzkt was paying out denunciation rewards. We have changed this default behavior to not pay them. The tzkt test compares the actual rewards from tzkt with the actual rewards from rpc. But rpc actual rewards (calculated by the protocol) take into account denunciation rewards. So we now have a discrepancy. To solve it, I am hardcoding the reward value that we expect, for these 2 tests where there is a double baking gain. This is a band-aid. We have several problems: * tests should not be cached. What should be cached is the tzkt api calls so they return faster than the real things (we talked about that before) * these tests should be generalized to tzkt, tzstats and api * the cycles we are testing against are pre-granada. we should replace with more recent interesting cycles Also there is a test mysteriously named "test staking balance issue" which was also failing. I assume it is a reference to [this](https://baking-bad.org/blog/2020/06/11/we-were-calculating-staking-balances-wrong/). This test attempts to match total staking balance value from rpc and tzkt in a cycle where the snapshot index was 15. An "assert_not_equal" test was previously passing and is now failing, which means that some values we expected not to match are now matching. Not sure how to interpret this! I removed this statement. @m-kus you wrote these tests and @utdrmac you are familiar with the "staking balance issue", perhaps you can make more sense of this test than I? Other changes * added comments so the next person is less confused than I was * indent the json cached data
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 anything to TRD 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 #415 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 returning only one value, it now returns several values (rewards from baking, endorsing, fees and revelations; accusation rewards; double baking losses). Various providers populate the same fields with custom logic. We end up with the below core logic executed independly of the provider chosen: ``` 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.
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 anything to TRD 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 returning only one value, it now returns several values (rewards from baking, endorsing, fees and revelations; accusation rewards; double baking losses). Various providers populate the same fields with custom logic. We end up with the below core logic executed independly of the provider chosen: ``` 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.
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 anything to TRD 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 returning only one value, it now returns several values (rewards from baking, endorsing, fees and revelations; accusation rewards; double baking losses). Various providers populate the same fields with custom logic. We end up with the below core logic executed independly of the provider chosen: ``` 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.
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.
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.
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`, `accusation_rewards`. 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 ``` The following is true: ``` total_reward_amount = rewards_and_fees + accusation_rewards - equivocation_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". This explains why we left `total_reward_amount` in the model. While it can be derived by other fields with tzstats or tzkt, this is not possible with RPC. 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.
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`, `accusation_rewards`. 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 ``` The `calculate` function now takes an extra arg `total_reward_amount` because this value is now controller by payment_producer and is no longer equivalent to `total_reward_amount` in the model. The following is true: ``` total_reward_amount = rewards_and_fees + accusation_rewards - equivocation_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". This explains why we left `total_reward_amount` in the model. While it can be derived by other fields with tzstats or tzkt, this is not possible with RPC. 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.
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. We introduce a new configuration option `payout_accusation_rewards`. This PR 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`, `accusation_rewards`. Here are the new variables for the payment_producer object model: ``` self.delegator_balance_dict = delegator_balance_dict 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 ``` 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 computed_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: computed_reward_amount = reward_model.total_reward_amount else: # omit denunciation rewards computed_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: computed_reward_amount = reward_model.total_reward_amount + reward_model.offline_losses else: # omit denunciation rewards and double baking loss computed_reward_amount = reward_model.rewards_and_fees + reward_model.offline_losses ``` The result of tris calculation is a new variable `computed_reward_amount` which is then passed to the `calculate` function as a new argument. Previously `calculate` was relying on `total_reward_amount` value in the model. The following is true: ``` total_reward_amount = rewards_and_fees + accusation_rewards - equivocation_losses ``` RPC can't distinguish between an accusation reward and a baking reward unless you parse every block (which is the job of an indexer). RPC provider will populate `total_reward_amount` but not `rewards_and_fees` or `accusation_rewards`. It will leave these fields to "None". This explains why we left `total_reward_amount` in the model. While it can be derived by other fields with tzstats or tzkt, this is not possible with RPC. 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.
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. We introduce a new configuration option `payout_accusation_rewards`. When using configure.py, the default choice is "False". This PR 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`, `accusation_rewards`. Here are the new variables for the payment_producer object model: ``` self.delegator_balance_dict = delegator_balance_dict 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 ``` The provider code populates these fields but no longer performs 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 computed_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: computed_reward_amount = reward_model.total_reward_amount else: # omit denunciation rewards computed_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: computed_reward_amount = reward_model.total_reward_amount + reward_model.offline_losses else: # omit denunciation rewards and double baking loss computed_reward_amount = reward_model.rewards_and_fees + reward_model.offline_losses ``` The result of tris calculation is a new variable `computed_reward_amount` which is then passed to the `calculate` function as a new argument. Previously `calculate` was relying on `total_reward_amount` value in the model. The following is true: ``` total_reward_amount = rewards_and_fees + accusation_rewards - equivocation_losses ``` RPC can't distinguish between an accusation reward and a baking reward unless you parse every block (which is the job of an indexer). RPC provider will populate `total_reward_amount` but not `rewards_and_fees` or `accusation_rewards`. It will leave these fields to "None". This explains why we left `total_reward_amount` in the model. While it can be derived by other fields with tzstats or tzkt, this is not possible with RPC. 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.
* 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. * We introduce a new configuration option `payout_accusation_rewards`. When using configure.py, the default choice is "False". * This PR 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. * Contributor: nicolasochem, Effort=Compensated * Reviewer: vkresch, Effort=3h
I am the maintainer of backerei and in the process of depcrecating it.
Backerei payout operations are different than TRD: its default,
non-configurable behaviour is to pay rewards assuming perfect behaviour
of the baker doing the payouts, while still accounting for imperfect
behaviour of other bakers. This acts as a liveness guarantee and ensures
maximum payouts for delegators, but without overpaying for other baker's
downtime, like the current "ideal/expected" payout does.
This PR adds this capability to TRD.
Nomenclature
The current master branch has "ideal" and "actual" payouts. I initially
thought about naming this new payout type "ideal_self" or "self-ideal"
but it makes much more sense to do some renaming:
expectedestimatedThe new names are more self-explanatory: "pay estimated rewards" convey
that these rewards are not realized yet, while "pay ideal rewards"
includes the fees and extra rewards, which makes sense.
A drawback is that this will require a configuration change to current
users and will need to be release-noted. But, we expect most users of
the old-style "ideal/expected" rewards to be using them to pay out
future cycles. This is not possible with the new style "ideal" type,
because this reward type expect the cycle to actually have run, so these
users will have an error upon upgrade, which will force them to upgrade
their configuration from
rewards_type: idealtorewards_type: expected.I am open to any suggestions about better naming for this new model.
Implementation
It only works with tzstats and tzkt API; at the moment, the TRDimplementation relying on Tezos Node RPC does not implement detailed
calculation of rewards, instead relying on unfreezing events to
determine the rewards. So, there is no way to add this feature without
implementing the detailed calculation itself. As a result, this type of
rewards will error out when attempted to run with a Tezos Node RPC
backend.
This feature was implemented in all 3 payment methods (tzkt, tzstats, rpc).
Both tzkt and tzstats "rewards" endpoint provide fields for missed
baking and endorsing rewards. When rewards_type.isIdeal is true, we add
these rewards to the "actual" rewards calculation, and this becomes our
total rewards.
While doing this, I observed that TRD was never paying out stolen blockrewards when used with the tzstats backend. This bug has been fixed.
I also observed that tzkt's rewards endpoints (but not tzstats) exposes
these fields:
missedOwnBlockFeesmissedExtraBlockFeesuncoveredOwnBlockFeesuncoveredExtraBlockFeesThese are the fees that the baker would have earned, had they baked the
blocks that they missed. We include them in the calculation because it
makes sense to do so. tzstats does not expose these lost fees, so there
is an ideal reward discrepancy between tzkt and tzstats backend when the
baker fails to bake one or more blocks in the cycle, with tzstats
backend reporting ideal payout lower than its true value. But this is
an edge case so it makes sense to enable ideal payouts with tzstats
backend anyway. We documented it.
For RPC we scan blocks with baking and endorsing rights and check whether
baking/endorsing happened. When it didn't, we add 40/1.25 tez to the rewards.