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

use dai for staking token #6

Closed

Conversation

tonisives
Copy link
Contributor

It is confusing that the staking.test.js has the DAI ERC20 variable, but DAI is actually not used in Staking. Currently staking and reward tokens are the same.

This pull request uses DAI as the staked token.

I used

const rewardToken = await ethers.getContract("RewardToken")
instead of
const rewardToken = await deployments.get("RewardToken")

because otherwise the deployemnt failed with unknown function rewardToken.approve

@PatrickAlphaC
Copy link
Contributor

Actually, I think it would be better just to remove dai from the tests altogether, I like the reward token being the staked token as well.

What do you think?

@tonisives
Copy link
Contributor Author

Yeah it would make sense to remove the unused DAI variable from staking.test.js.

Btw if we used DAI for staking, it would brake the frontend code as well. So better to keep it as it is.

@PatrickAlphaC
Copy link
Contributor

@tonisives front end code? What front end code

@tonisives
Copy link
Contributor Author

I referenced this repository from Chainlink workshop Build a Full Stack DeFi Application: Code Along

I think the frontend also uses RT for both staking and rewards, so DAI would not work there.

@tonisives
Copy link
Contributor Author

I created another pull request that removes the unused DAI variable here

@PatrickAlphaC
Copy link
Contributor

Thank you!!

Will close this one then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants