Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Assert.sol - Bug in Assert.notEqual for list types #770

Closed
sabhiram opened this issue Jan 24, 2018 · 4 comments
Closed

Assert.sol - Bug in Assert.notEqual for list types #770

sabhiram opened this issue Jan 24, 2018 · 4 comments

Comments

@sabhiram
Copy link
Contributor

When working on #769, I was reviewing your pattern for error reporting and noticed that you have a bug in the case where two equal length arrays with different values will fail the Assert.notEqual check.

Lets say a contains [1,2] and b contains [3, 4], you would expect Assert.notEqual(a, b, "a not equal to b"); to return true since they are not equal to each other. However, due to the way that this case is handled, the function incorrectly treats them as equal.

Here is a short test case to illustrate the point. Below, the testEqual_GOOD() does the right thing and asserts, while the testNotEqual_BAD() throws an assert even though the two arrays are clearly not equal (but have the same length). Notice that if the arrays are of different lengths - everything is a-ok (testNotEqual_GOOD()).

	int[] a;
	int[] b;
	int[] c;
	function beforeAll() public {
		a.push(1);
		a.push(2);
		b.push(3);
		b.push(4);
		c.push(5);
	}

	// Should assert and it does.
	function testEqual_GOOD() public {
		Assert.equal(a, b, "a and b are equal"); 
	}

	// Should not assert (a and b are in fact not equal).
	function testNotEqual_BAD() public {
		Assert.notEqual(a, b, "a and b are not equal");
	}

	// Should not assert (a and c are in fact not equal).
	function testNotEqual_GOOD() public {
		Assert.notEqual(a, c, "a and c are not equal");
	}

This error occurs due to the fact that you guys employ a negation trick on the call to _report for these cases since the variable r captures the equality of the lengths of the arrays being compared. If the lengths do not match, this works great. The bug lives in the code where on a match (not equal is true) - the code sets r to true which is later negated.

I will send you a patch shortly :)

@sabhiram
Copy link
Contributor Author

sabhiram commented Jan 24, 2018

Also, here is the output from truffle test:

    1) testEqual_GOOD

    Events emitted during test:
    ---------------------------

    TestEvent(result: <indexed>, message: a and b are equal)

    ---------------------------
    2) testNotEqual_BAD

    Events emitted during test:
    ---------------------------

    TestEvent(result: <indexed>, message: a and b are not equal)

    ---------------------------
    ✓ testNotEqual_GOOD (38ms)


  1 passing (631ms)
  2 failing

  1) Test testEqual_GOOD:
     Error: a and b are equal

  2) Test testNotEqual_BAD:
     Error: a and b are not equal

@sabhiram
Copy link
Contributor Author

This actually will also fail when the very first element of the arrays match. The code goes like this:

    function notEqual(int[] arrA, int[] arrB, string message) public returns (bool) {
        var r = arrA.length == arrB.length;
        if (r) {
            for (uint i = 0; i < arrA.length; i++) {
                if (arrA[i] == arrB[i]) {
                    r = true;
                    break;
                }
            }
        }
        _report(!r, message);
    }

So if a = [1,2] and b = [1,4] - it will consider them equal and throw an assert when checking notEqual.

@sabhiram
Copy link
Contributor Author

PR#769 contains the fix for this as well.

@sabhiram
Copy link
Contributor Author

Closing this in favor of trufflesuite/truffle-core#101

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant