Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(drive-abci): skip state transition txs if time limit is reached on prepare_proposal #2041

Merged
merged 5 commits into from
Aug 8, 2024
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
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 @@

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 @@ -37,11 +38,13 @@
/// state transitions, processing state transitions, or executing events.
pub(in crate::execution) fn process_raw_state_transitions(
&self,
raw_state_transitions: &Vec<Vec<u8>>,

Check warning on line 41 in packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/mod.rs

View workflow job for this annotation

GitHub Actions / Rust packages (drive-abci) / Linting

writing `&Vec` instead of `&[_]` involves a new object where a slice will do

warning: writing `&Vec` instead of `&[_]` involves a new object where a slice will do --> packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/mod.rs:41:32 | 41 | raw_state_transitions: &Vec<Vec<u8>>, | ^^^^^^^^^^^^^ help: change this to: `&[Vec<u8>]` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg = note: `#[warn(clippy::ptr_arg)]` on by default
block_platform_state: &PlatformState,
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 @@
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
Loading