-
Notifications
You must be signed in to change notification settings - Fork 162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ambit claim for ledger event rewards #1080
base: master
Are you sure you want to change the base?
Conversation
19d3fd1
to
5bed79a
Compare
5bed79a
to
cf6df5f
Compare
* MIR payments from the reserves. | ||
* Refunds of the stake pool deposit payment when a pool is de-registered. | ||
|
||
Currently, the ledger provides the following reward related events: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably worth noting that the list below is the name of the wrapped events from the Cardano API. The corresponding ledger events are currently nested according to the ledger rule hierarchy (though we may change this is the near future, a clean up is on order)
ShelleyLedgerEventTICK . NewEpochEvent . DeltaRewardEvent . RupdEvent
ShelleyLedgerEventTICK . NewEpochEvent . MirEvent
ShelleyLedgerEventTICK . NewEpochEvent . EpochEvent . PoolReapEvent
ShelleyLedgerEventTICK . NewEpochEvent . TotalRewardEvent
* `LEDeltaReward` | ||
* `LEMirTransfer` | ||
* `LERetiredPools` | ||
* `LETotalRewards` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* `LETotalRewards` | |
* `LERewardEvent` |
* A single `LETotalRewards` event should be emitted for every epoch in the Shelley era and later, | ||
even if there are no rewards for that epoch (in which case the event reward map will be `mempty`). | ||
* `LETotalRewards` should include all payments to stake addresses for a given epoch. That means | ||
all staking rewards (member and owner), all MIR payments and all stake pool deposit refunds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is currently no such LETotalRewards
event in the Cardano API, do you meant the LERewardEvent
? The MIR payments and pool deposit refunds come from totally different ledger rules, we cannot make the event described here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is currently no such LETotalRewards event in the Cardano API,
db-sync
currently does not use cardano-api
for anything ledger event related.
Is there any accomodation ledger
might make the use of these events by the current one and only consumer of these events easier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently does not use cardano-api for anything ledger event related.
oh! Where does LETotalRewards
come from? It's not from cardano-ledger.
Is there any accomodation ledger might make the use of these events by the current one and only consumer of these events easier?
Absolutely! We just can't glue these events together in the way you are proposing, not with serious consequences
* `LETotalRewards` should include all payments to stake addresses for a given epoch. That means | ||
all staking rewards (member and owner), all MIR payments and all stake pool deposit refunds. | ||
* `LEDeltaReward` should only ever contain pool membership or pool ownership rewards. | ||
* `LEDeltaReward` may contain rewards to stake addresses that have been de-registered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some extra details that might be helpful.
In the Alonzo era, and previous eras:
- any stake address inside the stake distribution snapshot which is deregestered at the time the reward calculation begins will not be included in this event.
- any stake address inside the stake distribution snapshot which is deregestered after the reward calculation begins, and is not registered at the time of the epoch boundary, will have it's lovelace given to the treasury.
In the Babbage era:
- any stake address inside the stake distribution snapshot which is not registered at the time of the epoch boundary will have it's lovelace given to the treasury.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Era specific behavior for any event is a huge pain in the neck for db-sync
.
* `LEDeltaReward` may contain rewards to stake addresses that have been de-registered. | ||
* The sum of all `LEDeltaReward`, `LEMirTransfer` and `LERetiredPools` amounts for an epoch should | ||
always be the same as the sum of `LETotalRewards` event amounts for that epoch. | ||
* `LETotalRewards` will not contain rewards to stake addresses that have been de-registered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This requirement (that the total reward event sum is equal to the sum of the other events) is inconsistent with the idea that the LETotalRewards
does not contain the de-registered accounts, but the LEDeltaReward
might include them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😢 Do we have a different idea of the meaning of the word "total"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sum of all
LEDeltaReward
,LEMirTransfer
andLERetiredPools
amounts for an epoch should
always be the same as the sum ofLETotalRewards
event amounts for that epoch.
LEDeltaReward
may contain rewards to stake addresses that have been de-registered.
LETotalRewards
will not contain rewards to stake addresses that have been de-registered.
These three requirements are inconsistent. A reward for a stake addresses that has been de-registered would be in the sum of "all LEDeltaReward
, LEMirTransfer
and LERetiredPools
amounts" by the second quote above, but not in the sum of "LETotalRewards
event amounts" by the third quote above. Therefore the sums will not be the same in the presence of stake addresses that have been de-registered.
* The sum of all `LEDeltaReward`, `LEMirTransfer` and `LERetiredPools` amounts for an epoch should | ||
always be the same as the sum of `LETotalRewards` event amounts for that epoch. | ||
* `LETotalRewards` will not contain rewards to stake addresses that have been de-registered. | ||
* The `LETotalRewards` must be the last reward related event for a given epoch to be emitted from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ledger will make no guarantees about the ordering of the events corresponding to a given block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I can theoretically get an LEDeltaReward
for epoch N
after I get the LETotalRewards
for that same epoch? That does not make a lot of sense. More importantly, it makes the LETotalRewards
useless from the POV of db-sync
using it for validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that is theoretically possible. The event is still useful, we use it in the ledger property tests to ensure that the delta rewards are what we expect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal of all of these events is to be used by db-sync
which is currently the only consumer of ledger events. If its not useful for db-sync
is not useful.
@JaredCorduan My attempt to ask for what would make my life easier has obviously failed. How about you actually document what you can provide me with? It would be important to document everything I can rely on and as much as possible about what I cannot rely on. Hopefully the things I can rely on is a lot longer than the list of things I cannot rely on. Feel free to add that to the current document, below the text I wrote or add it here as a comment. |
I don't think it failed at all! I think we are making great progress! the ledger team's top priority is Babbage right now, but after that we should have time to document the events. |
No description provided.