Introduce bitMatrix to replace mat.Dense

[liamsi]

This PR introduces a structure that replaces mat.Dense and gets rid of the gonum dependency.
Hence, it is half the story of closes #22.

It could use some light fuzzing but given that this is essentially a simple bit-mask I don't think that is urgent.

Can open a separate PR to actually use the structure, or, replace mat.Dense in here.

[liamsi]

As requested, this PR also replaces gonum now. It would have been nice to measure the impact with benchmarks but we already know the ballpark of improvment (see #22).

[adlerjohn]

Is there a reason not to use big.Int as the backing type instead of a []uint64?

[evan-forbes]

TestBlockRecovery fails sometimes using this branch, but always passes with the version that the lazyledger-core master branch is using. I'm still investigating though. It might just be the test.

func TestFuzzTestBlockRecovery(t *testing.T) {
	ctx, _ := context.WithTimeout(context.Background(), time.Second*10)
	for {
		select {
		case <-ctx.Done():
			return
		default:
			TestBlockRecovery(t)
		}
	}
}

    --- FAIL: TestFuzzTestBlockRecovery/missing_1/2_shares#295 (0.00s)
        read_test.go:220: 
                Error Trace:    read_test.go:220
                Error:          Received unexpected error:
                                failed to solve data square
                Test:           TestFuzzTestBlockRecovery/missing_1/2_shares#295

edit: see below comment, the square was being repaired fine, but an error was being returned along with the properly repaired square.

[evan-forbes]

Ahh I think this is issue for the failing lazyledger-core test mentioned in the other comment, the square is being repaired fine, but an error is being returned. Reverting this change allows for the other tests to pass.

[liamsi]

Good catch!! I'll revert these changes in the for-loop here. They are orthogonal to the bit-mask anyways.

[liamsi]

Done in 3eca580.
I'll retest with the above test to make sure that this fixed the issue.

[liamsi]

An alternative could be to change that if-else-if to: if !progressMade && !solved (or !(progressMade || solved)) and make the for loop for !solved. That should be equivalent. I think the for-loop would be easier to grasp directly as "repeat until solved". Currently, you either trust the comment Keep repeating until the square is solved or you have to scroll 105 complex looking lines of code to make sure that this what is indeed happening: https://github.com/lazyledger/rsmt2d/blob/master/extendeddatacrossword.go#L92-L207

[liamsi]

Either way, thanks for finding that issue and for pointing me to the solution even :-)

[liamsi]

Is there a reason not to use big.Int as the backing type instead of a []uint64?

Is there a good reason to use bigInt instead?

With big.Int can not directly operate on the underlying []uint to operate on the bits and would need to use Bits() which returns almost what we need (and already use). Almost because the underlying "words" used are Word uint and uint's size depends on the platform used: https://golang.org/pkg/builtin/#uint

uint is an unsigned integer type that is at least 32 bits in size.

Then, big.Int is signed which do not need.

Using big.Int is possible but it would just make the code more difficult to read imo.

[liamsi]

I've sneaked in another unrelated change here: int(math.Pow(float64(width), 2)) is just a complicated way of computing width*width.

[adlerjohn]

Saw that, and approve.

[liamsi]

can someone with write access approve the PR (or request changes): cc @musalbas @evan-forbes

[evan-forbes]

awesome 👍 👍