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/pricer fixes #446

Closed
wants to merge 18 commits into from
Closed

Feat/pricer fixes #446

wants to merge 18 commits into from

Conversation

aparnakr
Copy link
Contributor

@aparnakr aparnakr commented Dec 7, 2021

Task: Pricer fixes

Enable anyone to set prices for all assets

In the setExpiryPriceInOracle function, if anyone is calling the function, checks that the expiryTimestamp is less than the expiry timestamp of the passed in roundId. Also checks that the expiryTimestamp is grater than the expiry timestamp of the previous roundId. This ensures that someone cannot pass in an old price or too new of a price.

Code

  • Unit test 100% coverage
  • Does your code follow the naming and code documentation guidelines?

Copy link
Contributor

@aleone aleone left a comment

Choose a reason for hiding this comment

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

Generally lgtm, think we should change revert message to be distinct for each require. I also notice the build is failing re tests/coverage on github.

Copy link
Member

@haythemsellami haythemsellami left a comment

Choose a reason for hiding this comment

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

Approved for now, left few comments, need to fix build also.

@@ -26,7 +26,7 @@ To deploy a new `ChainlinkPricer.sol`, it is recommended to use the `deployChain
**Input**

```sh
truffle exec scripts/deployChainlinkPricer.js --network mainnet --controller 0x7d78c401c69c56cb21f4bf80c53afd92be0BBBBB --pool 0xc02aaa39b223fe8d0a0e5c4f27ead9083c7AAaaa --weth 0x5f4eC3Df9cbd43714FE2740f5E3616155cAGAGAG --oracle 0xef196aA0e3Cb8EA6d5720557C3B611Eff6OOOOOO --gasPrice 50000000000
truffle exec scripts/deployChainlinkPricer.js --network mainnet --bot 0xfacb407914655562d6619b0048a612B1795dF783 --asset 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599 --aggregator 0xF4030086522a5bEEa4988F8cA5B36dbC97BeE88c --oracle 0x789cD7AB3742e23Ce0952F6Bc3Eb3A73A0E08833 --gasPrice 100000000000
Copy link
Member

Choose a reason for hiding this comment

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

Is this command correct ?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep! This was wrong before! :)

Comment on lines +108 to +112

## Verification of Contracts
```sh
truffle run verify ChainlinkPricer@0x17FDF6cE88517bf0Cff19e014E509B543DD78432 --network mainnet
```
Copy link
Member

Choose a reason for hiding this comment

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

Why we are adding this to README ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because I imagine whoever wants to deploy also needs to verify right? Would you add it to packag.json instead? :)

@aparnakr aparnakr closed this Jan 31, 2022
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.

4 participants