-
Notifications
You must be signed in to change notification settings - Fork 39
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
Guaranteed Minimum APR Program #327
base: development
Are you sure you want to change the base?
Conversation
contracts/vaults/LiquidityLock.sol
Outdated
IToken iToken = IToken(PROTOCOL.underlyingToLoanPool(base)); | ||
claimID = keccak256(abi.encode(address(iToken),msg.sender,endTimestamp)); | ||
require(claims[claimID].token == address(0), "already used id"); | ||
require(block.timestamp <= endTimestamp && endTimestamp-block.timestamp<tokenSettings.maxLockTime, "invalid ending time"); |
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.
endTimestamp-block.timestamp<tokenSettings.maxLockTime
is sufficient enough to check. it will fail anyway if block.timestamp <= endTimestamp
is false
claimID = keccak256(abi.encode(address(iToken),msg.sender,endTimestamp)); | ||
require(claims[claimID].token == address(0), "already used id"); | ||
require(block.timestamp <= endTimestamp && endTimestamp-block.timestamp<tokenSettings.maxLockTime, "invalid ending time"); | ||
IERC20(base).safeTransferFrom(msg.sender, address(this), depositAmount); |
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.
at this point base
can be any contract maliciously provided externally, I don't see how this can be reused later but probably we should sanitize it.
TREASURY = t; | ||
} | ||
|
||
function lock(address base, uint256 depositAmount, uint256 endTimestamp) external returns (bytes32 claimID) { |
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.
What of instead of sending endstamp we send days from 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.
What of instead of sending endstamp we send days from now
why that would be better?
Settings memory tokenSettings = lockSettings[base]; | ||
IToken iToken = IToken(PROTOCOL.underlyingToLoanPool(base)); | ||
claimID = keccak256(abi.encode(address(iToken),msg.sender,endTimestamp)); | ||
require(claims[claimID].token == address(0), "already used id"); |
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.
would be nice to check if base != address(0)
contracts/vaults/LiquidityLock.sol
Outdated
IToken iToken = IToken(PROTOCOL.underlyingToLoanPool(base)); | ||
claimID = keccak256(abi.encode(address(iToken),msg.sender,endTimestamp)); | ||
require(claims[claimID].token == address(0), "already used id"); | ||
require(block.timestamp <= endTimestamp && endTimestamp-block.timestamp<tokenSettings.maxLockTime, "invalid ending time"); |
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 block.timestamp <= endTimestamp it might be <
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 the timestamps are the same no issues arise so it is fine to leave within the check.
No description provided.