Skip to content

Commit

Permalink
Merge pull request #2281 from eqlabs/sistemd/starknet_getTransactionS…
Browse files Browse the repository at this point in the history
…tatus-add-failure-reason

feat(rpc): add `failure_reason` to `starknet_getTransactionStatus`
  • Loading branch information
sistemd authored Oct 4, 2024
2 parents dfe0de0 + 9aec626 commit 6058abd
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 25 deletions.
12 changes: 9 additions & 3 deletions crates/gateway-types/src/reply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,17 +187,17 @@ pub mod call {

/// Used to deserialize replies to Starknet transaction requests.
///
/// We only care about the statuses so we ignore other fields.
/// Please note that this does not have to be backwards compatible:
/// since we only ever use it to deserialize replies from the Starknet
/// feeder gateway.
#[serde_as]
#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq)]
#[derive(Clone, Debug, Deserialize, PartialEq, Eq)]
pub struct TransactionStatus {
pub status: Status,
pub finality_status: transaction_status::FinalityStatus,
#[serde(default)]
pub execution_status: transaction_status::ExecutionStatus,
pub transaction_failure_reason: Option<transaction_status::TransactionFailureReason>,
pub revert_error: Option<String>,
}

/// Types used when deserializing get_transaction replies.
Expand All @@ -224,6 +224,12 @@ pub mod transaction_status {
Reverted,
Rejected,
}

#[derive(Clone, Debug, Deserialize, PartialEq, Eq)]
pub struct TransactionFailureReason {
pub code: String,
pub error_message: String,
}
}

/// Types used when deserializing L2 transaction related data.
Expand Down
25 changes: 18 additions & 7 deletions crates/rpc/src/dto/receipt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,23 @@ pub enum TxnStatus {
AcceptedOnL1,
}

#[derive(Copy, Clone, Debug, PartialEq)]
#[derive(Clone, Debug, PartialEq)]
pub enum TxnExecutionStatus {
Succeeded,
Reverted,
Reverted {
// Revert reason optional for backward compatibility with gateway.
reason: Option<String>,
},
}

impl From<&pathfinder_common::receipt::ExecutionStatus> for TxnExecutionStatus {
fn from(value: &pathfinder_common::receipt::ExecutionStatus) -> Self {
use pathfinder_common::receipt::ExecutionStatus;
match value {
ExecutionStatus::Succeeded => Self::Succeeded,
ExecutionStatus::Reverted { .. } => Self::Reverted,
ExecutionStatus::Reverted { reason } => Self::Reverted {
reason: Some(reason.clone()),
},
}
}
}
Expand Down Expand Up @@ -91,7 +96,7 @@ impl SerializeForVersion for TxnExecutionStatus {
fn serialize(&self, serializer: Serializer) -> Result<serialize::Ok, serialize::Error> {
match self {
TxnExecutionStatus::Succeeded => "SUCCEEDED",
TxnExecutionStatus::Reverted => "REVERTED",
TxnExecutionStatus::Reverted { .. } => "REVERTED",
}
.serialize(serializer)
}
Expand All @@ -108,7 +113,12 @@ impl SerializeForVersion for TxnExecutionStatusWithRevertReason<'_> {
serializer.serialize_field("execution_status", &TxnExecutionStatus::Succeeded)?;
}
ExecutionStatus::Reverted { reason } => {
serializer.serialize_field("execution_status", &TxnExecutionStatus::Reverted)?;
serializer.serialize_field(
"execution_status",
&TxnExecutionStatus::Reverted {
reason: Some(reason.clone()),
},
)?;
serializer.serialize_field("revert_reason", reason)?;
}
}
Expand Down Expand Up @@ -449,8 +459,9 @@ mod tests {
}

