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

Fix serde issue #93

Merged
merged 9 commits into from Mar 21, 2024
Merged

Fix serde issue #93

merged 9 commits into from Mar 21, 2024

Conversation

survived
Copy link
Contributor

Turns out that #[serde(flatten)] is buggy, and, specifically, it does not preserve is_human_readable flag, which broke (de)serialization code and compatibility with old key shares.

See the issue for more details on the serde problem: serde-rs/serde#2704

PR adds a workaround that manually reimplements flatten functionality.

Copy link

github-actions bot commented Mar 21, 2024

The spec was successfully compiled. PDF is available here.

Copy link

github-actions bot commented Mar 21, 2024

Benchmark Result

Benchmarks
RUST_TESTS_SEED=f5c49763d5ba00c0a8d24d436fd248e9bad2718953ce0c3ea64f8a51a2a19cbb
n = 3

Non-threshold DKG
Protocol Performance:
  - Protocol took 439.33µs to complete
In particular:
  - Stage: 9.04µs
    - Setup networking: 8.78µs (97.1%)
    - Unstaged: 260.00ns (2.9%)
  - Round 1: 149.46µs
    - Compute execution id: 4.25µs (2.8%)
    - Sample x_i, rid_i, chain_code: 65.19µs (43.6%)
    - Sample schnorr commitment: 59.48µs (39.8%)
    - Commit to public data: 20.23µs (13.5%)
    - Unstaged: 310.00ns (0.2%)
  - Round 2: 5.49µs
    - Hash received msgs (reliability check): 5.27µs (96.0%)
    - Unstaged: 220.00ns (4.0%)
  - Round 3: 301.00ns
    - Assert other parties hashed messages (reliability check): 170.00ns (56.5%)
    - Unstaged: 131.00ns (43.5%)
  - Round 4: 36.29µs
    - Validate decommitments: 30.45µs (83.9%)
    - Calculate chain_code: 722.00ns (2.0%)
    - Calculate challege rid: 4.71µs (13.0%)
    - Prove knowledge of `x_i`: 230.00ns (0.6%)
    - Unstaged: 179.00ns (0.5%)
  - Round 5: 238.76µs
    - Validate schnorr proofs: 238.47µs (99.9%)
    - Unstaged: 281.00ns (0.1%)


Threshold DKG
Protocol Performance:
  - Protocol took 1.25ms to complete
In particular:
  - Stage: 1.94µs
    - Setup networking: 1.87µs (96.4%)
    - Unstaged: 70.00ns (3.6%)
  - Round 1: 203.55µs
    - Compute execution id: 1.32µs (0.6%)
    - Sample rid_i, schnorr commitment, polynomial, chain_code: 179.25µs (88.1%)
    - Commit to public data: 22.74µs (11.2%)
    - Unstaged: 230.00ns (0.1%)
  - Round 2: 1.09µs
    - Hash received msgs (reliability check): 922.00ns (84.4%)
    - Unstaged: 170.00ns (15.6%)
  - Round 3: 330.00ns
    - Assert other parties hashed messages (reliability check): 191.00ns (57.9%)
    - Unstaged: 139.00ns (42.1%)
  - Round 4: 778.93µs
    - Validate decommitments: 44.25µs (5.7%)
    - Validate data size: 201.00ns (0.0%)
    - Validate Feldmann VSS: 358.52µs (46.0%)
    - Compute rid: 170.00ns (0.0%)
    - Compute chain_code: 651.00ns (0.1%)
    - Compute Ys: 359.69µs (46.2%)
    - Compute sigma: 361.00ns (0.0%)
    - Calculate challenge: 14.66µs (1.9%)
    - Prove knowledge of `sigma_i`: 300.00ns (0.0%)
    - Unstaged: 131.00ns (0.0%)
  - Round 5: 267.34µs
    - Validate schnorr proofs: 265.83µs (99.4%)
    - Derive resulting public key and other data: 1.32µs (0.5%)
    - Unstaged: 190.00ns (0.1%)


Auxiliary data generation protocol
Protocol Performance:
  - Protocol took 9.52s to complete
