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

Fix !isToken0 && fee != 0 && protocolFee != 0 test cases #148

Merged
merged 11 commits into from
Oct 23, 2024

Conversation

kinrezC
Copy link
Collaborator

@kinrezC kinrezC commented Oct 22, 2024

  • Fixes !isToken0 test cases
  • Fixes FEE=30 tests
  • Fixes FEE=30 && PROTOCOL_FEE=50 tests

@kinrezC kinrezC changed the base branch from main to feat/eth-support-2 October 22, 2024 18:43
@kinrezC kinrezC requested a review from kadenzipfel October 22, 2024 20:52
@kinrezC kinrezC changed the title Fix !isToken0 cases Fix !isToken0 && fee != 0 && protocolFee != 0 test cases Oct 22, 2024
forge test -vvv --via-ir
id: test1

- name: Run forge tests isToken0 (true) fee (300) protocolFee (50)
run: |
export IS_TOKEN_0=TRUE
export FEE=300
export FEE=30
export PROTOCOL_FEE=50
forge test -vvv --via-ir
id: test2
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should add a run with USING_ETH=TRUE here as well

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nvm, I see this is done in #154

Comment on lines 220 to +227
}
} else {
revert InvalidSwapAfterMaturitySufficientProceeds();
}
}

if (block.timestamp > endingTime && !insufficientProceeds) {
revert InvalidSwapAfterMaturitySufficientProceeds();
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the reason for moving this? Are they not logically the same since if state.totalProceeds < minimumProceeds, we set insufficientProceeds = true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah i think this was an artifact of me debugging, I will revert it.

@kinrezC kinrezC mentioned this pull request Oct 23, 2024
@kinrezC kinrezC changed the base branch from feat/eth-support-2 to main October 23, 2024 19:31
@kinrezC kinrezC changed the base branch from main to feat/eth-support-2 October 23, 2024 19:31
@kinrezC kinrezC merged commit 0c358e1 into feat/eth-support-2 Oct 23, 2024
1 check passed
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.

2 participants