#[rstest]
#[case::accepted_on_l2(TxnExecutionStatus::Succeeded, "SUCCEEDED")]
#[case::accepted_on_l1(TxnExecutionStatus::Reverted, "REVERTED")]
#[case::succeeded(TxnExecutionStatus::Succeeded, "SUCCEEDED")]
#[case::reverted_missing_reason(TxnExecutionStatus::Reverted { reason: None }, "REVERTED")]
#[case::reverted_with_reason(TxnExecutionStatus::Reverted { reason: Some("Reverted because".to_string()) }, "REVERTED")]
fn txn_execution_status(#[case] input: TxnExecutionStatus, #[case] expected: &str) {
let expected = json!(expected);
let encoded = input.serialize(Serializer::default()).unwrap();
Expand Down
32 changes: 31 additions & 1 deletion crates/rpc/src/jsonrpc/websocket/logic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ async fn transaction_status_subscription(
match gateway.transaction(transaction_hash).await {
Ok(tx_status) => {
num_consecutive_errors = 0;
let update = match (tx_status.finality_status, tx_status.execution_status) {
let update = match (tx_status.finality_status, &tx_status.execution_status) {
(_, ExecutionStatus::Rejected) => Some(TransactionStatusUpdate::Rejected),
(FinalityStatus::NotReceived, _) => {
// "NOT_RECEIVED" status is never sent to the client.
Expand Down Expand Up @@ -1206,26 +1206,36 @@ mod tests {
status: Status::NotReceived,
finality_status: FinalityStatus::NotReceived,
execution_status: ExecutionStatus::Succeeded,
transaction_failure_reason: None,
revert_error: None,
},
TransactionStatus {
status: Status::Received,
finality_status: FinalityStatus::Received,
execution_status: ExecutionStatus::Succeeded,
transaction_failure_reason: None,
revert_error: None,
},
TransactionStatus {
status: Status::NotReceived,
finality_status: FinalityStatus::NotReceived,
execution_status: ExecutionStatus::Succeeded,
transaction_failure_reason: None,
revert_error: None,
},
TransactionStatus {
status: Status::Received,
finality_status: FinalityStatus::Received,
execution_status: ExecutionStatus::Succeeded,
transaction_failure_reason: None,
revert_error: None,
},
TransactionStatus {
status: Status::AcceptedOnL1,
finality_status: FinalityStatus::AcceptedOnL1,
execution_status: ExecutionStatus::Succeeded,
transaction_failure_reason: None,
revert_error: None,
},
]
.into_iter()
Expand Down Expand Up @@ -1291,26 +1301,36 @@ mod tests {
status: Status::NotReceived,
finality_status: FinalityStatus::NotReceived,
execution_status: ExecutionStatus::Succeeded,
transaction_failure_reason: None,
revert_error: None,
},
TransactionStatus {
status: Status::Received,
finality_status: FinalityStatus::Received,
execution_status: ExecutionStatus::Succeeded,
transaction_failure_reason: None,
revert_error: None,
},
TransactionStatus {
status: Status::NotReceived,
finality_status: FinalityStatus::NotReceived,
execution_status: ExecutionStatus::Succeeded,
transaction_failure_reason: None,
revert_error: None,
},
TransactionStatus {
status: Status::Received,
finality_status: FinalityStatus::Received,
execution_status: ExecutionStatus::Succeeded,
transaction_failure_reason: None,
revert_error: None,
},
TransactionStatus {
status: Status::AcceptedOnL1,
finality_status: FinalityStatus::AcceptedOnL1,
execution_status: ExecutionStatus::Reverted,
transaction_failure_reason: None,
revert_error: None,
},
]
.into_iter()
Expand Down Expand Up @@ -1376,26 +1396,36 @@ mod tests {
status: Status::NotReceived,
finality_status: FinalityStatus::NotReceived,
execution_status: ExecutionStatus::Succeeded,
transaction_failure_reason: None,
revert_error: None,
},
TransactionStatus {
status: Status::Received,
finality_status: FinalityStatus::Received,
execution_status: ExecutionStatus::Succeeded,
transaction_failure_reason: None,
revert_error: None,
},
TransactionStatus {
status: Status::NotReceived,
finality_status: FinalityStatus::NotReceived,
execution_status: ExecutionStatus::Succeeded,
transaction_failure_reason: None,
revert_error: None,
},
TransactionStatus {
status: Status::Received,
finality_status: FinalityStatus::Received,
execution_status: ExecutionStatus::Succeeded,
transaction_failure_reason: None,
revert_error: None,
},
TransactionStatus {
status: Status::Rejected,
finality_status: FinalityStatus::NotReceived,
execution_status: ExecutionStatus::Rejected,
transaction_failure_reason: None,
revert_error: None,
},
]
.into_iter()
Expand Down
72 changes: 58 additions & 14 deletions crates/rpc/src/method/get_transaction_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use pathfinder_common::TransactionHash;

use crate::context::RpcContext;
use crate::dto::TxnExecutionStatus;
use crate::RpcVersion;

#[derive(Debug, PartialEq, Eq)]
pub struct Input {
Expand All @@ -22,7 +23,10 @@ impl crate::dto::DeserializeForVersion for Input {
#[derive(Debug, PartialEq)]
pub enum Output {
Received,
Rejected,
Rejected {
// Reject error message optional for backward compatibility with gateway.
error_message: Option<String>,
},
AcceptedOnL1(TxnExecutionStatus),
AcceptedOnL2(TxnExecutionStatus),
}
Expand Down Expand Up @@ -96,16 +100,24 @@ pub async fn get_transaction_status(context: RpcContext, input: Input) -> Result

match (tx.finality_status, tx.execution_status) {
(GatewayFinalityStatus::NotReceived, _) => Err(Error::TxnHashNotFound),
(_, GatewayExecutionStatus::Rejected) => Ok(Output::Rejected),
(_, GatewayExecutionStatus::Rejected) => Ok(Output::Rejected {
error_message: tx
.transaction_failure_reason
.map(|reason| reason.error_message),
}),
(GatewayFinalityStatus::Received, _) => Ok(Output::Received),
(GatewayFinalityStatus::AcceptedOnL1, GatewayExecutionStatus::Reverted) => {
Ok(Output::AcceptedOnL1(TxnExecutionStatus::Reverted))
Ok(Output::AcceptedOnL1(TxnExecutionStatus::Reverted {
reason: tx.revert_error,
}))
}
(GatewayFinalityStatus::AcceptedOnL1, GatewayExecutionStatus::Succeeded) => {
Ok(Output::AcceptedOnL1(TxnExecutionStatus::Succeeded))
}
(GatewayFinalityStatus::AcceptedOnL2, GatewayExecutionStatus::Reverted) => {
Ok(Output::AcceptedOnL2(TxnExecutionStatus::Reverted))
Ok(Output::AcceptedOnL2(TxnExecutionStatus::Reverted {
reason: tx.revert_error,
}))
}
(GatewayFinalityStatus::AcceptedOnL2, GatewayExecutionStatus::Succeeded) => {
Ok(Output::AcceptedOnL2(TxnExecutionStatus::Succeeded))
Expand All @@ -119,17 +131,26 @@ impl Output {
use crate::dto::TxnStatus;
match self {
Output::Received => TxnStatus::Received,
Output::Rejected => TxnStatus::Rejected,
Output::Rejected { .. } => TxnStatus::Rejected,
Output::AcceptedOnL1(_) => TxnStatus::AcceptedOnL1,
Output::AcceptedOnL2(_) => TxnStatus::AcceptedOnL2,
}
}

fn execution_status(&self) -> Option<TxnExecutionStatus> {
match self {
Output::Received | Output::Rejected => None,
Output::AcceptedOnL1(x) => Some(*x),
Output::AcceptedOnL2(x) => Some(*x),
Output::Received | Output::Rejected { .. } => None,
Output::AcceptedOnL1(x) => Some(x.clone()),
Output::AcceptedOnL2(x) => Some(x.clone()),
}
}

fn failure_reason(&self) -> Option<String> {
match self {
Output::Rejected { error_message } => error_message.clone(),
Output::AcceptedOnL1(TxnExecutionStatus::Reverted { reason }) => reason.clone(),
Output::AcceptedOnL2(TxnExecutionStatus::Reverted { reason }) => reason.clone(),
_ => None,
}
}
}
Expand All @@ -142,6 +163,10 @@ impl crate::dto::serialize::SerializeForVersion for Output {
let mut serializer = serializer.serialize_struct()?;
serializer.serialize_field("finality_status", &self.finality_status())?;
serializer.serialize_optional("execution_status", self.execution_status())?;
// Delete check once rustc gives you a friendly reminder
if serializer.version != RpcVersion::V07 {
serializer.serialize_optional("failure_reason", self.failure_reason())?;
}
serializer.end()
}
}
Expand All @@ -156,14 +181,14 @@ mod tests {
use super::*;

#[rstest::rstest]
#[case::rejected(Output::Rejected, json!({"finality_status":"REJECTED"}))]
#[case::rejected(Output::Rejected { error_message: None }, json!({"finality_status":"REJECTED"}))]
#[case::reverted(Output::Received, json!({"finality_status":"RECEIVED"}))]
#[case::accepted_on_l1_succeeded(
Output::AcceptedOnL1(TxnExecutionStatus::Succeeded),
json!({"finality_status":"ACCEPTED_ON_L1","execution_status":"SUCCEEDED"})
)]
#[case::accepted_on_l2_reverted(
Output::AcceptedOnL2(TxnExecutionStatus::Reverted),
Output::AcceptedOnL2(TxnExecutionStatus::Reverted{ reason: None }),
json!({"finality_status":"ACCEPTED_ON_L2","execution_status":"REVERTED"})
)]
fn output_serialization(#[case] output: Output, #[case] expected: serde_json::Value) {
Expand Down Expand Up @@ -211,7 +236,7 @@ mod tests {
}

#[tokio::test]
async fn rejected() {
async fn rejected_with_error_message() {
let input = Input {
// Transaction hash known to be rejected by the testnet gateway.
transaction_hash: transaction_hash!(
Expand All @@ -221,7 +246,16 @@ mod tests {
let context = RpcContext::for_tests();
let status = get_transaction_status(context, input).await.unwrap();

assert_eq!(status, Output::Rejected);
assert_eq!(
status,
Output::Rejected {
error_message: Some(
"Transaction is too big to fit a batch; Its gas_weight weights 5214072 while \
the batch upper bound is set to 5000000.0."
.to_string()
)
}
);
}

#[tokio::test]
Expand All @@ -233,13 +267,23 @@ mod tests {
let status = get_transaction_status(context.clone(), input)
.await
.unwrap();
assert_eq!(status, Output::AcceptedOnL2(TxnExecutionStatus::Reverted));
assert_eq!(
status,
Output::AcceptedOnL2(TxnExecutionStatus::Reverted {
reason: Some("Reverted because".to_string())
})
);

let input = Input {
transaction_hash: transaction_hash_bytes!(b"pending reverted"),
};
let status = get_transaction_status(context, input).await.unwrap();
assert_eq!(status, Output::AcceptedOnL2(TxnExecutionStatus::Reverted));
assert_eq!(
status,
Output::AcceptedOnL2(TxnExecutionStatus::Reverted {
reason: Some("Reverted!".to_string())
})
);
}

#[tokio::test]
Expand Down
1 change: 1 addition & 0 deletions crates/rpc/src/v08.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::method::subscribe_pending_transactions::SubscribePendingTransactions;
pub fn register_routes() -> RpcRouterBuilder {
RpcRouter::builder(crate::RpcVersion::V08)
.register("starknet_syncing", crate::method::syncing)
.register("starknet_getTransactionStatus", crate::method::get_transaction_status)
.register("starknet_subscribeNewHeads", SubscribeNewHeads)
.register("starknet_subscribePendingTransactions", SubscribePendingTransactions)
.register("starknet_subscribeEvents", SubscribeEvents)
Expand Down

0 comments on commit 6058abd

Please sign in to comment.