Skip to content

Commit

Permalink
SafeERC20.trySafeTransfer{,from} (#5483)
Browse files Browse the repository at this point in the history
  • Loading branch information
Amxx authored Feb 3, 2025
1 parent c089efa commit 19c2f2f
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 0 deletions.
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

0 comments on commit 19c2f2f

Please sign in to comment.