Enable the max object nesting check #12129
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
junkil-park
requested review from
davidiw,
movekevin and
wrwg
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #12129 +/- ##
=========================================
- Coverage 70.3% 70.3% -0.1%
=========================================
Files 850 850
Lines 189122 189123 +1
=========================================
- Hits 132976 132971 -5
- Misses 56146 56152 +6 ☔ View full report in Codecov by Sentry. |
davidiw
reviewed
Feb 21, 2024
There was a problem hiding this comment.
thanks for getting to this so quickly! do we really need the control flag here? It seems like it would be just as much effort to update the package to disable the feature? this is also in the critical path of object transfers, right? do we want this extra lookup each time?
gedigi
approved these changes
Feb 21, 2024
junkil-park
force-pushed
the
jpark/max-object-nesting-check
branch
from
ac550f0 to
f7ef4fd
Compare
junkil-park
added
CICD:run-e2e-tests
junkil-park
force-pushed
the
jpark/max-object-nesting-check
branch
from
f7ef4fd to
0401506
Compare
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.
junkil-park
force-pushed
the
jpark/max-object-nesting-check
branch
from
0401506 to
4c33417
Compare
junkil-park
added
CICD:run-framework-upgrade-test
and removed
CICD:run-framework-upgrade-test
labels
I made it feature-gated for safety reasons during our network upgrade process. I will make sure to clean up the feature gate in the next release to remove the lookup overhead in the path. |
This comment has been minimized.
This comment has been minimized.
junkil-park
force-pushed
the
jpark/max-object-nesting-check
branch
from
4c33417 to
6443eb5
Compare
junkil-park
added
CICD:run-framework-upgrade-test
and removed
CICD:run-framework-upgrade-test
labels
junkil-park
force-pushed
the
jpark/max-object-nesting-check
branch
from
6443eb5 to
9a13308
Compare
movekevin
approved these changes
Feb 22, 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.
✅ Forge suite
|
✅ Forge suite
|
❌ Forge suite
|
junkil-park
added a commit
that referenced
this pull request
Feb 22, 2024
junkil-park
added a commit
that referenced
this pull request
Feb 22, 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR enables the max object nesting check
Test Plan
aptos move test