-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: utilize split in existing rewards calculation #117
base: release/rewards-v2
Are you sure you want to change the base?
Conversation
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.
Looks good, one clarification
ELSE | ||
total_staker_operator_payout - floor(total_staker_operator_payout * 0.10) | ||
total_staker_operator_payout - floor(total_staker_operator_payout * COALESCE(oas.split, 1000) / 10000.0) |
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 know what type the output of COALESCE(oas.split, 1000) / 10000.0
is? We want the total_staker_operator_payout * split
to be a numeric * numeric
and not a numeric * float
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.
I mean the existing 0.10
is a float / double. So I've stuck with that. I believe the floor does round it down to a whole number.
pkg/rewards/5_goldRfaeStakers.go
Outdated
@@ -106,12 +107,23 @@ staker_operator_total_tokens AS ( | |||
FLOOR(staker_proportion * tokens_per_day_decimal) as total_staker_operator_payout | |||
FROM staker_proportion | |||
), | |||
-- Calculate the token breakdown for each (staker, operator) pair | |||
-- Include the operator_pi_split_snapshots table | |||
operator_pi_splits_cte AS ( |
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.
Nit: we don't use cte in naming anywhere
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.
I did that to prevent a conflict with the existing operator_pi_splits
table in the DB. Needed something to explicitly differentiate it.
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.
Not sure why this is needed at all. Why not just reference operator_pi_split_snapshots where you need it? You don’t really gain any advantage by selecting just a few columns from it
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.
Ah good point. I don't know why I did it that way. Fixed in 0eb7412
Utilize the
OperatorAVSSplit
andOperatorPISplit
in the existing rewards v1 and Programmatic incentives calculation (instead of the hardcoded 10% split).