-
Notifications
You must be signed in to change notification settings - Fork 220
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
test partial fill #114
base: main
Are you sure you want to change the base?
test partial fill #114
Conversation
const covenBalanceBefore = await covenContract.balanceOf(alice.address) | ||
await router['execute(bytes,bytes[],uint256)'](commands, inputs, DEADLINE) | ||
const covenBalanceAfter = await covenContract.balanceOf(alice.address) | ||
expect(covenBalanceAfter.sub(covenBalanceBefore)).to.eq(numCovensNFTX) |
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.
we need to make sure that the user gets their money back for the NFT's they did not receive
expect(owner1Before.toLowerCase()).to.eq(params1.offerer) | ||
expect(owner0After).to.eq(alice.address) | ||
expect(owner1After).to.eq(alice.address) | ||
expect(ethDelta).to.eq(gasSpent) // eth spent only on gas bc trade came from DAI |
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.
if this was a partial fill, does some ether get stuck in the router?? might be good to check that router balance is 0 and that Alice got refunded for her partial fill....I may have forgetten to do this in my other partial fill tests. 😬
it('completes a trade for ERC20 --> ETH --> NFTs, invalid Seaport order', async () => { | ||
const maxAmountIn = expandTo18DecimalsBN(100_000) | ||
// in this case there is leftover dai in the router, and the unspent eth gets sent to alice | ||
await daiContract.transfer(router.address, maxAmountIn) |
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.
this should now use permit2 to transfer in the tokens 👯
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.
you can just add a
dai.approve(permit2)
permit2.approve(router, amount)
so that then during v2 it can pull in the tokens from the user :)
it('completes a trade for ERC20 --> ETH --> NFTs with Seaport, fulfillAvailableAdvancedOrders fill', async () => { | ||
const maxAmountIn = expandTo18DecimalsBN(100_000) | ||
// in this case there is leftover dai in the router and all eth gets spent on the nfts | ||
await daiContract.transfer(router.address, maxAmountIn) |
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.
aaand again
basically these dai transfers used to be our way to make tests before we could integrate with permit2 :)
No description provided.