lurk-lab / arecibo

An advanced fork of Nova
https://lurk-lang.org/
MIT License
68 stars 28 forks source link

Compatibility enforcements between Rust and Solidity #337

Open storojs72 opened 5 months ago

storojs72 commented 5 months ago

Nova verifier implemented in Arecibo consists from a bunch of cryptographic blocks (Poseidon, KeccakTranscript, IPA, etc.) each having its Solidity counterpart in solidity-verifier project. Historically, Solidity codebase growing started from lurk-lab/Nova. Currently we have a situation when Solidity building blocks guarantee compatibility only with some specific (various) versions of their Rust references, for example:

In order to prevent our Solidity codebase obsoleting, it is necessary to enforce Rust <-> Solidity compatibility per building blocks. Apart from e2e integration testing that we are performing in feature branches of solidity-verifier (where we have "terminal" code in a form of contracts) that guarantees complex compatibility of the whole verification flow (which is now obsolete and hence unfortunately is not able to consider frequent low-level changes happening in Arecibo) we can have in Arecibo bunch of unit-tests (one per each building block that we have in Solidity) - each of them will produce correspondent solidity unit-test with input/output data dynamically generated using particular Arecibo commit. This can be achieved with a help of Solidity templating in Rust (initial experiments for IPA are here). E.g. we can establish branch-protection rule at Arecibo repository that prevents merging PR if failure occurs on CI job with following set of steps (for single IPA case, which can be extended for every building block):

As Arecibo is reference - it should dominate in making decision whether accept or not the particular PR that breaks compatibility. But with these new settings - such breaking changes should be thoroughly justified.

I would start from enforcing single block - IPA compatibility and see how it will work.

@samuelburnham, happy to communicate on overall idea or its lower details more. Your additional input is highly appreciated!

samuelburnham commented 5 months ago

This is great, automated compatibility between the two repos would be amazing. I think we can hash out the CI details after the unit tests are set up, but my first thought is to have CI open a noisy issue in Solidity-verifier and/or Arecibo when breaking changes are merged rather than having to bypass required status checks.

storojs72 commented 4 months ago

Ok, PR for IPA is up, once we merge it, we are ready to making appropriate CI changes.

As it is mentioned in PR description, once IPA compatibility test is generated:

cargo test test_solidity_compatibility_ipa --release -- --ignored --nocapture > ipa-compatibility.t.sol
catipa-compatibility.t.sol | sed '1,3d' | sed -n -e :a -e '1,4!{P;N;D;};N;ba' > tmp.file && mv tmp.file ipa-compatibility.t.sol

next, ipa-compatibility.t.sol can be added to the solidity-verifier, and executed via forge

mv ipa-compatibility.t.sol solidity-verifier/test
forge test --match-path test/ipa-compatibility.t.sol -vv

the result of execution - failed/successful - will indicate whether IPA in Rust is incompatible/compatible with IPA in Solidity.

storojs72 commented 4 months ago

my first thought is to have CI open a noisy issue in Solidity-verifier and/or Arecibo when breaking changes are merged rather than having to bypass required status checks

@samuelburnham, I think noisy issue should appear in Solidity-verifier. At the same time, I would say we need some explicit blocker or warning at Arecibo's PRs that break compatibility in order to force PR' author to think twice if the task can be solved without breaking changes. If the answer is NO and potential PR's benefit exceeds necessity of extra-work at Solidity side of things - then this extra-work should happen