Refactor HasherLookup

[tohrnii]

Describe your changes

Partly addressing #360.

• Remove HasherState from HasherLookup and get the state from main_trace instead.
• Refactor hash_span_block to avoid using multiple is_memoized checks.

Benchmark:
Execution time for keccak256 reduced from 91.833 ms to 87.36 ms, an improvement of ~4.8% after removing HasherState from HasherLookup.

Checklist before requesting a review

• Repo forked and branch created from next according to naming convention.
• Commit messages and codestyle follow conventions.
• Relevant issues are linked in the PR description.
• Tests added for new functionality.
• Documentation/comments updated according to changes.

[grjte]

Thanks @tohrnii , that's a great improvement on keccak! I left a few comments inline. Also, it would be great if you can search the processor crate for "TODO" and make any other updates related to removing the state from the HasherLookup. I think there are a couple other ones still hanging around

[grjte]

Thanks @tohrnii! There are just a couple more small things that I left inline

[grjte]

Do we still need the state parameter and the code below? The point of this comment was that I thought we wouldn't need these once the state was removed from the lookups, since this method appends the rows to the trace directly. Have I missed something?

[tohrnii]

I think we need it to get the final state to check that the resultant state digest is the same as the expected hash.
I thought your comment was to avoid copying the state for intermediate states after removing lookups but since we only copy the trace once at the end in each case now, we only copy the final state.

[grjte]

Hmm yeah, I see. However, the result (which comes from the digest in this state array) and expected_hash are only checked in debug mode, which means this state is only used in debug mode, so I'm not sure we want to be passing it around and going through this extra work when not in debug.

I think what we want is to move that debug_assert_eq from the chiplets module into the hasher module. We would then check the assertion in debug mode in 2 cases:

1. after building the trace for the first time (in the control block merge methods before saving the memoized version)
2. after copying the trace (inside the copy_trace method) so that the work required for the check only occurs in debug mode

@bobbinth what are your thoughts?

If we do this, we can do it as a separate PR so this one can be merged

[grjte]

Approved, thanks @tohrnii! As noted, I think there's something we want to revisit, but we can wait for Bobbin to weigh in and then address it in a future PR