rp/poseidon arrays

[richardpringle]

The main change of this PR is the ArithmeticSpongeParams type (found here).

What used to be a Vec<Vec<F>> is now [[F; FULL_ROUNDS]] where FULL_ROUNDS is a const generic of type usize. This means the data can now sit on either the stack or the heap, and is contiguous rather than spread across the heap.

Making this change had wide spread affects to the all the dependent functions, types and traits. Now many of them are generic over a usize. There's really only one file that had logical changes and that's poseidon/src/permutation.rs. Extra allocations were removed and the code was cleaned up a little to now handle the arrays instead of Vecs.

I benchmarked locally and there is small consistent speed up (though that was not the goal).

[dannywillems]

Next steps:

• check benchmarks
  • this is the first thing to check because the hash function is a critical part of the whole protocol. If we get worse performances, we should abort this task immediately. However, there is a small probability that we get something worse. If we get better performances, we should absolutely documented it, and the CHANGELOG should contain the benchmarks, in addition to this PR.
• check if updating MinaProtocol/Mina with this version doesn't break
• as well as o1js
• fix the different lints
• clean the history
• rename ROUNDS into FULL_ROUNDS

Probably as a follow-up:

• add a new type parameter PARTIAL_ROUNDS as a Poseidon instance contains two types of rounds, so-called "partial" and "full". Our current Poseidon instance do not use any partial rounds, but if we change it in the future, we would need it. As discussed, if the parameter is not used right now, we should not have it in the codebase now.

[richardpringle]

Some of these comments are for myself, others are for the reviewer

[richardpringle]

To get the exact same behaviour as before, these should actually be reversed:

Suggested change

	state
	.iter_mut()
	.zip(params.round_constants[0].iter())
	.for_each(|(s, x)| {
	s.add_assign(x);
	});
	params
	.round_constants[0]
	.iter()
	.zip(state.iter_mut())
	.for_each(|(x, s)| {
	s.add_assign(x);
	});

In practice though, the round_constants[0] and state are always the same length.

@dannywillems, thoughts? Should I change this.

[dannywillems]

Yes, they must be the same length.

[richardpringle]

@dannywillems, should we add an issue to add an assertion here? Could mark it with "good first issue" and hope someone else does it

[richardpringle]

Running CI on github.com/mina

[richardpringle]

Next steps:

• check benchmarks

  • this is the first thing to check because the hash function is a critical part of the whole protocol. If we get worse performances, we should abort this task immediately. However, there is a small probability that we get something worse. If we get better performances, we should absolutely documented it, and the CHANGELOG should contain the benchmarks, in addition to this PR.

• check if updating MinaProtocol/Mina with this version doesn't break

• as well as o1js

• fix the different lints

• clean the history

• rename ROUNDS into FULL_ROUNDS

Probably as a follow-up:

• add a new type parameter PARTIAL_ROUNDS as a Poseidon instance contains two types of rounds, so-called "partial" and "full". Our current Poseidon instance do not use any partial rounds, but if we change it in the future, we would need it. As discussed, if the parameter is not used right now, we should not have it in the codebase now.

• Consistent (small) speedup locally
• Mina doesn't break, see my comment above.
• o1js was a little harder to test, but @Trivo25 got things working locally for him. It required a change to the initial memory pages on the web, but that just puts the web inline with nodejs. Not only should the speedup help on wasm-side, but there should be less memory pressure as there are fewer allocations now. @Trivo25 may have been a little concerned about the new memory usage, but it's memory that was always used; now it just exists on initialization instead of being allocated dynamically.
• lints are fixed
• history is clean (every commit is formatted and passes the linter - I would bet the tests pass too, but I didn't check that as it's not a requirement)
• ROUNDS is now FULL_ROUNDS
• and finally, I will create the PARTIAL_ROUNDS ticket once CI is passing and this PR is approved

[dannywillems]

I guess later we can remove PERM_HALF_ROUNDS_FULL from the params.

[richardpringle]

@dannywillems, is there an issue for doing the partial round stuff? We can reference this comment in there

[richardpringle]

Added a bunch of follow up issues from the comments (mine and @dannywillems's)