Skip to content
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

contracts: Sushiswap OHM/LQTY rewards #704

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

bingen
Copy link
Collaborator

@bingen bingen commented Sep 22, 2021

No description provided.

@bingen bingen added enhancement New feature or request backend labels Sep 22, 2021
@bingen bingen self-assigned this Sep 22, 2021
@bingen bingen changed the title contracts: Sushiswap ohm rewards contracts: Sushiswap OHM/LQTY rewards Sep 22, 2021
Copy link
Collaborator

@RickGriff RickGriff left a comment

Choose a reason for hiding this comment

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

Great, thanks - just worth commenting the decimal issue mentioned below, and ensuring that deployment uses the right multiplier values.


function onSushiReward (uint256, address, address to, uint256 sushiAmount, uint256) onlyMCV2 override external {
// OHM rewards
uint256 ohmPendingReward = sushiAmount.mul(ohmRewardMultiplier) / REWARD_TOKEN_DIVISOR;
Copy link
Collaborator

@RickGriff RickGriff Sep 24, 2021

Choose a reason for hiding this comment

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

I noticed the OHM token uses 9-digit decimal precision:
https://etherscan.io/address/0x383518188c0c6d7730d91b2c03a03c837814a899#readContract
and SUSHI and LQTY use 18.

It seems here that ohmRewardMultiplier and lqtyRewardMultiplier variables are 18-digit decimal precision, and that's why they're divided by REWARD_TOKEN_DIVISOR.

Perhaps we can add comments to note that:

  • These multiplier variables are 18-digit precision
  • The value of ohmRewardMultiplier should take into account the different decimal precision of SUSHI and OHM when it is set at construction.

e.g. if we wanted to reward 2 OHM per SUSHI, the raw value of ohmRewardMultiplier should be 2e9. Then when calculating the OHM pending reward, we'd get:

ohmPendingReward = sushiAmount * 2e9 / 1e18

= sushiAmount * 2 / 1e9

Which is equivalent to 2 OHM per SUSHI, given OHM's 9-digit precision.

Basically at deployment we just need to remember to pass an _ohmRewardMultiplier on the order of 1e9, and _lqtyRewardMultiplier should be on the order of 1e18.

I think tests don't catch this because the OZ ERC20.sol hardcodes decimals = 18.

Copy link
Collaborator

@RickGriff RickGriff left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Collaborator

@RickGriff RickGriff left a comment

Choose a reason for hiding this comment

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

Makes sense!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants