References to MiMC instead of Poseidon

In several places in the codebase, the MiMC hash function is named when in fact the Poseidon hash function is used. This can at times make the code confusing to follow and misleading, and thus it could cause mistakes. We list these occurrences below.

Merkle tree root-hash calculation in assignInputCommitment and assignToggleCommitment

In the sdk repository's sdk/app.go, the assignInputCommitment and assignToggleCommitment functions use the Poseidon hash for calculating the commitments and the Merkle tree root hash. However, some variable names are referring to MiMC instead. Additionally, mimc_bn254.BlockSize, a constant genuinely arising from a MiMC implementation, is used. This constant happens to have a value that is compatible with its use, so the code currently functions correctly, nevertheless. The following snippet from assignToggleCommitment shows the confusing parts of the two functions (assignInputCommitment is analogous):

func (q *BrevisApp) assignToggleCommitment(in *CircuitInput) {
    // ...

    hasher := utils.NewPoseidonBn254()

    // ...

    for {
		if elementCount == 1 {
			in.TogglesCommitment = leafs[0]
			return
		}
		for i := 0; i < elementCount/2; i++ {
! 			var mimcBlockBuf0, mimcBlockBuf1 [mimc_bn254.BlockSize]byte
! 			leafs[2*i].FillBytes(mimcBlockBuf0[:])
! 			leafs[2*i+1].FillBytes(mimcBlockBuf1[:])
			hasher.Reset()
! 			hasher.Write(new(big.Int).SetBytes(mimcBlockBuf0[:]))
! 			hasher.Write(new(big.Int).SetBytes(mimcBlockBuf1[:]))
			result, err := hasher.Sum()
			if err != nil {
				panic(fmt.Sprintf("failed to hash merkle tree: %s", err.Error()))
			}
			leafs[i] = result
		}
		elementCount = elementCount / 2
	}
}

We recommend to rename the variables and replace mimc_bn254.BlockSize by a suitable constant relating to Poseidon (for example by adding a BlockSize constant to utils/poseidon_bn254.go in the zk-hash repository).

As a side remark, code duplication could be reduced by factoring out the root-hash computation that is currently present in both assignInputCommitment and assignToggleCommitment into a separate function.

Comment in BuildCircuitInput

Also in sdk/app.go, the BrevisApp.BuildCircuitInput starts with the following comments:

func (q *BrevisApp) BuildCircuitInput(app AppCircuit) (CircuitInput, error) {

	// 1. mimc hash data at each position to generate and assign input commitments and toggles commitment
	// 2. dry-run user circuit to generate output and output commitment

The first comment line should use "Poseidon hash data" instead of "mimc hash data".

Documentation comments for functions in poseidon_bn254_circuit.go

In the zk-hash repository's file poseidon/poseidon_bn254_circuit.go, the type PoseidonCircuit and function NewBn254PoseidonCircuit have comments using "MiMC" where they should use "Poseidon":

! // MiMC contains the params of the Mimc hash func and the curves on which it is implemented
type PoseidonCircuit struct {
	preimage []frontend.Variable // state storage. data is updated when Write() is called. Sum sums the data.
	api      frontend.API        // underlying constraint system
}

! // NewMiMC returns a MiMC instance, that can be used in a gnark circuit
func NewBn254PoseidonCircuit(api frontend.API) (PoseidonCircuit, error) {
	return PoseidonCircuit{
		api: api,
	}, nil
}