consensus query for ledger reward provenance #2830
Conversation
| @@ -285,6 +293,7 @@ querySupportedVersion = \case | |||
| GetLedgerTip -> (>= v1) | |||
| GetEpochNo -> (>= v1) | |||
| GetNonMyopicMemberRewards {} -> (>= v1) | |||
| GetRewardProvenance -> (>= v2) | |||
There was a problem hiding this comment.
@mrBliss is it okay to add this new query to version 2 like I've done? Is there anywhere else that I need to document it?
There was a problem hiding this comment.
Unfortunately, adding this new query requires adding a new version, as V2 is already out in the wild and does not include this new query 🙁. If you plan on adding another query soon, I would try to get it in before this version is released, otherwise yet another version will be needed.
I'll push a commit to this PR to show what needs to be done to add a new version. It can be used as a template for the next queries. cc: @nfrisby.
FYI, the new version is needed because nodes using V2 are already released and they are not aware of the new GetRewardProvenance query. So when they receive such a query, they will fail to decode it and disconnect. The sender of the query will be left in the dark. By adding a new version and requiring it for the new query (done in this function), the sender of the new query will see an error when sending it to a node that doesn't support the new version.
There was a problem hiding this comment.
That makes sense, thank you for the explanation!
There was a problem hiding this comment.
@JaredCorduan Thanks for doing the work!
I'll push some fixes to this PR.
| GetRewardProvenance | ||
| :: Query (ShelleyBlock era) (SL.RewardProvenance (EraCrypto era)) |
There was a problem hiding this comment.
I would move it to the bottom of the list so that it is chronological, which I think is useful in querySupportedVersion.
There was a problem hiding this comment.
Yep, I agree, that's nicer.
| sameDepIndex GetRewardProvenance _ | ||
| = Nothing |
There was a problem hiding this comment.
You'll also need a case for a match:
sameDepIndex GetRewardProvenance GetRewardProvenance
= Just Refl| GetRewardProvenance -> | ||
| snd $ SL.getRewardInfo globals st |
There was a problem hiding this comment.
Can you move this one to the bottom too, for consistency? (Same for all other places you had to touch)
| @@ -285,6 +293,7 @@ querySupportedVersion = \case | |||
| GetLedgerTip -> (>= v1) | |||
| GetEpochNo -> (>= v1) | |||
| GetNonMyopicMemberRewards {} -> (>= v1) | |||
| GetRewardProvenance -> (>= v2) | |||
There was a problem hiding this comment.
Unfortunately, adding this new query requires adding a new version, as V2 is already out in the wild and does not include this new query 🙁. If you plan on adding another query soon, I would try to get it in before this version is released, otherwise yet another version will be needed.
I'll push a commit to this PR to show what needs to be done to add a new version. It can be used as a template for the next queries. cc: @nfrisby.
FYI, the new version is needed because nodes using V2 are already released and they are not aware of the new GetRewardProvenance query. So when they receive such a query, they will fail to decode it and disconnect. The sender of the query will be left in the dark. By adding a new version and requiring it for the new query (done in this function), the sender of the new query will see an error when sending it to a node that doesn't support the new version.
I will probably force-push, so that the history is clean (and each commit is buildable). |
This commit adds a new consensus query which returns a lot of data surrounding the reward calculation in the ledger. There is data concerning the overall calculation, but also detailed information about each individual stake pool's rewards and performance. As this is a new query, a new version number is needed.
mrBliss
force-pushed
the
jc/query-reward-provenance
branch
from
1762372
to
0c2b19e
Compare
|
@nfrisby Adding you as a reviewer, as you will be doing this in the future 🙂. Unfortunately, adding new queries requires adding a new version. See the the last commit for a template on what to do. The golden test results are generated automatically by running the golden tests. |
Thank you for cleaning this up for me @mrBliss ! |
|
Build succeeded: |
This commit adds a new consensus query which returns a lot of data surrounding the reward calculation in the ledger. There is data concerning the overall calculation, but also detailed information about each individual stake pool's rewards and performance.
I updated the ledger package to master in order to get the CBOR instances for the reward provenance.
Resolves: CAD-2387