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

Add custom Serialize/Deserialize impl for EphemeralPublicKey, Pepper, Groth16ZkpAndStatement, G1Bytes, and G2Bytes, which output hex when used to serialize json #12295

Merged
merged 21 commits into from
Mar 1, 2024

Conversation

rex1fernando
Copy link
Contributor

@rex1fernando rex1fernando commented Feb 29, 2024

Description

This PR adds Serialize/Deserialize implementations for the following types:

  • EphemeralPublicKey
  • Pepper
  • Groth16ZkpAndStatement
  • G1Bytes
  • G2Bytes

instead of using the derive macros. The purpose of this is so that serde_json serializes these two types as hex strings, while bcs behavior is still preserved.

Test plan

This PR includes a new module aptos_types::unit_tests::keyless_serialization_test, which has test cases to test the new serialization/deserialization behavior for each of the types above.

Copy link

trunk-io bot commented Feb 29, 2024

⏱️ 16h 40m total CI duration on this PR
Job Cumulative Duration Recent Runs
windows-build 4h 5m 🟩🟩🟩🟩🟩 (+9 more)
rust-unit-tests 3h 18m 🟩🟩🟩🟩 (+6 more)
execution-performance / single-node-performance 1h 42m 🟥🟩🟩
run-tests-main-branch 1h 31m 🟥🟥🟥🟥🟥 (+9 more)
rust-smoke-tests 1h 28m 🟥🟥🟩
rust-lints 47m 🟩🟩🟩🟩 (+6 more)
forge-e2e-test / forge 41m 🟩🟩🟩
rust-images / rust-all 40m 🟩🟩🟩
forge-compat-test / forge 39m 🟩🟩🟩
check 28m 🟩🟩🟩 (+12 more)
check-dynamic-deps 27m 🟩🟩🟩🟩🟩 (+9 more)
general-lints 20m 🟩🟩🟩🟩 (+6 more)
cli-e2e-tests / run-cli-tests 17m 🟩🟩🟩
semgrep/ci 6m 🟩🟩🟩🟩🟩 (+9 more)
node-api-compatibility-tests / node-api-compatibility-tests 3m 🟩🟩🟩
file_change_determinator 2m 🟩🟩🟩🟩🟩 (+7 more)
file_change_determinator 2m 🟩🟩🟩🟩 (+8 more)
permission-check 56s 🟩🟩🟩🟩🟩 (+9 more)
permission-check 46s 🟩🟩🟩🟩🟩 (+9 more)
permission-check 44s 🟩🟩🟩🟩🟩 (+9 more)
permission-check 43s 🟩🟩🟩🟩🟩 (+9 more)
file_change_determinator 37s 🟩🟩🟩
execution-performance / file_change_determinator 25s 🟩🟩🟩
determine-docker-build-metadata 16s 🟩🟩🟩
permission-check 12s 🟩🟩🟩

settingsfeedbackdocs ⋅ learn more about trunk.io

Copy link
Contributor

@alinush alinush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be sure to run ./scripts/rust_lint.sh since, judging from the authenticator.rs changes, I think you might've linted with an older cargo or rustc...

…G1Bytes, and G2Bytes so that things serialize as hex when using a human-readable target format. Add tests for the above and for Pepper.
@rex1fernando rex1fernando changed the title Add custom Serialize/Deserialize impl for EphemeralPublicKey and Pepper which output hex when used to serialize json Add custom Serialize/Deserialize impl for EphemeralPublicKey, Pepper, Groth16ZkpAndStatement, G1Bytes, and G2Bytes, which output hex when used to serialize json Feb 29, 2024
@rex1fernando rex1fernando enabled auto-merge (squash) March 1, 2024 00:57

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@rex1fernando rex1fernando disabled auto-merge March 1, 2024 01:25
@rex1fernando rex1fernando enabled auto-merge (squash) March 1, 2024 02:50

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@rex1fernando rex1fernando disabled auto-merge March 1, 2024 03:56
@rex1fernando rex1fernando enabled auto-merge (squash) March 1, 2024 04:03

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Mar 1, 2024

✅ Forge suite compat success on aptos-node-v1.9.5 ==> 14a109eccfc254fb23e11cba997f564efd4bc834

Compatibility test results for aptos-node-v1.9.5 ==> 14a109eccfc254fb23e11cba997f564efd4bc834 (PR)
1. Check liveness of validators at old version: aptos-node-v1.9.5
compatibility::simple-validator-upgrade::liveness-check : committed: 5430 txn/s, latency: 5957 ms, (p50: 4900 ms, p90: 9600 ms, p99: 14600 ms), latency samples: 206360
2. Upgrading first Validator to new version: 14a109eccfc254fb23e11cba997f564efd4bc834
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 1137 txn/s, latency: 25005 ms, (p50: 26800 ms, p90: 35300 ms, p99: 36800 ms), latency samples: 56880
3. Upgrading rest of first batch to new version: 14a109eccfc254fb23e11cba997f564efd4bc834
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 457 txn/s, submitted: 681 txn/s, expired: 223 txn/s, latency: 25278 ms, (p50: 18300 ms, p90: 48300 ms, p99: 57200 ms), latency samples: 43486
4. upgrading second batch to new version: 14a109eccfc254fb23e11cba997f564efd4bc834
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 2142 txn/s, latency: 13203 ms, (p50: 13900 ms, p90: 16300 ms, p99: 18400 ms), latency samples: 102840
5. check swarm health
Compatibility test for aptos-node-v1.9.5 ==> 14a109eccfc254fb23e11cba997f564efd4bc834 passed
Test Ok

Copy link
Contributor

github-actions bot commented Mar 1, 2024

✅ Forge suite realistic_env_max_load success on 14a109eccfc254fb23e11cba997f564efd4bc834

two traffics test: inner traffic : committed: 7494 txn/s, latency: 5244 ms, (p50: 5000 ms, p90: 6300 ms, p99: 12500 ms), latency samples: 3229940
two traffics test : committed: 100 txn/s, latency: 1796 ms, (p50: 1700 ms, p90: 2000 ms, p99: 5200 ms), latency samples: 1680
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.255, avg: 0.207", "QsPosToProposal: max: 0.349, avg: 0.271", "ConsensusProposalToOrdered: max: 0.469, avg: 0.433", "ConsensusOrderedToCommit: max: 0.319, avg: 0.297", "ConsensusProposalToCommit: max: 0.750, avg: 0.730"]
Max round gap was 1 [limit 4] at version 1517828. Max no progress secs was 3.9664931 [limit 15] at version 1517828.
Test Ok

@rex1fernando rex1fernando merged commit bfbd4f6 into main Mar 1, 2024
74 of 80 checks passed
@rex1fernando rex1fernando deleted the rex/add_epk_and_pepper_deserialization_hex branch March 1, 2024 04:36
alinush pushed a commit that referenced this pull request Mar 4, 2024
… Groth16ZkpAndStatement, G1Bytes, and G2Bytes, which output hex when used to serialize json (#12295)

* Expose G1Bytes and G2Bytes

* Expose g{1,2}_projective_str_to_affine

* Expose g{1,2}_projective_str_to_affine

* Rename jwk method

* typo

* Add custom Serialize/Deserialize impl for EphemeralPublicKey and Pepper so that serde_json serializes as hex, while bcs still serializes normally

* rust fmt

* Lint fixes

* Add custom serialization/deserialization for Groth16ZkpAndStatement, G1Bytes, and G2Bytes so that things serialize as hex when using a human-readable target format. Add tests for the above and for Pepper.

* rust linter corrections

* Add copyright

* Remove to_hex and from_hex, and don't panic on unsuccessful deserialization. Add tests for G1Bytes and G2Bytes, and do length sanity checks during tests.

* remove from_hex and to_hex from EphemeralPublicKey
alinush added a commit that referenced this pull request Mar 4, 2024
* Add custom Serialize/Deserialize impl for EphemeralPublicKey, Pepper, Groth16ZkpAndStatement, G1Bytes, and G2Bytes, which output hex when used to serialize json (#12295)

* e2e Move tests for keyless + feature gating (#12296)

* add support for passkey-based EPKs & merge non-malleability signature into ephemeral signature (#12333)

Co-authored-by: Rex Fernando <[email protected]>
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.

3 participants