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

rebase with upstream main #3

Merged
merged 40 commits into from
Dec 20, 2024
Merged

rebase with upstream main #3

merged 40 commits into from
Dec 20, 2024

Conversation

tserg
Copy link

@tserg tserg commented Dec 20, 2024

The Ekubo tests are passing locally. There is just one thing to highlight in the comment below. We can open the PR against the upstream once this is merged.

In the earlier PR, I suggested:

Additionally, we should add a restriction that the quote token cannot be added as an asset, because its price would always be 1 from Ekubo, which I don't think makes sense.

Thinking about this again today, I think it might not be an issue. Let's say the quote asset is USDC, collateral asset is USDC, and borrow asset is STRK. Assuming the threshold for STRK is 80%, then even though the price of 1 USDC is always 1 USDC, but we can still liquidate this position if STRK's price > 0.8 USDC. Maybe we can do away with this requirement first, and raise it in the upstream PR?

Comment on lines -148 to +163
vendor::erc20::{IERC20MetadataDispatcher, IERC20MetadataDispatcherTrait},
vendor::erc20::{ERC20ABIDispatcher as IERC20Dispatcher, ERC20ABIDispatcherTrait}, units::INFLATION_FEE,
Copy link
Author

Choose a reason for hiding this comment

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

I reverted back to ERC20ABIDispatcher because that is already now used for token approvals.

@tserg tserg requested a review from milancermak December 20, 2024 03:45
Copy link

@milancermak milancermak left a comment

Choose a reason for hiding this comment

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

I'm guessing there weren't any conflicts so I'm optimistically approving.

@tserg
Copy link
Author

tserg commented Dec 20, 2024

I'm guessing there weren't any conflicts so I'm optimistically approving.

Yes! I will open a PR against the main repo to check the CI.

@tserg tserg merged commit 2e5fc6b into feat/ekubo-oracle Dec 20, 2024
0 of 4 checks passed
@milancermak milancermak deleted the chore/rebase branch December 20, 2024 11:26
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.

3 participants