Skip to content
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

Added approveAndCall to TBTC token #5

Merged
merged 2 commits into from
Jul 5, 2021
Merged

Added approveAndCall to TBTC token #5

merged 2 commits into from
Jul 5, 2021

Conversation

pdyraga
Copy link
Member

@pdyraga pdyraga commented Jun 10, 2021

Depends on #4

approveAndCall is not a part of ERC20 - it was specified in ERC827 and
slightly diverged for some implementations. This one is identical to
approveAndCall behavior of KEEP token.

approveAndCall executes receiveApproval function on the spender as
specified in IReceiveApproval interface. Approves spender to withdraw
from the caller multiple times, up to the value amount. If this function
is called again, it overwrites the current allowance with the provided
value. Reverts if the approval operation reverted or if receiveApproval
call on the spender reverted.

@pdyraga pdyraga requested a review from a team June 10, 2021 15:23
approveAndCall is not a part of ERC20 - it was specified in ERC827 and
slightly diverged for some implementations. This one is identical to
approveAndCall behavior of KEEP token.

approveAndCall executes receiveApproval function on the spender as
specified in IReceiveApproval interface. Approves spender to withdraw
from the caller multiple times, up to the value amount. If this function
is called again, it overwrites the current allowance with the provided
value. Reverts if the approval operation reverted or if receiveApproval
call on the spender reverted.
@pdyraga pdyraga force-pushed the approve-and-call branch from 3ecf33c to 4799cdf Compare June 10, 2021 15:28

pragma solidity <0.9.0;

import "@openzeppelin/contracts/token/ERC20/SafeERC20.sol";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove this import

Copy link
Member Author

Choose a reason for hiding this comment

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

🔥 in d671d1a

@@ -5,6 +5,37 @@ pragma solidity <0.9.0;
import "./ERC20WithPermit.sol";
import "./MisfundRecovery.sol";

interface IReceiveApproval {
Copy link
Contributor

Choose a reason for hiding this comment

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

In TBTC v1 this interface is named ITokenRecipient. Was there a reason why we don't want to name it the same?

Copy link
Member Author

Choose a reason for hiding this comment

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

In retrospect, I think ITokenRecipient is too broad. It is about a specific way of receiving approval and maybe receiving tokens if the implementation decides for that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, I'm okay with both, but what I liked about ITokenRecipient is that it tells me right away in its name that this contract will receive tokens. IReceiveApproval(spender).receiveApproval also looks a bit redundant.

Base automatically changed from misfunds to main July 5, 2021 08:30
@pdyraga pdyraga marked this pull request as ready for review July 5, 2021 08:52
@dimpar dimpar merged commit db23320 into main Jul 5, 2021
@dimpar dimpar deleted the approve-and-call branch July 5, 2021 08:53
@pdyraga pdyraga added this to the solidity/v0.1.0 milestone Aug 17, 2021
@pdyraga pdyraga added the ⛓️ solidity Solidity contracts label Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⛓️ solidity Solidity contracts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants