feat: Change proof commit and find next key

[bernard-avalabs]

Why this should be merged

Builds on #1641. This is likely the last in the current series of Change Proof PRs. It implements fwd_db_verify_and_commit_change_proof and fwd_change_proof_find_next_key.

How this works

Mostly follows the implementation from range proofs with some additional checks that are specific to change proofs. The find next key approach is the same basic one used in range proofs and should later be replaced.

How this was tested

Added end-to-end tests for committing a change proof, applying multiple rounds of change proofs with different start keys determined by calling find next key, and testing change proofs with deletes instead of just puts.

[alarso16]

Haha good catch, but will you fix the entire line?

[bernard-avalabs]

I'll rewrite the comment. This was something that I was planning to come back to after catching the error, but I must have forgotten.

[alarso16]

Suggested change

	root2, err := db2.Update(batch[:50])
	_, err := db2.Update(batch[:50])

Shouldn't these match? Just adds what seems like more mental overhead then necessary. Then you only need root1, root2, gotRoot

[alarso16]

Suggested change

	change_proof, err := db1.ChangeProof(root1, root1_updated, nothing(), nothing(), changeProofLenUnbounded)
	changeProof, err := db1.ChangeProof(root1, root1_updated, nothing(), nothing(), changeProofLenUnbounded)

camelCase is conventional in Go

[alarso16]

Suggested change

startKey = append([]byte{}, startKey...) // copy to new slice to avoid use-after-free

I believe startKey is copied when it's returned from the API, so this is unnecessary

[alarso16]

Suggested change

	value: append([]byte{}, nextRange.StartKey()...), // copy to new slice to avoid use-after-free
	value: StartKey(),

[alarso16]

This seems prone to busy loop forever if there's ever a bug. Do we know the exact amount of proofs this should take? or can we add some debugging with t.Logf? Just think this test will be very hard to debug

[bernard-avalabs]

I will add a loop limit.

[alarso16]

Can you combine the tests above? It's very conventional for functional in Go to have a test table like this:

tests := []struct {
    name string,
    params any, // put everything necessary in other fields
}

for _, tt := range tests {
    t.Run(tt.name, func(t *testing.T) {
        r := require.New(t) // optional, but make sure you use the new `*testing.T`
        actualTest(tt.Params) // or write the test here
    })
}

[bernard-avalabs]

I didn't know that this is Go convention. I'll make this change.