In particular:
  - Stage: 12.48µs
    - Retrieve auxiliary data: 271.00ns (2.2%)
    - Setup networking: 11.07µs (88.7%)
    - Precompute execution id and shared state: 1.06µs (8.5%)
    - Unstaged: 80.00ns (0.6%)
  - Round 1: 1.16s
    - Retrieve primes (p and q): 70.00ns (0.0%)
    - Compute paillier decryption key (N): 4.60µs (0.0%)
    - Generate auxiliary params r, λ, t, s: 9.05ms (0.8%)
    - Prove Πprm (ψˆ_i): 1.15s (99.2%)
    - Sample random bytes: 2.38µs (0.0%)
    - Compute hash commitment and sample decommitment: 305.79µs (0.0%)
    - Unstaged: 251.00ns (0.0%)
  - Round 2: 1.36µs
    - Hash received msgs (reliability check): 1.01µs (74.3%)
    - Unstaged: 350.00ns (25.7%)
  - Round 3: 391.00ns
    - Assert other parties hashed messages (reliability check): 190.00ns (48.6%)
    - Unstaged: 201.00ns (51.4%)
  - Round 4: 5.88s
    - Validate round 1 decommitments: 609.35µs (0.0%)
    - Validate П_prm (ψ_i): 2.28s (38.7%)
    - Add together shared random bytes: 1.66µs (0.0%)
    - Compute П_mod (ψ_i): 3.43s (58.4%)
    - Assemble security params for П_fac (ф_i): 6.38µs (0.0%)
    - Compute П_fac (ф_i^j): 171.30ms (2.9%)
    - Unstaged: 1.06µs (0.0%)
  - Round 5: 2.48s
    - Validate ψ_j (П_mod): 2.31s (93.1%)
    - Validate ф_j (П_fac): 171.90ms (6.9%)
    - Assemble auxiliary info: 146.07µs (0.0%)
    - Unstaged: 2.20µs (0.0%)


Signing protocol
Protocol Performance:
  - Protocol took 4.16s to complete
In particular:
  - Stage: 142.53µs
    - Map t-out-of-n protocol to t-out-of-t: 70.53µs (49.5%)
    - Retrieve auxiliary data: 67.52µs (47.4%)
    - Precompute execution id and security params: 691.00ns (0.5%)
    - Setup networking: 3.70µs (2.6%)
    - Unstaged: 90.00ns (0.1%)
  - Round 1: 188.43ms
    - Generate local ephemeral secrets (k_i, y_i, p_i, v_i): 49.18µs (0.0%)
    - Encrypt G_i and K_i: 107.33ms (57.0%)
    - Prove ψ0_j: 81.04ms (43.0%)
    - Unstaged: 883.00ns (0.0%)
  - Round 2: 15.27µs
    - Hash received msgs (reliability check): 15.04µs (98.5%)
    - Unstaged: 230.00ns (1.5%)
  - Round 3: 2.23s
    - Assert other parties hashed messages (reliability check): 571.00ns (0.0%)
    - Verify psi0 proofs: 268.49ms (12.1%)
    - Sample random r, hat_r, s, hat_s, beta, hat_beta: 53.02µs (0.0%)
    - Encrypt D_ji: 70.18ms (3.2%)
    - Encrypt F_ji: 35.73ms (1.6%)
    - Encrypt hat_D_ji: 558.79ms (25.1%)
    - Encrypt hat_F_ji: 35.92ms (1.6%)
    - Prove psi_ji: 790.86ms (35.5%)
    - Prove psiˆ_ji: 208.49ms (9.4%)
    - Prove psi_prime_ji : 257.55ms (11.6%)
    - Unstaged: 1.75µs (0.0%)
  - Round 4: 1.56s
    - Retrieve auxiliary data: 5.60µs (0.0%)
    - Validate psi: 164.27ms (10.5%)
    - Validate hat_psi: 164.43ms (10.5%)
    - Validate psi_prime: 933.79ms (59.9%)
    - Compute Gamma, Delta_i, delta_i, chi_i: 215.12ms (13.8%)
    - Prove psi_prime_prime: 81.12ms (5.2%)
    - Unstaged: 381.00ns (0.0%)
  - Presig output: 185.08ms
    - Validate psi_prime_prime: 184.95ms (99.9%)
    - Calculate presignature: 134.23µs (0.1%)
    - Unstaged: 691.00ns (0.0%)
  - Partial signing: 9.78µs
  - Signature reconstruction: 201.41µs


