Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch the zkID Groth16 VK from on-chain #11895

Merged
merged 28 commits into from
Feb 8, 2024
Merged

Fetch the zkID Groth16 VK from on-chain #11895

merged 28 commits into from
Feb 8, 2024

Conversation

alinush
Copy link
Contributor

@alinush alinush commented Feb 5, 2024

Description

Currently, the VK was hardcoded in Rust (see #11772), but it needs to be upgradeable via on-chain governance in Move in case of bugfixes or other issues.

Note: You can run ./scripts/authenticator_regenerate.sh to regenerate all API files and protobuf files (and other dependencies when modifying the Rust authenticator code). Otherwise, you'll get CI/CD failures.

Copy link

trunk-io bot commented Feb 5, 2024

⏱️ 62h 33m total CI duration on this PR
Job Cumulative Duration Recent Runs
rust-unit-tests 17h 39m 🟩🟥🟩 (+31 more)
windows-build 11h 11m 🟩🟩🟩🟩🟩 (+31 more)
rust-move-unit-coverage 10h 35m 🟩🟩🟩 (+29 more)
rust-move-tests 6h 31m 🟩🟩🟩🟩 (+29 more)
rust-lints 3h 10m 🟩🟩🟩 (+30 more)
run-tests-main-branch 3h 8m 🟥🟥🟥🟥 (+30 more)
check 2h 11m 🟩🟩🟩🟩 (+30 more)
general-lints 1h 27m 🟩🟩🟩🟩 (+30 more)
rust-smoke-tests 1h 21m 🟩🟩
check-dynamic-deps 1h 16m 🟩🟩🟩🟩🟩 (+31 more)
execution-performance / single-node-performance 1h 12m 🟩🟩🟩🟩
rust-images / rust-all 40m 🟩🟩🟩
forge-e2e-test / forge 38m 🟩🟩
forge-compat-test / forge 35m 🟩🟩
cli-e2e-tests / run-cli-tests 25m 🟩🟩🟩
semgrep/ci 13m 🟩🟩🟩🟩🟩 (+31 more)
file_change_determinator 7m 🟩🟩🟩🟩🟩 (+32 more)
file_change_determinator 6m 🟩🟩🟩🟩🟩 (+31 more)
node-api-compatibility-tests / node-api-compatibility-tests 3m 🟩🟩🟩
permission-check 2m 🟩🟩🟩🟩🟩 (+31 more)
permission-check 2m 🟩🟩🟩🟩🟩 (+31 more)
permission-check 2m 🟩🟩🟩🟩🟩 (+31 more)
permission-check 2m 🟩🟩🟩🟩🟩 (+31 more)
file_change_determinator 40s 🟩🟩🟩🟩
execution-performance / file_change_determinator 37s 🟩🟩🟩🟩
determine-docker-build-metadata 12s 🟩🟩🟩🟩
permission-check 11s 🟩🟩🟩🟩

🚨 1 job on the last run was significantly faster/slower than expected

Job Duration vs 7d avg Delta
run-tests-main-branch 6m 4m +45%

settingsfeedbackdocs ⋅ learn more about trunk.io

@alinush alinush requested a review from heliuchuan February 5, 2024 17:14
@alinush alinush force-pushed the alin/zkid.move branch 3 times, most recently from 8074647 to d3f96f0 Compare February 6, 2024 04:34
@alinush alinush force-pushed the alin/zkid.move branch 2 times, most recently from 2af01b6 to fd99a67 Compare February 6, 2024 04:42
Copy link

codecov bot commented Feb 6, 2024

Codecov Report

Attention: 267 lines in your changes are missing coverage. Please review.

Comparison is base (6371800) 69.8% compared to head (4c052a5) 71.2%.
Report is 2 commits behind head on main.

❗ Current head 4c052a5 differs from pull request most recent head 9fbb893. Consider uploading reports for the commit 9fbb893 to get more accurate results

Files Patch % Lines
types/src/bn254_circom.rs 0.0% 109 Missing ⚠️
aptos-move/aptos-vm/src/zkid_validation.rs 0.0% 75 Missing ⚠️
types/src/zkid.rs 0.0% 69 Missing ⚠️
types/src/jwks/rsa/mod.rs 0.0% 11 Missing ⚠️
types/src/transaction/authenticator.rs 0.0% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main   #11895       +/-   ##
===========================================
+ Coverage    69.8%    71.2%     +1.3%     
===========================================
  Files        2199      796     -1403     
  Lines      416774   183369   -233405     
===========================================
- Hits       291116   130613   -160503     
+ Misses     125658    52756    -72902     

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

@alinush alinush enabled auto-merge (squash) February 7, 2024 23:09

This comment has been minimized.

This comment has been minimized.

@alinush alinush disabled auto-merge February 7, 2024 23:32
@alinush alinush enabled auto-merge (squash) February 8, 2024 02:04

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Feb 8, 2024

✅ Forge suite compat success on aptos-node-v1.8.3 ==> 9fbb893fc5f9a136ca7a80efae566f8907f80216

Compatibility test results for aptos-node-v1.8.3 ==> 9fbb893fc5f9a136ca7a80efae566f8907f80216 (PR)
1. Check liveness of validators at old version: aptos-node-v1.8.3
compatibility::simple-validator-upgrade::liveness-check : committed: 3249 txn/s, latency: 6627 ms, (p50: 5400 ms, p90: 9300 ms, p99: 24900 ms), latency samples: 201460
2. Upgrading first Validator to new version: 9fbb893fc5f9a136ca7a80efae566f8907f80216
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1791 txn/s, latency: 16424 ms, (p50: 19000 ms, p90: 22400 ms, p99: 22600 ms), latency samples: 93180
3. Upgrading rest of first batch to new version: 9fbb893fc5f9a136ca7a80efae566f8907f80216
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 1280 txn/s, latency: 21456 ms, (p50: 19900 ms, p90: 28900 ms, p99: 30000 ms), latency samples: 64020
4. upgrading second batch to new version: 9fbb893fc5f9a136ca7a80efae566f8907f80216
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 3374 txn/s, latency: 9458 ms, (p50: 9900 ms, p90: 14600 ms, p99: 15700 ms), latency samples: 134980
5. check swarm health
Compatibility test for aptos-node-v1.8.3 ==> 9fbb893fc5f9a136ca7a80efae566f8907f80216 passed
Test Ok

Copy link
Contributor

github-actions bot commented Feb 8, 2024

✅ Forge suite realistic_env_max_load success on 9fbb893fc5f9a136ca7a80efae566f8907f80216

two traffics test: inner traffic : committed: 7729 txn/s, latency: 4937 ms, (p50: 4500 ms, p90: 6200 ms, p99: 10500 ms), latency samples: 3339260
two traffics test : committed: 100 txn/s, latency: 2108 ms, (p50: 2100 ms, p90: 2400 ms, p99: 3900 ms), latency samples: 1840
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.241, avg: 0.200", "QsPosToProposal: max: 0.220, avg: 0.149", "ConsensusProposalToOrdered: max: 0.556, avg: 0.524", "ConsensusOrderedToCommit: max: 0.463, avg: 0.442", "ConsensusProposalToCommit: max: 0.998, avg: 0.966"]
Max round gap was 1 [limit 4] at version 1311964. Max no progress secs was 4.611399 [limit 15] at version 1311964.
Test Ok

@alinush alinush merged commit 2d7288f into main Feb 8, 2024
43 checks passed
@alinush alinush deleted the alin/zkid.move branch February 8, 2024 05:27
alexfilip2 pushed a commit to bwarelabs/aptos-core that referenced this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants