Skip to content
This repository has been archived by the owner on Aug 2, 2024. It is now read-only.

Commit

Permalink
fix: bugs after rebase and refactor (#1578)
Browse files Browse the repository at this point in the history
  • Loading branch information
apoorvsadana authored May 6, 2024
1 parent 79e7f43 commit b2730f6
Show file tree
Hide file tree
Showing 10 changed files with 143 additions and 91 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Next release

- fix: txn hash calculation and refactor
- chore: remove all da/settlement related code
- fix: re-execute txs instead of simulating for txn receipts
- chore: rebase on latest blockifier
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion crates/client/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ where
C: HeaderBackend<B> + 'static,
{
pub fn current_spec_version(&self) -> RpcResult<String> {
Ok("0.4.0".to_string())
Ok("0.7.0".to_string())
}
}

Expand Down
21 changes: 16 additions & 5 deletions crates/pallets/starknet/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,10 @@ pub mod pallet {
}
_ => run_revertible_transaction(&transaction, &mut state, &block_context, true, charge_fee),
}
.map_err(|_| Error::<T>::TransactionExecutionFailed)?;
.map_err(|e| {
log!(error, "Invoke transaction execution failed: {:?}", e);
Error::<T>::TransactionExecutionFailed
})?;

Self::emit_and_store_tx_and_fees_events(
transaction.tx_hash,
Expand Down Expand Up @@ -569,7 +572,10 @@ pub mod pallet {
// Execute
let tx_execution_infos =
run_non_revertible_transaction(&transaction, &mut state, &Self::get_block_context(), true, charge_fee)
.map_err(|_| Error::<T>::TransactionExecutionFailed)?;
.map_err(|e| {
log!(error, "Declare transaction execution failed: {:?}", e);
Error::<T>::TransactionExecutionFailed
})?;

Self::emit_and_store_tx_and_fees_events(
transaction.tx_hash(),
Expand Down Expand Up @@ -616,7 +622,10 @@ pub mod pallet {
// Execute
let tx_execution_infos =
run_non_revertible_transaction(&transaction, &mut state, &Self::get_block_context(), true, charge_fee)
.map_err(|_| Error::<T>::TransactionExecutionFailed)?;
.map_err(|e| {
log!(error, "Deploy account transaction execution failed: {:?}", e);
Error::<T>::TransactionExecutionFailed
})?;

Self::emit_and_store_tx_and_fees_events(
transaction.tx_hash,
Expand Down Expand Up @@ -666,8 +675,10 @@ pub mod pallet {

// Execute
let tx_execution_infos =
execute_l1_handler_transaction(&transaction, &mut state, &Self::get_block_context())
.map_err(|_| Error::<T>::TransactionExecutionFailed)?;
execute_l1_handler_transaction(&transaction, &mut state, &Self::get_block_context()).map_err(|e| {
log!(error, "L1 Handler transaction execution failed: {:?}", e);
Error::<T>::TransactionExecutionFailed
})?;

Self::emit_and_store_tx_and_fees_events(
transaction.tx_hash,
Expand Down
41 changes: 29 additions & 12 deletions crates/pallets/starknet/src/simulations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use sp_runtime::DispatchError;
use starknet_api::transaction::TransactionVersion;

use crate::blockifier_state_adapter::BlockifierStateAdapter;
use crate::{Config, Error, Pallet};
use crate::{log, Config, Error, Pallet};

type ReExecutionResult = Result<
Vec<(TransactionExecutionInfo, Option<CommitmentStateDiff>)>,
Expand Down Expand Up @@ -49,8 +49,22 @@ impl<T: Config> Pallet<T> {
let fee_res_iterator = transactions
.into_iter()
.map(|tx| match Self::execute_account_transaction(&tx, &mut state, &block_context, simulation_flags) {
Ok(execution_info) if !execution_info.is_reverted() => Ok(execution_info),
Err(_) | Ok(_) => Err(Error::<T>::TransactionExecutionFailed),
Ok(execution_info) => {
if !execution_info.is_reverted() {
Ok(execution_info)
} else {
log!(
debug,
"Transaction execution reverted during fee estimation: {:?}",
execution_info.revert_error
);
Err(Error::<T>::TransactionExecutionFailed)
}
}
Err(e) => {
log!(debug, "Transaction execution failed during fee estimation: {:?}", e);
Err(Error::<T>::TransactionExecutionFailed)
}
})
.map(|exec_info_res| {
exec_info_res.map(|exec_info| {
Expand Down Expand Up @@ -102,7 +116,7 @@ impl<T: Config> Pallet<T> {
)?;

let result = res.0.map_err(|e| {
log::error!("Transaction execution failed during simulation: {e}");
log!(debug, "Transaction execution failed during simulation: {:?}", e);
PlaceHolderErrorTypeForFailedStarknetExecution
});

Expand Down Expand Up @@ -135,7 +149,7 @@ impl<T: Config> Pallet<T> {
let mut state = BlockifierStateAdapter::<T>::default();

let tx_execution_result = Self::execute_message(&message, &mut state, &block_context).map_err(|e| {
log::error!("Transaction execution failed during simulation: {e}");
log!(debug, "Transaction execution failed during simulation: {:?}", e);
PlaceHolderErrorTypeForFailedStarknetExecution
});

Expand All @@ -157,14 +171,17 @@ impl<T: Config> Pallet<T> {
let tx_execution_infos = match message.execute(&mut cached_state, &Self::get_block_context(), true, true) {
Ok(execution_info) if !execution_info.is_reverted() => Ok(execution_info),
Err(e) => {
log::error!(
"Transaction execution failed during fee estimation: {e} {:?}",
log!(
debug,
"Transaction execution failed during fee estimation: {:?} {:?}",
e,
std::error::Error::source(&e)
);
Err(Error::<T>::TransactionExecutionFailed)
}
Ok(execution_info) => {
log::error!(
log!(
debug,
"Transaction execution reverted during fee estimation: {}",
// Safe due to the `match` branch order
execution_info.revert_error.unwrap()
Expand Down Expand Up @@ -205,7 +222,7 @@ impl<T: Config> Pallet<T> {

transactions_before.iter().try_for_each(|tx| {
Self::execute_transaction(tx, &mut state, &block_context, &SimulationFlags::default()).map_err(|e| {
log::error!("Failed to reexecute a tx: {}", e);
log!(debug, "Failed to reexecute a tx: {}", e);
PlaceHolderErrorTypeForFailedStarknetExecution
})?;

Expand All @@ -224,14 +241,14 @@ impl<T: Config> Pallet<T> {
&SimulationFlags::default(),
)
.map_err(|e| {
log::error!("Failed to reexecute a tx: {}", e);
log!(debug, "Failed to reexecute a tx: {}", e);
PlaceHolderErrorTypeForFailedStarknetExecution
});

let res = res
.map(|r| if with_state_diff { (r, Some(transactional_state.to_state_diff())) } else { (r, None) });
commit_transactional_state(transactional_state).map_err(|e| {
log::error!("Failed to commit state changes: {:?}", e);
log!(debug, "Failed to commit state changes: {:?}", e);
PlaceHolderErrorTypeForFailedStarknetExecution
})?;

Expand Down Expand Up @@ -316,7 +333,7 @@ impl<T: Config> Pallet<T> {
// Once the state diff of this tx is generated, we apply those changes on the original state
// so that next txs being simulated are ontop of this one (avoid nonce error)
commit_transactional_state(transactional_state).map_err(|e| {
log::error!("Failed to commit state changes: {:?}", e);
log!(error, "Failed to commit state changes: {:?}", e);
PlaceHolderErrorTypeForFailedStarknetExecution
})?;

Expand Down
10 changes: 8 additions & 2 deletions crates/pallets/starknet/src/transaction_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ impl<T: Config> Pallet<T> {
}
}
// TODO: have more granular error mapping
.map_err(|_| InvalidTransaction::BadProof)
.map_err(|e| {
log!(debug, "Error in pre_validate_unsigned_tx: {:?}", e);
InvalidTransaction::BadProof
})
}
Transaction::L1HandlerTransaction(transaction) => {
Self::ensure_l1_message_not_executed(&transaction.tx.nonce)
Expand Down Expand Up @@ -141,7 +144,10 @@ impl<T: Config> Pallet<T> {
Ok(None)
}
}
.map_err(|_| InvalidTransaction::BadProof)?;
.map_err(|e| {
log!(debug, "Error in validate_unsigned_tx: {:?}", e);
InvalidTransaction::BadProof
})?;

Ok(())
}
Expand Down
3 changes: 3 additions & 0 deletions crates/primitives/simulations/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ impl From<Vec<SimulationFlagForEstimateFee>> for SimulationFlags {
}
}

// estimate_fee does not charge fees or do any balance checks
flags_out.charge_fee = false;

flags_out
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/primitives/transactions/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ serde_json = { workspace = true, optional = true }
thiserror = { workspace = true, optional = true }

# Other optional
log = "0.4.20"
parity-scale-codec = { workspace = true, features = [
"derive",
], optional = true }
Expand Down
Loading

0 comments on commit b2730f6

Please sign in to comment.