Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ pub enum FeatureFlag {
PublicStructEnumArgs,
MultisigScript,
TransactionLimits,
VersionedTransactionValidation,
}

fn generate_features_blob(writer: &CodeWriter, data: &[u64]) {
Expand Down Expand Up @@ -425,6 +426,9 @@ impl From<FeatureFlag> for AptosFeatureFlag {
FeatureFlag::PublicStructEnumArgs => AptosFeatureFlag::PUBLIC_STRUCT_ENUM_ARGS,
FeatureFlag::MultisigScript => AptosFeatureFlag::MULTISIG_SCRIPT,
FeatureFlag::TransactionLimits => AptosFeatureFlag::TRANSACTION_LIMITS,
FeatureFlag::VersionedTransactionValidation => {
AptosFeatureFlag::VERSIONED_TRANSACTION_VALIDATION
},
}
}
}
Expand Down Expand Up @@ -614,6 +618,9 @@ impl From<AptosFeatureFlag> for FeatureFlag {
AptosFeatureFlag::PUBLIC_STRUCT_ENUM_ARGS => FeatureFlag::PublicStructEnumArgs,
AptosFeatureFlag::MULTISIG_SCRIPT => FeatureFlag::MultisigScript,
AptosFeatureFlag::TRANSACTION_LIMITS => FeatureFlag::TransactionLimits,
AptosFeatureFlag::VERSIONED_TRANSACTION_VALIDATION => {
FeatureFlag::VersionedTransactionValidation
},
}
}
}
Expand Down
1 change: 1 addition & 0 deletions aptos-move/aptos-vm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ pub mod system_module_names;
pub mod testing;
pub mod transaction_metadata;
mod transaction_validation;
mod transaction_validation_versioned;
pub mod validator_txns;
pub mod verifier;

Expand Down
10 changes: 10 additions & 0 deletions aptos-move/aptos-vm/src/system_module_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,13 @@ pub static TRANSACTION_FEE_MODULE: Lazy<ModuleId> = Lazy::new(|| {
});

pub const EMIT_FEE_STATEMENT: &IdentStr = ident_str!("emit_fee_statement");

pub static TRANSACTION_VALIDATION_MODULE: Lazy<ModuleId> = Lazy::new(|| {
ModuleId::new(
account_config::CORE_CODE_ADDRESS,
ident_str!("transaction_validation").to_owned(),
)
});

pub const VERSIONED_PROLOGUE_NAME: &IdentStr = ident_str!("versioned_prologue");
pub const VERSIONED_EPILOGUE_NAME: &IdentStr = ident_str!("versioned_epilogue");
217 changes: 108 additions & 109 deletions aptos-move/aptos-vm/src/transaction_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ use aptos_types::{
fee_statement::FeeStatement,
move_utils::as_move_value::AsMoveValue,
on_chain_config::Features,
transaction::{
MultisigTransactionPayload, ReplayProtector, TransactionExecutableRef, TxnLimitsRequest,
},
transaction::{MultisigTransactionPayload, ReplayProtector, TransactionExecutableRef},
};
use aptos_vm_logging::log_schema::AdapterLogSchema;
use fail::fail_point;
Expand Down Expand Up @@ -64,11 +62,6 @@ pub static APTOS_TRANSACTION_VALIDATION: Lazy<TransactionValidation> =
unified_prologue_fee_payer_v2_name: Identifier::new("unified_prologue_fee_payer_v2")
.unwrap(),
unified_epilogue_v2_name: Identifier::new("unified_epilogue_v2").unwrap(),

// V3 prologues support voting-power-based high-txn-limits.
unified_prologue_v3_name: Identifier::new("unified_prologue_v3").unwrap(),
unified_prologue_fee_payer_v3_name: Identifier::new("unified_prologue_fee_payer_v3")
.unwrap(),
});

