From a3cae4dd527a32cb6976fdb1025ac7f2dea7d03d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20S=C3=A1?= Date: Fri, 15 Dec 2017 15:45:34 +0000 Subject: [PATCH 1/4] Extending the test suite with non-critical tests (future-proofing) --- test/TestBytesLib1.sol | 24 ++++++++++++++++++++---- test/TestBytesLib2.sol | 23 +++++++++++++++++++---- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/test/TestBytesLib1.sol b/test/TestBytesLib1.sol index d0d6695..3477666 100755 --- a/test/TestBytesLib1.sol +++ b/test/TestBytesLib1.sol @@ -10,6 +10,7 @@ contract TestBytesLib1 { using BytesLib for bytes; bytes storageCheckBytes = hex"aabbccddeeff"; + bytes storageCheckBytesZeroLength = hex""; bytes storageBytes4 = hex"f00dfeed"; bytes storageBytes31 = hex"f00d000000000000000000000000000000000000000000000000000000feed"; @@ -28,16 +29,31 @@ contract TestBytesLib1 { function testSanityCheck() public { // Assert library sanity checks + // + // Please don't change the ordering of the var definitions + // the order is purposeful for testing zero-length arrays bytes memory checkBytes = hex"aabbccddeeff"; + bytes memory checkBytesZeroLength = hex""; bytes memory checkBytesRight = hex"aabbccddeeff"; - bytes memory checkBytesWrong = hex"000000"; + bytes memory checkBytesZeroLengthRight = hex""; + bytes memory checkBytesWrongLength = hex"000000"; + bytes memory checkBytesWrongContent = hex"aabbccddee00"; AssertBytes.equal(checkBytes, checkBytesRight, "Sanity check should be checking equal bytes arrays out."); - AssertBytes.notEqual(checkBytes, checkBytesWrong, "Sanity check should be checking different bytes arrays out."); + AssertBytes.notEqual(checkBytes, checkBytesWrongLength, "Sanity check should be checking different length bytes arrays out."); + AssertBytes.notEqual(checkBytes, checkBytesWrongContent, "Sanity check should be checking different content bytes arrays out."); - AssertBytes.equalStorage(storageCheckBytes, checkBytesRight, "Sanity check should be checking equal bytes arrays out."); - AssertBytes.notEqualStorage(storageCheckBytes, checkBytesWrong, "Sanity check should be checking different bytes arrays out."); + AssertBytes.equalStorage(storageCheckBytes, checkBytesRight, "Sanity check should be checking equal bytes arrays out. (Storage)"); + AssertBytes.notEqualStorage(storageCheckBytes, checkBytesWrongLength, "Sanity check should be checking different length bytes arrays out. (Storage)"); + AssertBytes.notEqualStorage(storageCheckBytes, checkBytesWrongContent, "Sanity check should be checking different content bytes arrays out. (Storage)"); + + // Zero-length checks + AssertBytes.equal(checkBytesZeroLength, checkBytesZeroLengthRight, "Sanity check should be checking equal zero-length bytes arrays out."); + AssertBytes.notEqual(checkBytesZeroLength, checkBytes, "Sanity check should be checking different length bytes arrays out."); + + AssertBytes.equalStorage(storageCheckBytesZeroLength, checkBytesZeroLengthRight, "Sanity check should be checking equal zero-length bytes arrays out. (Storage)"); + AssertBytes.notEqualStorage(storageCheckBytesZeroLength, checkBytes, "Sanity check should be checking different length bytes arrays out. (Storage)"); } /** diff --git a/test/TestBytesLib2.sol b/test/TestBytesLib2.sol index ca1adcd..a904015 100755 --- a/test/TestBytesLib2.sol +++ b/test/TestBytesLib2.sol @@ -17,16 +17,31 @@ contract TestBytesLib2 { function testSanityCheck() public { // Assert library sanity checks + // + // Please don't change the ordering of the var definitions + // the order is purposeful for testing zero-length arrays bytes memory checkBytes = hex"aabbccddeeff"; + bytes memory checkBytesZeroLength = hex""; bytes memory checkBytesRight = hex"aabbccddeeff"; - bytes memory checkBytesWrong = hex"000000"; + bytes memory checkBytesZeroLengthRight = hex""; + bytes memory checkBytesWrongLength = hex"000000"; + bytes memory checkBytesWrongContent = hex"aabbccddee00"; AssertBytes.equal(checkBytes, checkBytesRight, "Sanity check should be checking equal bytes arrays out."); - AssertBytes.notEqual(checkBytes, checkBytesWrong, "Sanity check should be checking different bytes arrays out."); + AssertBytes.notEqual(checkBytes, checkBytesWrongLength, "Sanity check should be checking different length bytes arrays out."); + AssertBytes.notEqual(checkBytes, checkBytesWrongContent, "Sanity check should be checking different content bytes arrays out."); - AssertBytes.equalStorage(storageCheckBytes, checkBytesRight, "Sanity check should be checking equal bytes arrays out."); - AssertBytes.notEqualStorage(storageCheckBytes, checkBytesWrong, "Sanity check should be checking different bytes arrays out."); + AssertBytes.equalStorage(storageCheckBytes, checkBytesRight, "Sanity check should be checking equal bytes arrays out. (Storage)"); + AssertBytes.notEqualStorage(storageCheckBytes, checkBytesWrongLength, "Sanity check should be checking different length bytes arrays out. (Storage)"); + AssertBytes.notEqualStorage(storageCheckBytes, checkBytesWrongContent, "Sanity check should be checking different content bytes arrays out. (Storage)"); + + // Zero-length checks + AssertBytes.equal(checkBytesZeroLength, checkBytesZeroLengthRight, "Sanity check should be checking equal zero-length bytes arrays out."); + AssertBytes.notEqual(checkBytesZeroLength, checkBytes, "Sanity check should be checking different length bytes arrays out."); + + AssertBytes.equalStorage(storageCheckBytesZeroLength, checkBytesZeroLengthRight, "Sanity check should be checking equal zero-length bytes arrays out. (Storage)"); + AssertBytes.notEqualStorage(storageCheckBytesZeroLength, checkBytes, "Sanity check should be checking different length bytes arrays out. (Storage)"); } /** From 87983d70c8f317dd4b2dd4e3061ddf6231f7f1ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20S=C3=A1?= Date: Fri, 15 Dec 2017 16:31:04 +0000 Subject: [PATCH 2/4] Fix for storage equality method Thanks to the most recent extension of the test suite another bug handling zero-length storage/memory equality was found --- contracts/AssertBytes.sol | 64 ++++++++++++++++++++------------------- contracts/BytesLib.sol | 64 ++++++++++++++++++++------------------- test/TestBytesLib1.sol | 2 +- test/TestBytesLib2.sol | 3 +- 4 files changed, 69 insertions(+), 64 deletions(-) diff --git a/contracts/AssertBytes.sol b/contracts/AssertBytes.sol index a26b6f9..2abc508 100755 --- a/contracts/AssertBytes.sol +++ b/contracts/AssertBytes.sol @@ -105,40 +105,42 @@ library AssertBytes { // slength can contain both the length and contents of the array // if length < 32 bytes so let's prepare for that // v. http://solidity.readthedocs.io/en/latest/miscellaneous.html#layout-of-state-variables-in-storage - switch lt(slength, 32) - case 1 { - // blank the last byte which is the length - fslot := mul(div(fslot, 0x100), 0x100) + if iszero(iszero(slength)) { + switch lt(slength, 32) + case 1 { + // blank the last byte which is the length + fslot := mul(div(fslot, 0x100), 0x100) - if iszero(eq(fslot, mload(add(_b, 0x20)))) { - // unsuccess: - returnBool := 0 - } - } - default { - // cb is a circuit breaker in the for loop since there's - // no said feature for inline assembly loops - // cb = 1 - don't breaker - // cb = 0 - break - let cb := 1 - - // get the keccak hash to get the contents of the array - mstore(0x0, _a_slot) - let sc := keccak256(0x0, 0x20) - - let mc := add(_b, 0x20) - let end := add(mc, mlength) - - // the next line is the loop condition: - // while(uint(mc < end) + cb == 2) - for {} eq(add(lt(mc, end), cb), 2) { - sc := add(sc, 1) - mc := add(mc, 0x20) - } { - if iszero(eq(sload(sc), mload(mc))) { + if iszero(eq(fslot, mload(add(_b, 0x20)))) { // unsuccess: returnBool := 0 - cb := 0 + } + } + default { + // cb is a circuit breaker in the for loop since there's + // no said feature for inline assembly loops + // cb = 1 - don't breaker + // cb = 0 - break + let cb := 1 + + // get the keccak hash to get the contents of the array + mstore(0x0, _a_slot) + let sc := keccak256(0x0, 0x20) + + let mc := add(_b, 0x20) + let end := add(mc, mlength) + + // the next line is the loop condition: + // while(uint(mc < end) + cb == 2) + for {} eq(add(lt(mc, end), cb), 2) { + sc := add(sc, 1) + mc := add(mc, 0x20) + } { + if iszero(eq(sload(sc), mload(mc))) { + // unsuccess: + returnBool := 0 + cb := 0 + } } } } diff --git a/contracts/BytesLib.sol b/contracts/BytesLib.sol index 0de2556..67a773e 100755 --- a/contracts/BytesLib.sol +++ b/contracts/BytesLib.sol @@ -271,40 +271,42 @@ library BytesLib { // slength can contain both the length and contents of the array // if length < 32 bytes so let's prepare for that // v. http://solidity.readthedocs.io/en/latest/miscellaneous.html#layout-of-state-variables-in-storage - switch lt(slength, 32) - case 1 { - // blank the last byte which is the length - fslot := mul(div(fslot, 0x100), 0x100) + if iszero(iszero(slength)) { + switch lt(slength, 32) + case 1 { + // blank the last byte which is the length + fslot := mul(div(fslot, 0x100), 0x100) - if iszero(eq(fslot, mload(add(_postBytes, 0x20)))) { - // unsuccess: - success := 0 + if iszero(eq(fslot, mload(add(_b, 0x20)))) { + // unsuccess: + returnBool := 0 + } } - } - default { - // cb is a circuit breaker in the for loop since there's - // no said feature for inline assembly loops - // cb = 1 - don't breaker - // cb = 0 - break - let cb := 1 - - // get the keccak hash to get the contents of the array - mstore(0x0, _preBytes_slot) - let sc := keccak256(0x0, 0x20) - - let mc := add(_postBytes, 0x20) - let end := add(mc, mlength) + default { + // cb is a circuit breaker in the for loop since there's + // no said feature for inline assembly loops + // cb = 1 - don't breaker + // cb = 0 - break + let cb := 1 - // the next line is the loop condition: - // while(uint(mc < end) + cb == 2) - for {} eq(add(lt(mc, end), cb), 2) { - sc := add(sc, 1) - mc := add(mc, 0x20) - } { - if iszero(eq(sload(sc), mload(mc))) { - // unsuccess: - success := 0 - cb := 0 + // get the keccak hash to get the contents of the array + mstore(0x0, _a_slot) + let sc := keccak256(0x0, 0x20) + + let mc := add(_b, 0x20) + let end := add(mc, mlength) + + // the next line is the loop condition: + // while(uint(mc < end) + cb == 2) + for {} eq(add(lt(mc, end), cb), 2) { + sc := add(sc, 1) + mc := add(mc, 0x20) + } { + if iszero(eq(sload(sc), mload(mc))) { + // unsuccess: + returnBool := 0 + cb := 0 + } } } } diff --git a/test/TestBytesLib1.sol b/test/TestBytesLib1.sol index 3477666..393498a 100755 --- a/test/TestBytesLib1.sol +++ b/test/TestBytesLib1.sol @@ -37,7 +37,7 @@ contract TestBytesLib1 { bytes memory checkBytesRight = hex"aabbccddeeff"; bytes memory checkBytesZeroLengthRight = hex""; - bytes memory checkBytesWrongLength = hex"000000"; + bytes memory checkBytesWrongLength = hex"aa0000"; bytes memory checkBytesWrongContent = hex"aabbccddee00"; AssertBytes.equal(checkBytes, checkBytesRight, "Sanity check should be checking equal bytes arrays out."); diff --git a/test/TestBytesLib2.sol b/test/TestBytesLib2.sol index a904015..cd2b1cc 100755 --- a/test/TestBytesLib2.sol +++ b/test/TestBytesLib2.sol @@ -10,6 +10,7 @@ contract TestBytesLib2 { using BytesLib for bytes; bytes storageCheckBytes = hex"aabbccddeeff"; + bytes storageCheckBytesZeroLength = hex""; /** * Sanity Checks @@ -25,7 +26,7 @@ contract TestBytesLib2 { bytes memory checkBytesRight = hex"aabbccddeeff"; bytes memory checkBytesZeroLengthRight = hex""; - bytes memory checkBytesWrongLength = hex"000000"; + bytes memory checkBytesWrongLength = hex"aa0000"; bytes memory checkBytesWrongContent = hex"aabbccddee00"; AssertBytes.equal(checkBytes, checkBytesRight, "Sanity check should be checking equal bytes arrays out."); From 070fde731804a9f695c5a75f60fe21422c360552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20S=C3=A1?= Date: Fri, 15 Dec 2017 16:35:15 +0000 Subject: [PATCH 3/4] Fixing var names for the equalStorage method in BytesLib --- contracts/BytesLib.sol | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contracts/BytesLib.sol b/contracts/BytesLib.sol index 67a773e..2adba64 100755 --- a/contracts/BytesLib.sol +++ b/contracts/BytesLib.sol @@ -277,9 +277,9 @@ library BytesLib { // blank the last byte which is the length fslot := mul(div(fslot, 0x100), 0x100) - if iszero(eq(fslot, mload(add(_b, 0x20)))) { + if iszero(eq(fslot, mload(add(_postBytes, 0x20)))) { // unsuccess: - returnBool := 0 + success := 0 } } default { @@ -290,10 +290,10 @@ library BytesLib { let cb := 1 // get the keccak hash to get the contents of the array - mstore(0x0, _a_slot) + mstore(0x0, _preBytes_slot) let sc := keccak256(0x0, 0x20) - let mc := add(_b, 0x20) + let mc := add(_postBytes, 0x20) let end := add(mc, mlength) // the next line is the loop condition: @@ -304,7 +304,7 @@ library BytesLib { } { if iszero(eq(sload(sc), mload(mc))) { // unsuccess: - returnBool := 0 + success := 0 cb := 0 } } From 49d24229c691b92b0ee77ac323b4463135dfd7c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20S=C3=A1?= Date: Fri, 15 Dec 2017 16:36:06 +0000 Subject: [PATCH 4/4] Bump version number to deploy fix to EPM --- ethpm.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ethpm.json b/ethpm.json index 41cc749..3c97f15 100644 --- a/ethpm.json +++ b/ethpm.json @@ -1,6 +1,6 @@ { "package_name": "bytes", - "version": "0.0.2", + "version": "0.0.3", "description": "Solidity bytes tightly packed arrays utility library.", "authors": [ "Gonçalo Sá " diff --git a/package.json b/package.json index 6a4a54d..f4f3d6a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "solidity-bytes-utils", - "version": "0.0.2", + "version": "0.0.3", "description": "Solidity bytes tightly packed arrays utility library.", "main": "truffle.js", "repository": {