Skip to content

Commit

Permalink
DeltaApplicationError & predicted value fixes, error fallbacks (#14422)
Browse files Browse the repository at this point in the history
* DeltaApplicationError & latest predicted value fixes, fallbacks to sequential

* addressing comments
  • Loading branch information
gelash authored Oct 2, 2024
1 parent 8d138df commit 18bcbd2
Show file tree
Hide file tree
Showing 38 changed files with 402 additions and 274 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion aptos-move/aptos-aggregator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ repository = { workspace = true }
rust-version = { workspace = true }

[dependencies]
aptos-logger = { workspace = true }
aptos-types = { workspace = true }
bcs = { workspace = true }
claims = { workspace = true }
Expand Down
3 changes: 2 additions & 1 deletion aptos-move/aptos-aggregator/src/aggregator_v1_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ use crate::{
bounded_math::{BoundedMath, SignedU128},
delta_math::DeltaHistory,
resolver::AggregatorV1Resolver,
types::{expect_ok, DelayedFieldsSpeculativeError, DeltaApplicationFailureReason},
types::{DelayedFieldsSpeculativeError, DeltaApplicationFailureReason},
};
use aptos_types::{
error::expect_ok,
state_store::{state_key::StateKey, table::TableHandle},
PeerId,
};
Expand Down
7 changes: 5 additions & 2 deletions aptos-move/aptos-aggregator/src/delayed_change.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@

use crate::{
delta_change_set::{DeltaOp, DeltaWithMax},
types::{code_invariant_error, DelayedFieldValue, DelayedFieldsSpeculativeError, PanicOr},
types::{DelayedFieldValue, DelayedFieldsSpeculativeError},
};
use aptos_types::{
delayed_fields::SnapshotToStringFormula,
error::{code_invariant_error, PanicOr},
};
use aptos_types::delayed_fields::SnapshotToStringFormula;

#[derive(Clone, Debug, Eq, PartialEq)]
pub enum DelayedApplyChange<I: Clone> {
Expand Down
14 changes: 7 additions & 7 deletions aptos-move/aptos-aggregator/src/delayed_field_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ use crate::{
delayed_change::{ApplyBase, DelayedApplyChange, DelayedChange},
delta_change_set::DeltaWithMax,
resolver::DelayedFieldResolver,
types::{
code_invariant_error, expect_ok, DelayedFieldValue, DelayedFieldsSpeculativeError, PanicOr,
ReadPosition,
},
types::{DelayedFieldValue, DelayedFieldsSpeculativeError, ReadPosition},
};
use aptos_types::delayed_fields::{
calculate_width_for_constant_string, calculate_width_for_integer_embedded_string,
SnapshotToStringFormula,
use aptos_types::{
delayed_fields::{
calculate_width_for_constant_string, calculate_width_for_integer_embedded_string,
SnapshotToStringFormula,
},
error::{code_invariant_error, expect_ok, PanicOr},
};
use move_binary_format::errors::PartialVMResult;
use move_vm_types::delayed_values::delayed_field_id::DelayedFieldID;
Expand Down
7 changes: 3 additions & 4 deletions aptos-move/aptos-aggregator/src/delta_change_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@
use crate::{
bounded_math::{BoundedMath, SignedU128},
delta_math::{merge_data_and_delta, merge_two_deltas, DeltaHistory},
types::{
code_invariant_error, DelayedFieldsSpeculativeError, DeltaApplicationFailureReason, PanicOr,
},
types::{DelayedFieldsSpeculativeError, DeltaApplicationFailureReason},
};
use aptos_types::error::{code_invariant_error, PanicOr};

#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub struct DeltaWithMax {
Expand Down Expand Up @@ -219,7 +218,7 @@ mod test {
FakeAggregatorView,
};
use aptos_types::{
delayed_fields::PanicError,
error::PanicError,
state_store::{
state_key::StateKey,
state_value::{StateValue, StateValueMetadata},
Expand Down
5 changes: 3 additions & 2 deletions aptos-move/aptos-aggregator/src/delta_math.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
use crate::{
bounded_math::{ok_overflow, ok_underflow, BoundedMath, SignedU128},
types::{
expect_ok, DelayedFieldsSpeculativeError, DeltaApplicationFailureReason,
DeltaHistoryMergeOffsetFailureReason, PanicOr,
DelayedFieldsSpeculativeError, DeltaApplicationFailureReason,
DeltaHistoryMergeOffsetFailureReason,
},
};
use aptos_types::error::{expect_ok, PanicOr};

/// Tracks values seen by aggregator. In particular, stores information about
/// the biggest and the smallest deltas that were applied successfully during
Expand Down
7 changes: 2 additions & 5 deletions aptos-move/aptos-aggregator/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,10 @@ use crate::{
aggregator_v1_extension::{addition_v1_error, subtraction_v1_error},
bounded_math::SignedU128,
delta_change_set::{serialize, DeltaOp},
types::{
code_invariant_error, DelayedFieldValue, DelayedFieldsSpeculativeError,
DeltaApplicationFailureReason, PanicOr,
},
types::{DelayedFieldValue, DelayedFieldsSpeculativeError, DeltaApplicationFailureReason},
};
use aptos_types::{
delayed_fields::PanicError,
error::{code_invariant_error, PanicError, PanicOr},
state_store::{
state_key::StateKey,
state_value::{StateValue, StateValueMetadata},
Expand Down
6 changes: 2 additions & 4 deletions aptos-move/aptos-aggregator/src/tests/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ use crate::{
bounded_math::{BoundedMath, SignedU128},
delta_change_set::serialize,
resolver::{TAggregatorV1View, TDelayedFieldView},
types::{
code_invariant_error, expect_ok, DelayedFieldValue, DelayedFieldsSpeculativeError, PanicOr,
},
types::{DelayedFieldValue, DelayedFieldsSpeculativeError},
};
use aptos_types::{
delayed_fields::PanicError,
error::{code_invariant_error, expect_ok, PanicError, PanicOr},
state_store::{
state_key::StateKey,
state_value::{StateValue, StateValueMetadata},
Expand Down
75 changes: 1 addition & 74 deletions aptos-move/aptos-aggregator/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

use crate::bounded_math::SignedU128;
use aptos_logger::error;
use aptos_types::delayed_fields::PanicError;
use aptos_types::error::{code_invariant_error, NonPanic, PanicError, PanicOr};
use move_binary_format::errors::PartialVMError;
use move_core_types::{
value::{IdentifierMappingKind, MoveTypeLayout},
Expand All @@ -20,84 +19,12 @@ use move_vm_types::{
values::{Struct, Value},
};

// Wrapping another error, to add a variant that represents
// something that should never happen - i.e. a code invariant error,
// which we would generally just panic, but since we are inside of the VM,
// we cannot do that.
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum PanicOr<T: std::fmt::Debug> {
CodeInvariantError(String),
Or(T),
}

impl<T: std::fmt::Debug> PanicOr<T> {
pub fn map_non_panic<E: std::fmt::Debug>(self, f: impl FnOnce(T) -> E) -> PanicOr<E> {
match self {
PanicOr::CodeInvariantError(msg) => PanicOr::CodeInvariantError(msg),
PanicOr::Or(value) => PanicOr::Or(f(value)),
}
}
}

pub fn code_invariant_error<M: std::fmt::Debug>(message: M) -> PanicError {
let msg = format!(
"Delayed materialization code invariant broken (there is a bug in the code), {:?}",
message
);
error!("{}", msg);
PanicError::CodeInvariantError(msg)
}

pub fn expect_ok<V, E: std::fmt::Debug>(value: Result<V, E>) -> Result<V, PanicError> {
value.map_err(|e| code_invariant_error(format!("Expected Ok, got Err({:?})", e)))
}

impl<T: std::fmt::Debug> From<PanicError> for PanicOr<T> {
fn from(err: PanicError) -> Self {
match err {
PanicError::CodeInvariantError(e) => PanicOr::CodeInvariantError(e),
}
}
}

pub trait NonPanic {}

impl<T: std::fmt::Debug + NonPanic> From<T> for PanicOr<T> {
fn from(err: T) -> Self {
PanicOr::Or(err)
}
}

impl From<DelayedFieldsSpeculativeError> for PartialVMError {
fn from(err: DelayedFieldsSpeculativeError) -> Self {
PartialVMError::from(PanicOr::from(err))
}
}

impl<T: std::fmt::Debug> From<&PanicOr<T>> for StatusCode {
fn from(err: &PanicOr<T>) -> Self {
match err {
PanicOr::CodeInvariantError(_) => {
StatusCode::DELAYED_MATERIALIZATION_CODE_INVARIANT_ERROR
},
PanicOr::Or(_) => StatusCode::SPECULATIVE_EXECUTION_ABORT_ERROR,
}
}
}

impl<T: std::fmt::Debug> From<PanicOr<T>> for PartialVMError {
fn from(err: PanicOr<T>) -> Self {
match err {
PanicOr::CodeInvariantError(msg) => {
PartialVMError::new(StatusCode::DELAYED_MATERIALIZATION_CODE_INVARIANT_ERROR)
.with_message(msg)
},
PanicOr::Or(err) => PartialVMError::new(StatusCode::SPECULATIVE_EXECUTION_ABORT_ERROR)
.with_message(format!("{:?}", err)),
}
}
}

/// Different reasons for why applying new start_value doesn't
/// satisfy history bounds
#[derive(Clone, Debug, PartialEq, Eq)]
Expand Down
5 changes: 2 additions & 3 deletions aptos-move/aptos-vm-types/src/change_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,10 @@ use aptos_aggregator::{
delayed_change::DelayedChange,
delta_change_set::{serialize, DeltaOp},
resolver::AggregatorV1Resolver,
types::code_invariant_error,
};
use aptos_types::{
contract_event::ContractEvent,
delayed_fields::PanicError,
error::{code_invariant_error, PanicError},
state_store::{
state_key::{inner::StateKeyInner, StateKey},
state_value::StateValueMetadata,
Expand Down Expand Up @@ -193,7 +192,7 @@ impl VMChangeSet {
let (key, value) = element?;
if acc.insert(key, value).is_some() {
Err(PartialVMError::new(
StatusCode::DELAYED_MATERIALIZATION_CODE_INVARIANT_ERROR,
StatusCode::DELAYED_FIELD_OR_BLOCKSTM_CODE_INVARIANT_ERROR,
)
.with_message(
"Found duplicate key across resource change sets.".to_string(),
Expand Down
5 changes: 2 additions & 3 deletions aptos-move/aptos-vm-types/src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@ use crate::{
};
use aptos_aggregator::{
delayed_change::DelayedChange, delta_change_set::DeltaOp, resolver::AggregatorV1Resolver,
types::code_invariant_error,
};
use aptos_types::{
contract_event::ContractEvent,
delayed_fields::PanicError,
error::{code_invariant_error, PanicError},
fee_statement::FeeStatement,
state_store::state_key::StateKey,
transaction::{TransactionAuxiliaryData, TransactionOutput, TransactionStatus},
Expand Down Expand Up @@ -167,7 +166,7 @@ impl VMOutput {
self.try_materialize(resolver)?;
self.into_transaction_output().map_err(|e| {
VMStatus::error(
StatusCode::DELAYED_MATERIALIZATION_CODE_INVARIANT_ERROR,
StatusCode::DELAYED_FIELD_OR_BLOCKSTM_CODE_INVARIANT_ERROR,
Some(e.to_string()),
)
})
Expand Down
5 changes: 3 additions & 2 deletions aptos-move/aptos-vm-types/src/tests/test_change_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ use aptos_aggregator::{
delta_change_set::DeltaWithMax,
};
use aptos_types::{
delayed_fields::{PanicError, SnapshotToStringFormula},
delayed_fields::SnapshotToStringFormula,
error::PanicError,
state_store::{state_key::StateKey, state_value::StateValueMetadata},
transaction::ChangeSet as StorageChangeSet,
write_set::{WriteOp, WriteSetMut},
Expand Down Expand Up @@ -199,7 +200,7 @@ macro_rules! assert_invariant_violation {
assert!(
err.major_status() == StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR
|| err.major_status()
== StatusCode::DELAYED_MATERIALIZATION_CODE_INVARIANT_ERROR
== StatusCode::DELAYED_FIELD_OR_BLOCKSTM_CODE_INVARIANT_ERROR
);
};

Expand Down
4 changes: 2 additions & 2 deletions aptos-move/aptos-vm/src/block_executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use aptos_infallible::Mutex;
use aptos_types::{
block_executor::config::BlockExecutorConfig,
contract_event::ContractEvent,
delayed_fields::PanicError,
error::PanicError,
executable::ExecutableTestType,
fee_statement::FeeStatement,
state_store::{state_key::StateKey, state_value::StateValueMetadata, StateView, StateViewId},
Expand Down Expand Up @@ -436,7 +436,7 @@ impl BlockAptosVM {
Err(BlockExecutionError::FatalBlockExecutorError(PanicError::CodeInvariantError(
err_msg,
))) => Err(VMStatus::Error {
status_code: StatusCode::DELAYED_MATERIALIZATION_CODE_INVARIANT_ERROR,
status_code: StatusCode::DELAYED_FIELD_OR_BLOCKSTM_CODE_INVARIANT_ERROR,
sub_status: None,
message: Some(err_msg),
}),
Expand Down
4 changes: 2 additions & 2 deletions aptos-move/aptos-vm/src/block_executor/vm_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl ExecutorTask for AptosExecutorTask {
vm_status.message().cloned().unwrap_or_default(),
)
} else if vm_status.status_code()
== StatusCode::DELAYED_MATERIALIZATION_CODE_INVARIANT_ERROR
== StatusCode::DELAYED_FIELD_OR_BLOCKSTM_CODE_INVARIANT_ERROR
{
ExecutionStatus::DelayedFieldsCodeInvariantError(
vm_status.message().cloned().unwrap_or_default(),
Expand All @@ -98,7 +98,7 @@ impl ExecutorTask for AptosExecutorTask {
err.message().cloned().unwrap_or_default(),
)
} else if err.status_code()
== StatusCode::DELAYED_MATERIALIZATION_CODE_INVARIANT_ERROR
== StatusCode::DELAYED_FIELD_OR_BLOCKSTM_CODE_INVARIANT_ERROR
{
ExecutionStatus::DelayedFieldsCodeInvariantError(
err.message().cloned().unwrap_or_default(),
Expand Down
4 changes: 2 additions & 2 deletions aptos-move/aptos-vm/src/data_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ use crate::{
use aptos_aggregator::{
bounded_math::SignedU128,
resolver::{TAggregatorV1View, TDelayedFieldView},
types::{DelayedFieldValue, DelayedFieldsSpeculativeError, PanicOr},
types::{DelayedFieldValue, DelayedFieldsSpeculativeError},
};
use aptos_table_natives::{TableHandle, TableResolver};
use aptos_types::{
delayed_fields::PanicError,
error::{PanicError, PanicOr},
on_chain_config::{ConfigStorage, Features, OnChainConfig},
state_store::{
errors::StateviewError,
Expand Down
6 changes: 3 additions & 3 deletions aptos-move/aptos-vm/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ pub fn convert_prologue_error(
e @ VMStatus::Error {
status_code:
StatusCode::SPECULATIVE_EXECUTION_ABORT_ERROR
| StatusCode::DELAYED_MATERIALIZATION_CODE_INVARIANT_ERROR,
| StatusCode::DELAYED_FIELD_OR_BLOCKSTM_CODE_INVARIANT_ERROR,
..
} => e,
status @ VMStatus::ExecutionFailure { .. } | status @ VMStatus::Error { .. } => {
Expand Down Expand Up @@ -207,7 +207,7 @@ pub fn convert_epilogue_error(
e @ VMStatus::Error {
status_code:
StatusCode::SPECULATIVE_EXECUTION_ABORT_ERROR
| StatusCode::DELAYED_MATERIALIZATION_CODE_INVARIANT_ERROR,
| StatusCode::DELAYED_FIELD_OR_BLOCKSTM_CODE_INVARIANT_ERROR,
..
} => e,
status => {
Expand Down Expand Up @@ -237,7 +237,7 @@ pub fn expect_only_successful_execution(
e @ VMStatus::Error {
status_code:
StatusCode::SPECULATIVE_EXECUTION_ABORT_ERROR
| StatusCode::DELAYED_MATERIALIZATION_CODE_INVARIANT_ERROR,
| StatusCode::DELAYED_FIELD_OR_BLOCKSTM_CODE_INVARIANT_ERROR,
..
} => e,
status => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ use aptos_aggregator::{
delayed_change::{ApplyBase, DelayedApplyChange, DelayedChange},
delta_change_set::DeltaWithMax,
resolver::{TAggregatorV1View, TDelayedFieldView},
types::{
code_invariant_error, expect_ok, DelayedFieldValue, DelayedFieldsSpeculativeError, PanicOr,
},
types::{DelayedFieldValue, DelayedFieldsSpeculativeError},
};
use aptos_types::{
delayed_fields::PanicError,
error::{code_invariant_error, expect_ok, PanicError, PanicOr},
state_store::{
errors::StateviewError,
state_key::StateKey,
Expand Down
Loading

0 comments on commit 18bcbd2

Please sign in to comment.