/// On-chain functions used to validate transactions
Expand All @@ -94,10 +87,6 @@ pub struct TransactionValidation {
pub unified_prologue_v2_name: Identifier,
pub unified_prologue_fee_payer_v2_name: Identifier,
pub unified_epilogue_v2_name: Identifier,

// V3 prologues support voting-power-based high-txn-limits.
pub unified_prologue_v3_name: Identifier,
pub unified_prologue_fee_payer_v3_name: Identifier,
}

impl TransactionValidation {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Expand Down Expand Up @@ -133,6 +122,18 @@ pub(crate) fn run_script_prologue(
traversal_context: &mut TraversalContext,
is_simulation: bool,
) -> Result<(), VMStatus> {
if features.is_versioned_transaction_validation_enabled() {
return crate::transaction_validation_versioned::run_prologue(
session,
module_storage,
serialized_signers,
txn_data,
log_context,
traversal_context,
is_simulation,
);
}

let txn_replay_protector = txn_data.replay_protector();
let txn_authentication_key = txn_data.authentication_proof().optional_auth_key();
let txn_gas_price = txn_data.gas_unit_price();
Expand Down Expand Up @@ -166,103 +167,88 @@ pub(crate) fn run_script_prologue(
}
};

let (prologue_function_name, mut serialized_args) =
if let (true, Some(fee_payer_auth_key)) = (
txn_data.fee_payer().is_some(),
txn_data
.fee_payer_authentication_proof
.as_ref()
.map(|proof| proof.optional_auth_key()),
) {
let serialized_args = vec![
serialized_signers.sender(),
serialized_signers
.fee_payer()
.ok_or_else(|| VMStatus::error(StatusCode::UNREACHABLE, None))?,
txn_authentication_key
.as_move_value()
.simple_serialize()
.unwrap(),
fee_payer_auth_key
.as_move_value()
.simple_serialize()
.unwrap(),
replay_protector_move_value,
MoveValue::vector_address(txn_data.secondary_signers())
.simple_serialize()
.unwrap(),
MoveValue::Vector(secondary_auth_keys)
.simple_serialize()
.unwrap(),
MoveValue::U64(txn_gas_price.into())
.simple_serialize()
.unwrap(),
MoveValue::U64(txn_max_gas_units.into())
.simple_serialize()
.unwrap(),
MoveValue::U64(txn_expiration_timestamp_secs)
.simple_serialize()
.unwrap(),
MoveValue::U8(chain_id.id()).simple_serialize().unwrap(),
MoveValue::Bool(is_simulation).simple_serialize().unwrap(),
];
(
if features.is_transaction_limits_enabled() {
&APTOS_TRANSACTION_VALIDATION.unified_prologue_fee_payer_v3_name
} else if features.is_transaction_payload_v2_enabled() {
&APTOS_TRANSACTION_VALIDATION.unified_prologue_fee_payer_v2_name
} else {
&APTOS_TRANSACTION_VALIDATION.unified_prologue_fee_payer_name
},
serialized_args,
)
} else {
let serialized_args = vec![
serialized_signers.sender(),
txn_authentication_key
.as_move_value()
.simple_serialize()
.unwrap(),
replay_protector_move_value,
MoveValue::vector_address(txn_data.secondary_signers())
.simple_serialize()
.unwrap(),
MoveValue::Vector(secondary_auth_keys)
.simple_serialize()
.unwrap(),
MoveValue::U64(txn_gas_price.into())
.simple_serialize()
.unwrap(),
MoveValue::U64(txn_max_gas_units.into())
.simple_serialize()
.unwrap(),
MoveValue::U64(txn_expiration_timestamp_secs)
.simple_serialize()
.unwrap(),
MoveValue::U8(chain_id.id()).simple_serialize().unwrap(),
MoveValue::Bool(is_simulation).simple_serialize().unwrap(),
];
(
if features.is_transaction_limits_enabled() {
&APTOS_TRANSACTION_VALIDATION.unified_prologue_v3_name
} else if features.is_transaction_payload_v2_enabled() {
&APTOS_TRANSACTION_VALIDATION.unified_prologue_v2_name
} else {
&APTOS_TRANSACTION_VALIDATION.unified_prologue_name
},
serialized_args,
)
};

// Append the user's staking-backed request when dispatching to v3 prologue.
// Governance scripts pass None (they don't need Move-side validation).
if features.is_transaction_limits_enabled() {
let user_request = txn_data.txn_limits.as_ref().and_then(|v| match v {
TxnLimitsRequest::Staking(req) => Some(req),
TxnLimitsRequest::ApprovedGovernanceScript => None,
});
serialized_args.push(bcs::to_bytes(&user_request).unwrap());
}
let (prologue_function_name, serialized_args) = if let (true, Some(fee_payer_auth_key)) = (
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this is only format change.

txn_data.fee_payer().is_some(),
txn_data
.fee_payer_authentication_proof
.as_ref()
.map(|proof| proof.optional_auth_key()),
) {
let serialized_args = vec![
serialized_signers.sender(),
serialized_signers
.fee_payer()
.ok_or_else(|| VMStatus::error(StatusCode::UNREACHABLE, None))?,
txn_authentication_key
.as_move_value()
.simple_serialize()
.unwrap(),
fee_payer_auth_key
.as_move_value()
.simple_serialize()
.unwrap(),
replay_protector_move_value,
MoveValue::vector_address(txn_data.secondary_signers())
.simple_serialize()
.unwrap(),
MoveValue::Vector(secondary_auth_keys)
.simple_serialize()
.unwrap(),
MoveValue::U64(txn_gas_price.into())
.simple_serialize()
.unwrap(),
MoveValue::U64(txn_max_gas_units.into())
.simple_serialize()
.unwrap(),
MoveValue::U64(txn_expiration_timestamp_secs)
.simple_serialize()
.unwrap(),
MoveValue::U8(chain_id.id()).simple_serialize().unwrap(),
MoveValue::Bool(is_simulation).simple_serialize().unwrap(),
];
(
if features.is_transaction_payload_v2_enabled() {
&APTOS_TRANSACTION_VALIDATION.unified_prologue_fee_payer_v2_name
} else {
&APTOS_TRANSACTION_VALIDATION.unified_prologue_fee_payer_name
},
serialized_args,
)
} else {
let serialized_args = vec![
serialized_signers.sender(),
txn_authentication_key
.as_move_value()
.simple_serialize()
.unwrap(),
replay_protector_move_value,
MoveValue::vector_address(txn_data.secondary_signers())
.simple_serialize()
.unwrap(),
MoveValue::Vector(secondary_auth_keys)
.simple_serialize()
.unwrap(),
MoveValue::U64(txn_gas_price.into())
.simple_serialize()
.unwrap(),
MoveValue::U64(txn_max_gas_units.into())
.simple_serialize()
.unwrap(),
MoveValue::U64(txn_expiration_timestamp_secs)
.simple_serialize()
.unwrap(),
MoveValue::U8(chain_id.id()).simple_serialize().unwrap(),
MoveValue::Bool(is_simulation).simple_serialize().unwrap(),
];
(
if features.is_transaction_payload_v2_enabled() {
&APTOS_TRANSACTION_VALIDATION.unified_prologue_v2_name
} else {
&APTOS_TRANSACTION_VALIDATION.unified_prologue_name
},
serialized_args,
)
};

session
.execute_function_bypass_visibility(
Expand Down Expand Up @@ -503,6 +489,19 @@ fn run_epilogue(
traversal_context: &mut TraversalContext,
is_simulation: bool,
) -> VMResult<()> {
if features.is_versioned_transaction_validation_enabled() {
return crate::transaction_validation_versioned::run_epilogue(
session,
module_storage,
serialized_signers,
gas_remaining,
fee_statement,
txn_data,
traversal_context,
is_simulation,
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main rationale of change is to avoid adding new functions but keep only 1 set for prologue and epilogue.

let txn_gas_price = txn_data.gas_unit_price();
let txn_max_gas_units = txn_data.max_gas_amount();
let is_orderless_txn = txn_data.is_orderless();
Expand Down
Loading
Loading