-
Notifications
You must be signed in to change notification settings - Fork 111
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
Refactor ConstraintsLib and InputLib plus added new flags #204
Conversation
Codecov ReportBase: 0.00% // Head: 0.00% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #204 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 18 19 +1
Lines 420 418 -2
Branches 104 104
======================================
+ Misses 420 418 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
contracts/OrderRFQLib.sol
Outdated
@@ -5,12 +5,32 @@ pragma solidity 0.8.17; | |||
import "@1inch/solidity-utils/contracts/libraries/ECDSA.sol"; | |||
import "@1inch/solidity-utils/contracts/libraries/AddressLib.sol"; | |||
|
|||
type TraitsRFQ is uint256; | |||
|
|||
library TraitsRFQLib { |
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.
Why "Traits" instead of "Details"?
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.
It is kind of restrictions
e29b34a
to
97ae79d
Compare
97ae79d
to
827a0fb
Compare
b9f8c36
to
6b0b779
Compare
a280b0b
to
d896752
Compare
// proceed only if taker is willing to execute permit and its length is enough to store address | ||
address token = address(bytes20(permit)); | ||
bytes calldata permitCalldata = permit[20:]; | ||
IERC20(token).safePermit(permitCalldata); // TODO: inline both arguments |
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.
Why not TODO now?
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.
Can be refactored simultaneously in multiple places
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.
LGTM
=>