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

SafeERC20.trySafeTransfer{,from} #5483

Merged
merged 2 commits into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/brown-seals-sing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`SafeERC20`: Add `trySafeTransfer` and `trySafeTransferFrom` that do not revert and return false if the transfer is not successful.
14 changes: 14 additions & 0 deletions contracts/token/ERC20/utils/SafeERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,20 @@ library SafeERC20 {
_callOptionalReturn(token, abi.encodeCall(token.transferFrom, (from, to, value)));
}

/**
* @dev Variant of {safeTransfer} that returns a bool instead of reverting if the operation is not successful.
*/
function trySafeTransfer(IERC20 token, address to, uint256 value) internal returns (bool) {
return _callOptionalReturnBool(token, abi.encodeCall(token.transfer, (to, value)));
}

/**
* @dev Variant of {safeTransferFrom} that returns a bool instead of reverting if the operation is not successful.
*/
function trySafeTransferFrom(IERC20 token, address from, address to, uint256 value) internal returns (bool) {
return _callOptionalReturnBool(token, abi.encodeCall(token.transferFrom, (from, to, value)));
}

/**
* @dev Increase the calling contract's allowance toward `spender` by `value`. If `token` returns no value,
* non-reverting calls are assumed to be successful.
Expand Down
36 changes: 36 additions & 0 deletions test/token/ERC20/utils/SafeERC20.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,24 @@ describe('SafeERC20', function () {
.withArgs(this.token);
});

it('returns false on trySafeTransfer', async function () {
await expect(this.mock.$trySafeTransfer(this.token, this.receiver, 0n))
.to.emit(this.mock, 'return$trySafeTransfer')
.withArgs(false);
});

it('reverts on transferFrom', async function () {
await expect(this.mock.$safeTransferFrom(this.token, this.mock, this.receiver, 0n))
.to.be.revertedWithCustomError(this.mock, 'SafeERC20FailedOperation')
.withArgs(this.token);
});

it('returns false on trySafeTransferFrom', async function () {
await expect(this.mock.$trySafeTransferFrom(this.token, this.mock, this.receiver, 0n))
.to.emit(this.mock, 'return$trySafeTransferFrom')
.withArgs(false);
});

it('reverts on increaseAllowance', async function () {
// Call to 'token.allowance' does not return any data, resulting in a decoding error (revert without reason)
await expect(this.mock.$safeIncreaseAllowance(this.token, this.spender, 0n)).to.be.revertedWithoutReason();
Expand Down Expand Up @@ -94,12 +106,24 @@ describe('SafeERC20', function () {
.withArgs(this.token);
});

it('returns false on trySafeTransfer', async function () {
await expect(this.mock.$trySafeTransfer(this.token, this.receiver, 0n))
.to.emit(this.mock, 'return$trySafeTransfer')
.withArgs(false);
});

it('reverts on transferFrom', async function () {
await expect(this.mock.$safeTransferFrom(this.token, this.mock, this.receiver, 0n))
.to.be.revertedWithCustomError(this.mock, 'SafeERC20FailedOperation')
.withArgs(this.token);
});

it('returns false on trySafeTransferFrom', async function () {
await expect(this.mock.$trySafeTransferFrom(this.token, this.mock, this.receiver, 0n))
.to.emit(this.mock, 'return$trySafeTransferFrom')
.withArgs(false);
});

it('reverts on increaseAllowance', async function () {
await expect(this.mock.$safeIncreaseAllowance(this.token, this.spender, 0n))
.to.be.revertedWithCustomError(this.mock, 'SafeERC20FailedOperation')
Expand Down Expand Up @@ -357,11 +381,23 @@ function shouldOnlyRevertOnErrors() {
.withArgs(this.mock, this.receiver, 10n);
});

it('returns true on trySafeTransfer', async function () {
await expect(this.mock.$trySafeTransfer(this.token, this.receiver, 10n))
.to.emit(this.mock, 'return$trySafeTransfer')
.withArgs(true);
});

it("doesn't revert on transferFrom", async function () {
await expect(this.mock.$safeTransferFrom(this.token, this.owner, this.receiver, 10n))
.to.emit(this.token, 'Transfer')
.withArgs(this.owner, this.receiver, 10n);
});

it('returns true on trySafeTransferFrom', async function () {
await expect(this.mock.$trySafeTransferFrom(this.token, this.owner, this.receiver, 10n))
.to.emit(this.mock, 'return$trySafeTransferFrom')
.withArgs(true);
});
});

describe('approvals', function () {
Expand Down