-
Notifications
You must be signed in to change notification settings - Fork 254
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
feature: stake-dao adapters and tests for DeFi-SDK integration #150
base: interactive
Are you sure you want to change the base?
Conversation
function setSdveCrv(address newSdveCrv) external { | ||
sdveCrv = newSdveCrv; | ||
} |
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.
is there a reason it may be useful?
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.
yes, so that you can update the sdveCrv address when it's changed in the future, without redeploying StakeDaoTokenAdapter.sol
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.
okay, but I will break the integration anyway
so I think there are 2 things you should do
- inherit from Ownable contract from this repo and use
onlyOwner
here - notify us about change of
sdveCrv
address, so we change it here and in our backend
* @author Elephant memory/strength | ||
*/ | ||
contract StakeDaoTokenAdapter is TokenAdapter { | ||
address public sdveCrv = 0x478bBC744811eE8310B461514BDc29D03739084D; |
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.
we usually save such addresses as address internal constant
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.
it might have a new address in the future, hence not a constant
|
||
if (token == sdveCrv) { | ||
components[0] = Component({ | ||
token: sdveCrv, |
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.
token: sdveCrv, | |
token: CRV, |
it should be CRV address as it's component's address
function setSdveCrv(address newSdveCrv) external { | ||
sdveCrv = newSdveCrv; | ||
} |
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.
okay, but I will break the integration anyway
so I think there are 2 things you should do
- inherit from Ownable contract from this repo and use
onlyOwner
here - notify us about change of
sdveCrv
address, so we change it here and in our backend
4 StakeDAO assets to be integrated (along with the underlying token after the comma)
Tests have been written for (1) and (4), as for those of (2), (3) will follow exact pattern of (1)
For (4), the actual underlying asset i.e. veCrv is non-tradable, so you may derive the price for sdveCrv from CRV, as 1 sdveCrv is minted for 1 CRV deposited into the vault, and hence I've set its rate as 1e18 in the Component, based off this understanding.