From 1c4a3e635464e3b3367639041131ff0ac73decb5 Mon Sep 17 00:00:00 2001 From: wojo Date: Wed, 4 Dec 2024 15:18:37 +0100 Subject: [PATCH 1/3] Add NotReceived case handling in adaptTransactionStatus The NotReceived case in adaptTransactionStatus should return nil, nil instead of falling through to default case. This allows the error to naturally propagate from the original ErrTxnHashNotFound in TransactionStatus without logging unnecessary errors for the common case of transactions not being found. --- rpc/transaction.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rpc/transaction.go b/rpc/transaction.go index 8e3c3956f5..55b56a8ad5 100644 --- a/rpc/transaction.go +++ b/rpc/transaction.go @@ -588,6 +588,9 @@ func (h *Handler) TransactionStatus(ctx context.Context, hash felt.Felt) (*Trans h.log.Errorw("Failed to adapt transaction status", "err", err) return nil, ErrTxnHashNotFound } + if status == nil { + return nil, ErrTxnHashNotFound + } return status, nil } @@ -751,6 +754,8 @@ func adaptTransactionStatus(txStatus *starknet.TransactionStatus) (*TransactionS status.Finality = TxnStatusAcceptedOnL2 case starknet.Received: status.Finality = TxnStatusReceived + case starknet.NotReceived: + return nil, nil default: return nil, fmt.Errorf("unknown finality status: %v", finalityStatus) } From 30add4ee5e50b31fd25aab4cf710ddeca55c773c Mon Sep 17 00:00:00 2001 From: Rian Hughes Date: Thu, 5 Dec 2024 11:16:30 +0200 Subject: [PATCH 2/3] dont log error for not_received responses from the fgw (#2304) * dont log error for not_received responses from the fgw * lint --- rpc/transaction.go | 10 ++++------ starknet/compiler/rust/src/lib.rs | 15 +++++++++++---- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/rpc/transaction.go b/rpc/transaction.go index 55b56a8ad5..e6b539598d 100644 --- a/rpc/transaction.go +++ b/rpc/transaction.go @@ -585,13 +585,11 @@ func (h *Handler) TransactionStatus(ctx context.Context, hash felt.Felt) (*Trans status, err := adaptTransactionStatus(txStatus) if err != nil { - h.log.Errorw("Failed to adapt transaction status", "err", err) + if !errors.Is(err, fmt.Errorf("%s", ErrTxnHashNotFound.Message)) { + h.log.Errorw("Failed to adapt transaction status", "err", err) + } return nil, ErrTxnHashNotFound } - if status == nil { - return nil, ErrTxnHashNotFound - } - return status, nil } return nil, txErr @@ -755,7 +753,7 @@ func adaptTransactionStatus(txStatus *starknet.TransactionStatus) (*TransactionS case starknet.Received: status.Finality = TxnStatusReceived case starknet.NotReceived: - return nil, nil + return nil, fmt.Errorf("%s", ErrTxnHashNotFound.Message) default: return nil, fmt.Errorf("unknown finality status: %v", finalityStatus) } diff --git a/starknet/compiler/rust/src/lib.rs b/starknet/compiler/rust/src/lib.rs index a1027a3e3b..fa45765cbc 100644 --- a/starknet/compiler/rust/src/lib.rs +++ b/starknet/compiler/rust/src/lib.rs @@ -1,6 +1,8 @@ -use cairo_lang_starknet_classes::casm_contract_class::{CasmContractClass, StarknetSierraCompilationError}; +use cairo_lang_starknet_classes::casm_contract_class::{ + CasmContractClass, StarknetSierraCompilationError, +}; use std::ffi::{c_char, CStr, CString}; -use std::panic::{self,AssertUnwindSafe}; +use std::panic::{self, AssertUnwindSafe}; #[no_mangle] #[allow(clippy::not_unsafe_ptr_arg_deref)] @@ -25,9 +27,14 @@ pub extern "C" fn compileSierraToCasm(sierra_json: *const c_char, result: *mut * } }; - let mut casm_class_result: Option> = None; + let mut casm_class_result: Option> = + None; let compilation_result = panic::catch_unwind(AssertUnwindSafe(|| { - casm_class_result = Some(CasmContractClass::from_contract_class(sierra_class, true, usize::MAX)); + casm_class_result = Some(CasmContractClass::from_contract_class( + sierra_class, + true, + usize::MAX, + )); })); if let Err(_) = compilation_result { unsafe { From cd9a8b1a2ef92ec4d9de32414b75d6eaa45f9a26 Mon Sep 17 00:00:00 2001 From: wojo Date: Thu, 5 Dec 2024 14:27:29 +0100 Subject: [PATCH 3/3] Improve transaction status error handling Introduce a dedicated error variable for transaction not found cases in the adapter layer. This separates internal error handling from API errors and follows Go best practices by using errors.Is() for comparisons. The change: - Adds errTransactionNotFound for internal error handling - Replaces string-based error comparison with errors.Is() - Maintains existing API behavior while improving code maintainability --- rpc/transaction.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rpc/transaction.go b/rpc/transaction.go index e6b539598d..8157b4caa8 100644 --- a/rpc/transaction.go +++ b/rpc/transaction.go @@ -565,6 +565,8 @@ func (h *Handler) AddTransaction(ctx context.Context, tx BroadcastedTransaction) }, nil } +var errTransactionNotFound = fmt.Errorf("transaction not found") + func (h *Handler) TransactionStatus(ctx context.Context, hash felt.Felt) (*TransactionStatus, *jsonrpc.Error) { receipt, txErr := h.TransactionReceiptByHash(hash) switch txErr { @@ -585,7 +587,7 @@ func (h *Handler) TransactionStatus(ctx context.Context, hash felt.Felt) (*Trans status, err := adaptTransactionStatus(txStatus) if err != nil { - if !errors.Is(err, fmt.Errorf("%s", ErrTxnHashNotFound.Message)) { + if !errors.Is(err, errTransactionNotFound) { h.log.Errorw("Failed to adapt transaction status", "err", err) } return nil, ErrTxnHashNotFound @@ -753,7 +755,7 @@ func adaptTransactionStatus(txStatus *starknet.TransactionStatus) (*TransactionS case starknet.Received: status.Finality = TxnStatusReceived case starknet.NotReceived: - return nil, fmt.Errorf("%s", ErrTxnHashNotFound.Message) + return nil, errTransactionNotFound default: return nil, fmt.Errorf("unknown finality status: %v", finalityStatus) }