[vm] Versioned enum-based transaction prologue and epilogue#19468
[vm] Versioned enum-based transaction prologue and epilogue#19468georgemitenkov wants to merge 2 commits intomainfrom
Conversation
51c8091 to
9d425e3
Compare
9d425e3 to
5a1a2f4
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5a1a2f4. Configure here.
352ee72 to
639ba2e
Compare
|
@security-bug-bot |
639ba2e to
efe85f4
Compare
| }); | ||
| serialized_args.push(bcs::to_bytes(&user_request).unwrap()); | ||
| } | ||
| let (prologue_function_name, serialized_args) = if let (true, Some(fee_payer_auth_key)) = ( |
There was a problem hiding this comment.
Note: this is only format change.
There was a problem hiding this comment.
Security Review — PR #19468: Versioned Enum-Based Transaction Prologue and Epilogue
Scope: 14 files changed, +1390 / -905 lines. Rust VM dispatch refactored, new Move enums PrologueArgs/EpilogueArgs, feature flag 112 (VERSIONED_TRANSACTION_VALIDATION).
| Severity | Count |
|---|---|
| MEDIUM | 1 |
| LOW | 2 |
| INFO | 1 |
Overall risk: LOW-MEDIUM. The architectural change is sound. One path-coverage gap (fee-statement emission gating) exists under non-default feature configurations; no current exploit on mainnet. BCS layout is verified-compatible and auth key logic is semantically unchanged.
Recommendation: REVIEW BEFORE MERGE (address finding #1 before enabling on a network where EMIT_FEE_STATEMENT is explicitly disabled)
Finding 1 — [MEDIUM] Versioned epilogue unconditionally emits fee statement; legacy path respects EMIT_FEE_STATEMENT gate
File: aptos-move/framework/aptos-framework/sources/transaction_validation.move:974
Blast radius: every transaction executed through versioned_epilogue once flag 112 is on
Description: The legacy run_epilogue in transaction_validation_legacy.rs guards the fee-statement emit behind features.is_emit_fee_statement_enabled() (which requires both MODULE_EVENT and EMIT_FEE_STATEMENT to be on, see aptos_features.rs:386). The new versioned_epilogue in Move calls transaction_fee::emit_fee_statement(fee_statement) unconditionally after unified_epilogue_v2 completes, with no corresponding feature check.
// transaction_validation.move line ~974
transaction_fee::emit_fee_statement(fee_statement); // no feature gateOn mainnet both flags are in default_features() so the behavior is identical. The mismatch surfaces on any network where flag 112 is enabled but EMIT_FEE_STATEMENT or MODULE_EVENT is disabled, or if governance later disables one while keeping flag 112. The delta means switching from the legacy path to the versioned path changes observable event emission.
Concrete impact: Path coverage gap / behavioral divergence under non-default flag combos. No current exploit on mainnet where both feature flags are on by default.
Finding 2 — [LOW] .expect() panics on BCS serialization of prologue/epilogue arguments
File: aptos-move/aptos-vm/src/transaction_validation.rs:98,166
Blast radius: every transaction routed through the versioned path
Description:
bcs::to_bytes(&args).expect("Failed to serialize prologue arguments")
bcs::to_bytes(&args).expect("Failed to serialize epilogue arguments")bcs::to_bytes can fail if the Serde serializer returns an error. For current types this is effectively infallible, but .expect() on a transaction-critical path in the VM violates the project's CLAUDE.md preference for checked error handling with descriptive messages over bare unwrap/expect. The existing emit_fee_statement call in the legacy path (line 656) also uses .expect() with the same pattern. If future additions to PrologueArgs/EpilogueArgs introduce a type with a fallible Serialize impl, this will panic the VM.
Concrete impact: No current exploit. Style/robustness concern; follow-on enum variants could introduce a panicking serialization path.
Finding 3 — [LOW] is_transaction_limits_enabled silently returns false when flag 111 is on and flag 112 is off
File: types/src/on_chain_config/aptos_features.rs:499-503
Description: The PR makes TRANSACTION_LIMITS (flag 111) enforcement depend on VERSIONED_TRANSACTION_VALIDATION (flag 112) also being enabled. Any network/test that turns on flag 111 without flag 112 silently skips transaction limit enforcement with no error or warning — is_transaction_limits_enabled() just returns false. The comment documents the dependency, but there is no assertion or log to detect the misconfiguration.
pub fn is_transaction_limits_enabled(&self) -> bool {
self.is_enabled(FeatureFlag::TRANSACTION_LIMITS)
&& self.is_enabled(FeatureFlag::VERSIONED_TRANSACTION_VALIDATION) // silent AND
}Concrete impact: Feature-flag misconfiguration risk. A node operator enabling flag 111 alone would observe no transaction limit enforcement without any diagnostic signal.
Finding 4 — [INFO] BCS layout: Rust/Move enum field order verified compatible
PrologueArgs::V1, EpilogueArgs::V1, ReplayProtector, UserTxnLimitsRequest, RequestedMultipliers, and FeeStatement all have matching field order and types between Rust #[derive(Serialize)] and Move enums/structs. The Move VM's own round-trip tests in move-vm/types/src/values/serialization_tests.rs cover this surface. The variant discriminant encoding (u32 index via Serde/BCS) is consistent between the two layers. No layout mismatch identified.
Fee-payer auth key check in versioned_prologue
The new if (fee_payer_address != signer::address_of(&sender)) guard before the fee-payer auth key block is semantically correct: when sender == fee_payer, prologue_common already validates the sender's auth key against the same account (the sender_address == gas_payer_address branch in prologue_common), so the second check would be a no-op. For distinct fee payers the check still runs as before. No auth key validation gap.
Test Coverage
| Changed Function | Coverage | Note |
|---|---|---|
versioned_prologue (Move) |
Exercised by 9 fee_payer e2e + 19 txn e2e | Flag 112 in default_features |
versioned_epilogue (Move) |
Same e2e suite | Emit path not gated in tests |
PrologueBuilder::build (Rust) |
Covered by e2e | No unit test for BCS round-trip |
EpilogueBuilder::build (Rust) |
Covered by e2e | Same |
| Flag 111 on / flag 112 off combination | Not tested | Silent limits bypass |
Blast Radius
run_prologue / run_epilogue in transaction_validation.rs are called from transaction_validation_legacy.rs when flag 112 is on; legacy is called from aptos_vm.rs for every user transaction. The versioned path touches all user transactions on networks with flag 112 enabled.
Sent by Cursor Automation: Security Review Bot
| pub unified_prologue_fee_payer_v3_name: Identifier, | ||
| } | ||
|
|
||
| impl TransactionValidation { |
There was a problem hiding this comment.
[LOW] .expect() on a VM-critical serialization path.
bcs::to_bytes can technically fail if a type's Serialize impl errors. All current field types are infallible, but as new variants are added to PrologueArgs this becomes a maintenance footgun. Per project convention, prefer propagating errors (or at minimum use unwrap_or_else with a descriptive panic) so reviewers of future enum variants understand the contract.
efe85f4 to
05eec9c
Compare
| is_simulation, | ||
| is_orderless_txn, | ||
| ); | ||
| transaction_fee::emit_fee_statement(fee_statement); |
There was a problem hiding this comment.
Important: I folded fee statement emission here so we do not go out of VM just to do that.
| fee_payer_public_key_hash == option::some(account::get_authentication_key(fee_payer_address)), | ||
| error::invalid_argument(PROLOGUE_EINVALID_ACCOUNT_AUTH_KEY) | ||
| fun versioned_prologue(sender: signer, fee_payer: signer, args: PrologueArgs) { | ||
| match (args) { |
There was a problem hiding this comment.
So what will the prologue look like in the future, when we have more than 1 version? Mainly want to understand what logic we can share and what we have to duplicate.
There was a problem hiding this comment.
It will match over 1-2 variants all the time: because older versions V1, V2 are part of code, they do not need to be maintained here.
So say we roll this out, and later create V2:
- Then we add V2 match here which handles this, V1 still exists during rollout
- Once fully rolled out, we make V1 => abort, nothing needs to be maintained
versioned_prologueitself never changes signature.
There was a problem hiding this comment.
On Rust side, we do this:
- Builder used to produce V1.
- Now with V2, we still populate builder fields independently (no nested if-elses), but if new feature enabled, build V2 variant out of builder to pass it.
- This code for compatibility should still handle V1 but arguably less nested if-elses?
There was a problem hiding this comment.
Got it. So there will be a brief time when V1 and V2 may share some logic (during rollout), but once rollout is done, we can remove the V1 logic completely (probably in the next framework release), right?
There was a problem hiding this comment.
yes, that is the intention
| is_simulation, | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
I haven't thought about this super thoroughly, but what do we want to do with the old code below?
- Is there a way to make it use the new enum-based epilogue as well?
- Or are we planning to remove it in the future?
Asking cuz right now there seems to be more code duplication when it comes to prologue/epilogue logic in the AptosVM
There was a problem hiding this comment.
There is no way because old Move validation uses args and that code is on-chain :( I was hoping with new VM we can roll it out and use transaction_validation_versioned straight away (at least post-testing) so we do not need to maintain any legacy code there.
There was a problem hiding this comment.
The main rationale of change is to avoid adding new functions but keep only 1 set for prologue and epilogue.
Replace the proliferating per-version prologue/epilogue functions with a single versioned entry point each. PrologueArgs and EpilogueArgs enums allow adding new arguments as new variants without creating new on-chain functions, and old variants can be pruned once fully rolled out — unlike the current 18+ functions that can never be deleted. V1 matches unified_prologue_fee_payer_v2 / unified_epilogue_v2. Both signers (sender + fee_payer) are always passed. Rust side uses a builder pattern with bcs::to_bytes for clean serialization. Gated behind VERSIONED_TRANSACTION_VALIDATION feature flag (111). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
05eec9c to
a68b19b
Compare




Summary
Introduces versioned enum-based transaction validation to replace the per-version prologue/epilogue Move functions.
Every new prologue/epilogue argument currently requires new on-chain functions that can never be deleted (lagging nodes may still call them by name). This creates an ever-growing set of functions and deeply nested if-else trees on the Rust side.
The new design adds a single
versioned_prologue(sender, fee_payer, args: PrologueArgs)andversioned_epilogue(account, gas_payer, args: EpilogueArgs)where the args are versioned enums. New arguments become new enum variants (V2, V3, ...). Old variants can be pruned once fully rolled out since they are match arms rather than named entry points. The Rust side uses a builder pattern (PrologueBuilder/EpilogueBuilder) that selects the variant based on feature flags andbcs::to_bytesfor serialization.V1 matches the current latest prologue and epilogue, carrying
txn_limits_request: Option<UserTxnLimitsRequest>. Gated behind theVERSIONED_TRANSACTION_VALIDATIONfeature flag (112).Subsumes
unified_prologue_v3Because
PrologueArgs::V1already carriestxn_limits_request, the separateunified_prologue_v3/unified_prologue_fee_payer_v3functions added by the TRANSACTION_LIMITS PR (#19109) are redundant. This PR removes them along with their Rust dispatch inaptos-move/aptos-vm/src/transaction_validation.rs. Theunified_prologue_v2/unified_prologue_fee_payer_v2shims (which previously delegated to v3) now callprologue_commondirectly withtxn_limits_request = option::none().Feature relationship
TRANSACTION_LIMITS(111) — gates payload-side acceptance ofUserTxnLimitsRequestintoTransactionMetadata(aptos-move/aptos-vm/src/transaction_metadata.rs:84). When off,txn_data.txn_limitsis alwaysNone.VERSIONED_TRANSACTION_VALIDATION(112) — gates whether the VM dispatches throughversioned_prologue. The V1 variant is the only path that plumbstxn_limits_requestintoprologue_common.For Move-side enforcement of staking-backed transaction limits, 112 must be enabled wherever 111 is active. The legacy non-versioned dispatch path retained on this branch selects only between
unified_prologue_v2/unified_prologue(no v3 tier remains).Test plan
cargo check -p aptos-vmcompilesscripts/cargo_build_aptos_cached_packages.shGenerated with Claude Code
Note
High Risk
Changes core transaction prologue/epilogue dispatch and feature gating, which directly affects transaction validity and fee/sequence-number handling. Bugs in BCS enum layout compatibility or flag rollout ordering could cause transaction failures or consensus divergence across nodes.
Overview
Introduces a new feature flag
VERSIONED_TRANSACTION_VALIDATIONand routes transaction prologue/epilogue execution through new on-chain entry pointsversioned_prologue/versioned_epiloguethat take BCS-serialized, versioned enum args built on the Rust side.Refactors validation to drop the dedicated v3 prologue/fee-payer prologue functions and associated Rust dispatch logic;
TRANSACTION_LIMITSenforcement is now tied to the versioned path (requires bothTRANSACTION_LIMITSandVERSIONED_TRANSACTION_VALIDATION).Updates framework Move code and docs to define the new
PrologueArgs/EpilogueArgsenums, emit fee statements from the versioned epilogue path, and adjuststransaction_feehelpers (newstorage_fee_refund_octas, friend visibility tweaks, deprecated resource moved).Reviewed by Cursor Bugbot for commit a68b19b. Bugbot is set up for automated code reviews on this repo. Configure here.