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

OZ Audit #212

Merged
merged 11 commits into from
Dec 18, 2023
Merged

OZ Audit #212

merged 11 commits into from
Dec 18, 2023

Conversation

marktoda
Copy link
Collaborator

This commit merges in all OZ audit commits from development into main. These were previously reviewed
#190
#189
#191
#193
#192
#194
#196
#197
#199

marktoda and others added 11 commits August 21, 2023 15:58
This commit fixes a couple typos.

Fixes: N-03
Gas limit for native transfer can cause some reasonable smart contract
wallets that perform operations in their receive function to be unable
to receive native output from uniswapX. compatibilty preferred here

Resolves M-02
Previously there was ambiguous undefined behavior for if startTime and
endTime were the same - This commit removes that ambiguity by enforcing
startTime !== endTime
document magic numbers in orderquoter

Fixes: L-08
* Remove unused return names

* Update OrderQuoter.sol
@@ -28,7 +28,7 @@ library DutchDecayLib {
view
returns (uint256 decayedAmount)
{
if (decayEndTime < decayStartTime) {
if (decayEndTime <= decayStartTime) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

we really wanna revert if theyre the same? is it because they shouldnt be using a decay and should just have a limit order?

Copy link
Collaborator Author

@marktoda marktoda Dec 18, 2023

Choose a reason for hiding this comment

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

#194

I think it was just undefined behavior what we should do in that case - should we use startAmount or endAmount? with the current impl we used endAmount which is better for the filler and could be unintended by the swapper

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@marktoda marktoda merged commit 855b495 into main Dec 18, 2023
2 checks 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