Skip to content

Conversation

kevincheng96
Copy link
Contributor

This generalizes the logic used in the recently audited WBTCPriceFeed to create a MultiplicativePriceFeed that derives its price from multiplying the prices from two other price feeds together.

This is a flexible wrapper price feed that can be used to generate prices for any asset that does not have a price feed that fits Comet's expected price denomination. e.g. if we wanted to add cbETH to cUSDCv3, there is no Chainlink price feed for cbETH / USD, so we would need to multiply the cbETH / ETH and ETH / USD price feeds together to get cbETH / USD

@kevincheng96 kevincheng96 added openzeppelin PRs and issues related to OZ audit needs audit These changes will need an audit labels Jun 8, 2023
@kevincheng96 kevincheng96 requested a review from scott-silver June 8, 2023 00:26
@kevincheng96 kevincheng96 marked this pull request as ready for review June 8, 2023 00:26
@kevincheng96 kevincheng96 force-pushed the kevin/multiplicative-price-feed branch from c93fab7 to 044600f Compare June 8, 2023 00:35
Comment on lines +45 to +56
constructor(address priceFeedA_, address priceFeedB_, uint8 decimals_, string memory description_) {
priceFeedA = priceFeedA_;
priceFeedB = priceFeedB_;
uint8 priceFeedADecimals = AggregatorV3Interface(priceFeedA_).decimals();
uint8 priceFeedBDecimals = AggregatorV3Interface(priceFeedB_).decimals();
combinedScale = signed256(10 ** (priceFeedADecimals + priceFeedBDecimals));

if (decimals_ > 18) revert BadDecimals();
decimals = decimals_;
description = description_;
priceFeedScale = int256(10 ** decimals);
}

Check notice

Code scanning / Semgrep

Semgrep Finding: rules.solidity.performance.non-payable-constructor

Consider making costructor payable to save gas.
Comment on lines +45 to +56
constructor(address priceFeedA_, address priceFeedB_, uint8 decimals_, string memory description_) {
priceFeedA = priceFeedA_;
priceFeedB = priceFeedB_;
uint8 priceFeedADecimals = AggregatorV3Interface(priceFeedA_).decimals();
uint8 priceFeedBDecimals = AggregatorV3Interface(priceFeedB_).decimals();
combinedScale = signed256(10 ** (priceFeedADecimals + priceFeedBDecimals));

if (decimals_ > 18) revert BadDecimals();
decimals = decimals_;
description = description_;
priceFeedScale = int256(10 ** decimals);
}

Check warning

Code scanning / Semgrep

Semgrep Finding: compound.solidity.missing-constructor-sanity-checks

There're no sanity checks for the constructor argument description_.
Comment on lines +45 to +56
constructor(address priceFeedA_, address priceFeedB_, uint8 decimals_, string memory description_) {
priceFeedA = priceFeedA_;
priceFeedB = priceFeedB_;
uint8 priceFeedADecimals = AggregatorV3Interface(priceFeedA_).decimals();
uint8 priceFeedBDecimals = AggregatorV3Interface(priceFeedB_).decimals();
combinedScale = signed256(10 ** (priceFeedADecimals + priceFeedBDecimals));

if (decimals_ > 18) revert BadDecimals();
decimals = decimals_;
description = description_;
priceFeedScale = int256(10 ** decimals);
}

Check warning

Code scanning / Semgrep

Semgrep Finding: compound.solidity.missing-constructor-sanity-checks

There're no sanity checks for the constructor argument priceFeedA_.
Comment on lines +45 to +56
constructor(address priceFeedA_, address priceFeedB_, uint8 decimals_, string memory description_) {
priceFeedA = priceFeedA_;
priceFeedB = priceFeedB_;
uint8 priceFeedADecimals = AggregatorV3Interface(priceFeedA_).decimals();
uint8 priceFeedBDecimals = AggregatorV3Interface(priceFeedB_).decimals();
combinedScale = signed256(10 ** (priceFeedADecimals + priceFeedBDecimals));

if (decimals_ > 18) revert BadDecimals();
decimals = decimals_;
description = description_;
priceFeedScale = int256(10 ** decimals);
}

Check warning

Code scanning / Semgrep

Semgrep Finding: compound.solidity.missing-constructor-sanity-checks

There're no sanity checks for the constructor argument priceFeedB_.
Copy link
Contributor

@scott-silver scott-silver left a comment

Choose a reason for hiding this comment

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

nice! 🙌

@kevincheng96 kevincheng96 added ready for audit This pull request is ready / waiting to be audited and removed needs audit These changes will need an audit labels Jun 9, 2023
@kevincheng96
Copy link
Contributor Author

Forgot to merge this.

Here is the final audit report from OZ: https://gist.github.com/antonleviathan/2938185330642deffe141f682375678b

@kevincheng96 kevincheng96 added audited These changes have been audited and removed ready for audit This pull request is ready / waiting to be audited labels Aug 3, 2023
@kevincheng96 kevincheng96 merged commit ccb6e9b into main Aug 3, 2023
@kevincheng96 kevincheng96 deleted the kevin/multiplicative-price-feed branch August 3, 2023 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audited These changes have been audited openzeppelin PRs and issues related to OZ audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants