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

Basis trading contract v1 #4

Open
wants to merge 95 commits into
base: main
Choose a base branch
from
Open

Basis trading contract v1 #4

wants to merge 95 commits into from

Conversation

deepsp94
Copy link
Contributor

@deepsp94 deepsp94 commented May 2, 2022

  1. Contract is meant to hold only one user's weth deposit.
  2. Update function checks funding and goes long or stays idle
  3. Withdraw function works when contract is long or idle

@mikeyrf mikeyrf changed the title [WIP] Basis trading contract v1 Basis trading contract v1 May 19, 2022
@mikeyrf
Copy link
Contributor

mikeyrf commented May 19, 2022

Except for some small changes, this looks good so far. Need integration tests for interacting with Overlay v1-core, but then good to merge.

Haven't thoroughly reviewed security wise, but will once move beyond the MVP.

contracts/EthBasisTrade.sol Outdated Show resolved Hide resolved
contracts/EthBasisTrade.sol Outdated Show resolved Hide resolved
@deepsp94
Copy link
Contributor Author

@mikeyrf i'm good to review and merge this branch when you're back.
Still have one concern here though:

Without the chain.mine() snippet, this test sometimes fails. Attached logs.
unwind_fail_log.pdf

Would be nice if you could help improve this test.

@mikeyrf
Copy link
Contributor

mikeyrf commented Jun 13, 2022

Sounds good. Let's get to this when I'm back, but some things to check:

  • Possibly due to precision loss when storing midTick and entryTick in Position.Info but this should be good to within 1bps. Your error is around 4 bps
  • Possibly due to some time elapsed between state reported values for your expect and the timestamp associated with the unwind tx. Where the time elapsed would create more funding payments and changes to the TWAP

To check whether it's related to the TWAP changing, you can use the mock feed setup in the v1 core tests (where you set the price manually).

@deepsp94
Copy link
Contributor Author

Hey @mikeyrf, this test passes now since I'm using the mock feed. Looks like it was a TWAP issue after all.

def test_pos_unwind_as_expected(eth_basis_trade, state, mock_market,

However, to be able to use the mock market in the unwind and build functions of my contract, i had to expose it as an argument.
Here:

address _marketAddress

and here:
address _marketAddress

This is how the unwind (and build too) test looked before. I was using the weth/ovl market by default: https://github.com/overlay-market/v1-vaults/blame/64515b894b0f8ce80180571132ee5629f28f0122/contracts/EthBasisTrade.sol#L164

Lemme know your thoughts/if you think there's a better way to go about it.

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

2 participants