mit-dci / zksigma

A library for generating non-interactive proofs of knowledge without trusted setup
MIT License
29 stars 10 forks source link

Change big.NewInt(0) to BigZero where appropriate #15

Closed gertjaap closed 5 years ago

gertjaap commented 5 years ago

I changed big.NewInt(0) to BigZero in places where I think this is appropriate. Tests succeed after these changes

Nabeelperson commented 5 years ago

@gertjaap using BigZero for comparisons and proof generation I agree with but for assignment statements I am hesitant about it. For instance value := BigZero, is this a deep copy assignment or a shallow copy that just copies the pointer? I had some bugs with my attempt to implement bullet proofs because of the lack of deep copy for *big.Int. Let me know what you think, at least in the GoDocs I could not find an answer to this question.

Nabeelperson commented 5 years ago

I ran a quick experiment, and when you assign value := BigZero is actually copies the pointer as I suspected. For deep copy it should be value:= new(big.Int).Set(BigZero). I will push a commit to master that implements your BigZero behaviour.

Nabeelperson commented 5 years ago

If you add this to the bottom of cypto_test.go and run this unit test you'll see the deep copy problem.

func TestBigZeroAssignment(t *testing.T) {
    TestBigZero := big.NewInt(0)

    assign1 := TestBigZero // assign will be using TestBigZero pointer from here on
    assign1.Add(assign1, big.NewInt(1)) // TestBigZero looks like it does not change but actually does

    assign2 := TestBigZero // assign2 = TestBigZero = 1

    if assign1.Cmp(assign2) == 0 {
        t.Fatalf("THIS TEST WILL FAIL FOR DEMO PURPOSES: should not be equal %v", TestBigZero)
    }

}