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

ZenBull strategy oracle #517

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

iamsahu
Copy link
Contributor

@iamsahu iamsahu commented Jan 23, 2023

Please note that there is a difference of 4 wei in the value quoted by the oracle as compared to the calculation provided by the opyn team.

@iamsahu iamsahu marked this pull request as ready for review January 25, 2023 05:15
@iamsahu iamsahu requested a review from alcueca January 25, 2023 05:15
Copy link
Contributor

@alcueca alcueca left a comment

Choose a reason for hiding this comment

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

The math can be made clearer, and casting fixed. A test harness is also a good idea at this point.

packages/foundry/contracts/oracles/opyn/ZenBullOracle.sol Outdated Show resolved Hide resolved
packages/foundry/contracts/oracles/opyn/ZenBullOracle.sol Outdated Show resolved Hide resolved
packages/foundry/contracts/oracles/opyn/ZenBullOracle.sol Outdated Show resolved Hide resolved
@@ -0,0 +1,59 @@
// SPDX-License-Identifier: BUSL-1.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Please deploy on a fork, and use that to code a test harness that verifies that the ZenBullPrice obtained is within an order of magnitude of something. If ZenBull is expected to stay within 0.1 ETH and 10 ETH most of the time, that would be a good range to test against.

- remapping overridden for forge to make it compatible in env directory
- upgraded npm pacakge version
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.

2 participants