Skip to content

Commit

Permalink
feat(drive-abci): skip state transition txs if time limit is reached …
Browse files Browse the repository at this point in the history
…on prepare_proposal (dashpay#2041)

Co-authored-by: Quantum Explorer <[email protected]>
  • Loading branch information
ogabrielides and QuantumExplorer authored Aug 8, 2024
1 parent 4c56692 commit 6055b33
Show file tree
Hide file tree
Showing 18 changed files with 373 additions and 128 deletions.
29 changes: 16 additions & 13 deletions packages/rs-drive-abci/src/abci/app/execution_result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,39 +7,42 @@ use dpp::version::PlatformVersion;
use dpp::version::TryIntoPlatformVersioned;
use tenderdash_abci::proto::abci::ExecTxResult;

impl TryIntoPlatformVersioned<ExecTxResult> for StateTransitionExecutionResult {
impl TryIntoPlatformVersioned<Option<ExecTxResult>> for StateTransitionExecutionResult {
type Error = Error;

fn try_into_platform_versioned(
self,
platform_version: &PlatformVersion,
) -> Result<ExecTxResult, Self::Error> {
) -> Result<Option<ExecTxResult>, Self::Error> {
let response = match self {
StateTransitionExecutionResult::SuccessfulExecution(_, actual_fees) => ExecTxResult {
code: 0,
gas_used: actual_fees.total_base_fee() as SignedCredits,
..Default::default()
},
StateTransitionExecutionResult::UnpaidConsensusError(error) => ExecTxResult {
StateTransitionExecutionResult::SuccessfulExecution(_, actual_fees) => {
Some(ExecTxResult {
code: 0,
gas_used: actual_fees.total_base_fee() as SignedCredits,
..Default::default()
})
}
StateTransitionExecutionResult::UnpaidConsensusError(error) => Some(ExecTxResult {
code: HandlerError::from(&error).code(),
info: error.response_info_for_version(platform_version)?,
gas_used: 0,
..Default::default()
},
}),
StateTransitionExecutionResult::PaidConsensusError(error, actual_fees) => {
ExecTxResult {
Some(ExecTxResult {
code: HandlerError::from(&error).code(),
info: error.response_info_for_version(platform_version)?,
gas_used: actual_fees.total_base_fee() as SignedCredits,
..Default::default()
}
})
}
StateTransitionExecutionResult::InternalError(message) => ExecTxResult {
StateTransitionExecutionResult::InternalError(message) => Some(ExecTxResult {
code: HandlerError::Internal(message).code(),
// TODO: That would be nice to provide more information about the error for debugging
info: String::default(),
..Default::default()
},
}),
StateTransitionExecutionResult::NotExecuted(_) => None,
};

Ok(response)
Expand Down
10 changes: 10 additions & 0 deletions packages/rs-drive-abci/src/abci/config.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Configuration of ABCI Application server
use dpp::prelude::TimestampMillis;
use serde::{Deserialize, Serialize};

// We allow changes in the ABCI configuration, but there should be a social process
Expand Down Expand Up @@ -32,6 +33,10 @@ pub struct AbciConfig {
// Note it is parsed directly in PlatformConfig::from_env() so here we just set defaults.
#[serde(default)]
pub log: crate::logging::LogConfigs,

/// Maximum time limit (in ms) to process state transitions in block proposals
#[serde(default = "AbciConfig::default_tx_processing_time_limit")]
pub tx_processing_time_limit: TimestampMillis,
}

impl AbciConfig {
Expand All @@ -42,6 +47,10 @@ impl AbciConfig {
pub(crate) fn default_genesis_core_height() -> u32 {
1
}

pub(crate) fn default_tx_processing_time_limit() -> TimestampMillis {
8000
}
}

impl Default for AbciConfig {
Expand All @@ -52,6 +61,7 @@ impl Default for AbciConfig {
genesis_core_height: AbciConfig::default_genesis_core_height(),
chain_id: "chain_id".to_string(),
log: Default::default(),
tx_processing_time_limit: AbciConfig::default_tx_processing_time_limit(),
}
}
}
20 changes: 14 additions & 6 deletions packages/rs-drive-abci/src/abci/handler/prepare_proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,13 @@ where
.expect("transaction must be started");

// Running the proposal executes all the state transitions for the block
let mut run_result =
app.platform()
.run_block_proposal(block_proposal, true, &platform_state, transaction)?;
let mut run_result = app.platform().run_block_proposal(
block_proposal,
true,
&platform_state,
transaction,
Some(&timer),
)?;

if !run_result.is_valid() {
// This is a system error, because we are proposing
Expand Down Expand Up @@ -151,13 +155,17 @@ where
// We shouldn't include in the block any state transitions that produced an internal error
// during execution
StateTransitionExecutionResult::InternalError(..) => TxAction::Removed,
// State Transition was not executed as it reached the maximum time limit
StateTransitionExecutionResult::NotExecuted(..) => TxAction::Delayed,
};

let tx_result: ExecTxResult =
let tx_result: Option<ExecTxResult> =
state_transition_execution_result.try_into_platform_versioned(platform_version)?;

if tx_action != TxAction::Removed {
tx_results.push(tx_result);
if let Some(result) = tx_result {
if tx_action != TxAction::Removed {
tx_results.push(result);
}
}

tx_records.push(TxRecord {
Expand Down
7 changes: 6 additions & 1 deletion packages/rs-drive-abci/src/abci/handler/process_proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ where
false,
&platform_state,
transaction,
None,
)?;

if !run_result.is_valid() {
Expand Down Expand Up @@ -255,7 +256,11 @@ where
| StateTransitionExecutionResult::PaidConsensusError(..)
)
})
.map(|execution_result| execution_result.try_into_platform_versioned(platform_version))
.filter_map(|execution_result| {
execution_result
.try_into_platform_versioned(platform_version)
.transpose()
})
.collect::<Result<_, _>>()?;

let response = proto::ResponseProcessProposal {
Expand Down
20 changes: 19 additions & 1 deletion packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ where
check_tx_result.unique_identifiers = state_transition.unique_identifiers();

let validation_result = state_transition_to_execution_event_for_check_tx(
&platform_ref,
platform_ref,
state_transition,
check_tx_level,
platform_version,
Expand Down Expand Up @@ -368,6 +368,8 @@ mod tests {
&BlockInfo::default(),
&transaction,
platform_version,
false,
None,
)
.expect("expected to process state transition");

Expand Down Expand Up @@ -479,6 +481,8 @@ mod tests {
&BlockInfo::default(),
&transaction,
platform_version,
false,
None,
)
.expect("expected to process state transition");

Expand Down Expand Up @@ -678,6 +682,8 @@ mod tests {
&BlockInfo::default(),
&transaction,
platform_version,
false,
None,
)
.expect("expected to process state transition");

Expand Down Expand Up @@ -829,6 +835,8 @@ mod tests {
&BlockInfo::default(),
&transaction,
platform_version,
false,
None,
)
.expect("expected to process state transition");

Expand Down Expand Up @@ -974,6 +982,8 @@ mod tests {
&BlockInfo::default(),
&transaction,
platform_version,
false,
None,
)
.expect("expected to process state transition");

Expand Down Expand Up @@ -1088,6 +1098,8 @@ mod tests {
&BlockInfo::default(),
&transaction,
platform_version,
false,
None,
)
.expect("expected to process state transition");

Expand Down Expand Up @@ -1167,6 +1179,8 @@ mod tests {
&BlockInfo::default(),
&transaction,
platform_version,
false,
None,
)
.expect("expected to process state transition");

Expand Down Expand Up @@ -1294,6 +1308,8 @@ mod tests {
&BlockInfo::default(),
&transaction,
platform_version,
false,
None,
)
.expect("expected to process state transition");

Expand Down Expand Up @@ -1411,6 +1427,8 @@ mod tests {
&BlockInfo::default(),
&transaction,
platform_version,
false,
None,
)
.expect("expected to process state transition");

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::error::execution::ExecutionError;
use crate::error::Error;
use crate::metrics::HistogramTiming;
use crate::platform_types::epoch_info::v0::EpochInfoV0Methods;
use crate::platform_types::platform::Platform;
use crate::platform_types::platform_state::v0::PlatformStateV0Methods;
Expand Down Expand Up @@ -46,6 +47,7 @@ where
known_from_us: bool,
platform_state: &PlatformState,
transaction: &Transaction,
timer: Option<&HistogramTiming>,
) -> Result<ValidationResult<block_execution_outcome::v0::BlockExecutionOutcome, Error>, Error>
{
// Epoch information is always calculated with the last committed platform version
Expand Down Expand Up @@ -127,6 +129,7 @@ Your software version: {}, latest supported protocol version: {}."#,
platform_state,
block_platform_state,
block_platform_version,
timer,
),
version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch {
method: "run_block_proposal".to_string(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::execution::types::block_state_info::v0::{
BlockStateInfoV0Getters, BlockStateInfoV0Methods, BlockStateInfoV0Setters,
};
use crate::execution::types::{block_execution_context, block_state_info};

use crate::metrics::HistogramTiming;
use crate::platform_types::block_execution_outcome;
use crate::platform_types::block_proposal;
use crate::platform_types::epoch_info::v0::{EpochInfoV0Getters, EpochInfoV0Methods};
Expand Down Expand Up @@ -67,6 +67,7 @@ where
last_committed_platform_state: &PlatformState,
mut block_platform_state: PlatformState,
platform_version: &'static PlatformVersion,
timer: Option<&HistogramTiming>,
) -> Result<ValidationResult<block_execution_outcome::v0::BlockExecutionOutcome, Error>, Error>
{
tracing::trace!(
Expand Down Expand Up @@ -310,6 +311,8 @@ where
&block_info,
transaction,
platform_version,
known_from_us,
timer,
)?;

// Pool withdrawals into transactions queue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ mod v0;

use crate::error::execution::ExecutionError;
use crate::error::Error;
use crate::metrics::HistogramTiming;
use crate::platform_types::platform::Platform;
use crate::platform_types::platform_state::PlatformState;
use crate::platform_types::state_transitions_processing_result::StateTransitionsProcessingResult;
Expand Down Expand Up @@ -42,6 +43,8 @@ where
block_info: &BlockInfo,
transaction: &Transaction,
platform_version: &PlatformVersion,
proposing_state_transitions: bool,
timer: Option<&HistogramTiming>,
) -> Result<StateTransitionsProcessingResult, Error> {
match platform_version
.drive_abci
Expand All @@ -55,6 +58,8 @@ where
block_info,
transaction,
platform_version,
proposing_state_transitions,
timer,
),
version => Err(Error::Execution(ExecutionError::UnknownVersionMismatch {
method: "process_raw_state_transitions".to_string(),
Expand Down
Loading

0 comments on commit 6055b33

Please sign in to comment.