Update transaction_metadata.rs #15215

junkil-park · 2024-11-06T19:12:04Z

Added a comment to explain fee_payer_authentication_key in transaction_metadat.rs

Type of Change

trunk-io · 2024-11-06T19:12:08Z

⏱️ 1h 30m total CI duration on this PR

Slowest 15 Jobs	Cumulative Duration	Recent Runs
execution-performance / single-node-performance	23m	🟩
rust-move-tests	10m	🟩
rust-move-tests	10m	🟩
rust-move-tests	9m	🟩
rust-cargo-deny	7m	🟩 🟩 🟩 🟩
rust-doc-tests	5m	🟩
rust-move-tests	5m	🟥
test-target-determinator	4m	🟩
execution-performance / test-target-determinator	4m	🟩
check-dynamic-deps	4m	🟩 🟩 🟩 🟩
check	4m	🟩
general-lints	2m	🟩 🟩 🟩 🟩
fetch-last-released-docker-image-tag	2m	🟩
semgrep/ci	1m	🟩 🟩 🟩 🟩
file_change_determinator	55s	🟩 🟩 🟩 🟩

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

georgemitenkov · 2024-11-06T19:29:13Z

aptos-move/aptos-vm/src/transaction_metadata.rs

@@ -55,6 +55,9 @@ impl TransactionMetadata {
                .collect(),
            sequence_number: txn.sequence_number(),
            fee_payer: txn.authenticator_ref().fee_payer_address(),
+            // `fee_payer_authentication_key` is `None` if the TransactionAuthenticator
+            // lacks an authenticator for the fee payer. It is `Some([])` if the
+            // authenticator for the fee payer is a NoAccountAuthenticator.
            fee_payer_authentication_key: txn.authenticator().fee_payer_signer().map(|signer| {


Can we have this as a doc string /// None if ..., ... for the field in TransactionMetadata, rather then somewhere in the constructor?. Also let's add a unit test that checks this invariant

Also, you can use [TransactionAuthenticator] to make structs searchable in IDE + when refactoring and checking usages (works with ///). Otherwise we change names, but the comments stay stale :(

Added a comment to explain `fee_payer_authentication_key` in transaction_metadat.rs

github-actions · 2024-11-07T21:05:44Z

✅ Forge suite `realistic_env_max_load` success on `e1fd0f129dd7257aff21e2f735094ee5c84be081`

two traffics test: inner traffic : committed: 14644.23 txn/s, latency: 2713.62 ms, (p50: 2700 ms, p70: 2700, p90: 2700 ms, p99: 3000 ms), latency samples: 5568120
two traffics test : committed: 99.95 txn/s, latency: 1540.18 ms, (p50: 1400 ms, p70: 1400, p90: 1600 ms, p99: 10900 ms), latency samples: 1740
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 1.983, avg: 1.528", "ConsensusProposalToOrdered: max: 0.312, avg: 0.290", "ConsensusOrderedToCommit: max: 0.360, avg: 0.350", "ConsensusProposalToCommit: max: 0.649, avg: 0.640"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.88s no progress at version 2756676 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.51s no progress at version 2756674 (avg 8.51s) [limit 15].
Test Ok

github-actions · 2024-11-07T21:07:33Z

✅ Forge suite `framework_upgrade` success on `1086a5e00d773704731ab84fb4ed3538613b2250` ==> `e1fd0f129dd7257aff21e2f735094ee5c84be081`

Compatibility test results for 1086a5e00d773704731ab84fb4ed3538613b2250 ==> e1fd0f129dd7257aff21e2f735094ee5c84be081 (PR)
Upgrade the nodes to version: e1fd0f129dd7257aff21e2f735094ee5c84be081
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1306.22 txn/s, submitted: 1309.38 txn/s, failed submission: 3.16 txn/s, expired: 3.16 txn/s, latency: 2318.27 ms, (p50: 2100 ms, p70: 2400, p90: 3400 ms, p99: 4600 ms), latency samples: 115740
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1366.38 txn/s, submitted: 1368.09 txn/s, failed submission: 1.71 txn/s, expired: 1.71 txn/s, latency: 2416.04 ms, (p50: 2100 ms, p70: 2700, p90: 3900 ms, p99: 5400 ms), latency samples: 112080
5. check swarm health
Compatibility test for 1086a5e00d773704731ab84fb4ed3538613b2250 ==> e1fd0f129dd7257aff21e2f735094ee5c84be081 passed
Upgrade the remaining nodes to version: e1fd0f129dd7257aff21e2f735094ee5c84be081
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1282.96 txn/s, submitted: 1287.04 txn/s, failed submission: 4.08 txn/s, expired: 4.08 txn/s, latency: 2310.69 ms, (p50: 2000 ms, p70: 2400, p90: 4000 ms, p99: 5600 ms), latency samples: 113300
Test Ok

github-actions · 2024-11-07T21:08:59Z

✅ Forge suite `compat` success on `1086a5e00d773704731ab84fb4ed3538613b2250` ==> `e1fd0f129dd7257aff21e2f735094ee5c84be081`

Compatibility test results for 1086a5e00d773704731ab84fb4ed3538613b2250 ==> e1fd0f129dd7257aff21e2f735094ee5c84be081 (PR)
1. Check liveness of validators at old version: 1086a5e00d773704731ab84fb4ed3538613b2250
compatibility::simple-validator-upgrade::liveness-check : committed: 14188.11 txn/s, latency: 2319.96 ms, (p50: 2000 ms, p70: 2100, p90: 2500 ms, p99: 6700 ms), latency samples: 458660
2. Upgrading first Validator to new version: e1fd0f129dd7257aff21e2f735094ee5c84be081
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 6755.82 txn/s, latency: 4214.56 ms, (p50: 4800 ms, p70: 5100, p90: 5300 ms, p99: 5300 ms), latency samples: 122920
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 6787.23 txn/s, latency: 4762.22 ms, (p50: 5200 ms, p70: 5300, p90: 6100 ms, p99: 6600 ms), latency samples: 224420
3. Upgrading rest of first batch to new version: e1fd0f129dd7257aff21e2f735094ee5c84be081
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 5909.24 txn/s, latency: 4712.29 ms, (p50: 5200 ms, p70: 5400, p90: 6100 ms, p99: 6300 ms), latency samples: 116300
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6459.30 txn/s, latency: 5024.84 ms, (p50: 5300 ms, p70: 5500, p90: 6800 ms, p99: 7300 ms), latency samples: 215980
4. upgrading second batch to new version: e1fd0f129dd7257aff21e2f735094ee5c84be081
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 9562.93 txn/s, latency: 2922.80 ms, (p50: 3300 ms, p70: 3500, p90: 3600 ms, p99: 3700 ms), latency samples: 169360
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 8853.44 txn/s, latency: 3578.43 ms, (p50: 3700 ms, p70: 4000, p90: 5200 ms, p99: 5400 ms), latency samples: 286640
5. check swarm health
Compatibility test for 1086a5e00d773704731ab84fb4ed3538613b2250 ==> e1fd0f129dd7257aff21e2f735094ee5c84be081 passed
Test Ok

junkil-park requested review from davidiw, wrwg, zekun000, vgao1996 and georgemitenkov as code owners November 6, 2024 19:12

georgemitenkov reviewed Nov 6, 2024

View reviewed changes

junkil-park force-pushed the jpark/comment_fee_payer_authentication_key branch from f6483f4 to f95bef9 Compare November 6, 2024 19:57

georgemitenkov approved these changes Nov 6, 2024

View reviewed changes

Update transaction_metadata.rs

e1fd0f1

Added a comment to explain `fee_payer_authentication_key` in transaction_metadat.rs

junkil-park force-pushed the jpark/comment_fee_payer_authentication_key branch from f95bef9 to e1fd0f1 Compare November 7, 2024 17:09

junkil-park requested a review from JohnChangUK November 7, 2024 17:10

junkil-park enabled auto-merge (squash) November 7, 2024 20:37

This comment has been minimized.

Sign in to view

rahxephon89 approved these changes Nov 7, 2024

View reviewed changes

junkil-park merged commit a2c4706 into main Nov 7, 2024
92 checks passed

junkil-park deleted the jpark/comment_fee_payer_authentication_key branch November 7, 2024 21:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update transaction_metadata.rs #15215

Update transaction_metadata.rs #15215

junkil-park commented Nov 6, 2024

trunk-io bot commented Nov 6, 2024 •

edited

Loading

georgemitenkov Nov 6, 2024 •

edited

Loading

georgemitenkov Nov 6, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Nov 7, 2024

github-actions bot commented Nov 7, 2024

github-actions bot commented Nov 7, 2024

Update transaction_metadata.rs #15215

Update transaction_metadata.rs #15215

Conversation

junkil-park commented Nov 6, 2024

Type of Change

trunk-io bot commented Nov 6, 2024 • edited Loading

georgemitenkov Nov 6, 2024 • edited Loading

Choose a reason for hiding this comment

georgemitenkov Nov 6, 2024 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Nov 7, 2024

✅ Forge suite realistic_env_max_load success on e1fd0f129dd7257aff21e2f735094ee5c84be081

github-actions bot commented Nov 7, 2024

✅ Forge suite framework_upgrade success on 1086a5e00d773704731ab84fb4ed3538613b2250 ==> e1fd0f129dd7257aff21e2f735094ee5c84be081

github-actions bot commented Nov 7, 2024

✅ Forge suite compat success on 1086a5e00d773704731ab84fb4ed3538613b2250 ==> e1fd0f129dd7257aff21e2f735094ee5c84be081

trunk-io bot commented Nov 6, 2024 •

edited

Loading

georgemitenkov Nov 6, 2024 •

edited

Loading

georgemitenkov Nov 6, 2024 •

edited

Loading

✅ Forge suite `realistic_env_max_load` success on `e1fd0f129dd7257aff21e2f735094ee5c84be081`

✅ Forge suite `framework_upgrade` success on `1086a5e00d773704731ab84fb4ed3538613b2250` ==> `e1fd0f129dd7257aff21e2f735094ee5c84be081`

✅ Forge suite `compat` success on `1086a5e00d773704731ab84fb4ed3538613b2250` ==> `e1fd0f129dd7257aff21e2f735094ee5c84be081`