introduce adjusted early payout mode #540
Conversation
This allows bakers to pay out cycles as soon as they are computed, 5 cycles before they actually run, without losing money. This is done with a two phase reward calculation: * phase 1 calculates estimated rewards based on baking and endorsing rights, and pays them out * phase 2 recalculates rewards after the cycle actually runs. The difference between what should have been paid and what was paid out is called the overestimate. TRD tries to substract it as an adjustment during the ongoing payout. I urge you to judge this feature not by its complexity, but by what it brings to the table. It definitely makes the code more complex, but it allows newer bakers to pay out rewards early (6-9 days after delegating). This will make delegation and self-custody more popular. Feature ======= This PR does not change how TRD works when using a release_override of 0 (pay rewards after they unfreeze) or -5 (pay rewards after cycle runs). So the vast majority of users will not use this new code. To activate adjusted early payout, set `release_override` to -11. Any time you try to pay a cycle that has not run yet, it will pay estimated rewards and later substract an adjustment. We removed "estimated" payout mode since anyone wishing to pay early will prefer adjusted early payout. The valid payout modes are now "actual" and "ideal". When used in combination with a release_override of -11, it will pay estimated payouts and will later adjust to the wanted "actual" or "ideal" amount. This does not always work: it assumes delegator keeps delegating. When a delegator leaves or drains their account, TRD may not be able to claim back all dues. In this case, it will just claim back as much as possible and pay nothing (adjustment will be equal to reward amount, so payout will be zero). It also works in reverse. In most situations the actual/ideal reward is lower than the estimation, because of missed bakes and endorsements from the baker and other bakers which result in lower rewards. But actual/ideal also pay out fees and stolen block rewards, so it can occasionally happen that the adjustment is positive and the rewards are more than expected. Regular payouts are the most precise and still the default recommendation. Using adjusted early payouts still exposes the baker to reduced earnings, but less so than just paying expected rewards. It can be a way for bakers to differenciate. Why not make this behavior configurable with a toggle? To avoid configuration complexity, it is better to assume that people will want this feature whenever they pay early. Plus, "ideal" vs "actual" is now relevant when doing early payouts, so a toggle would not be enough anyway. It's best to make the behavior implicit. Calculation CSV file ==================== We introduce 3 new columns: overestimate, adjustment, and adjusted amount. Outside of the early payout mode (release_override 0 and -5), all three columns will show zero value and the rest of the colums will be identical to master branch today. In adjusted early payout, the columns are populated as follows: * "overestimate" is the difference between what we paid and what we should have paid. If we paid too much, it is positive. When the payout just happened, the values in the column show as "pending" as we do not know yet the value, until the cycle runs. * "adjustment" is the value substracted from the payout amount due to a previous overestimate. It is equal to the negative of the overestimate of the just completed cycle (which is in a different csv file 6 cycles earlier!), or the full payout amount if the overestimate is above. In the latter case, it means we are not recovering all losses. * adjusted_amount is the sum of the payout amount for the cycle being paid and the adjustment (generally a negative value). If the overestimate is greater than the payout amount, this value will be zero. Adjusted_amount is the value actually being paid to the delegator. This extra data makes it possible for the baker and their delegates to interpret payout values as needed. Logging ======= The log displays evidence of recalculation of completed cycle (here 436) and displays total adjustment (82,125,609 mutez) and the unrecovered amount (15,201,928 mutez). ``` 2021-12-31 11:17:15,670 - producer - INFO - Checking for pending payments: checking 442 <= 437 - (5 + 1) - -11 2021-12-31 11:17:15,671 - producer - WARNING - Please note that you are doing payouts for future rewards!!! These rewards are not earned yet, they are an estimation. 2021-12-31 11:17:15,671 - producer - INFO - Payment cycle is 442 2021-12-31 11:17:15,672 - producer - INFO - Checking for potential adjustment for recently completed cycle 436. 2021-12-31 11:17:15,672 - producer - INFO - TRD ran for cycle: 436, calculating adjustments. 2021-12-31 11:17:15,697 - producer - INFO - Computing rewards for cycle 436. 2021-12-31 11:17:16,740 - producer - INFO - Using actual rewards for payouts calculations 2021-12-31 11:17:16,741 - producer - INFO - Total rewards before processing is 129,163,176 mutez. 2021-12-31 11:17:16,745 - producer - INFO - Total rewards after processing is 129,163,176 mutez. 2021-12-31 11:17:16,745 - producer - INFO - Sum of amounts allocated to delegators is 129,163,170 mutez 2021-12-31 11:17:16,746 - producer - INFO - Difference between total rewards and sum of amounts allocated to delegators is 6 mutez. This is due to floating point arithmetic. (max allowed diff is 10) 2021-12-31 11:17:16,746 - producer - INFO - We overestimated payout for cycle 436 by 97,327,535.0 mutez. 2021-12-31 11:17:16,804 - producer - INFO - Calculation report is created at '' 2021-12-31 11:17:16,804 - producer - INFO - Computing rewards for cycle 442. 2021-12-31 11:17:17,969 - producer - INFO - Using estimated rewards for payouts calculations 2021-12-31 11:17:17,971 - producer - INFO - Total rewards before processing is 149,609,375 mutez. 2021-12-31 11:17:17,975 - producer - INFO - Total rewards after processing is 149,609,375 mutez. 2021-12-31 11:17:17,975 - producer - INFO - Total adjustment for past early payout is -82,125,609.0 mutez. 2021-12-31 11:17:17,975 - producer - INFO - Adjusted total rewards is 67,483,766.0 mutez. 2021-12-31 11:17:17,975 - producer - INFO - Sum of amounts allocated to delegators is 67,483,757.0 mutez 2021-12-31 11:17:17,975 - producer - INFO - Difference between total rewards and sum of amounts allocated to delegators is 9.0 mutez. This is due to floating point arithmetic. (max allowed diff is 10) 2021-12-31 11:17:17,976 - producer - INFO - After early payout of cycle 436, 15,201,928.0 mutez were not recovered. 2021-12-31 11:17:18,004 - producer - INFO - Calculation report is created at '' 2021-12-31 11:17:19,006 - consumer0 - INFO - Starting payments for cycle 442 ``` When adjusted early payout is not used, the logs show usual information. Implementation ============== A new function in payment_producer called `recompute_payouts` is called in case of advance payouts. It calculates overestimate and returns an `adjustment` map containing the amount to recover by address. This map is passed to the calculation logic, which attempts to recover the amounts. `recompute_payouts` also modifies the calculation csv file of a previous payout to record overestimate once computed. We add the new utility `csv_calculation_file_parser` which can read and write this file. We verified that reading and writing this file results in the exact same file. This utility is also abole to read an "old style" computation csv file that does not contain the three new columns. They will be added at subsequent write. `compute_payouts` function has been extended to also call the reward api model and the `calculate` function. This used to be done in `try_to_pay`. `recompute_payouts` calls `compute_payouts` whenever the overestimate is absent from the file. This results in reduced duplication of data. The actual payouts use the `adjusted_amount` value of the reward log instead of `amount` as previously. We grepped the code for all instances of `rl.amount` to make sure this happens in every case. In particular, in case of not empty founders and owners map, or payout redirection, we make sure that the "merge" still works and adjusted amounts are being merged. Tenderbake impact ================= Tenderbake affects the baking and endorsing rewards computation, and it also deprecates the unfreezing period. But it does not affect the new mechanism introduced here. In particular, there will still be an estimated amount, ideal and actual rewards. Merging this does not affect the tenderbake workload. Testing ======= We tested this against an actual baker that pays estimated rewards. We also tested the non-early payout. We compared csv files before/after and verified that the adjustments match with the overestimates and that the amounts being paid out are indeed adjusted. We ensured that the csv files show the same values than master when used in "normal" mode. The way adjusted rewards are calculated seem to introduce floating point variance of 1 mutez. We modified the tzkt integration test to be tolerant of this 1 tez difference, in order to make it pass. Documentation ============= We added a documentation page called "payout timing" to explain the release_override parameter.
|
First, awesome work. This is super interesting, and great clear explanation.
This is the biggest risk and should be in big red bold letters. This is reason enough to put it behind an explicit configuration option. At the same time I'd love to see a TZIP that allows delegators to 'commit' to a baker for a certain amount of cycles. |
Thanks for looking into the PR. The text is already in bold. I think a configuration option is redundant as you already have to choose |
|
I agree, it is implicit, this is why I liked the new section in the docs you created explaining fully the options when using release override at -11, and -5. No need of a parameter to a parameter. |
|
@ericlavoie here is an extract of a csv file for adjusted amount calculation for a recent payment on a baker that is using this mode |
|
This looks good to me, your work is detailed and extremely well documented, I asked others more knowledgeable to this code base to weight in. |
|
@nicolasochem I am troubled by this here:
I cannot say if people do want to adjust their payouts. To my knowledge there are bakers that use the estimated payouts mode and I think they have no desire to adjust it or change the mode. We would need to reach out to them when we remove the feature. What made you think you should remove the mode? |
Yes, do you know who these bakers are and can you reach out? I would think that they would happily switch to adjusted early payouts: their payouts will be more accurate and they will lose less tez. In the current state of the PR, anyone using "estimated" payout will have to switch their config to either "actual" or "ideal". If they leave release_override to -11, they will still pay estimated rewards, but the future payouts will be adjusted to match "actual" or "ideal" rewards (based on rewards_type) once we are able to compute them. 2 reasons why I opted to remove the mode:
What's the alternative then? We could have 4 options Or 2 configuration parameters: This is ugly too. In the PR currently, when release_override is -11, we pay estimated rewards. No need for a "mode". This is simpler and less confusing. Don't forget that a vast majority of bakers today don't care because they don't pay in advace. |
When the calculation csv file is first generated in early payout, the overestimate is set as `pending`. Later, this field is overwritten with the actual overestimate. I accidentally broke it during a code refactor, and I didn't relaize until today. Today marks 5 cycles after I deployed this PR for a production baker, so it's the first time a csv file that says `pending` is actually overwritten. Before, csv files in the format currently on master branch were being overwritten (without any overestimate row at all).
The problem is that IMO the payout mode is a "customer promise" by the baker - if I personally think it is useful or if I would do it like that is not up to discussion. You should always consult the other maintainer before you intend to cancel features of TRD. I don't see myself in the position to decide for the community a change like that. I have to think how we can proceed here as I don't want the new feature to not be merged. @utdrmac Do you see a way of knowing or signaling bakers that use this mode? |
|
Maybe it's not clear. The feature is still here. You can still pay 11 cycles in advance. The way to configure it is changing. Also, after you run it for a while, it starts adjusting payouts so the baker loses less money. I didn't make the adjustment optional since I see no good reason anyone would disable it. |
The issue is one of backward compatibility, anyone updating their code, that use estimated, this will now fail, like we all know not eveyone keeps up to date. Question we collect stats data from bakers (if they opted in) can we see how many are using this parameter? The other option is to leave the parameter (and mark it deprecated) and handle the cases, I assume using estimated means someone is using a value of -6 to -11 for release_override. If that is the case to be backward compatible, if someone has set estimated, we can default to actual. Can this work and resolve the issue. |
I suppose so, though I'm not familiar with the stats collection infra.
Yes, it works, though the hidden behavior makes me uneasy. Instead, I just pushed a commit that simply fails TRD in error when rewards_type is set to estimated: People using this mode, who upgrade their TRD, will come here and gain all the necessary context. Is this acceptable? |
|
May I suggest you lead with: |
|
Use the doc section that explains the new parameter |
The mode "estimated" was meant as a kind of baker insurrance. "However bad we are at baking - you get what was estimated". It was always clear that bakers can lose money with it. It is no unwanted behavior that needed fixing. We have to figure out how many people are troubled by depricating it. I asked BakingBad for help. |
Ah ok, bakingbad has bakers insurance, meaning if you miss a block or endorsement, the delegate still get the expected reward. It's a different feature. |
|
@jdsika I hope you're right. Although one baker has this feature, I can't say it's in high demand among the Tezos community. |
|
@jdsika when we started the Baking Bad project in 2018, there were a lot of payment strategies, but all bakers ended up with just one because it works great and everyone is happy with that, actually. |
|
I tend to agree, this use case is ultra fringe at best, I don't see myself ever using it, too much risk. Paying before unfreezing makes sense at the earliest. And seeing that very few use it. Is it worth the complexity that will need to be maintained. |
No, the current implementation does not do that. I would like only 0, -5 and -11 to be allowed, yes. I didn't implement because it wouldn't work on test networks where PRESERVED_CYCLES is different, so it would not be -5/-11 then. This has to be tested against test networks, so maybe that can be a followup discussion.
I will have to maintain this branch regardless of whether we merge it. Payout mode estimated is already in the code (not added by me) so there is already something to maintain (arguably there are less lines of code than what I want to merge). "Ultra fringe" for today's bakers and delegators. How about everyone who stakes on (edited: coinbase actually pays 40 days later) |
|
@nicolasochem can you add a warning at the startup of TRD that tells the user something like: "You have configured TRD to payout estimated rewards X cycles ahead of their potential realization. There might be unforseen attack vectors like being unable to adjust rewards of delegators that leave your baker. Are you aware of the risks? y/N" -> users acknowledges. |
|
@jdsika I can't add a Y/N dialog, unless I also implement a Why is an additional warning needed? There is already a warning:
Today, if people pay estimated rewards from master branch, there is no adjustment, so this "attack vector" is much worse than in this PR. I have added an extra line. Is this acceptable? |
These are the only values being used in practice. Also fix a cosmetic issue with logs.
When adding #540 (advanced early payout) I made an assumption that "payable" should be set to false if the adjustment is bigger than the payout, resulting in an adjusted amount of 0. It turns out, there are other reasons for adjusted_amount to be zero (such as a payout redirect). The code handles it later. The positive check affects this and results in unexpected description of the payout in csv file. Consequently I am removing this >0 check.
* fix "not in payment log" status in calculation When adding #540 (advanced early payout) I made an assumption that "payable" should be set to false if the adjustment is bigger than the payout, resulting in an adjusted amount of 0. It turns out, there are other reasons for adjusted_amount to be zero (such as a payout redirect). The code handles it later. The positive check affects this and results in unexpected description of the payout in csv file. Consequently I am removing this >0 check. * Fixed a bug in the validation of args for not existing objects * Contributor: nicolasochem, Effort=Compensated * Reviewer/Debugger: jdsika, Effort=2h
This allows bakers to pay out cycles as soon as they are computed, 5
cycles before they actually run, without losing money.
This is done with a two phase reward calculation:
rights, and pays them out
difference between what should have been paid and what was paid out is
called the overestimate. TRD tries to substract it as an adjustment
during the ongoing payout.
I urge you to judge this feature not by its complexity, but by what it
brings to the table. It definitely makes the code more complex, but it
allows newer bakers to pay out rewards early (6-9 days after
delegating). This will make delegation and self-custody more popular.
Feature
This PR does not change how TRD works when using a release_override of 0
(pay rewards after they unfreeze) or -5 (pay rewards after cycle runs).
So the vast majority of users will not use this new code.
To activate adjusted early payout, set
release_overrideto -11. Anytime you try to pay a cycle that has not run yet, it will pay estimated
rewards and later substract an adjustment.
We removed "estimated" payout mode since anyone wishing to pay early
will prefer adjusted early payout. The valid payout modes are now
"actual" and "ideal". When used in combination with a release_override
of -11, it will pay estimated payouts and will later adjust to the
wanted "actual" or "ideal" amount.
This does not always work: it assumes delegator keeps delegating. When a
delegator leaves or drains their account, TRD may not be able to claim
back all dues. In this case, it will just claim back as much as possible
and pay nothing (adjustment will be equal to reward amount, so payout
will be zero).
It also works in reverse. In most situations the actual/ideal reward is
lower than the estimation, because of missed bakes and endorsements from
the baker and other bakers which result in lower rewards. But
actual/ideal also pay out fees and stolen block rewards, so it can
occasionally happen that the adjustment is positive and the rewards are
more than expected.
Regular payouts are the most precise and still the default
recommendation. Using adjusted early payouts still exposes the baker to
reduced earnings, but less so than just paying expected rewards. It can
be a way for bakers to differenciate.
Why not make this behavior configurable with a toggle? To avoid
configuration complexity, it is better to assume that people will want
this feature whenever they pay early. Plus, "ideal" vs "actual" is
now relevant when doing early payouts, so a toggle would not be enough
anyway. It's best to make the behavior implicit.
Calculation CSV file
We introduce 3 new columns: overestimate, adjustment, and adjusted
amount.
Outside of the early payout mode (release_override 0 and -5), all three
columns will show zero value and the rest of the colums will be
identical to master branch today.
In adjusted early payout, the columns are populated as follows:
should have paid. If we paid too much, it is positive. When the payout
just happened, the values in the column show as "pending" as we do not
know yet the value, until the cycle runs.
previous overestimate. It is equal to the negative of the
overestimate of the just completed cycle (which is in a different csv
file 6 cycles earlier!), or the full payout amount if the overestimate
is above. In the latter case, it means we are not recovering all
losses.
paid and the adjustment (generally a negative value). If the
overestimate is greater than the payout amount, this value will be
zero. Adjusted_amount is the value actually being paid to the
delegator.
This extra data makes it possible for the baker and their delegates to
interpret payout values as needed.
Logging
The log displays evidence of recalculation of completed cycle (here 436)
and displays total adjustment (82,125,609 mutez) and the unrecovered
amount (15,201,928 mutez).
When adjusted early payout is not used, the logs show usual information.
Implementation
A new function in payment_producer called
recompute_payoutsis calledin case of advance payouts. It calculates overestimate and returns an
adjustmentmap containing the amount to recover by address. This mapis passed to the calculation logic, which attempts to recover the
amounts.
recompute_payoutsalso modifies the calculation csv file of a previouspayout to record overestimate once computed. We add the new utility
csv_calculation_file_parserwhich can read and write this file. Weverified that reading and writing this file results in the exact same
file.
This utility is also abole to read an "old style" computation csv file
that does not contain the three new columns. They will be added at
subsequent write.
compute_payoutsfunction has been extended to also call the reward apimodel and the
calculatefunction. This used to be done intry_to_pay.recompute_payoutscallscompute_payoutswhenever theoverestimate is absent from the file. This results in reduced
duplication of data.
The actual payouts use the
adjusted_amountvalue of the reward loginstead of
amountas previously. We grepped the code for all instancesof
rl.amountto make sure this happens in every case. In particular,in case of not empty founders and owners map, or payout redirection, we
make sure that the "merge" still works and adjusted amounts are being
merged.
Tenderbake impact
Tenderbake affects the baking and endorsing rewards computation, and it
also deprecates the unfreezing period. But it does not affect
the new mechanism introduced here. In particular, there will still be an
estimated amount, ideal and actual rewards. Merging this does not affect
the tenderbake workload.
Testing
We tested this against an actual baker that pays estimated rewards. We
also tested the non-early payout. We compared csv files before/after and
verified that the adjustments match with the overestimates and that the
amounts being paid out are indeed adjusted. We ensured that the csv
files show the same values than master when used in "normal" mode.
The way adjusted rewards are calculated seem to introduce floating point
variance of 1 mutez. We modified the tzkt integration test to be
tolerant of this 1 tez difference, in order to make it pass.
Documentation
We added a documentation page called "payout timing" to explain the
release_override parameter.
Update: