Skip to content

Commit

Permalink
Unsigned conversion #2111 (#2123)
Browse files Browse the repository at this point in the history
* Add signed to unsigned conversion to SafeCast

* Update SafeCast exception message

* Add test for SafeCast int to uint conversion

- Update SafeCastMock
- Add tests for SafeCast int256 to uint256

* Update SafeCast int to uint definition

Apply suggestions from code review.

Co-Authored-By: Nicolás Venturo <nicolas.venturo@gmail.com>

* Update test for SafeCast int to uint conversion

* Update SafeCast test after code review

- Change "downcasts" to "casts"
- Move test closer to its function

* Fix error in SafeCast toUint256 description

* Fix breaking error in SafeCast

* Add uint256 to int256 conversion to SafeCast

- Add function
- Add mock
- Add test

* Update SafeCast unsigned to signed conversion

- Update error in conversion to be more clear
- Update constants in test to be powers of 2 instead of shifts

* Add changelog entry

* Update SafeCast tests

- Add minus in INT256_MIN for clarity

Co-Authored-By: Nicolás Venturo <nicolas.venturo@gmail.com>

Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
  • Loading branch information
pepelu and nventuro committed Mar 27, 2020
1 parent 7554ea8 commit 4476a2d
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### New features
* `AccessControl`: new contract for managing permissions in a system, replacement for `Ownable` and `Roles`. ([#2112](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2112))
* `SafeCast`: new functions to convert to and from signed and unsigned values: `toUint256` and `toInt256`. ([#2123](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2123))

### Breaking changes
* `ERC721`: `burn(owner, tokenId)` was removed, use `burn(owner)` instead. ([#2125](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2125))
Expand Down
9 changes: 9 additions & 0 deletions contracts/mocks/SafeCastMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@ import "../utils/SafeCast.sol";

contract SafeCastMock {
using SafeCast for uint;
using SafeCast for int;

function toUint256(int a) public pure returns (uint256) {
return a.toUint256();
}

function toInt256(uint a) public pure returns (int256) {
return a.toInt256();
}

function toUint128(uint a) public pure returns (uint128) {
return a.toUint128();
Expand Down
24 changes: 24 additions & 0 deletions contracts/utils/SafeCast.sol
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,28 @@ library SafeCast {
require(value < 2**8, "SafeCast: value doesn\'t fit in 8 bits");
return uint8(value);
}

/**
* @dev Converts a signed int256 into an unsigned uint256.
*
* Requirements:
*
* - input must be greater than or equal to 0.
*/
function toUint256(int256 value) internal pure returns (uint256) {
require(value >= 0, "SafeCast: value must be positive");
return uint256(value);
}

/**
* @dev Converts an unsigned uint256 into a signed int256.
*
* Requirements:
*
* - input must be less than or equal to maxInt256.
*/
function toInt256(uint256 value) internal pure returns (int256) {
require(value < 2**255, "SafeCast: value doesn't fit in an int256");
return int256(value);
}
}
70 changes: 70 additions & 0 deletions test/utils/SafeCast.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,74 @@ describe('SafeCast', async () => {
}

[8, 16, 32, 64, 128].forEach(bits => testToUint(bits));

describe('toUint256', () => {
const maxInt256 = new BN('2').pow(new BN(255)).subn(1);
const minInt256 = new BN('2').pow(new BN(255)).neg();
const maxUint256 = new BN('2').pow(new BN(256)).subn(1);

it('casts 0', async function () {
expect(await this.safeCast.toUint256(0)).to.be.bignumber.equal('0');
});

it('casts 1', async function () {
expect(await this.safeCast.toUint256(1)).to.be.bignumber.equal('1');
});

it(`casts INT256_MAX (${maxInt256})`, async function () {
expect(await this.safeCast.toUint256(maxInt256)).to.be.bignumber.equal(maxInt256);
});

it('reverts when casting -1', async function () {
await expectRevert(
this.safeCast.toUint256(-1),
'SafeCast: value must be positive'
);
});

it(`reverts when casting INT256_MIN (${minInt256})`, async function () {
await expectRevert(
this.safeCast.toUint256(minInt256),
'SafeCast: value must be positive'
);
});

it(`reverts when casting UINT256_MAX (${maxUint256})`, async function () {
await expectRevert(
this.safeCast.toUint256(maxUint256),
'SafeCast: value must be positive'
);
});
});

describe('toInt256', () => {
const maxUint256 = new BN('2').pow(new BN(256)).subn(1);
const maxInt256 = new BN('2').pow(new BN(255)).subn(1);

it('casts 0', async function () {
expect(await this.safeCast.toInt256(0)).to.be.bignumber.equal('0');
});

it('casts 1', async function () {
expect(await this.safeCast.toInt256(1)).to.be.bignumber.equal('1');
});

it(`casts INT256_MAX (${maxInt256})`, async function () {
expect(await this.safeCast.toInt256(maxInt256)).to.be.bignumber.equal(maxInt256);
});

it(`reverts when casting INT256_MAX + 1 (${maxInt256.addn(1)})`, async function () {
await expectRevert(
this.safeCast.toInt256(maxInt256.addn(1)),
'SafeCast: value doesn\'t fit in an int256'
);
});

it(`reverts when casting UINT256_MAX (${maxUint256})`, async function () {
await expectRevert(
this.safeCast.toInt256(maxUint256),
'SafeCast: value doesn\'t fit in an int256'
);
});
});
});

0 comments on commit 4476a2d

Please sign in to comment.