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

feat: generic chainlink financing adapter #329

Draft
wants to merge 4 commits into
base: financing-chainlink-adapter
Choose a base branch
from

Conversation

mcchan1
Copy link
Contributor

@mcchan1 mcchan1 commented Jun 28, 2021

Goal is to accommodate any 8 digit (eg. ETH/USD) or 18 digit priceFeed from Chainlink (e.g. USDC/ETH).

  • made priceFeed public
  • remove constructor() and create configureDao()
  • create decimalsChainlink = priceFeed.decimals()
  • checks in processProposal depending on whether its 8 or 18 digits decimals
  • TODO - refactor financing-chainlink.test.js

Goal is to accommodate any 8 digit (eg. ETH/USD)  or 18 digit priceFeed from Chainlink (e.g. USDC/ETH).
- made priceFeed public
- create decimalsChainlink = priceFeed.decimals()
- checks in processProposal depending on whether its 8 or 18 digits decimals
@mcchan1 mcchan1 requested a review from fforbeck June 28, 2021 19:47
fforbeck and others added 2 commits June 28, 2021 18:05
- change function name `_usdToEth` to `_convertToWeiValue`
- return 8 decimals in FakeChainlinkPriceFeed.sol
- `it` block in tests
@fforbeck fforbeck added the enhancement New feature or request label Jun 30, 2021
@fforbeck
Copy link
Contributor

depends on ##331

@fforbeck fforbeck changed the title A more generic chainlink financing feat: generic chainlink financing adapter Sep 22, 2021
@fforbeck fforbeck removed the blocked label Nov 5, 2021
@fforbeck
Copy link
Contributor

fforbeck commented Nov 5, 2021

@mcchan1
this pr is now unblocked. I've pushed the changes that allow the creation of Configuration proposals with address type configs. It solves the issue: #331

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

Successfully merging this pull request may close these issues.

2 participants