Skip to content

Add Safe Transfer#12

Merged
Joeysantoro merged 2 commits intoPOZ-Combinedfrom
CS-Safe-Transfer
Jan 30, 2021
Merged

Add Safe Transfer#12
Joeysantoro merged 2 commits intoPOZ-Combinedfrom
CS-Safe-Transfer

Conversation

@Joeysantoro
Copy link
Contributor

@Joeysantoro Joeysantoro commented Jan 28, 2021

Fixes OZ audit issue L09

Did not add for this line

fei().transferFrom(msg.sender, address(pair), amountFei);
because the OpenZeppelin ERC20 cannot return false and reverts on failure.

@wuestholz
Copy link

Did you consider doing the same at

fei().transferFrom(msg.sender, address(pair), amountFei);
?

@Joeysantoro
Copy link
Contributor Author

Did you consider doing the same at

fei().transferFrom(msg.sender, address(pair), amountFei);

?

How about adding an "assert" since the fei contract is not expected to ever return false

@Joeysantoro Joeysantoro merged commit 0d66119 into POZ-Combined Jan 30, 2021
@Joeysantoro Joeysantoro added the OZ/CS Fix Combo A PR which addresses an issue from both OpenZeppelin and ConsenSys label Feb 6, 2021
@xklob xklob deleted the CS-Safe-Transfer branch September 19, 2021 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OZ/CS Fix Combo A PR which addresses an issue from both OpenZeppelin and ConsenSys

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments