Add an entry function to update the federated jwkset #14579

heliuchuan · 2024-09-10T07:40:27Z

Description

Add an entry function to the framework that allows for updating an issuers JWK set.

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Key Areas to Review

Checklist

I have read and followed the CONTRIBUTING doc
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I identified and added all stakeholders and component owners affected by this change as reviewers
I tested both happy and unhappy path of the functionality
I have made corresponding changes to the documentation

trunk-io · 2024-09-10T07:40:30Z

⏱️ 10h 40m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
execution-performance / single-node-performance	1h 57m	🟩 🟥 🟩 🟥 🟩 (+1 more)
forge-framework-upgrade-test / forge	51m	⬜ 🟩 ⬜ 🟩
forge-compat-test / forge	49m	⬜ 🟩 ⬜ 🟩
forge-e2e-test / forge	48m	🟩 🟩 ⬜ 🟩
execution-performance / test-target-determinator	26m	🟩 🟩 🟩 🟩 🟩 (+1 more)
test-target-determinator	23m	🟩 🟩 🟩 ⬜ 🟩 (+1 more)
general-lints	22m	🟩 🟩 🟩 ⬜ 🟩 (+8 more)
rust-cargo-deny	21m	🟩 🟩 🟩 ⬜ 🟩 (+8 more)
check	21m	🟩 🟩 🟩 ⬜ 🟩 (+1 more)
check-dynamic-deps	18m	🟩 🟩 🟩 🟩 🟩 (+10 more)
rust-move-unit-coverage	15m	🟩
rust-move-unit-coverage	14m	🟩
rust-move-unit-coverage	14m	🟩
rust-move-unit-coverage	11m	🟩
rust-move-tests	10m	🟩
rust-move-tests	10m	🟩
rust-move-tests	10m	🟩
rust-move-unit-coverage	10m	🟩
rust-move-unit-coverage	9m	🟩
rust-move-unit-coverage	9m	🟩
rust-move-tests	9m	🟩
rust-move-unit-coverage	9m	🟩
rust-move-tests	9m	🟩
rust-move-tests	9m	🟩
rust-move-tests	9m	🟩
rust-move-tests	8m	🟩
rust-move-unit-coverage	8m	⬜
semgrep/ci	6m	🟩 🟩 🟩 🟩 🟩 (+10 more)
rust-move-unit-coverage	6m	⬜
rust-move-tests	6m	⬜
rust-move-unit-coverage	5m	⬜
rust-move-tests	5m	⬜
rust-doc-tests	5m	🟩
rust-doc-tests	4m	🟥
rust-doc-tests	4m	🟥
rust-move-tests	4m	🟥
rust-doc-tests	4m	🟥
rust-doc-tests	4m	🟥
file_change_determinator	3m	🟩 🟩 🟩 🟩 🟩 (+10 more)
file_change_determinator	3m	🟩 🟩 🟩 🟩 🟩 (+10 more)
rust-move-unit-coverage	1m	⬜
rust-move-tests	1m	⬜
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+1 more)
rust-doc-tests	1m	⬜
rust-move-tests	1m	⬜
rust-move-unit-coverage	52s	⬜
permission-check	47s	🟩 🟩 🟩 🟩 🟩 (+10 more)
permission-check	45s	🟩 🟩 🟩 🟩 🟩 (+10 more)
permission-check	42s	🟩 🟩 🟩 🟩 🟩 (+10 more)
permission-check	39s	🟩 🟩 🟩 🟩 🟩 (+10 more)
determine-docker-build-metadata	15s	🟩 🟩 🟩 🟩 🟩 (+1 more)
permission-check	13s	🟩 🟩 🟩 🟩 🟩 (+1 more)
Backport PR	4s	🟥
permission-check	2s	🟩
rust-move-tests	1s	⬜
rust-move-unit-coverage	1s	⬜
rust-move-tests	1s	⬜

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

codecov · 2024-09-10T07:51:06Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.8%. Comparing base (63a3dea) to head (6791062).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #14579   +/-   ##
=======================================
  Coverage    59.8%    59.8%           
=======================================
  Files         850      850           
  Lines      206876   206876           
=======================================
  Hits       123713   123713           
  Misses      83163    83163

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

aptos-move/framework/aptos-framework/sources/jwks.move

alinush

...but address @gregnazario's comments!

gregnazario

We actually are using the Any type?

I thought we had avoided that. Why not use an enum (or something that will look like an enum)?

I see the usage of this function, to build up all the internal types, just let's please ensure that we have documentation around all the different short names on the structs accordingly.

aptos-move/framework/aptos-framework/sources/jwks.move

gregnazario · 2024-09-11T20:54:11Z

aptos-move/framework/aptos-framework/sources/jwks.move

+    ///
+    /// The `iss` parameter is the value of the `iss` claim on the JWTs that are to be verified by the JWK set.
+    /// `kid_vec`, `alg_vec`, `e_vec`, `n_vec` are String vectors of the JWK attributes `kid`, `alg`, `e` and `n` respectively.
+    ///


I'm guessing this is all in the original documentation for patch_federated_jwks? Though I don't see it.

What is iss?

alg is the algorithm

kty is the key type

kid is key id?

e is some cryptographic e

n is some cryptographic n

Co-authored-by: Greg Nazario <[email protected]>

This reverts commit 64f5a3f.

github-actions · 2024-09-12T21:26:58Z

✅ Forge suite `realistic_env_max_load` success on `6791062b6023ea7f5c518a5878a4eafa6d613c20`

two traffics test: inner traffic : committed: 12680.92 txn/s, latency: 3140.76 ms, (p50: 2700 ms, p70: 3000, p90: 4200 ms, p99: 6300 ms), latency samples: 4821560
two traffics test : committed: 99.89 txn/s, latency: 2936.74 ms, (p50: 2600 ms, p70: 2900, p90: 4400 ms, p99: 5400 ms), latency samples: 1760
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.235, avg: 0.218", "QsPosToProposal: max: 0.228, avg: 0.183", "ConsensusProposalToOrdered: max: 0.322, avg: 0.295", "ConsensusOrderedToCommit: max: 0.490, avg: 0.459", "ConsensusProposalToCommit: max: 0.788, avg: 0.754"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.64s no progress at version 2676594 (avg 0.21s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 7.47s no progress at version 2676590 (avg 7.47s) [limit 15].
Test Ok

github-actions · 2024-09-12T21:28:07Z

✅ Forge suite `framework_upgrade` success on `d1bf834728a0cf166d993f4728dfca54f3086fb0` ==> `6791062b6023ea7f5c518a5878a4eafa6d613c20`

Compatibility test results for d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> 6791062b6023ea7f5c518a5878a4eafa6d613c20 (PR)
Upgrade the nodes to version: 6791062b6023ea7f5c518a5878a4eafa6d613c20
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1312.70 txn/s, submitted: 1316.10 txn/s, failed submission: 3.41 txn/s, expired: 3.41 txn/s, latency: 2411.26 ms, (p50: 2100 ms, p70: 2400, p90: 4200 ms, p99: 5700 ms), latency samples: 115640
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1176.29 txn/s, submitted: 1179.07 txn/s, failed submission: 2.77 txn/s, expired: 2.77 txn/s, latency: 2696.91 ms, (p50: 2400 ms, p70: 3000, p90: 4500 ms, p99: 6300 ms), latency samples: 101880
5. check swarm health
Compatibility test for d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> 6791062b6023ea7f5c518a5878a4eafa6d613c20 passed
Upgrade the remaining nodes to version: 6791062b6023ea7f5c518a5878a4eafa6d613c20
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1233.81 txn/s, submitted: 1235.87 txn/s, failed submission: 2.07 txn/s, expired: 2.07 txn/s, latency: 2492.81 ms, (p50: 2400 ms, p70: 2700, p90: 3700 ms, p99: 5400 ms), latency samples: 107540
Test Ok

github-actions · 2024-09-12T21:28:19Z

✅ Forge suite `compat` success on `d1bf834728a0cf166d993f4728dfca54f3086fb0` ==> `6791062b6023ea7f5c518a5878a4eafa6d613c20`

Compatibility test results for d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> 6791062b6023ea7f5c518a5878a4eafa6d613c20 (PR)
1. Check liveness of validators at old version: d1bf834728a0cf166d993f4728dfca54f3086fb0
compatibility::simple-validator-upgrade::liveness-check : committed: 9930.77 txn/s, latency: 3284.66 ms, (p50: 2700 ms, p70: 3500, p90: 5200 ms, p99: 7600 ms), latency samples: 342580
2. Upgrading first Validator to new version: 6791062b6023ea7f5c518a5878a4eafa6d613c20
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 7641.23 txn/s, latency: 3605.05 ms, (p50: 3900 ms, p70: 4200, p90: 4800 ms, p99: 4900 ms), latency samples: 140700
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 7768.06 txn/s, latency: 4071.98 ms, (p50: 4000 ms, p70: 4200, p90: 6300 ms, p99: 6500 ms), latency samples: 259220
3. Upgrading rest of first batch to new version: 6791062b6023ea7f5c518a5878a4eafa6d613c20
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 6903.33 txn/s, latency: 3984.19 ms, (p50: 4200 ms, p70: 4400, p90: 5400 ms, p99: 5500 ms), latency samples: 144780
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 7080.85 txn/s, latency: 4490.59 ms, (p50: 4700 ms, p70: 4900, p90: 6800 ms, p99: 7100 ms), latency samples: 238520
4. upgrading second batch to new version: 6791062b6023ea7f5c518a5878a4eafa6d613c20
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 11506.24 txn/s, latency: 2458.89 ms, (p50: 2600 ms, p70: 2800, p90: 3000 ms, p99: 3200 ms), latency samples: 204860
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 10648.47 txn/s, latency: 3060.42 ms, (p50: 2800 ms, p70: 3100, p90: 4700 ms, p99: 6800 ms), latency samples: 349040
5. check swarm health
Compatibility test for d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> 6791062b6023ea7f5c518a5878a4eafa6d613c20 passed
Test Ok

