Bump/aptos node v1.27.0 rc #176
Conversation
WalkthroughThis update spans multiple components. In essence, dependency revisions have been applied in Cargo.toml, while several native, JSON, and compiler modules have been modified. Changes include updating dependency versions and Git revisions, renaming methods (e.g., from get_struct_type to get_struct_name), revising serialization/deserialization to use a new context‑based approach, removing version parameters from type signatures, and introducing a new function value extension adapter within the module storage layer. Together, the modifications refine method signatures, error handling, and internal data processing across the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant NativeTableCtx as NativeTableContext
participant FExtension as FunctionValueExtensionAdapter
Client->>NativeTableCtx: Call into_change_set(optional extension)
NativeTableCtx->>FExtension: Request function argument context
FExtension-->>NativeTableCtx: Provide function argument types
NativeTableCtx-->>Client: Return TableChangeSet
sequenceDiagram
participant User
participant Session as SessionExt
participant ModStorage as ModuleStorage
User->>Session: finish() / publish module
Session->>ModStorage: as_function_value_extension()
ModStorage-->>Session: Provide function extension adapter
Session->>Session: Resolve struct info via get_struct_name(&Type, module_storage)
Session-->>User: Return updated session/publish result
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libmovevm/src/move_api/bytecode.rs (1)
87-90: Track implementation of function signature token conversion.The placeholder for converting function signature tokens to API MoveType needs implementation.
Would you like me to create an issue to track the implementation of the function signature token conversion to API MoveType?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
Cargo.toml(2 hunks)crates/compiler/src/built_package.rs(1 hunks)crates/compiler/src/unit_test_factory.rs(1 hunks)crates/json/src/json_to_move.rs(16 hunks)crates/json/src/json_to_value.rs(3 hunks)crates/json/src/move_to_json.rs(13 hunks)crates/natives/src/account.rs(1 hunks)crates/natives/src/event.rs(3 hunks)crates/natives/src/from_bcs.rs(2 hunks)crates/natives/src/move_stdlib/bcs.rs(3 hunks)crates/natives/src/move_stdlib/unit_test.rs(1 hunks)crates/natives/src/string_utils.rs(1 hunks)crates/natives/src/table.rs(11 hunks)crates/storage/src/code_scale.rs(1 hunks)crates/storage/src/code_storage.rs(2 hunks)crates/storage/src/initia_storage.rs(1 hunks)crates/storage/src/module_cache.rs(2 hunks)crates/storage/src/module_storage.rs(5 hunks)crates/vm/src/publish.rs(1 hunks)crates/vm/src/session.rs(3 hunks)crates/vm/src/verifier/transaction_arg_validation.rs(2 hunks)libmovevm/src/move_api/bytecode.rs(1 hunks)libmovevm/src/move_api/move_types.rs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Rust libmovevm
- GitHub Check: rust lint
🔇 Additional comments (62)
crates/natives/src/table.rs (15)
13-13: Added import forFunctionValueExtensionandValueSerDeContext.No functional concerns; this import is required for the subsequent serialization/deserialization enhancements.
31-31: No substantive changes.This appears to be an added blank line with no functional impact.
123-126: Extendedinto_change_setsignature to includefunction_extensionparameter.This additional parameter allows for optional function-value-specific serialization. Ensure downstream callers provide the appropriate argument or
Noneas needed.
149-149: Addedfunction_extensionargument toserialize.These lines use the updated
serializefunction to incorporate optional function context. Implementation appears correct.Also applies to: 153-153
370-374: Serializing key with optional function extension innative_add_box.Passing
Some(context.function_value_extension())is consistent with the updated function signature. Looks good.
377-377: Invokingget_or_create_global_valueafter key serialization.No issues; correctly retrieves or creates the global value for the table entry.
415-419: Serializing key with optional function extension innative_borrow_box.Aligns with the same pattern as other native functions. Implementation looks consistent.
423-423: Obtaining global value reference post-serialization.No concerns; logic is coherent and mirrors the pattern in related functions.
461-465: Serializing key with optional function extension innative_contains_box.Implementation follows the established approach for table key serialization.
469-469: Retrieving global value reference fornative_contains_box.No new functionality concerns; consistent with the rest of the table lookups.
502-506: Serializing key with optional function extension innative_remove_box.Maintains the same serialization pattern. No issues found.
510-510: Retrieving global value reference innative_remove_box.Behavior is in line with other usage of this API. Looks fine.
736-751: Newserializefunction with optional function extension.Implementation is clear and properly handles the
Nonecase by defaulting to a context without function argument deserialization.
753-769: Newdeserializefunction with optional function extension.Supports function-argument-aware deserialization when provided. No concerns.
864-870: Usingdeserializewith optional function extension inload_table_entry.Consistently integrates the new function extension parameter. Implementation looks good.
crates/compiler/src/unit_test_factory.rs (1)
28-28: PassingNonetointo_change_set.This indicates no function-value-specific serialization is needed for these tests. The usage is correct and appears intentional.
crates/natives/src/move_stdlib/bcs.rs (3)
12-12: Necessary import for the new context-based serialization approach.
This addition enables more flexible handling of values during serialization.
72-75: Context-aware serialization is a clear improvement.
UsingValueSerDeContextwithwith_legacy_signerandwith_func_args_deserializationensures more robust handling of the value being serialized.
144-148: Consistent usage of delayed fields during serialization size calculation.
ApplyingValueSerDeContext::new().with_delayed_fields_serde()maintains a standardized approach across serialization.crates/json/src/json_to_move.rs (18)
1-1: New import needed for string parsing.
AddingFromStrsupports the parsing of numeric and address-like strings in subsequent code.
10-10: Introduced shorter, typed error handling.
ImportingPartialVMResultandVMResultaligns with more granular error reporting.
14-14: Identifier import supports naming resolution.
IncludingIdentifieris essential for retrieving fully qualified struct names.
22-22: Expanded Type enum usage.
Pulling inType::{self, *}facilitates the new type-centric approach across functions.
30-34: Clearer struct resolution method signature.
Changing toget_struct_name(&self, ty: &Type, ...) -> PartialVMResult<Option<(ModuleId, Identifier)>>makes the code more direct and type-driven.
137-144: Refined struct resolution logic for JSON deserialization.
Utilizingget_struct_nameand mapping errors withLocation::Undefinedcorrectly differentiates missing struct metadata from other failure cases.
216-229: Unified handling of struct instantiations.
Fetching the struct name and any type arguments with additional error checks ensures consistent resolution flow for generic structures.
686-704: Test helper function for struct creation.
Providing a utility that returns(Type, StructNameIndex, (ModuleId, Identifier))simplifies test setup and improves clarity.
712-718: Expanded test setup for string deserialization.
Demonstrates how a new struct index is inserted and tested, ensuring coverage for string scenarios.
739-764: Multi-struct test coverage.
Initializing mock states for multiple modules and struct names ensures robust testing of composite or interrelated types.
785-791: Validating struct instantiation of ‘Metadata’.
The test scenario checks correct resolution of generic struct arguments, improving type coverage.
820-827: Ensuring invalid struct instantiation is handled.
A negative test ensures the code correctly rejects mismatch with “Metadata2.”
848-854: Object address validation in tests.
Properly verifies error scenarios when address or resource references are invalid.
885-886: Option type testing for null checks.
Covers “none” state, guaranteeing JSON null is handled gracefully.
912-913: FixedPoint32 deserialization test.
Checks for correct scaling and rejects negative values, ensuring safe conversion.
949-952: FixedPoint64 deserialization test.
Similar validation ensures correct scaling logic and safe numeric interpretation.
981-983: BigUint test coverage added.
Verifies large integer parsing, reinforcing reliability.
1011-1014: BigDecimal boundary checks.
Ensures that negative values are rejected and scaling is applied correctly.crates/storage/src/code_scale.rs (2)
43-43: Removed the now-unnecessary NoVersion type parameter.
Streamlines themodule_codefield.
49-49: Constructor updated to reflect NoVersion removal.
Simplifies the interface by excluding redundant version data.crates/storage/src/initia_storage.rs (1)
11-14: LGTM! Import additions align with the codebase changes.The new imports prepare the module for enhanced type handling and error management capabilities, consistent with changes across other files.
Also applies to: 17-18, 20-21
crates/natives/src/move_stdlib/unit_test.rs (1)
40-40: Verify the impact of switching to master signers in tests.The change from
Value::signertoValue::master_signercould affect test behavior and permissions. This change aligns with similar modifications innative_create_signer, but we should ensure it doesn't unintentionally elevate privileges in test scenarios.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for other signer-related test functions that might need similar updates rg -A 3 "Value::signer|Value::master_signer" --type rustLength of output: 638
Master signer usage in tests is consistent and appears intentional.
- The search confirmed that both tests in
crates/natives/src/move_stdlib/unit_test.rsand usage incrates/natives/src/account.rsnow consistently useValue::master_signer.- There is no evidence of the legacy
Value::signerremaining in test contexts, suggesting that the change is deliberate and aligned with broader signer's handling updates.- The impact on test behavior and permissions seems controlled, indicating no unintended escalation of privileges.
crates/natives/src/from_bcs.rs (1)
47-51: LGTM! Enhanced deserialization with context-aware approach.The new implementation using
ValueSerDeContextprovides better control over deserialization by:
- Supporting legacy signers
- Enabling function-specific argument deserialization
- Maintaining existing error handling
crates/vm/src/session.rs (2)
64-66: LGTM! Enhanced table change set with function value extension.The addition of
as_function_value_extension()provides better context during change set processing, improving type safety.
129-135: LGTM! Improved struct type resolution with clearer naming.The rename from
get_struct_typetoget_struct_namebetter reflects the method's purpose. The signature changes also improve type safety by:
- Taking a reference to
Typeinstead ofStructNameIndex- Returning
PartialVMResult<Option<(ModuleId, Identifier)>>for better error handlingcrates/natives/src/event.rs (2)
125-136: Improved event serialization with better error handling.The new implementation uses ValueSerDeContext with legacy signer and function arguments deserialization support, providing a more robust serialization approach.
173-182: Consistent deserialization approach with improved error handling.The deserialization logic now matches the serialization approach, using ValueSerDeContext for consistency.
crates/storage/src/module_cache.rs (1)
98-98: Simplified module code creation by removing version parameter.The removal of the version parameter streamlines the module cache implementation without losing functionality.
Also applies to: 123-123
crates/compiler/src/built_package.rs (1)
67-68: Verify the impact of the new empty vector parameter.The addition of
vec![]as a parameter tocompile_package_no_exitmight affect how compilation errors or warnings are processed.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for other usages of compile_package_no_exit to ensure consistent parameter usage rg -A 2 "compile_package_no_exit"Length of output: 298
Impact of the
vec![]Parameter VerifiedOur investigation shows that the updated call:
new_config.compile_package_no_exit(&package_path, vec![], &mut stderr())?;is the only usage of
compile_package_no_exitin the codebase. This usage is consistent with the new function signature—passing an empty vector appears deliberate. No other discrepancies or adverse effects regarding error/warning processing were found.
- Location: crates/compiler/src/built_package.rs at lines 67-68
- Finding: The empty vector parameter is intentionally provided and does not conflict with how
compile_package_no_exitis used elsewhere.crates/storage/src/code_storage.rs (1)
24-27: LGTM! Enhanced type safety with new imports.The addition of these imports improves type safety and error handling capabilities without affecting runtime behavior.
crates/vm/src/publish.rs (1)
69-71: Verify the impact of the new compatibility parameter.The addition of a third parameter (
false) toCompatibility::new()might affect module compatibility checking. Please confirm:
- The purpose of this third parameter
- Its impact on backward compatibility
- Whether existing modules will continue to work as expected
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for other usages of Compatibility::new to ensure consistent behavior ast-grep --pattern 'Compatibility::new($_, $_, $_)'Length of output: 133
Compatibility Flag Impact Verification
- The third parameter in
Compatibility::new(true, false, false)is a new flag intended to enable or disable additional compatibility checks or behavior.- In its current usage (set to
false), this parameter disables the new behavior, thereby preserving legacy compatibility checks.- A repository-wide search shows that this three-parameter invocation appears only in
crates/vm/src/publish.rs, indicating that its impact is localized.- As a result, existing modules should continue to work as expected without being affected by any new compatibility criteria.
crates/natives/src/string_utils.rs (1)
188-194: Verify the Signer struct layout change.The change from
next()tonth(1)suggests a modification in the Signer struct layout. This change aligns with the switch tomaster_signerbut needs verification:
- Confirm that the address field is now the second field in the Signer struct
- Ensure all other code accessing Signer struct fields has been updated accordingly
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for other places accessing Signer struct fields ast-grep --pattern 'value_as::<Struct>()?.unpack()?'Length of output: 54
Let's gather additional data to verify the Signer struct layout change. The initial ast-grep command didn't return any results, so we'll now search for the definition of the
Signerstruct and references tomaster_signerin the codebase to confirm that its address field is indeed the second field and check that other code has updated accordingly.
🏁 Script executed:
#!/bin/bash # Search for the definition of the Signer struct (with surrounding context) rg -A 10 -B 10 "pub struct Signer" -t rust # Search for references to master_signer to see how it's used in the codebase rg "master_signer" -t rustLength of output: 287
Verification of Signer Struct Layout Update
The repository references tomaster_signerin bothcrates/natives/src/account.rsandcrates/natives/src/move_stdlib/unit_test.rsconfirm that the Signer is now being constructed viaValue::master_signer(). This strongly implies that the layout has been updated so that the address is the second field (accessed vianth(1)). Moreover, our searches did not reveal any other code accessing the Signer struct’s fields in the old layout, suggesting the change has been uniformly applied.crates/vm/src/verifier/transaction_arg_validation.rs (2)
218-226: LGTM! Improved error handling in struct type validation.The change from index-based to name-based struct type fetching with proper error handling makes the code more robust and maintainable.
381-388: LGTM! Consistent improvement in struct name resolution.The change maintains consistency with
is_valid_txn_argby using the same robust name-based approach with proper error handling.crates/json/src/move_to_json.rs (2)
72-75: LGTM! Added deterministic JSON object sorting.The addition of
sort_all_objects()ensures consistent JSON output, which is crucial for deterministic behavior in serialization.
77-103: LGTM! Consistent field renaming across struct variants.The renaming of fields to
_type_and_fieldsimproves naming consistency across the codebase.crates/json/src/json_to_value.rs (1)
427-462: LGTM! Enhanced value serialization with context.The switch to
ValueSerDeContextprovides a more structured and maintainable approach to value serialization in tests.crates/storage/src/module_storage.rs (2)
181-187: LGTM! Improved error handling for checksum mismatches.The addition of explicit error handling with clear messaging for checksum mismatches enhances debuggability.
419-467: LGTM! Well-structured function value extension implementation.The addition of
FunctionValueExtensionAdapterand related traits provides a clean way to extendModuleStoragefunctionality while maintaining good separation of concerns.libmovevm/src/move_api/move_types.rs (2)
7-7: LGTM! Import paths updated correctly.The changes correctly update the import paths for
AbilityandAbilitySetto use them frommove_core_types::abilityinstead ofmove_binary_format::file_format. This aligns with the dependency updates.Also applies to: 11-11
1230-1230: LGTM! Test imports aligned with main code changes.The test module's import of
AbilitySethas been correctly updated to match the new import path used in the main code.Cargo.toml (2)
95-95: LGTM! Updated serde_json to latest compatible version.The serde_json dependency has been updated to version 1.0.138, which is a minor version bump.
119-140: LGTM! Move dependencies updated consistently.All Move dependencies have been updated to use the same Git revision (255673e8f2757ed8634a7d65d7aca11de7016cde) from the Aptos Core repository. This is consistent with the PR objective of bumping the Aptos node version.
Let's verify if this is the latest stable revision:
❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Check the latest tags and commits in the Aptos Core repository gh api repos/aptos-labs/aptos-core/commits?per_page=1Length of output: 4498
Action: Confirm the Intended Commit Revision
All Move dependencies in Cargo.toml are now uniformly pinned to commit
255673e8f2757ed8634a7d65d7aca11de7016cde. However, our query shows that the most recent commit in the Aptos Core repository is
80b98ce2c8cda854f3bccf0e8649d33606e624eb(dated 2025‑02‑12).Before merging, please verify whether the selected commit (255673e8…) is intentionally chosen as the stable reference for the Aptos node bump—or if it should be updated to the very latest commit. This confirmation is critical to ensure that we’re tracking the intended revision for stability and compatibility.
| let address = safely_pop_arg!(arguments, AccountAddress); | ||
| Ok(smallvec![Value::signer(address)]) | ||
| Ok(smallvec![Value::master_signer(address)]) |
There was a problem hiding this comment.
Verify the security implications of using master_signer.
The change from Value::signer(address) to Value::master_signer(address) could grant elevated privileges. Please ensure this is intentional and that proper access controls are in place.
Summary by CodeRabbit
New Features
Refactor