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

[move] rotate_authentication_key_call should not modify OriginatingAddress #12108

Merged
merged 2 commits into from
Feb 21, 2024

Conversation

davidiw
Copy link
Contributor

@davidiw davidiw commented Feb 20, 2024

because the auth key isn't verified, the OriginatingAddress table can be manipulated with false entries, therefore eliminate the ability for the unverified auth key rotations from manipulating it.

Description

Test Plan

@davidiw davidiw requested a review from gregnazario February 20, 2024 06:51
Copy link

codecov bot commented Feb 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (67f372a) 71.4% compared to head (ab6e51a) 71.4%.

Additional details and impacted files
@@            Coverage Diff            @@
##             main   #12108     +/-   ##
=========================================
- Coverage    71.4%    71.4%   -0.1%     
=========================================
  Files         810      810             
  Lines      184821   184821             
=========================================
- Hits       132070   132037     -33     
- Misses      52751    52784     +33     

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

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.

Fixed your lints & comments too

davidiw and others added 2 commits February 20, 2024 17:07
…dress

because the auth key isn't verified, the OriginatingAddress table can be
manipulated with false entries, therefore eliminate the ability for the
unverified auth key rotations from manipulating it.
@alinush alinush enabled auto-merge (squash) February 21, 2024 01:08

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on ab6e51a431c278bb904a699fd450c50755e53c8e

two traffics test: inner traffic : committed: 7384 txn/s, latency: 5309 ms, (p50: 5100 ms, p90: 6300 ms, p99: 13200 ms), latency samples: 3189900
two traffics test : committed: 100 txn/s, latency: 2323 ms, (p50: 2100 ms, p90: 2600 ms, p99: 9000 ms), latency samples: 1780
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.223, avg: 0.202", "QsPosToProposal: max: 0.400, avg: 0.343", "ConsensusProposalToOrdered: max: 0.563, avg: 0.517", "ConsensusOrderedToCommit: max: 0.495, avg: 0.473", "ConsensusProposalToCommit: max: 1.020, avg: 0.990"]
Max round gap was 1 [limit 4] at version 1480618. Max no progress secs was 7.183143 [limit 15] at version 1480618.
Test Ok

Copy link
Contributor

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

Compatibility test results for aptos-node-v1.9.5 ==> ab6e51a431c278bb904a699fd450c50755e53c8e (PR)
1. Check liveness of validators at old version: aptos-node-v1.9.5
compatibility::simple-validator-upgrade::liveness-check : committed: 6213 txn/s, latency: 5320 ms, (p50: 4800 ms, p90: 8400 ms, p99: 15800 ms), latency samples: 217460
2. Upgrading first Validator to new version: ab6e51a431c278bb904a699fd450c50755e53c8e
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 593 txn/s, submitted: 664 txn/s, expired: 70 txn/s, latency: 35712 ms, (p50: 41600 ms, p90: 56600 ms, p99: 60400 ms), latency samples: 49839
3. Upgrading rest of first batch to new version: ab6e51a431c278bb904a699fd450c50755e53c8e
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 335 txn/s, submitted: 600 txn/s, expired: 264 txn/s, latency: 32132 ms, (p50: 34100 ms, p90: 57400 ms, p99: 61800 ms), latency samples: 26810
4. upgrading second batch to new version: ab6e51a431c278bb904a699fd450c50755e53c8e
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 2108 txn/s, latency: 12893 ms, (p50: 14300 ms, p90: 15400 ms, p99: 19600 ms), latency samples: 103320
5. check swarm health
Compatibility test for aptos-node-v1.9.5 ==> ab6e51a431c278bb904a699fd450c50755e53c8e passed
Test Ok

@alinush alinush merged commit d771cec into main Feb 21, 2024
76 of 83 checks passed
@alinush alinush deleted the david-rotate branch February 21, 2024 01:44
alinush added a commit that referenced this pull request Feb 21, 2024
sherry-x pushed a commit that referenced this pull request Feb 21, 2024
zjma added a commit that referenced this pull request Feb 22, 2024
* Fix `iss`-related bug in Groth16 path & refactor (#12017)

Co-authored-by: Oliver <[email protected]>

* [aptosvm] Simplify VM flows (#11888)

* Duplicated logic for creating the gas meter for view functions has been removed.
* Duplicated logic for calculating gas used for view functions has been removed.
* There was unreachable code in failure transaction cleanup, where the discarded
status has been returned immediately, but then re-checked again. The first check
is shifted inside.
* No more default transaction metadata.
* Scripts are now validated consistently.
* Simplifies transaction execution function signature to avoid `Option<String>`.
* Removes duplicated features from `AptosVM` and keeps them in `MoveVMExt`.
* Fixes a bug when script hash was not computed for `RunOnAbort`.

Related tests are moved  to `move-e2e-tests`.

* [Compiler V2] Critical edge elimination (#11894)

Implement a pass to eliminate critical edges by splitting them with empty blocks

* [consensus configs] reduce sending block size from 2500 to 1900 (#12091)

### Description

The block output limit is no longer hit with p2p txns.

### Test Plan

Forge `realistic_env_max_load` TPS improves.

* [Indexer-grpc] Add profiling support. (#12034)

* Minor aggregator cleanup (#12013)

* Minor aggregator cleanup

* Addressing PR comments

* [move] rotate_authentication_key_call should not modify OriginatingAddress (#12108)

Co-authored-by: Alin Tomescu <[email protected]>

* [Data Streaming Service] Add dynamic prefetching support

* [Data Streaming Service] Add dynamic prefetching unit tests.

* [Data Streaming Service] Update existing integration tests.

* [State Sync] Add backpressure to fast sync receiver.

* Update perf baseline for gas charging coverage improvements (reducing throughput) (#12124)

* Reduce latency of cloning network sender using Arc pointers (#12103)

* Avoid cloning network sender using Arc pointers

* Removing a clone

* 100 node sweep test

* Removing a few clone operations

* reset forge test

* Removing some clones

* Removing clones

* adopt AIP-61 terminology for consistency (#12123)

adopt AIP-61 terminology for consistency

* [Consensus] Remove non-decoupled execution and refactor for cleaner interfaces (#12104)

* fix jwk key logging (#12090)

* remove spurious error lines (#12137)

* randomness #1: types update from randomnet (#12106)

* types update from randomnet

* update

* lint

* lint

* All validators broadcast commit vote messages (#12059)

* All validators broadcast commit messages

* Forge testing

* Increase timeout for forge

* test forge realistic_env_workload_sweep_test

* run realistic_env_workload_sweep_test

* run realistic_env_workload_sweep_test

* run sweep test

* increase forge runner duration

* forge testing

* Letting the proposer also broadcast commit decision for backward compatibility

* removing forge changes

* Added a TODO

* [vm] Resource access control: runtime engine (#10544)

* [vm] Resource access control: runtime engine

Implements the runtime engine for resource access control:

- a representation of access control specifiers in `loaded_data::runtime_access_specifiers`.
- a loader for access specifiers in `runtime::loader::access_specifier_loader`.
- a new stateful object representing the access control logic in `runtime::access_control`.
- finally the use of the `AccessControlState` in `runtime::interpreter`.

* Addressing reviewer comments.

* Addressing reviewer comments.

* typo: PTLA -> PTAL

* Rebasing: adjusting to upstream changes

* Rebasing

* ObjectCodeDeployment API cleanup update (#12133)

* ObjectCodeDeployment API cleanup update (#12141)

* [Compiler-v2] porting more V1 unit tests to V2 (#12085)

* update tests

* fix bug

* fix-12116

* fix missing space

* add expected got

* remove live-var tests

* fix had_erros

* fix

* Enable the max object nesting check (#12129)

* Resolved the warning for unused variable (#12157)

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* update

* Squashed commit of the following:

commit a50ffec
Author: Zhoujun Ma <[email protected]>
Date:   Thu Feb 22 21:10:12 2024 +0000

    lint

commit 388350f
Author: zhoujun.ma <[email protected]>
Date:   Thu Feb 22 13:04:28 2024 -0800

    update

commit 76f7eca
Author: zhoujun.ma <[email protected]>
Date:   Thu Feb 22 12:56:04 2024 -0800

    update

commit a663542
Author: zhoujun.ma <[email protected]>
Date:   Thu Feb 22 12:54:18 2024 -0800

    update

commit b439449
Author: zhoujun.ma <[email protected]>
Date:   Thu Feb 22 12:34:14 2024 -0800

    update

commit 3378ceb
Author: zhoujun.ma <[email protected]>
Date:   Thu Feb 22 12:17:06 2024 -0800

    update

commit 6cd6685
Author: zhoujun.ma <[email protected]>
Date:   Thu Feb 22 12:15:05 2024 -0800

    update

commit 6d89f37
Author: zhoujun.ma <[email protected]>
Date:   Thu Feb 22 12:13:51 2024 -0800

    update

commit 980f257
Author: zhoujun.ma <[email protected]>
Date:   Thu Feb 22 12:12:04 2024 -0800

    update

commit 16e9349
Author: Zhoujun Ma <[email protected]>
Date:   Thu Feb 22 18:25:08 2024 +0000

    lint

---------

Co-authored-by: Alin Tomescu <[email protected]>
Co-authored-by: Oliver <[email protected]>
Co-authored-by: George Mitenkov <[email protected]>
Co-authored-by: Zekun Wang <[email protected]>
Co-authored-by: Brian (Sunghoon) Cho <[email protected]>
Co-authored-by: Guoteng Rao <[email protected]>
Co-authored-by: Satya Vusirikala <[email protected]>
Co-authored-by: David Wolinsky <[email protected]>
Co-authored-by: Josh Lind <[email protected]>
Co-authored-by: igor-aptos <[email protected]>
Co-authored-by: Sital Kedia <[email protected]>
Co-authored-by: Wolfgang Grieskamp <[email protected]>
Co-authored-by: Teng Zhang <[email protected]>
Co-authored-by: Junkil Park <[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