Feat: Extend tests for BLS12-381 with json tests from EIP-2537

[mrLSD]

Description

Extended tests for BLS12-381 with json tests from EIP-2537: https://github.com/ethereum/execution-spec-tests
for NEP-488 implementation

➡️ It's related to bug: #12928
➡️ Successfull tests pass: aurora-is-near/aurora-engine#990

This PR added json cases that were prepared: aurora-is-near/sputnikvm#78

In the tests added special restrictions, to avoid bug from: #12928

[olga24912]

Thank you for finding the bug and for the additional tests!

I will add all these tests in another PR, in a slightly different format, along with the bug fix from #12928.

[Longarithm]

@olga24912 should we close the PR? Or could you assign someone from @near/contract-runtime for review?

[mrLSD]

Hello @olga24912 , @Longarithm
I'm a bit confused about the intention of closing that PR. As it's an opensource contribution, I'm expecting to proposal for changes if in general you're agree about NEP-488 tests changes.
If you just close PR, it will mean declining my contribution to the project.
@olga24912 if you have some related changes, it'll be logically to propose for me changes or operate your own in that PR as co-author.

[olga24912]

@Longarithm, @mrLSD

Alright, let's continue with this PR if you think it's the better approach. I'll make my changes after this PR is merged to avoid conflicts.

[nagisa]

I'm left wondering if there's a strong reason to use JSON here. Why not put the test cases in some global const array in Rust code? Especially if we're skipping some of the test cases based on input string in the macro…

I guess there is value in using exactly what's in that other repository: it makes keeping up with upstream changes ever-so-slightly easier, for instance. At the same time to take advantage of all those benefits we'd still need a README explaining where these JSONs are coming from and how to update them. And that the license for these test files belongs to a different party and is just MIT (not MIT OR APACHE-2.0.)

Regardless additional test coverage is much appreciated and I don't see any issues with the implementation other than the easily addressable nitpick inline.

[nagisa]

I'd say lets include_str!() the json file instead of loading it at runtime. I wouldn't want to bet on any particular working directory for the tests being run.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.54%. Comparing base (8b3fc09) to head (a778bdf).
Report is 7 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #12929   +/-   ##
=======================================
  Coverage   70.54%   70.54%           
=======================================
  Files         851      851           
  Lines      174983   174983           
  Branches   174983   174983           
=======================================
+ Hits       123433   123445   +12     
+ Misses      46419    46406   -13     
- Partials     5131     5132    +1     

Flag	Coverage Δ
backward-compatibility	0.36% <ø> (ø)
db-migration	0.36% <ø> (ø)
genesis-check	1.42% <ø> (ø)
linux	70.36% <ø> (-0.01%)	⬇️
linux-nightly	70.19% <ø> (+0.02%)	⬆️
pytests	1.73% <ø> (ø)
sanity-checks	1.54% <ø> (ø)
unittests	70.38% <ø> (+<0.01%)	⬆️
upgradability	0.36% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mrLSD]

@nagisa

I'm left wondering if there's a strong reason to use JSON here. Why not put the test cases in some global const array in Rust code? Especially if we're skipping some of the test cases based on input string in the macro…

the reason was the same as used in that test csv files. Also test cases are quite big. Also in the future int possible just update json files to extend it after Ethereum Prague hard fork. As that test cases from execution-spec-tests.

Also it's possible to move json to csv. But I'm not sure it's good idea form MIT license point of view of execution-spec-tests.

Also if you're prefering include_str!() including tests in compilation time. @olga24912 please your vision on that. As it seems the same logic should be for csv firls.