Skip to content

Commit

Permalink
Merge pull request #1576 from eqlabs/krisztian/rpc-0.6.0-fix-non-opti…
Browse files Browse the repository at this point in the history 
…onal-trace-invocations

fix(rpc/v06): align `TRANSACTION_TRACE` output with the spec
  • Loading branch information
@kkovaacs
kkovaacs authored Nov 27, 2023
2 parents 9744131 + a2ee610 commit 0943598
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 48 deletions.
82 changes: 54 additions & 28 deletions crates/rpc/src/v06/method/simulate_transactions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,10 @@ pub async fn simulate_transactions(

let txs =
pathfinder_executor::simulate(state, transactions, skip_validate, skip_fee_charge)?;
let txs = txs.into_iter().map(Into::into).collect();
let txs = txs
.into_iter()
.map(TryInto::try_into)
.collect::<Result<Vec<_>, _>>()?;
Ok(SimulateTransactionOutput(txs))
})
.await
Expand Down Expand Up @@ -373,15 +376,19 @@ pub mod dto {
L1Handler(L1HandlerTxnTrace),
}

impl From<pathfinder_executor::types::TransactionTrace> for TransactionTrace {
fn from(trace: pathfinder_executor::types::TransactionTrace) -> Self {
impl TryFrom<pathfinder_executor::types::TransactionTrace> for TransactionTrace {
type Error = pathfinder_executor::CallError;

fn try_from(
trace: pathfinder_executor::types::TransactionTrace,
) -> Result<Self, Self::Error> {
use pathfinder_executor::types::TransactionTrace::*;
match trace {
Ok(match trace {
Declare(t) => Self::Declare(t.into()),
DeployAccount(t) => Self::DeployAccount(t.into()),
DeployAccount(t) => Self::DeployAccount(t.try_into()?),
Invoke(t) => Self::Invoke(t.into()),
L1Handler(t) => Self::L1Handler(t.into()),
}
L1Handler(t) => Self::L1Handler(t.try_into()?),
})
}
}

Expand Down Expand Up @@ -409,22 +416,33 @@ pub mod dto {
#[derive(Clone, Debug, Serialize, Eq, PartialEq)]
pub struct DeployAccountTxnTrace {
#[serde(default)]
pub constructor_invocation: Option<FunctionInvocation>,
pub constructor_invocation: FunctionInvocation,
#[serde(default)]
pub fee_transfer_invocation: Option<FunctionInvocation>,
#[serde(default)]
pub validate_invocation: Option<FunctionInvocation>,
pub state_diff: Option<StateDiff>,
}

impl From<pathfinder_executor::types::DeployAccountTransactionTrace> for DeployAccountTxnTrace {
fn from(trace: pathfinder_executor::types::DeployAccountTransactionTrace) -> Self {
Self {
constructor_invocation: trace.constructor_invocation.map(Into::into),
impl TryFrom<pathfinder_executor::types::DeployAccountTransactionTrace> for DeployAccountTxnTrace {
type Error = pathfinder_executor::CallError;

fn try_from(
trace: pathfinder_executor::types::DeployAccountTransactionTrace,
) -> Result<Self, Self::Error> {
Ok(Self {
constructor_invocation: trace
.constructor_invocation
.ok_or_else(|| {
Self::Error::Custom(anyhow::anyhow!(
"Missing constructor_invocation in trace"
))
})?
.into(),
fee_transfer_invocation: trace.fee_transfer_invocation.map(Into::into),
validate_invocation: trace.validate_invocation.map(Into::into),
state_diff: Some(trace.state_diff.into()),
}
})
}
}

Expand Down Expand Up @@ -475,17 +493,25 @@ pub mod dto {
#[serde_with::skip_serializing_none]
#[derive(Clone, Debug, Serialize, Eq, PartialEq)]
pub struct L1HandlerTxnTrace {
#[serde(default)]
pub function_invocation: Option<FunctionInvocation>,
pub function_invocation: FunctionInvocation,
pub state_diff: Option<StateDiff>,
}

impl From<pathfinder_executor::types::L1HandlerTransactionTrace> for L1HandlerTxnTrace {
fn from(trace: pathfinder_executor::types::L1HandlerTransactionTrace) -> Self {
Self {
function_invocation: trace.function_invocation.map(Into::into),
impl TryFrom<pathfinder_executor::types::L1HandlerTransactionTrace> for L1HandlerTxnTrace {
type Error = pathfinder_executor::CallError;

fn try_from(
trace: pathfinder_executor::types::L1HandlerTransactionTrace,
) -> Result<Self, Self::Error> {
Ok(Self {
function_invocation: trace
.function_invocation
.ok_or_else(|| {
Self::Error::Custom(anyhow::anyhow!("Missing function_invocation in trace"))
})?
.into(),
state_diff: Some(trace.state_diff.into()),
}
})
}
}

Expand All @@ -498,12 +524,14 @@ pub mod dto {
pub transaction_trace: TransactionTrace,
}

impl From<TransactionSimulation> for SimulatedTransaction {
fn from(tx: TransactionSimulation) -> Self {
dto::SimulatedTransaction {
impl TryFrom<TransactionSimulation> for SimulatedTransaction {
type Error = pathfinder_executor::CallError;

fn try_from(tx: TransactionSimulation) -> Result<Self, Self::Error> {
Ok(dto::SimulatedTransaction {
fee_estimation: tx.fee_estimation.into(),
transaction_trace: tx.trace.into(),
}
transaction_trace: tx.trace.try_into()?,
})
}
}

Expand Down Expand Up @@ -636,8 +664,7 @@ pub(crate) mod tests {
transaction_trace:
TransactionTrace::DeployAccount(
DeployAccountTxnTrace {
constructor_invocation: Some(
FunctionInvocation {
constructor_invocation: FunctionInvocation {
call_type: CallType::Call,
caller_address: felt!("0x0"),
calls: vec![],
Expand All @@ -653,7 +680,6 @@ pub(crate) mod tests {
result: vec![],
execution_resources: ExecutionResources::default(),
},
),
validate_invocation: Some(
FunctionInvocation {
call_type: CallType::Call,
Expand Down
61 changes: 43 additions & 18 deletions crates/rpc/src/v06/method/trace_block_transactions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,25 +80,43 @@ impl From<CallError> for TraceBlockTransactionsError {
}
}

impl From<TraceConversionError> for TraceBlockTransactionsError {
fn from(value: TraceConversionError) -> Self {
Self::Custom(anyhow::anyhow!(value.0))
}
}

pub(super) struct TraceConversionError(pub &'static str);

pub(crate) fn map_gateway_trace(
transaction: GatewayTransaction,
trace: GatewayTxTrace,
) -> TransactionTrace {
match transaction {
) -> Result<TransactionTrace, TraceConversionError> {
Ok(match transaction {
GatewayTransaction::Declare(_) => TransactionTrace::Declare(DeclareTxnTrace {
fee_transfer_invocation: trace.fee_transfer_invocation.map(Into::into),
validate_invocation: trace.validate_invocation.map(Into::into),
state_diff: None,
}),
GatewayTransaction::Deploy(_) => TransactionTrace::DeployAccount(DeployAccountTxnTrace {
constructor_invocation: trace.function_invocation.map(Into::into),
constructor_invocation: trace
.function_invocation
.ok_or(TraceConversionError(
"Constructor_invocation is missing from trace response",
))?
.into(),
fee_transfer_invocation: trace.fee_transfer_invocation.map(Into::into),
validate_invocation: trace.validate_invocation.map(Into::into),
state_diff: None,
}),
GatewayTransaction::DeployAccount(_) => {
TransactionTrace::DeployAccount(DeployAccountTxnTrace {
constructor_invocation: trace.function_invocation.map(Into::into),
constructor_invocation: trace
.function_invocation
.ok_or(TraceConversionError(
"constructor_invocation is missing from trace response",
))?
.into(),
fee_transfer_invocation: trace.fee_transfer_invocation.map(Into::into),
validate_invocation: trace.validate_invocation.map(Into::into),
state_diff: None,
Expand All @@ -118,10 +136,15 @@ pub(crate) fn map_gateway_trace(
state_diff: None,
}),
GatewayTransaction::L1Handler(_) => TransactionTrace::L1Handler(L1HandlerTxnTrace {
function_invocation: trace.function_invocation.map(Into::into),
function_invocation: trace
.function_invocation
.ok_or(TraceConversionError(
"function_invocation is missing from trace response",
))?
.into(),
state_diff: None,
}),
}
})
}

pub async fn trace_block_transactions(
Expand Down Expand Up @@ -200,11 +223,13 @@ pub async fn trace_block_transactions(

let result = traces
.into_iter()
.map(|(hash, trace)| Trace {
transaction_hash: hash,
trace_root: trace.into(),
.map(|(hash, trace)| {
Ok(Trace {
transaction_hash: hash,
trace_root: trace.try_into()?,
})
})
.collect();
.collect::<Result<Vec<_>, TraceBlockTransactionsError>>()?;

Ok(LocalExecution::Success(result))
})
Expand All @@ -221,25 +246,25 @@ pub async fn trace_block_transactions(
.block_traces(input.block_id)
.await
.context("Forwarding to feeder gateway")
.map_err(Into::into)
.map_err(TraceBlockTransactionsError::from)
.map(|trace| {
TraceBlockTransactionsOutput(
Ok(TraceBlockTransactionsOutput(
trace
.traces
.into_iter()
.zip(transactions.into_iter())
.map(|(trace, tx)| {
let transaction_hash = tx.hash();
let trace_root = map_gateway_trace(tx, trace);
let trace_root = map_gateway_trace(tx, trace)?;

Trace {
Ok(Trace {
transaction_hash,
trace_root,
}
})
})
.collect(),
)
})
.collect::<Result<Vec<_>, TraceBlockTransactionsError>>()?,
))
})?
}

#[cfg(test)]
Expand Down
10 changes: 8 additions & 2 deletions crates/rpc/src/v06/method/trace_transaction.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ impl From<super::trace_block_transactions::TraceBlockTransactionsError> for Trac
}
}

impl From<super::trace_block_transactions::TraceConversionError> for TraceTransactionError {
fn from(value: super::trace_block_transactions::TraceConversionError) -> Self {
Self::Custom(anyhow::anyhow!(value.0))
}
}

impl From<TraceTransactionError> for ApplicationError {
fn from(value: TraceTransactionError) -> Self {
match value {
Expand Down Expand Up @@ -198,7 +204,7 @@ pub async fn trace_transaction(

pathfinder_executor::trace_one(state, transactions, input.transaction_hash, true, true)
.map_err(TraceTransactionError::from)
.map(|x| LocalExecution::Success(x.into()))
.and_then(|x| Ok(LocalExecution::Success(x.try_into()?)))
})
.await
.context("trace_transaction: execution")??;
Expand All @@ -214,7 +220,7 @@ pub async fn trace_transaction(
.await
.context("Proxying call to feeder gateway")?;

let trace = map_gateway_trace(transaction, trace);
let trace = map_gateway_trace(transaction, trace)?;

Ok(TraceTransactionOutput(trace))
}
Expand Down

0 comments on commit 0943598

Please sign in to comment.