Skip to content

Improved SafeERC20 allowance handling#1407

Merged
nventuro merged 28 commits into
OpenZeppelin:masterfrom
Aniket-Engg:fix/#1404
Oct 18, 2018
Merged

Improved SafeERC20 allowance handling#1407
nventuro merged 28 commits into
OpenZeppelin:masterfrom
Aniket-Engg:fix/#1404

Conversation

@Aniket-Engg

Copy link
Copy Markdown
Contributor

Fixes #1404

@nventuro nventuro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome @Aniket-Engg, thanks! I'll think a bit about where to put the new allowance function declarations, so all we should do is improve the tests a bit to make usage clearer.

Comment thread contracts/token/ERC20/IERC20.sol Outdated
Comment thread contracts/token/ERC20/SafeERC20.sol
Comment thread contracts/mocks/SafeERC20Helper.sol Outdated
@Aniket-Engg

Copy link
Copy Markdown
Contributor Author

I think, build needs to re-run.

Comment thread contracts/token/ERC20/IERC20.sol Outdated
@nventuro

Copy link
Copy Markdown
Contributor

@Aniket-Engg thank you for your great help, as always! We need to get this fix in soon since we plan on finishing the release tomorrow, so I pushed directly to your branch - sorry about that!

@nventuro nventuro requested a review from come-maiz October 17, 2018 22:17
@Aniket-Engg

Copy link
Copy Markdown
Contributor Author

No problem at all @nventuro

@come-maiz come-maiz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

good deal. Thanks @Aniket-Engg and @nventuro

@nventuro nventuro merged commit 315f426 into OpenZeppelin:master Oct 18, 2018
come-maiz pushed a commit that referenced this pull request Oct 21, 2018
* signing prefix added

* Minor improvement

* Tests changed

* Successfully tested

* Minor improvements

* Minor improvements

* Revert "Dangling commas are now required. (#1359)"

This reverts commit a688977.

* updates

* fixes #1404

* approve failing test

* suggested changes done

* ISafeERC20 removed

* allowance methods in library

* Improved SafeERC20 tests.

* Fixed test coverage.

(cherry picked from commit 315f426)
@Aniket-Engg Aniket-Engg deleted the fix/#1404 branch October 22, 2018 06:59
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.

3 participants