soroban-rpc: simulateTransaction: try to return diagnostic events on …

…failure (#878)

cmd/soroban-rpc/internal/methods/simulate_transaction.go

@@ -35,11 +35,11 @@ type RestorePreamble struct {
 
 type SimulateTransactionResponse struct {
 	Error           string                       `json:"error,omitempty"`
-	TransactionData string                       `json:"transactionData"` // SorobanTransactionData XDR in base64
-	MinResourceFee  int64                        `json:"minResourceFee,string"`
+	TransactionData string                       `json:"transactionData,omitempty"` // SorobanTransactionData XDR in base64
+	MinResourceFee  int64                        `json:"minResourceFee,string,omitempty"`
 	Events          []string                     `json:"events,omitempty"`          // DiagnosticEvent XDR in base64
 	Results         []SimulateHostFunctionResult `json:"results,omitempty"`         // an array of the individual host function call results
-	Cost            SimulateTransactionCost      `json:"cost"`                      // the effective cpu and memory cost of the invoked transaction execution.
+	Cost            SimulateTransactionCost      `json:"cost,omitempty"`            // the effective cpu and memory cost of the invoked transaction execution.
 	RestorePreamble RestorePreamble              `json:"restorePreamble,omitempty"` // If present, it indicates that a prior RestoreFootprint is required
 	LatestLedger    int64                        `json:"latestLedger,string"`
 }
@@ -136,6 +136,7 @@ func NewSimulateTransactionHandler(logger *log.Entry, ledgerEntryReader db.Ledge
 		}
 
 		return SimulateTransactionResponse{
+			Error:           result.Error,
 			Results:         results,
 			Events:          result.Events,
 			TransactionData: result.TransactionData,

cmd/soroban-rpc/internal/preflight/preflight.go

@@ -77,6 +77,7 @@ type PreflightParameters struct {
 }
 
 type Preflight struct {
+	Error                     string
 	Events                    []string // DiagnosticEvents XDR in base64
 	TransactionData           string   // SorobanTransactionData XDR in base64
 	MinFee                    int64
@@ -147,7 +148,7 @@ func getFootprintExpirationPreflight(params PreflightParameters) (Preflight, err
 	C.free(unsafe.Pointer(opBodyCString))
 	C.free(unsafe.Pointer(footprintCString))
 
-	return GoPreflight(res)
+	return GoPreflight(res), nil
 }
 
 func getSimulationLedgerSeq(readTx db.LedgerEntryReadTx) (uint32, error) {
@@ -218,17 +219,14 @@ func getInvokeHostFunctionPreflight(params PreflightParameters) (Preflight, erro
 	C.free(unsafe.Pointer(invokeHostFunctionCString))
 	C.free(unsafe.Pointer(sourceAccountCString))
 
-	return GoPreflight(res)
+	return GoPreflight(res), nil
 }
 
-func GoPreflight(result *C.CPreflightResult) (Preflight, error) {
+func GoPreflight(result *C.CPreflightResult) Preflight {
 	defer C.free_preflight_result(result)
 
-	if result.error != nil {
-		return Preflight{}, errors.New(C.GoString(result.error))
-	}
-
 	preflight := Preflight{
+		Error:                     C.GoString(result.error),
 		Events:                    GoNullTerminatedStringSlice(result.events),
 		TransactionData:           C.GoString(result.transaction_data),
 		MinFee:                    int64(result.min_fee),
@@ -239,5 +237,5 @@ func GoPreflight(result *C.CPreflightResult) (Preflight, error) {
 		PreRestoreTransactionData: C.GoString(result.pre_restore_transaction_data),
 		PreRestoreMinFee:          int64(result.pre_restore_min_fee),
 	}
-	return preflight, nil
+	return preflight
 }

cmd/soroban-rpc/internal/preflight/preflight_test.go

@@ -305,7 +305,6 @@ func getPreflightParameters(t testing.TB, inMemory bool) PreflightParameters {
 }
 
 func TestGetPreflight(t *testing.T) {
-
 	params := getPreflightParameters(t, false)
 	_, err := GetPreflight(context.Background(), params)
 	require.NoError(t, err)

cmd/soroban-rpc/internal/test/simulate_transaction_test.go

@@ -526,6 +526,9 @@ func TestSimulateTransactionError(t *testing.T) {
 	result := simulateTransactionFromTxParams(t, client, params)
 	assert.Greater(t, result.LatestLedger, int64(0))
 	assert.Contains(t, result.Error, "MissingValue")
+	require.Len(t, result.Events, 1)
+	var event xdr.DiagnosticEvent
+	require.NoError(t, xdr.SafeUnmarshalBase64(result.Events[0], &event))
 }
 
 func TestSimulateTransactionMultipleOperations(t *testing.T) {

cmd/soroban-rpc/lib/preflight/src/fees.rs

@@ -364,7 +364,7 @@ fn compute_bump_footprint_rent_changes(
     for key in (&footprint).read_only.as_vec() {
         let unmodified_entry = ledger_storage
             .get(key, false)
-            .with_context(|| format!("cannot get ledger entry with key {key:?}"))?;
+            .with_context(|| format!("cannot find bump footprint ledger entry with key {key:?}"))?;
         let size = (key.to_xdr()?.len() + unmodified_entry.to_xdr()?.len()) as u32;
         let expirable_entry: Box<dyn ExpirableLedgerEntry> =
             (&unmodified_entry).try_into().map_err(|e: String| {
@@ -396,7 +396,7 @@ pub(crate) fn compute_restore_footprint_transaction_data_and_min_fee(
     let ConfigSettingEntry::StateExpiration(state_expiration) =
         ledger_storage.get_configuration_setting(ConfigSettingId::StateExpiration)?
         else {
-            bail!("get_fee_configuration(): unexpected config setting entry for StateExpiration key");
+            bail!("unexpected config setting entry for StateExpiration key");
         };
     let rent_changes = compute_restore_footprint_rent_changes(
         &footprint,
@@ -454,9 +454,9 @@ fn compute_restore_footprint_rent_changes(
     let mut rent_changes: Vec<LedgerEntryRentChange> =
         Vec::with_capacity(footprint.read_write.len());
     for key in footprint.read_write.as_vec() {
-        let unmodified_entry = ledger_storage
-            .get(key, true)
-            .with_context(|| format!("cannot get ledger entry with key {key:?}"))?;
+        let unmodified_entry = ledger_storage.get(key, true).with_context(|| {
+            format!("cannot find restore footprint ledger entry with key {key:?}")
+        })?;
         let size = (key.to_xdr()?.len() + unmodified_entry.to_xdr()?.len()) as u32;
         let expirable_entry: Box<dyn ExpirableLedgerEntry> =
             (&unmodified_entry).try_into().map_err(|e: String| {

cmd/soroban-rpc/lib/preflight/src/lib.rs

@@ -69,13 +69,16 @@ pub struct CPreflightResult {
 impl From<PreflightResult> for CPreflightResult {
     fn from(p: PreflightResult) -> Self {
         let mut result = Self {
-            error: null_mut(),
+            error: string_to_c(p.error),
             auth: xdr_vec_to_base64_c_null_terminated_char_array(p.auth),
             result: match p.result {
                 None => null_mut(),
                 Some(v) => xdr_to_base64_c(v),
             },
-            transaction_data: xdr_to_base64_c(p.transaction_data),
+            transaction_data: match p.transaction_data {
+                None => null_mut(),
+                Some(v) => xdr_to_base64_c(v),
+            },
             min_fee: p.min_fee,
             events: xdr_vec_to_base64_c_null_terminated_char_array(p.events),
             cpu_instructions: p.cpu_instructions,

cmd/soroban-rpc/lib/preflight/src/preflight.rs

@@ -11,6 +11,7 @@ use soroban_env_host::xdr::{
     SorobanAuthorizationEntry, SorobanCredentials, SorobanTransactionData, VecM,
 };
 use soroban_env_host::{DiagnosticLevel, Host, LedgerInfo};
+use std::collections::HashSet;
 use std::convert::{TryFrom, TryInto};
 use std::iter::FromIterator;
 use std::rc::Rc;
@@ -20,10 +21,12 @@ pub(crate) struct RestorePreamble {
     pub(crate) min_fee: i64,
 }
 
+#[derive(Default)]
 pub(crate) struct PreflightResult {
+    pub(crate) error: String,
     pub(crate) auth: Vec<SorobanAuthorizationEntry>,
     pub(crate) result: Option<ScVal>,
-    pub(crate) transaction_data: SorobanTransactionData,
+    pub(crate) transaction_data: Option<SorobanTransactionData>,
     pub(crate) min_fee: i64,
     pub(crate) events: Vec<DiagnosticEvent>,
     pub(crate) cpu_instructions: u64,
@@ -68,9 +71,9 @@ pub(crate) fn preflight_invoke_hf_op(
         .context("cannot set ledger info")?;
 
     // Run the preflight.
-    let result = host
+    let maybe_result = host
         .invoke_function(invoke_hf_op.host_function.clone())
-        .context("host invocation failed")?;
+        .context("host invocation failed");
     let auths: VecM<SorobanAuthorizationEntry> = if needs_auth_recording {
         let payloads = host.get_recorded_auth_payloads()?;
         VecM::try_from(
@@ -88,6 +91,19 @@ pub(crate) fn preflight_invoke_hf_op(
     let (storage, events) = host.try_finish().context("cannot finish host invocation")?;
 
     let diagnostic_events = host_events_to_diagnostic_events(&events);
+    let result = match maybe_result {
+        Ok(r) => r,
+        // If the invocation failed, try to at least add the diagnostic events
+        Err(e) => {
+            return Ok(PreflightResult {
+                // See https://docs.rs/anyhow/latest/anyhow/struct.Error.html#display-representations
+                error: format!("{e:?}"),
+                events: diagnostic_events,
+                ..Default::default()
+            });
+        }
+    };
+
     let invoke_host_function_with_auth = InvokeHostFunctionOp {
         host_function: invoke_hf_op.host_function,
         auth: auths.clone(),
@@ -103,33 +119,18 @@ pub(crate) fn preflight_invoke_hf_op(
     )
     .context("cannot compute resources and fees")?;
 
-    let entries = ledger_storage_rc.get_ledger_keys_requiring_restore();
-    let restore_preamble = if entries.is_empty() {
-        None
-    } else {
-        let read_write_vec: Vec<LedgerKey> = Vec::from_iter(entries);
-        let restore_footprint = LedgerFootprint {
-            read_only: VecM::default(),
-            read_write: read_write_vec.try_into()?,
-        };
-        let (transaction_data, min_fee) =
-            fees::compute_restore_footprint_transaction_data_and_min_fee(
-                restore_footprint,
-                &ledger_storage_rc,
-                bucket_list_size,
-                ledger_info.sequence_number,
-            )
-            .context("cannot compute restore preamble")?;
-        Some(RestorePreamble {
-            transaction_data,
-            min_fee,
-        })
-    };
+    let restore_preamble = compute_restore_preamble(
+        ledger_storage_rc.get_ledger_keys_requiring_restore(),
+        &ledger_storage_rc,
+        bucket_list_size,
+        ledger_info.sequence_number,
+    )
+    .context("cannot compute restore preamble")?;
 
     Ok(PreflightResult {
         auth: auths.to_vec(),
         result: Some(result),
-        transaction_data,
+        transaction_data: Some(transaction_data),
         min_fee,
         events: diagnostic_events,
         cpu_instructions: budget
@@ -139,6 +140,7 @@ pub(crate) fn preflight_invoke_hf_op(
             .get_mem_bytes_consumed()
             .context("cannot get consumed memory")?,
         restore_preamble,
+        ..Default::default()
     })
 }
 
@@ -168,6 +170,32 @@ fn recorded_auth_payload_to_xdr(
     Ok(result)
 }
 
+fn compute_restore_preamble(
+    entries: HashSet<LedgerKey>,
+    ledger_storage: &LedgerStorage,
+    bucket_list_size: u64,
+    current_ledger_seq: u32,
+) -> Result<Option<RestorePreamble>> {
+    if entries.is_empty() {
+        return Ok(None);
+    }
+    let read_write_vec: Vec<LedgerKey> = Vec::from_iter(entries);
+    let restore_footprint = LedgerFootprint {
+        read_only: VecM::default(),
+        read_write: read_write_vec.try_into()?,
+    };
+    let (transaction_data, min_fee) = fees::compute_restore_footprint_transaction_data_and_min_fee(
+        restore_footprint,
+        ledger_storage,
+        bucket_list_size,
+        current_ledger_seq,
+    )?;
+    Ok(Some(RestorePreamble {
+        transaction_data,
+        min_fee,
+    }))
+}
+
 fn host_events_to_diagnostic_events(events: &Events) -> Vec<DiagnosticEvent> {
     let mut res: Vec<DiagnosticEvent> = Vec::with_capacity(events.0.len());
     for e in &events.0 {
@@ -184,19 +212,19 @@ fn get_budget_from_network_config_params(ledger_storage: &LedgerStorage) -> Resu
     let ConfigSettingEntry::ContractComputeV0(compute) =
         ledger_storage.get_configuration_setting(ConfigSettingId::ContractComputeV0)?
         else {
-            bail!("get_budget_from_network_config_params(): unexpected config setting entry for ComputeV0 key");
+            bail!("unexpected config setting entry for ComputeV0 key");
         };
 
     let ConfigSettingEntry::ContractCostParamsCpuInstructions(cost_params_cpu) = ledger_storage
         .get_configuration_setting(ConfigSettingId::ContractCostParamsCpuInstructions)?
         else {
-            bail!("get_budget_from_network_config_params(): unexpected config setting entry for ComputeV0 key");
+            bail!("unexpected config setting entry for CostParamsCpuInstructions key");
         };
 
     let ConfigSettingEntry::ContractCostParamsMemoryBytes(cost_params_memory) =
         ledger_storage.get_configuration_setting(ConfigSettingId::ContractCostParamsMemoryBytes)?
         else {
-            bail!("get_budget_from_network_config_params(): unexpected config setting entry for ComputeV0 key");
+            bail!("unexpected config setting entry for CostParamsMemoryBytes key");
         };
 
     let budget = Budget::try_from_configs(
@@ -253,14 +281,9 @@ fn preflight_bump_footprint_expiration(
             current_ledger_seq,
         )?;
     Ok(PreflightResult {
-        auth: vec![],
-        result: None,
-        transaction_data,
+        transaction_data: Some(transaction_data),
         min_fee,
-        events: vec![],
-        cpu_instructions: 0,
-        memory_bytes: 0,
-        restore_preamble: None,
+        ..Default::default()
     })
 }
 
@@ -277,13 +300,8 @@ fn preflight_restore_footprint(
         current_ledger_seq,
     )?;
     Ok(PreflightResult {
-        auth: vec![],
-        result: None,
-        transaction_data,
+        transaction_data: Some(transaction_data),
         min_fee,
-        events: vec![],
-        cpu_instructions: 0,
-        memory_bytes: 0,
-        restore_preamble: None,
+        ..Default::default()
     })
 }