-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: add flash mint leverage extended contract #48
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes LGTM, few smol questions
import { LeveragedExtendedQuoteProvider } from './provider' | ||
|
||
const provider = LocalhostProvider | ||
const zeroExApi = ZeroExApiArbitrumSwapQuote |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In tests make sure to use this zeroExApi setup for Arbitrum
test('returns a tx for minting ETH2X (ETH)', async () => { | ||
const buildRequest = createBuildRequest(true, eth, 'ETH') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be the 2x token? (similar to L200)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is minting, outputToken is ETH2X which is the default: https://github.com/IndexCoop/flash-mint-sdk/pull/48/files#diff-63922e4c38d75ebb21208c533ae7d4a5bb2b0fd0c1bba0f80bb9ecc1a3d37a45R226. I was lazy with the request function 😅
symbol: IndexCoopBitcoin3xIndex.symbol, | ||
} | ||
|
||
describe.skip('BTC3X (Arbitrum)', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking to keep this test suite skipped?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes for now. As mentioned might have to skip the whole arbitrum tests suite to release the SDK due to the bug causing failed tx's.
src/utils/0x.test.ts
Outdated
@@ -50,7 +50,7 @@ describe('ZeroExApi', () => { | |||
const chainId = 137 | |||
const query = new URLSearchParams({ | |||
buyAmount: ONE, | |||
buyToken: DPI, | |||
buyToken: '0x1494CA1F11D487c2bBe4543E90080AeBa4BA3C2b', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious - why remove constant for the raw literal here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once again being lazy. I first deleted to replace everything with USDC, until I noticed I don't need USDC in every test. Readded.
Co-authored-by: 0xonramp <136734776+0xonramp@users.noreply.github.com>
Description
TODOs
FIXME
andTODO
s commented on this PRcheck to get correct issuance quotes w/ new function parameter