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.

Sign up for GitHub

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

Jump to bottom

Add Chainlink feeds and remove UniswapV3 feeds #133

Merged
merged 38 commits into from
Jan 18, 2024
Merged

Add Chainlink feeds and remove UniswapV3 feeds #133

merged 38 commits into from
Jan 18, 2024

Conversation

EperezOk
Copy link
Contributor

No description provided.

0xparashar and others added 13 commits August 29, 2022 23:08
@0xparashar
adding chainlink adapter to fetch prices as per requirement
76bcc64
@c-n-o-t-e
updated OverlayV1ChainlinkFeed contract
7d902e5
@0xparashar
Merge pull request #91 from c-n-o-t-e/feat/chainlink-oracle-adapter
d671f73 
updated OverlayV1ChainlinkFeed contract
@0xparashar
added mock and fixed inifinite loop, tested on mainnet aggregator & m…
c4a6594 
…ock aggregator both
@0xparashar
removing check gas function
b0fbc1b
@0xparashar
removed redundant aggregator calls, reducing gas usage by half
c1e8aaa
@0xparashar
adding unit tests passing
230ae2c
@0xparashar
Merge branch 'main' of github.com:overlay-market/v1-core into feat/ch…
49f530f 
…ainlink-oracle-adapter
@0xparashar
fixing lint issue
a541f55
@c-n-o-t-e
added overlay chainlink factory
1dc99b6
@c-n-o-t-e
added chainlink interface
d147b8b
@c-n-o-t-e
added chainlink interface
5abd310
@bigboydiamonds
Merge pull request #94 from c-n-o-t-e/feat/chainlink-oracle-adapter
28e5a82 
added overlay  chainlink factory
@EperezOk EperezOk changed the title hainlink feeds Add Chainlink feeds and remove UniswapV3 feeds Jan 11, 2024
EperezOk and others added 14 commits January 11, 2024 16:09
@EperezOk
fix: add chainlink-brownie-contracts pkg as dependency in brownie config
99a3cc4
@EperezOk
feat: add create and deploy scripts for chainlink feeds
eccff0e
@EperezOk
chore: remove uniswapv3 feeds, tests and scripts
4311c3f
@EperezOk
feat(tests/feeds): update real Chainlink aggregator address
b96f1ef
@magnetto90
removes uniswap feed adds chainlink feeds
26559e6
@magnetto90
Add Chainlink Brownie Contracts dependency
a78b71c
@magnetto90
forge install: chainlink
ecdb9c5 
v2.7.2
@magnetto90
Add Chainlink library remapping
f1ad88d
@magnetto90
Update test-python.yaml with environment variable exports and Arbitru…
c40606f 
…m configuration
@magnetto90
Update default network to Arbitrum main fork
093f28f
@magnetto90
Add .env file and export environment variables
1767084
@magnetto90
Update ARBISCAN_TOKEN in .env file
e535517
@EperezOk
fix: merge conflicts
c53eb84
@EperezOk
Merge pull request #145 from overlay-market/s/remove-uniswap-and-test
d29517c 
Update tests to use real Chainlink feed
This was referenced Jan 15, 2024
Closed
Merged
This was referenced Jan 16, 2024
Merged
Merged
magnetto90
magnetto90 previously approved these changes Jan 17, 2024
View reviewed changes
Copy link
Contributor

@magnetto90 magnetto90 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, but Chainlink contracts are not using MIT licence, but to not stop the progress I recommend fixing that on another PR.

@TomasCImach TomasCImach mentioned this pull request Jan 17, 2024
Open
TomasCImach
TomasCImach reviewed Jan 17, 2024
View reviewed changes
tests/markets/conftest.py Outdated Show resolved Hide resolved
tests/markets/conftest.py Outdated Show resolved Hide resolved
tests/markets/test_build.py Outdated
Copy link
Member

Choose a reason for hiding this comment

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

the change from 80000 -> 8000 was due to Arbitrum mainnet having less liquidity on uniswap pools. Now that we're moving to chainlink feeds, is this change needed? where does it fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@magnetto90 any idea about this?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should test it, not sure how it affects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made a commit moving from 8000 -> 80000, we'll see how that goes.

tests/markets/conftest.py Show resolved Hide resolved
@EperezOk
Copy link
Contributor Author

Before merging this, I think we should change the averageBlockTime to the actual value we are going to use when deploying new markets on Arbitrum. I would use averageBlockTime = 1, since averageBlockTime = 0 breaks some functions despite being more accurate.

@TomasCImach TomasCImach mentioned this pull request Jan 17, 2024
Open
@magnetto90
Copy link
Contributor

Before merging this, I think we should change the averageBlockTime to the actual value we are going to use when deploying new markets on Arbitrum. I would use averageBlockTime = 1, since averageBlockTime = 0 breaks some functions despite being more accurate.

Ok, I'm gonna test it now.

This was referenced Jan 17, 2024
Closed
Closed
Closed
@TomasCImach TomasCImach merged commit 7c9fe0d into staging Jan 18, 2024
2 checks passed
@TomasCImach TomasCImach deleted the chainlink-feeds branch January 18, 2024 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomasCImach TomasCImach TomasCImach approved these changes

@magnetto90 magnetto90 magnetto90 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@EperezOk @magnetto90 @TomasCImach @0xparashar @c-n-o-t-e @bigboydiamonds