Parse old precomputed blocks correctly by allowing Field.t to be deserialized from decimals

[glyh]

This is a revivial of #16922

This is encountered by JRWashburn on Discord. Context thread here.

Thank you @robinbb, sorry for us not being able to review your PR earlier :)

[SanabriaRusso]

!ci-build-me

[glyh]

This is untested, though. We should put some old blocks in our unit tests, suggested by @cjjdespres

But I think it would be even better if we just randomly sample some blocks from our archives and run the unit tests, which requires us to change the structure of our CI. WDYT? @SanabriaRusso @dkijania

[dkijania]

@glyh I'm always a fan of limited setup for every test. If this unit test requires old precomp block format, we should have dedicated test only for that case. IMHO it would be perfect if we can generate random precomputed block with old format. Such test will not only test the "Crème de la crème" but will have a good readability as it won't mislead us in the future into scratching out heads around why "real" and historic precomputed block is required for this.

I think also that such test should be a part of this PR (or in PR train). If you need any assistance I'm yours to help :)

[svv232]

approved too soon, we want to test dynamically.

[glyh]

!ci-build-me

[glyh]

!ci-build-me

[glyh]

wat

[glyh]

!ci-build-me

[glyh]

!ci-build-me

[glyh]

!ci-build-me

[cjjdespres]

Small nitpick (feel free to change in separate PR if this would delay things too much) - these field elements can appear in more than just precomputed blocks, so the error might not want to mention PCBs in particular

[cjjdespres]

These changes should fix the precomputed block parsing issue introduced in #15964. I think it might go slightly further, because it was a custom of_yojson just for Account_update.Authorization_kind.Proof that was replaced in that PR:

        | Proof of
            (Field.t
            [@version_asserted]
            [@to_yojson fun t -> `String (Snark_params.Tick.Field.to_string t)]
            [@of_yojson
              function
              | `String s ->
                  let field = Snark_params.Tick.Field.of_string s in
                  let s' = Snark_params.Tick.Field.to_string field in
                  if String.equal s s' then Ok field
                  else Error "Invalid JSON for field"
              | _ ->
                  Error "expected JSON string"] )

That's just an observation - having the field element parser accept both decimal and hex is fine, and is a clean way of fixing the problem.

Though, I don't know why the original code round-trips the string and compares it to the one it was parsing. That seems unnecessary to me, unless that was a workaround for some quirk in that of_string function.

[dannywillems]

Checking for crypto

[dannywillems]

Not sure it is relevant for future devs to mention "with an older precomputed block". Out of the context, it does not mean anything.

[dannywillems]

Refusing the changes as it does allow values larger than the modulus. It does not also enforce the format given as inputs as in 127ea64

[dannywillems]

I opened #17103

[dannywillems]

Approving for crypto. I would add a follow-up issue regarding the tests for the modulus. It seems to be out-of-scope of this fix, as the code is calling of_bigint as before.

[robinbb]

Thanks for the credit, and for the PR, @glyh .