Skip to content

Fix logging reward map #1083

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Fix logging reward map #1083

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cardano-db-sync/src/Cardano/DbSync/Era/Shelley/Validate.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ queryRewardMap (EpochNo epochNo) = do
`on` (\(rwd :& saddr) ->
rwd ^. Db.RewardAddrId ==. saddr ^. Db.StakeAddressId)
where_ (rwd ^. Db.RewardSpendableEpoch ==. val epochNo)
where_ (not_ $ rwd ^. Db.RewardType ==. val Db.RwdDepositRefund)
Copy link
Contributor

@erikd erikd Mar 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this means we no longer have a way of validating deposit refunds.

I really do not think this is a good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this function validates anything. This runs after the validation has failed.

We already don't validate deposit refunds.

Copy link
Contributor

@erikd erikd Mar 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation failure is at least in part due to the pool deposit refunds. One of the validation failures was:

 [2022-03-03 05:13:40.43 UTC]   e1581eab598e036f2bb6506204b5ad483e47fb817e880b1adfe5c04f09: 
    [(RwdDepositRefund,DbLovelace 500000000)] /= []

where the LHS is from the database and the RHS is from the ledger event.

This used to work.

Copy link
Contributor Author

@kderme kderme Mar 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation failure is at least in part due to the pool deposit refunds.

It's not really, the deposit refunds appear in the error message, but are not the reason of failure.

See also this existing comment in queryEpochRewardTotal

            -- For ... reasons ... pool deposit refunds are put into the rewards account
            -- but are not considered part of the total rewards for an epoch.

This was like this before. We do no validations of deposit refunds.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation detects any difference between what is in the db and what is provided by ledger.

The comment is almost certainly wrong (the code has gone through a lot of iterations and its easy to update the code while forgetting to update the comments). Before the latest update, this validation passed and now it does not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not just a comment. It's also in the query queryEpochRewardTotal

where_ (not_ $ rwd ^. Db.RewardType ==. val Db.RwdDepositRefund)

Before the latest update, this validation passed and now it does not.

Which update do you mean? This fixes an existing mismatch on master: The validation filters out deposits but the comment doesn't.

Copy link
Contributor

@erikd erikd Mar 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean whichever version of ledger was used in the 12.0.x release worked correctly. Then the legder people unilaterally changed things and broke db-sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This simply fixes the error message. It doesn't change and has nothing to do with the validation, the new changes in ledger which haven't been in master yet or whether this error message should appear or not.

In master there is no validation of deposits refunds.

orderBy [desc (saddr ^. Db.StakeAddressHashRaw)]
pure (saddr ^. Db.StakeAddressHashRaw, rwd ^. Db.RewardType, rwd ^. Db.RewardAmount)

Expand Down