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

Task/oracle test #170

Merged
merged 14 commits into from
Aug 18, 2020
Merged

Task/oracle test #170

merged 14 commits into from
Aug 18, 2020

Conversation

antoncoding
Copy link
Contributor

@antoncoding antoncoding commented Aug 17, 2020

Contract Changes

  • Separate CompoundOracleInterface and OracleInterface: CompoundOracleInterface is the one Oracle.sol use to interact with compound v1 oracle, and OracleInterface is just the interface for Oracle.sol
  • Add MockCompoundOracle and MockOracle accordingly, use MockCompoundOracle in Oracle's unit test.
  • Add setter function for priceOracle. Owner can update the address of compoundOracle.
  • Change variable name in OptionsContract.sol from compoundOracle to oracle
  • Remove OptionExchange in OptionsContract, declare that variable in Otoken, which is the only one using it.

await oracle.setAssetToCtoken(bat.address, cBat.address);

// update compound oracle price for BAT
const batPrice = '777857500000000';
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to use ganache fork for this and also test with mainnet data? (Also these tests will probs change since we need to use a different oracle now right?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the data I copied from mainnet, for unit tests I think it's okay that we just hardcode the value, because we don't have existing cicd workflow to run ganache fork now.

Copy link
Contributor

@aparnakr aparnakr left a comment

Choose a reason for hiding this comment

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

Looks good to me. It would also be good to test on ganache fork and pull mainnet data if we can, just so we don't mock too much of the data and we know for sure the math calculations we did make sense :)

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.

LGTM.

@antoncoding antoncoding merged commit dbd4946 into dev Aug 18, 2020
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

3 participants