Revert (v1.13)"enable REMOVE_DETAILED_ERROR_FROM_HASH CONCURRENT_FUNG…

…IBLE_AS… (#13478) * Revert "enable REMOVE_DETAILED_ERROR_FROM_HASH CONCURRENT_FUNGIBLE_ASSETS for devnet genesis" This reverts commit bf5010f. * adjust perf bench due to revert
aptos-labs · May 30, 2024 · 34a535e · 34a535e
1 parent b3af42c
commit 34a535e
Show file tree

Hide file tree

Showing 24 changed files with 61 additions and 108 deletions.
diff --git a/api/src/transactions.rs b/api/src/transactions.rs
@@ -34,7 +34,7 @@ use aptos_types::{
     mempool_status::MempoolStatusCode,
     transaction::{
         EntryFunction, ExecutionStatus, MultisigTransactionPayload, RawTransaction,
-        RawTransactionWithData, SignedTransaction, TransactionPayload,
+        RawTransactionWithData, SignedTransaction, TransactionPayload, TransactionStatus,
     },
     vm_status::StatusCode,
     APTOS_COIN_TYPE,
@@ -1376,11 +1376,10 @@ impl TransactionsApi {
         let version = ledger_info.version();
 
         // Ensure that all known statuses return their values in the output (even if they aren't supposed to)
-        let exe_status = match vm_status.clone().keep_or_discard() {
-            Ok(kept_vm_status) => kept_vm_status.into(),
-            Err(discarded_vm_status) => {
-                ExecutionStatus::MiscellaneousError(Some(discarded_vm_status))
-            },
+        let exe_status = match output.status().clone() {
+            TransactionStatus::Keep(exec_status) => exec_status,
+            TransactionStatus::Discard(status) => ExecutionStatus::MiscellaneousError(Some(status)),
+            _ => ExecutionStatus::MiscellaneousError(None),
         };
 
         let stats_key = match txn.payload() {

diff --git a/aptos-move/aptos-transactional-test-harness/tests/aptos_test_harness/borrow_in_loop.exp b/aptos-move/aptos-transactional-test-harness/tests/aptos_test_harness/borrow_in_loop.exp
@@ -4,7 +4,7 @@ task 1 'publish'. lines 4-26:
 Error: VMError with status STLOC_UNSAFE_TO_DESTROY_ERROR at location Module ModuleId { address: f75daa73fc071f93593335eb9033da804777eb94491650dd3f095ce6f778acb6, name: Identifier("m") } at index 0 for function definition at code offset 11 in function definition 0
 
 task 2 'run'. lines 28-28:
-Error: Failed to execute transaction. ExecutionStatus: MiscellaneousError(None)
+Error: Failed to execute transaction. ExecutionStatus: MiscellaneousError(Some(LINKER_ERROR))
 
 task 3 'run'. lines 30-44:
-Error: Failed to execute transaction. ExecutionStatus: MiscellaneousError(None)
+Error: Failed to execute transaction. ExecutionStatus: MiscellaneousError(Some(STLOC_UNSAFE_TO_DESTROY_ERROR))
diff --git a/aptos-move/aptos-transactional-test-harness/tests/aptos_test_harness/diamond_clicker.exp b/aptos-move/aptos-transactional-test-harness/tests/aptos_test_harness/diamond_clicker.exp
@@ -65,4 +65,4 @@ task 2 'publish'. lines 37-68:
 Error: VMError with status CALL_TYPE_MISMATCH_ERROR at location Module ModuleId { address: f75daa73fc071f93593335eb9033da804777eb94491650dd3f095ce6f778acb6, name: Identifier("game") } at index 0 for function definition at code offset 25 in function definition 0
 
 task 3 'run'. lines 70-70:
-Error: Failed to execute transaction. ExecutionStatus: MiscellaneousError(None)
+Error: Failed to execute transaction. ExecutionStatus: MiscellaneousError(Some(LINKER_ERROR))
diff --git a/aptos-move/e2e-move-tests/src/harness.rs b/aptos-move/e2e-move-tests/src/harness.rs
@@ -194,12 +194,11 @@ impl MoveHarness {
 
     /// Runs a signed transaction. On success, applies the write set.
     pub fn run_raw(&mut self, txn: SignedTransaction) -> TransactionOutput {
-        let mut output = self.executor.execute_transaction(txn);
+        let output = self.executor.execute_transaction(txn);
         if matches!(output.status(), TransactionStatus::Keep(_)) {
             self.executor.apply_write_set(output.write_set());
             self.executor.append_events(output.events().to_vec());
         }
-        output.fill_error_status();
         output
     }
 
@@ -237,12 +236,11 @@ impl MoveHarness {
         &mut self,
         txn_block: Vec<SignedTransaction>,
     ) -> Vec<TransactionOutput> {
-        let mut result = assert_ok!(self.executor.execute_block(txn_block));
-        for output in &mut result {
+        let result = assert_ok!(self.executor.execute_block(txn_block));
+        for output in &result {
             if matches!(output.status(), TransactionStatus::Keep(_)) {
                 self.executor.apply_write_set(output.write_set());
             }
-            output.fill_error_status();
         }
         result
     }
@@ -969,11 +967,7 @@ impl MoveHarness {
                 offset,
                 txns.len()
             );
-            let mut outputs = harness.run_block_get_output(txns);
-            let _ = outputs
-                .iter_mut()
-                .map(|t| t.fill_error_status())
-                .collect::<Vec<_>>();
+            let outputs = harness.run_block_get_output(txns);
             for (idx, (error, output)) in errors.into_iter().zip(outputs.iter()).enumerate() {
                 if error == SUCCESS {
                     assert_success!(

diff --git a/aptos-move/e2e-move-tests/src/tests/code_publishing.rs b/aptos-move/e2e-move-tests/src/tests/code_publishing.rs
@@ -182,17 +182,10 @@ fn code_publishing_upgrade_loader_cache_consistency() {
             |_| {},
         ),
     ];
-    let result = h.run_block_get_output(txns);
-    assert_success!(result[0].status().to_owned());
-    assert_success!(result[1].status().to_owned());
-    assert_eq!(
-        result[2]
-            .auxiliary_data()
-            .get_detail_error_message()
-            .unwrap()
-            .status_code(),
-        StatusCode::BACKWARD_INCOMPATIBLE_MODULE_UPDATE
-    )
+    let result = h.run_block(txns);
+    assert_success!(result[0]);
+    assert_success!(result[1]);
+    assert_vm_status!(result[2], StatusCode::BACKWARD_INCOMPATIBLE_MODULE_UPDATE)
 }
 
 #[test]

diff --git a/aptos-move/e2e-move-tests/src/tests/fee_payer.rs b/aptos-move/e2e-move-tests/src/tests/fee_payer.rs
@@ -271,8 +271,8 @@ fn test_account_not_exist_out_of_gas_with_fee_payer() {
     let result = h.run_raw(transaction);
 
     assert_eq!(
-        result.status().to_owned(),
-        TransactionStatus::Keep(ExecutionStatus::MiscellaneousError(Some(
+        result.status(),
+        &TransactionStatus::Keep(ExecutionStatus::MiscellaneousError(Some(
             StatusCode::EXECUTION_LIMIT_REACHED
         ))),
     );

diff --git a/aptos-move/e2e-move-tests/src/tests/vm.rs b/aptos-move/e2e-move-tests/src/tests/vm.rs
@@ -43,14 +43,8 @@ fn failed_transaction_cleanup_charges_gas(status_code: StatusCode) {
         )
         .1;
 
-    assert_eq!(
-        output
-            .auxiliary_data()
-            .get_detail_error_message()
-            .unwrap()
-            .status_code(),
-        status_code
-    );
+    assert_some!(output.auxiliary_data().get_detail_error_message());
+    println!("auxiliary data {:?}", output.auxiliary_data());
     let write_set: Vec<(&StateKey, &WriteOp)> = output
         .change_set()
         .concrete_write_set_iter()
@@ -61,6 +55,6 @@ fn failed_transaction_cleanup_charges_gas(status_code: StatusCode) {
     assert!(!output.status().is_discarded());
     assert_ok_eq!(
         output.status().as_kept_status(),
-        ExecutionStatus::MiscellaneousError(None)
+        ExecutionStatus::MiscellaneousError(Some(status_code))
     );
 }
diff --git a/...e2e-tests/goldens/language_e2e_testsuite__tests__scripts__script_bad_sig_function_dep.exp b/...e2e-tests/goldens/language_e2e_testsuite__tests__scripts__script_bad_sig_function_dep.exp
@@ -18,7 +18,9 @@ Ok(
             gas_used: 3,
             status: Keep(
                 MiscellaneousError(
-                    None,
+                    Some(
+                        TYPE_MISMATCH,
+                    ),
                 ),
             ),
             auxiliary_data: V1(

diff --git a/...ve/e2e-tests/goldens/language_e2e_testsuite__tests__scripts__script_code_unverifiable.exp b/...ve/e2e-tests/goldens/language_e2e_testsuite__tests__scripts__script_code_unverifiable.exp
@@ -18,7 +18,9 @@ Ok(
             gas_used: 3,
             status: Keep(
                 MiscellaneousError(
-                    None,
+                    Some(
+                        NEGATIVE_STACK_SIZE_WITHIN_BLOCK,
+                    ),
                 ),
             ),
             auxiliary_data: V1(

diff --git a/...uage_e2e_testsuite__tests__scripts__script_nested_type_argument_module_does_not_exist.exp b/...uage_e2e_testsuite__tests__scripts__script_nested_type_argument_module_does_not_exist.exp
@@ -18,7 +18,9 @@ Ok(
             gas_used: 3,
             status: Keep(
                 MiscellaneousError(
-                    None,
+                    Some(
+                        LINKER_ERROR,
+                    ),
                 ),
             ),
             auxiliary_data: V1(

diff --git a/...ests/goldens/language_e2e_testsuite__tests__scripts__script_non_existing_function_dep.exp b/...ests/goldens/language_e2e_testsuite__tests__scripts__script_non_existing_function_dep.exp
@@ -18,7 +18,9 @@ Ok(
             gas_used: 3,
             status: Keep(
                 MiscellaneousError(
-                    None,
+                    Some(
+                        LOOKUP_FAILED,
+                    ),
                 ),
             ),
             auxiliary_data: V1(

diff --git a/...tests/goldens/language_e2e_testsuite__tests__scripts__script_none_existing_module_dep.exp b/...tests/goldens/language_e2e_testsuite__tests__scripts__script_none_existing_module_dep.exp
@@ -18,7 +18,9 @@ Ok(
             gas_used: 3,
             status: Keep(
                 MiscellaneousError(
-                    None,
+                    Some(
+                        LINKER_ERROR,
+                    ),
                 ),
             ),
             auxiliary_data: V1(

diff --git a/...ns/language_e2e_testsuite__tests__scripts__script_type_argument_module_does_not_exist.exp b/...ns/language_e2e_testsuite__tests__scripts__script_type_argument_module_does_not_exist.exp
@@ -18,7 +18,9 @@ Ok(
             gas_used: 3,
             status: Keep(
                 MiscellaneousError(
-                    None,
+                    Some(
+                        LINKER_ERROR,
+                    ),
                 ),
             ),
             auxiliary_data: V1(

diff --git a/...ts/goldens/language_e2e_testsuite__tests__verify_txn__test_arbitrary_script_execution.exp b/...ts/goldens/language_e2e_testsuite__tests__verify_txn__test_arbitrary_script_execution.exp
@@ -18,7 +18,9 @@ Ok(
             gas_used: 3,
             status: Keep(
                 MiscellaneousError(
-                    None,
+                    Some(
+                        CODE_DESERIALIZATION_ERROR,
+                    ),
                 ),
             ),
             auxiliary_data: V1(

diff --git a/.../language_e2e_testsuite__tests__verify_txn__test_script_dependency_fails_verification.exp b/.../language_e2e_testsuite__tests__verify_txn__test_script_dependency_fails_verification.exp
@@ -18,7 +18,9 @@ Ok(
             gas_used: 3,
             status: Keep(
                 MiscellaneousError(
-                    None,
+                    Some(
+                        UNEXPECTED_VERIFIER_ERROR,
+                    ),
                 ),
             ),
             auxiliary_data: V1(

diff --git a/...2e_testsuite__tests__verify_txn__test_script_transitive_dependency_fails_verification.exp b/...2e_testsuite__tests__verify_txn__test_script_transitive_dependency_fails_verification.exp
@@ -18,7 +18,9 @@ Ok(
             gas_used: 3,
             status: Keep(
                 MiscellaneousError(
-                    None,
+                    Some(
+                        UNEXPECTED_VERIFIER_ERROR,
+                    ),
                 ),
             ),
             auxiliary_data: V1(

diff --git a/...anguage_e2e_testsuite__tests__verify_txn__test_type_tag_dependency_fails_verification.exp b/...anguage_e2e_testsuite__tests__verify_txn__test_type_tag_dependency_fails_verification.exp
@@ -18,7 +18,9 @@ Ok(
             gas_used: 3,
             status: Keep(
                 MiscellaneousError(
-                    None,
+                    Some(
+                        UNEXPECTED_VERIFIER_ERROR,
+                    ),
                 ),
             ),
             auxiliary_data: V1(

diff --git a/..._testsuite__tests__verify_txn__test_type_tag_transitive_dependency_fails_verification.exp b/..._testsuite__tests__verify_txn__test_type_tag_transitive_dependency_fails_verification.exp
@@ -18,7 +18,9 @@ Ok(
             gas_used: 3,
             status: Keep(
                 MiscellaneousError(
-                    None,
+                    Some(
+                        UNEXPECTED_VERIFIER_ERROR,
+                    ),
                 ),
             ),
             auxiliary_data: V1(

diff --git a/aptos-move/e2e-tests/src/executor.rs b/aptos-move/e2e-tests/src/executor.rs
@@ -543,17 +543,13 @@ impl FakeExecutor {
             },
             onchain: onchain_config,
         };
-        BlockAptosVM::execute_block::<
-            _,
-            NoOpTransactionCommitHook<AptosTransactionOutput, VMStatus>,
-        >(
+        BlockAptosVM::execute_block::<_, NoOpTransactionCommitHook<AptosTransactionOutput, VMStatus>>(
             self.executor_thread_pool.clone(),
             txn_block,
             &state_view,
             config,
             None,
-        )
-        .map(BlockOutput::into_transaction_outputs_forced)
+        ).map(BlockOutput::into_transaction_outputs_forced)
     }
 
     pub fn execute_transaction_block_with_state_view(
@@ -666,11 +662,9 @@ impl FakeExecutor {
         let mut outputs = self
             .execute_block(txn_block)
             .expect("The VM should not fail to startup");
-        let mut txn_output = outputs
+        outputs
             .pop()
-            .expect("A block with one transaction should have one output");
-        txn_output.fill_error_status();
-        txn_output
+            .expect("A block with one transaction should have one output")
     }
 
     pub fn execute_transaction_with_gas_profiler(

diff --git a/storage/aptosdb/src/db/include/aptosdb_reader.rs b/storage/aptosdb/src/db/include/aptosdb_reader.rs
@@ -888,19 +888,14 @@ impl AptosDB {
     ) -> Result<TransactionWithProof> {
         self.error_if_ledger_pruned("Transaction", version)?;
 
-        let mut proof = self
+        let proof = self
             .ledger_db
             .transaction_info_db()
             .get_transaction_info_with_proof(
                 version,
                 ledger_version,
                 self.ledger_db.transaction_accumulator_db(),
             )?;
-
-        let aux_data_opt = self.get_transaction_auxiliary_data_by_version(version).ok();
-        proof.inject_auxiliary_error_data(aux_data_opt);
-
-
         let transaction = self.ledger_db.transaction_db().get_transaction(version)?;
 
         // If events were requested, also fetch those.

diff --git a/testsuite/single_node_performance.py b/testsuite/single_node_performance.py
@@ -89,7 +89,7 @@ class RunGroupConfig:
     RunGroupConfig(expected_tps=22300, key=RunGroupKey("no-op"), included_in=LAND_BLOCKING_AND_C),
     RunGroupConfig(expected_tps=11900, key=RunGroupKey("no-op", module_working_set_size=1000), included_in=LAND_BLOCKING_AND_C),
     RunGroupConfig(expected_tps=13100, key=RunGroupKey("coin-transfer"), included_in=LAND_BLOCKING_AND_C | Flow.REPRESENTATIVE),
-    RunGroupConfig(expected_tps=40100, key=RunGroupKey("coin-transfer", executor_type="native"), included_in=LAND_BLOCKING_AND_C),
+    RunGroupConfig(expected_tps=39700, key=RunGroupKey("coin-transfer", executor_type="native"), included_in=LAND_BLOCKING_AND_C),
     RunGroupConfig(expected_tps=9000, key=RunGroupKey("account-generation"), included_in=LAND_BLOCKING_AND_C | Flow.REPRESENTATIVE),
     RunGroupConfig(expected_tps=30600, key=RunGroupKey("account-generation", executor_type="native"), included_in=Flow.CONTINUOUS),
     RunGroupConfig(expected_tps=19300, key=RunGroupKey("account-resource32-b"), included_in=Flow.CONTINUOUS),

diff --git a/types/src/on_chain_config/aptos_features.rs b/types/src/on_chain_config/aptos_features.rs
@@ -135,7 +135,6 @@ impl FeatureFlag {
             FeatureFlag::COIN_TO_FUNGIBLE_ASSET_MIGRATION,
             FeatureFlag::OBJECT_NATIVE_DERIVED_ADDRESS,
             FeatureFlag::DISPATCHABLE_FUNGIBLE_ASSET,
-            FeatureFlag::REMOVE_DETAILED_ERROR_FROM_HASH,
             FeatureFlag::CONCURRENT_FUNGIBLE_ASSETS,
         ]
     }

diff --git a/types/src/proof/definition.rs b/types/src/proof/definition.rs
@@ -11,7 +11,7 @@ use super::{
 use crate::{
     ledger_info::LedgerInfo,
     proof::accumulator::InMemoryTransactionAccumulator,
-    transaction::{TransactionAuxiliaryData, TransactionInfo, Version},
+    transaction::{TransactionInfo, Version},
 };
 use anyhow::{bail, ensure, format_err, Context, Result};
 #[cfg(any(test, feature = "fuzzing"))]
@@ -848,16 +848,6 @@ impl TransactionInfoWithProof {
         }
     }
 
-    /// Inject auxiliary error data into transaction info if auxiliary data is present
-    pub fn inject_auxiliary_error_data(
-        &mut self,
-        auxiliary_data_opt: Option<TransactionAuxiliaryData>,
-    ) {
-        if let Some(aux_data) = auxiliary_data_opt {
-            self.transaction_info.inject_auxiliary_error_data(aux_data);
-        }
-    }
-
     /// Returns the `ledger_info_to_transaction_info_proof` object in this proof.
     pub fn ledger_info_to_transaction_info_proof(&self) -> &TransactionAccumulatorProof {
         &self.ledger_info_to_transaction_info_proof