key-share/src/serde_fix.rs Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Mar 21, 2024

Crate direct deps

Direct deps
cggmp21-keygen v0.1.0 (/home/runner/work/cggmp21/cggmp21/pr_branch/cggmp21-keygen)
digest v0.10.6
futures v0.3.24
generic-ec v0.2.2
generic-ec-zkp v0.2.0
hex v0.4.3
key-share v0.2.1 (/home/runner/work/cggmp21/cggmp21/pr_branch/key-share)
paillier-zk v0.2.0
rand_chacha v0.3.1
rand_core v0.6.4
round-based v0.2.0
serde v1.0.188
serde_with v2.3.3
sha2 v0.10.6
thiserror v1.0.48
udigest v0.1.0

Compared to base branch

Diff
--- direct-deps-base	2024-03-21 17:42:17.591613907 +0000
+++ direct-deps-pr	2024-03-21 17:42:18.047612059 +0000
@@ -1 +1 @@
-cggmp21-keygen v0.1.0 (/home/runner/work/cggmp21/cggmp21/base_branch/cggmp21-keygen)
+cggmp21-keygen v0.1.0 (/home/runner/work/cggmp21/cggmp21/pr_branch/cggmp21-keygen)
@@ -4 +4 @@
-generic-ec v0.2.0
+generic-ec v0.2.2
@@ -7 +7 @@
-key-share v0.2.1 (/home/runner/work/cggmp21/cggmp21/base_branch/key-share)
+key-share v0.2.1 (/home/runner/work/cggmp21/cggmp21/pr_branch/key-share)

All deps

cargo tree
cggmp21 v0.2.0 (/home/runner/work/cggmp21/cggmp21/pr_branch/cggmp21)
├── cggmp21-keygen v0.1.0 (/home/runner/work/cggmp21/cggmp21/pr_branch/cggmp21-keygen)
│   ├── digest v0.10.6
│   │   ├── block-buffer v0.10.3
│   │   │   └── generic-array v0.14.6
│   │   │       ├── serde v1.0.188
│   │   │       │   └── serde_derive v1.0.188 (proc-macro)
│   │   │       │       ├── proc-macro2 v1.0.66
│   │   │       │       │   └── unicode-ident v1.0.4
│   │   │       │       ├── quote v1.0.33
│   │   │       │       │   └── proc-macro2 v1.0.66 (*)
│   │   │       │       └── syn v2.0.32
│   │   │       │           ├── proc-macro2 v1.0.66 (*)
│   │   │       │           ├── quote v1.0.33 (*)
│   │   │       │           └── unicode-ident v1.0.4
│   │   │       └── typenum v1.15.0
│   │   └── crypto-common v0.1.6
│   │       ├── generic-array v0.14.6 (*)
│   │       └── typenum v1.15.0
│   ├── futures v0.3.24
│   │   ├── futures-channel v0.3.24
│   │   │   ├── futures-core v0.3.24
│   │   │   └── futures-sink v0.3.24
│   │   ├── futures-core v0.3.24
│   │   ├── futures-executor v0.3.24
│   │   │   ├── futures-core v0.3.24
│   │   │   ├── futures-task v0.3.24
│   │   │   └── futures-util v0.3.24
│   │   │       ├── futures-channel v0.3.24 (*)
│   │   │       ├── futures-core v0.3.24
│   │   │       ├── futures-io v0.3.24
│   │   │       ├── futures-macro v0.3.24 (proc-macro)
│   │   │       │   ├── proc-macro2 v1.0.66 (*)
│   │   │       │   ├── quote v1.0.33 (*)
│   │   │       │   └── syn v1.0.101
│   │   │       │       ├── proc-macro2 v1.0.66 (*)
│   │   │       │       ├── quote v1.0.33 (*)
│   │   │       │       └── unicode-ident v1.0.4
│   │   │       ├── futures-sink v0.3.24
│   │   │       ├── futures-task v0.3.24
│   │   │       ├── memchr v2.5.0
│   │   │       ├── pin-project-lite v0.2.9
│   │   │       ├── pin-utils v0.1.0
│   │   │       └── slab v0.4.7
│   │   ├── futures-io v0.3.24
│   │   ├── futures-sink v0.3.24
│   │   ├── futures-task v0.3.24
│   │   └── futures-util v0.3.24 (*)
│   ├── generic-ec v0.2.2
│   │   ├── generic-ec-core v0.1.3
│   │   │   ├── generic-array v0.14.6 (*)
│   │   │   ├── rand_core v0.6.4
│   │   │   ├── serde v1.0.188 (*)
│   │   │   ├── subtle v2.4.1
│   │   │   └── zeroize v1.6.0
│   │   │       └── zeroize_derive v1.3.2 (proc-macro)
│   │   │           ├── proc-macro2 v1.0.66 (*)
│   │   │           ├── quote v1.0.33 (*)
│   │   │           ├── syn v1.0.101 (*)
│   │   │           └── synstructure v0.12.6
│   │   │               ├── proc-macro2 v1.0.66 (*)
│   │   │               ├── quote v1.0.33 (*)
│   │   │               ├── syn v1.0.101 (*)
│   │   │               └── unicode-xid v0.2.4
│   │   ├── hex v0.4.3
│   │   │   └── serde v1.0.188 (*)
│   │   ├── phantom-type v0.4.2
│   │   │   └── educe v0.4.19 (proc-macro)
│   │   │       ├── enum-ordinalize v3.1.11 (proc-macro)
│   │   │       │   ├── num-bigint v0.4.3
│   │   │       │   │   ├── num-integer v0.1.45
│   │   │       │   │   │   └── num-traits v0.2.15
│   │   │       │   │   └── num-traits v0.2.15
│   │   │       │   ├── num-traits v0.2.15
│   │   │       │   ├── proc-macro2 v1.0.66 (*)
│   │   │       │   ├── quote v1.0.33 (*)
│   │   │       │   └── syn v1.0.101 (*)
│   │   │       ├── proc-macro2 v1.0.66 (*)
│   │   │       ├── quote v1.0.33 (*)
│   │   │       └── syn v1.0.101 (*)
│   │   ├── rand_core v0.6.4
│   │   ├── serde v1.0.188 (*)
│   │   ├── serde_with v2.3.3
│   │   │   ├── serde v1.0.188 (*)
│   │   │   └── serde_with_macros v2.3.3 (proc-macro)
│   │   │       ├── darling v0.20.1
│   │   │       │   ├── darling_core v0.20.1
│   │   │       │   │   ├── fnv v1.0.7
│   │   │       │   │   ├── ident_case v1.0.1
│   │   │       │   │   ├── proc-macro2 v1.0.66 (*)
│   │   │       │   │   ├── quote v1.0.33 (*)
│   │   │       │   │   ├── strsim v0.10.0
│   │   │       │   │   └── syn v2.0.32 (*)
│   │   │       │   └── darling_macro v0.20.1 (proc-macro)
│   │   │       │       ├── darling_core v0.20.1 (*)
│   │   │       │       ├── quote v1.0.33 (*)
│   │   │       │       └── syn v2.0.32 (*)
│   │   │       ├── proc-macro2 v1.0.66 (*)
│   │   │       ├── quote v1.0.33 (*)
│   │   │       └── syn v2.0.32 (*)
│   │   ├── subtle v2.4.1
│   │   ├── udigest v0.1.0
│   │   │   ├── digest v0.10.6 (*)
│   │   │   └── udigest-derive v0.1.0 (proc-macro)
│   │   │       ├── proc-macro2 v1.0.66 (*)
│   │   │       ├── quote v1.0.33 (*)
│   │   │       └── syn v2.0.32 (*)
│   │   └── zeroize v1.6.0 (*)
│   ├── generic-ec-zkp v0.2.0
│   │   ├── generic-array v0.14.6 (*)
│   │   ├── generic-ec v0.2.2 (*)
│   │   ├── rand_core v0.6.4
│   │   ├── serde v1.0.188 (*)
│   │   ├── subtle v2.4.1
│   │   └── udigest v0.1.0 (*)
│   ├── hex v0.4.3 (*)
│   ├── key-share v0.2.1 (/home/runner/work/cggmp21/cggmp21/pr_branch/key-share)
│   │   ├── generic-ec v0.2.2 (*)
│   │   ├── generic-ec-zkp v0.2.0 (*)
│   │   ├── hex v0.4.3 (*)
│   │   ├── serde v1.0.188 (*)
│   │   ├── serde_with v2.3.3 (*)
│   │   └── thiserror v1.0.48
│   │       └── thiserror-impl v1.0.48 (proc-macro)
│   │           ├── proc-macro2 v1.0.66 (*)
│   │           ├── quote v1.0.33 (*)
│   │           └── syn v2.0.32 (*)
│   ├── rand_core v0.6.4
│   ├── round-based v0.2.0
│   │   ├── futures-util v0.3.24 (*)
│   │   ├── phantom-type v0.3.1
│   │   │   └── educe v0.4.19 (proc-macro) (*)
│   │   ├── round-based-derive v0.2.0 (proc-macro)
│   │   │   ├── proc-macro2 v1.0.66 (*)
│   │   │   ├── quote v1.0.33 (*)
│   │   │   └── syn v1.0.101 (*)
│   │   ├── thiserror v1.0.48 (*)
│   │   └── tracing v0.1.36
│   │       ├── cfg-if v1.0.0
│   │       ├── pin-project-lite v0.2.9
│   │       ├── tracing-attributes v0.1.22 (proc-macro)
│   │       │   ├── proc-macro2 v1.0.66 (*)
│   │       │   ├── quote v1.0.33 (*)
│   │       │   └── syn v1.0.101 (*)
│   │       └── tracing-core v0.1.29
│   │           └── once_cell v1.17.0
│   ├── serde v1.0.188 (*)
│   ├── serde_with v2.3.3 (*)
│   ├── sha2 v0.10.6
│   │   ├── cfg-if v1.0.0
│   │   ├── cpufeatures v0.2.12
│   │   └── digest v0.10.6 (*)
│   ├── thiserror v1.0.48 (*)
│   └── udigest v0.1.0 (*)
├── digest v0.10.6 (*)
├── futures v0.3.24 (*)
├── generic-ec v0.2.2 (*)
├── generic-ec-zkp v0.2.0 (*)
├── hex v0.4.3 (*)
├── key-share v0.2.1 (/home/runner/work/cggmp21/cggmp21/pr_branch/key-share) (*)
├── paillier-zk v0.2.0
│   ├── digest v0.10.6 (*)
│   ├── fast-paillier v0.1.0
│   │   ├── bytemuck v1.13.1
│   │   │   └── bytemuck_derive v1.4.1 (proc-macro)
│   │   │       ├── proc-macro2 v1.0.66 (*)
│   │   │       ├── quote v1.0.33 (*)
│   │   │       └── syn v2.0.32 (*)
│   │   ├── rand_core v0.6.4
│   │   ├── rug v1.21.0
│   │   │   ├── az v1.2.1
│   │   │   ├── gmp-mpfr-sys v1.6.1
│   │   │   │   └── libc v0.2.153
│   │   │   ├── libc v0.2.153
│   │   │   └── serde v1.0.188 (*)
│   │   ├── serde v1.0.188 (*)
│   │   └── thiserror v1.0.48 (*)
│   ├── generic-ec v0.2.2 (*)
│   ├── rand_core v0.6.4
│   ├── rug v1.21.0 (*)
│   ├── serde v1.0.188 (*)
│   ├── serde_with v3.0.0
│   │   ├── serde v1.0.188 (*)
│   │   └── serde_with_macros v3.0.0 (proc-macro)
│   │       ├── darling v0.20.1 (*)
│   │       ├── proc-macro2 v1.0.66 (*)
│   │       ├── quote v1.0.33 (*)
│   │       └── syn v2.0.32 (*)
│   └── thiserror v1.0.48 (*)
├── rand_chacha v0.3.1
│   ├── ppv-lite86 v0.2.17
│   └── rand_core v0.6.4
├── rand_core v0.6.4
├── round-based v0.2.0 (*)
├── serde v1.0.188 (*)
├── serde_with v2.3.3 (*)
├── sha2 v0.10.6 (*)
├── thiserror v1.0.48 (*)
└── udigest v0.1.0 (*)

Compared to base branch

Diff
--- all-deps-base	2024-03-21 17:42:17.715613405 +0000
+++ all-deps-pr	2024-03-21 17:42:18.167611570 +0000
@@ -6 +6 @@
-cggmp21-keygen v0.1.0 (/home/runner/work/cggmp21/cggmp21/base_branch/cggmp21-keygen)
+cggmp21-keygen v0.1.0 (/home/runner/work/cggmp21/cggmp21/pr_branch/cggmp21-keygen)
@@ -27 +27 @@
-generic-ec v0.2.0
+generic-ec v0.2.2
@@ -33 +33 @@
-key-share v0.2.1 (/home/runner/work/cggmp21/cggmp21/base_branch/key-share)
+key-share v0.2.1 (/home/runner/work/cggmp21/cggmp21/pr_branch/key-share)

Cargo.toml Outdated Show resolved Hide resolved
@survived survived marked this pull request as ready for review March 21, 2024 16:27
@survived
Copy link
Contributor Author

survived commented Mar 21, 2024

Only in this PR I've noticed that we serialize scalar in vss_setup as not compact. It makes the key share bigger.

This is how compact things are serialized:

"public_shares": [
"02a802d82b77530ccd16666bd84c3af9dffa54dc74fec5542547a346adc5c2407f",
"031d89dfbc7ec55ddea07be07e35828ce0d1dee71fd3cd92589d0053c5594cb5f0",
"022072e56f9a68a5ba216eac857e34936db294c601e5de3fe5c97030dbd163e943",
"02ac7a29bd7d541483a87fd109dec86687342bfd56e630f2432c781863a82ac601",
"0247b3373b6157a78b4381503b3b1cf8389ff5d94c80451d6c2bd0d40e5770ede7"
],

this is how VSS setup is serialized:

"vss_setup": {
"min_signers": 3,
"I": [
{
"curve": "secp256k1",
"scalar": "0000000000000000000000000000000000000000000000000000000000000001"
},
{
"curve": "secp256k1",
"scalar": "0000000000000000000000000000000000000000000000000000000000000002"
},
{
"curve": "secp256k1",
"scalar": "0000000000000000000000000000000000000000000000000000000000000003"
},
{
"curve": "secp256k1",
"scalar": "0000000000000000000000000000000000000000000000000000000000000004"
},
{
"curve": "secp256k1",
"scalar": "0000000000000000000000000000000000000000000000000000000000000005"
}
]
},

We can compress VSS setup by adding generic_ec::serde::PreferCompact that deserializes both compressed / not compressed, but serializes only the compact variant. Wdyt?

@maurges
Copy link
Contributor

maurges commented Mar 21, 2024

We can compress VSS setup by adding generic_ec::serde::PreferCompact that deserializes both compressed / not compressed, but serializes only the compact variant. Wdyt?

We should. I think it's what it should do by default, try both options on deserializing and always serialize as compact. You want to do it in this MR as well?

@survived
Copy link
Contributor Author

Better to do this in a new PR. We gotta release key-share crate, all versions are yanked

@maurges
Copy link
Contributor

maurges commented Mar 21, 2024

Looks good, let's update generic-ec and go

@survived
Copy link
Contributor Author

@maurges updated the generic-ec

key-share/CHANGELOG.md Outdated Show resolved Hide resolved
@survived survived merged commit bd75503 into m Mar 21, 2024
12 of 14 checks passed
@survived survived deleted the fix-serde branch March 21, 2024 17:43
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.

None yet

2 participants