-
Notifications
You must be signed in to change notification settings - Fork 3
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
#3 Lam airdrop2 #13
base: main
Are you sure you want to change the base?
#3 Lam airdrop2 #13
Conversation
curTokens[token] = true; | ||
} | ||
IERC20 tokenContract = IERC20(token); | ||
tokenContract.transferFrom(msg.sender, address(this), amount); |
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.
does transferFrom here work without an approve?
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.
No, it requires an approval before calling this function, that's what other interns also did =)).
I also provided other way to first transfer token to the contract and then add the token to the contract state later
emit TokenApproveIsSet(to, token, amount); | ||
} | ||
|
||
function setTokenApproveMultiple( |
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.
I think this function will still work if you pass in an array with one item, so there might not be a need for the setTokenApproval
function for single token addresses right? Unless it saves gas or something
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.
Yep, you're right. It's not gonna save much gas, but I just ensure that people will not have any problem if they forgot to pass in an array for just a single token =)))
event EthApproveIsSet(address to, uint256 amount); | ||
event TokenApproveIsSet(address to, address token, uint256 amount); | ||
event NewTokenAdded(address sender, address token, uint256 balance); | ||
event TokenIsAdded(address sender, address token, uint256 amount); | ||
event EthIsAdded(address sender, uint256 amount); | ||
event FundIsClaimed(address to); | ||
event EthIsClaimed(address to, uint256 amount); | ||
event TokenIsClaimed(address to, address token, uint256 amount); | ||
event AllTokensAreClaimed(address to); | ||
event ClaimedPartial(address to, address token, uint256 amount); | ||
event ClaimedEthPartial(address to, uint256 amount); |
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.
Nice use of events, I need to learn from you haha
contracts/FundDistribution.sol
Outdated
_transferToken(to, token, amount); | ||
} | ||
|
||
function sendTokenToWithRevertIfInsufficientFunds(address to, address token) public { |
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 done with one require
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.
yes I have one require below, I just offer generous option to withdraw for users
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.
yes what i mean is return boll and use require in exactly place then can prevent write too much function
if (tokenAvailable[to][token] > 0) emit ClaimedPartial(to, token, amount); | ||
else emit TokenIsClaimed(to, token, amount); | ||
} | ||
|
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.
For both contract may be check amount >0 then action to reduce gas?
New PR