fix: remove shorthash override for same field #1008
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reported by @readygo67 in #1000.
Description
For proof recursion, we are using "short-hash". When computing Fiat-Shamir challenges in-circuit then instead of emulating the whole MiMC hash in non-native, we instead use the native hash of the circuit and map the values to the target field. For this mapping, we have to perform binary decomposition of the inputs and outputs to ensure that the constructed elements fit into the field.
We had a premature optimization for the case where the target field and native field are the same, then we omitted the binary decomposition by using native hash instead. However, this doesn't work well with the rest of the stack (Fiat-Shamir transcript object and recursion implementations), as we do not exclude bits what the rest of the stack assumes are removed.
It would be difficult to retrofit as would require handling edge-cases in point marshalling, Fiat-Shamir and recursion gadgets, so it is better to remove the optimization in whole. This PR does exactly this.
Fixes #1000.
Type of change
How has this been tested?
How has this been benchmarked?
Not benchmarked
Checklist:
golangci-lint
does not output errors locally