Add an entry function to update the federated jwkset

81b7098

heliuchuan requested review from zjma, davidiw, movekevin and wrwg as code owners September 10, 2024 07:40

alinush reviewed Sep 10, 2024

View reviewed changes

aptos-move/framework/aptos-framework/sources/jwks.move Outdated Show resolved Hide resolved

alinush reviewed Sep 10, 2024

View reviewed changes

aptos-move/framework/aptos-framework/sources/jwks.move Outdated Show resolved Hide resolved

alinush reviewed Sep 10, 2024

View reviewed changes

aptos-move/framework/aptos-framework/sources/jwks.move Outdated Show resolved Hide resolved

heliuchuan added 4 commits September 10, 2024 15:33

make it an entry func

c748259

update

fc866bd

Merge branch 'main' into jwkupdate

eb397dd

update md

936fd79

gregnazario reviewed Sep 11, 2024

View reviewed changes

aptos-move/framework/aptos-framework/sources/jwks.move Outdated Show resolved Hide resolved

aptos-move/framework/aptos-framework/sources/jwks.move Show resolved Hide resolved

gregnazario reviewed Sep 11, 2024

View reviewed changes

aptos-move/framework/aptos-framework/sources/jwks.move Show resolved Hide resolved

alinush approved these changes Sep 11, 2024

View reviewed changes

heliuchuan added 2 commits September 11, 2024 15:57

update func doc

19f1c79

Merge branch 'main' into jwkupdate

c82201d

heliuchuan requested a review from gregnazario September 11, 2024 19:59

gregnazario reviewed Sep 11, 2024

View reviewed changes

gregnazario approved these changes Sep 11, 2024

View reviewed changes

heliuchuan and others added 3 commits September 11, 2024 19:52

Update aptos-move/framework/aptos-framework/sources/jwks.move

36b7574

Co-authored-by: Greg Nazario <[email protected]>

Merge branch 'main' into jwkupdate

f0f7085

update docs

54a8942

heliuchuan enabled auto-merge (squash) September 12, 2024 18:53

This comment has been minimized.

Sign in to view

update

9861999

This comment has been minimized.

Sign in to view

heliuchuan added 2 commits September 12, 2024 16:32

add ignore to hopefully fix doc lint

64f5a3f

Revert "add ignore to hopefully fix doc lint"

6826463

This reverts commit 64f5a3f.

This comment has been minimized.

Sign in to view

heliuchuan added 2 commits September 12, 2024 16:56

fix docs

0eab776

Merge branch 'main' into jwkupdate

6791062

This comment has been minimized.

Sign in to view

heliuchuan merged commit a6b2ecc into main Sep 12, 2024
49 checks passed

heliuchuan deleted the jwkupdate branch September 12, 2024 21:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an entry function to update the federated jwkset #14579

Add an entry function to update the federated jwkset #14579

heliuchuan commented Sep 10, 2024 •

edited

Loading

trunk-io bot commented Sep 10, 2024 •

edited

Loading

codecov bot commented Sep 10, 2024 •

edited

Loading

alinush left a comment

gregnazario left a comment

gregnazario Sep 11, 2024

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.

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.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Sep 12, 2024

github-actions bot commented Sep 12, 2024

github-actions bot commented Sep 12, 2024

Add an entry function to update the federated jwkset #14579

Add an entry function to update the federated jwkset #14579

Conversation

heliuchuan commented Sep 10, 2024 • edited Loading

Description

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

Key Areas to Review

Checklist

trunk-io bot commented Sep 10, 2024 • edited Loading

codecov bot commented Sep 10, 2024 • edited Loading

Codecov Report

alinush left a comment

Choose a reason for hiding this comment

gregnazario left a comment

Choose a reason for hiding this comment

gregnazario Sep 11, 2024

Choose a reason for hiding this comment

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.

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.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Sep 12, 2024

✅ Forge suite realistic_env_max_load success on 6791062b6023ea7f5c518a5878a4eafa6d613c20

github-actions bot commented Sep 12, 2024

✅ Forge suite framework_upgrade success on d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> 6791062b6023ea7f5c518a5878a4eafa6d613c20

github-actions bot commented Sep 12, 2024

✅ Forge suite compat success on d1bf834728a0cf166d993f4728dfca54f3086fb0 ==> 6791062b6023ea7f5c518a5878a4eafa6d613c20

heliuchuan commented Sep 10, 2024 •

edited

Loading

trunk-io bot commented Sep 10, 2024 •

edited

Loading

codecov bot commented Sep 10, 2024 •

edited

Loading

✅ Forge suite `realistic_env_max_load` success on `6791062b6023ea7f5c518a5878a4eafa6d613c20`

✅ Forge suite `framework_upgrade` success on `d1bf834728a0cf166d993f4728dfca54f3086fb0` ==> `6791062b6023ea7f5c518a5878a4eafa6d613c20`

✅ Forge suite `compat` success on `d1bf834728a0cf166d993f4728dfca54f3086fb0` ==> `6791062b6023ea7f5c518a5878a4eafa6d613c20`