From 93956eea441c107ca2ff8237b2fc8159c6932d94 Mon Sep 17 00:00:00 2001 From: Dmytro Kozhevin Date: Thu, 29 Feb 2024 21:05:08 -0500 Subject: [PATCH 1/2] Use the refactored version of soroban-simulation library. The refactored library encapsulates the network configuration in the library, so there is no longer a need to track the bucket list size from soroban-rpc. It also accepts the adjustment config that allows customizing the resource/refundable fee leeway without modifying the simulation crate itself. Besides the refactoring, soroban-simulation library is switched to use a more accurate recording mode implementation, which allows to decrease the necessary CPU instruction leeway significantly. --- Cargo.lock | 68 ++- Cargo.toml | 30 +- cmd/crates/soroban-rpc/src/txn.rs | 66 +-- .../internal/preflight/preflight.go | 55 +-- .../internal/preflight/preflight_test.go | 14 +- .../test/simulate_transaction_test.go | 6 +- cmd/soroban-rpc/lib/preflight.h | 7 +- cmd/soroban-rpc/lib/preflight/Cargo.toml | 4 +- cmd/soroban-rpc/lib/preflight/src/lib.rs | 440 ++++++++++++------ 9 files changed, 388 insertions(+), 302 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a730a8d5..ecd7f3dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1189,8 +1189,10 @@ checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" name = "preflight" version = "20.3.3" dependencies = [ + "anyhow", "base64 0.21.7", "libc", + "rand", "sha2", "soroban-env-host", "soroban-simulation", @@ -1547,9 +1549,8 @@ dependencies = [ [[package]] name = "soroban-builtin-sdk-macros" -version = "20.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "319fbfbf8a7fcaf9c69425d79d59819eedded4b3e56633fee10a4de1af833d51" +version = "20.2.2" +source = "git+https://github.com/stellar/rs-soroban-env?rev=8c9ab83c406bd86f56d52eae3e39dccf6b45b3da#8c9ab83c406bd86f56d52eae3e39dccf6b45b3da" dependencies = [ "itertools 0.11.0", "proc-macro2", @@ -1559,9 +1560,8 @@ dependencies = [ [[package]] name = "soroban-env-common" -version = "20.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b9d609330abbcc2d7fe185304de0c10ef1a95e64eb8effb6ee4faeea97668e0a" +version = "20.2.2" +source = "git+https://github.com/stellar/rs-soroban-env?rev=8c9ab83c406bd86f56d52eae3e39dccf6b45b3da#8c9ab83c406bd86f56d52eae3e39dccf6b45b3da" dependencies = [ "arbitrary", "crate-git-revision", @@ -1577,9 +1577,8 @@ dependencies = [ [[package]] name = "soroban-env-guest" -version = "20.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "280d73550935d482534abf3f897e89b40461b3401c3209163b3d0038f0b8b201" +version = "20.2.2" +source = "git+https://github.com/stellar/rs-soroban-env?rev=8c9ab83c406bd86f56d52eae3e39dccf6b45b3da#8c9ab83c406bd86f56d52eae3e39dccf6b45b3da" dependencies = [ "soroban-env-common", "static_assertions", @@ -1587,9 +1586,8 @@ dependencies = [ [[package]] name = "soroban-env-host" -version = "20.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd99f4e075f62e0faec118c568fbc70373793fb921148115d5f3f2563945c02d" +version = "20.2.2" +source = "git+https://github.com/stellar/rs-soroban-env?rev=8c9ab83c406bd86f56d52eae3e39dccf6b45b3da#8c9ab83c406bd86f56d52eae3e39dccf6b45b3da" dependencies = [ "backtrace", "curve25519-dalek", @@ -1614,9 +1612,8 @@ dependencies = [ [[package]] name = "soroban-env-macros" -version = "20.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c277d3828c200fef887ecd16bd24b2f16b7fc87207a23a94571ab909f2113d8" +version = "20.2.2" +source = "git+https://github.com/stellar/rs-soroban-env?rev=8c9ab83c406bd86f56d52eae3e39dccf6b45b3da#8c9ab83c406bd86f56d52eae3e39dccf6b45b3da" dependencies = [ "itertools 0.11.0", "proc-macro2", @@ -1629,9 +1626,8 @@ dependencies = [ [[package]] name = "soroban-ledger-snapshot" -version = "20.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a9cfeb46db19d0fb2e2c97d4e8aa102d660e0c80dc5412409a22dfd14241ca7" +version = "20.4.0" +source = "git+https://github.com/stellar/rs-soroban-sdk?rev=89efc3c211d41f1ab143bed0a09cd6af353bb098#89efc3c211d41f1ab143bed0a09cd6af353bb098" dependencies = [ "serde", "serde_json", @@ -1674,9 +1670,8 @@ dependencies = [ [[package]] name = "soroban-sdk" -version = "20.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "842c20c8503b137f8a8a5981009eb1f5841b96516f1485d7e1bf9c02afb227d1" +version = "20.4.0" +source = "git+https://github.com/stellar/rs-soroban-sdk?rev=89efc3c211d41f1ab143bed0a09cd6af353bb098#89efc3c211d41f1ab143bed0a09cd6af353bb098" dependencies = [ "bytes-lit", "rand", @@ -1691,9 +1686,8 @@ dependencies = [ [[package]] name = "soroban-sdk-macros" -version = "20.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0afc8337fadde3047fb774fa2abc3877a4a260b8e531868b65d5a1debc60b3b9" +version = "20.4.0" +source = "git+https://github.com/stellar/rs-soroban-sdk?rev=89efc3c211d41f1ab143bed0a09cd6af353bb098#89efc3c211d41f1ab143bed0a09cd6af353bb098" dependencies = [ "crate-git-revision", "darling", @@ -1711,9 +1705,8 @@ dependencies = [ [[package]] name = "soroban-simulation" -version = "20.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "210b093c6d08b8e85ef5f4e4a231d5fa25d1d2787d4fecd50e11040849f259ba" +version = "20.2.2" +source = "git+https://github.com/stellar/rs-soroban-env?rev=8c9ab83c406bd86f56d52eae3e39dccf6b45b3da#8c9ab83c406bd86f56d52eae3e39dccf6b45b3da" dependencies = [ "anyhow", "rand", @@ -1724,9 +1717,8 @@ dependencies = [ [[package]] name = "soroban-spec" -version = "20.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01f31b81de71543d314ef9252e11f2194606d6a4c8c350fbc701eeaa45a8e2d9" +version = "20.4.0" +source = "git+https://github.com/stellar/rs-soroban-sdk?rev=89efc3c211d41f1ab143bed0a09cd6af353bb098#89efc3c211d41f1ab143bed0a09cd6af353bb098" dependencies = [ "base64 0.13.1", "stellar-xdr", @@ -1736,9 +1728,8 @@ dependencies = [ [[package]] name = "soroban-spec-rust" -version = "20.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1468c9f0025166fc5e8853ba47cbd97d93c877757a5a600fcf71529e5b3b398c" +version = "20.4.0" +source = "git+https://github.com/stellar/rs-soroban-sdk?rev=89efc3c211d41f1ab143bed0a09cd6af353bb098#89efc3c211d41f1ab143bed0a09cd6af353bb098" dependencies = [ "prettyplease", "proc-macro2", @@ -1753,8 +1744,7 @@ dependencies = [ [[package]] name = "soroban-wasmi" version = "0.31.1-soroban.20.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "710403de32d0e0c35375518cb995d4fc056d0d48966f2e56ea471b8cb8fc9719" +source = "git+https://github.com/stellar/wasmi?rev=0ed3f3dee30dc41ebe21972399e0a73a41944aa0#0ed3f3dee30dc41ebe21972399e0a73a41944aa0" dependencies = [ "smallvec", "spin", @@ -2173,15 +2163,13 @@ checksum = "4f186bd2dcf04330886ce82d6f33dd75a7bfcf69ecf5763b89fcde53b6ac9838" [[package]] name = "wasmi_arena" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "104a7f73be44570cac297b3035d76b169d6599637631cf37a1703326a0727073" +version = "0.4.0" +source = "git+https://github.com/stellar/wasmi?rev=0ed3f3dee30dc41ebe21972399e0a73a41944aa0#0ed3f3dee30dc41ebe21972399e0a73a41944aa0" [[package]] name = "wasmi_core" version = "0.13.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dcf1a7db34bff95b85c261002720c00c3a6168256dcb93041d3fa2054d19856a" +source = "git+https://github.com/stellar/wasmi?rev=0ed3f3dee30dc41ebe21972399e0a73a41944aa0#0ed3f3dee30dc41ebe21972399e0a73a41944aa0" dependencies = [ "downcast-rs", "libm", diff --git a/Cargo.toml b/Cargo.toml index 389794b7..fffd35fa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,25 +12,25 @@ version = "20.3.3" rust-version = "1.74.0" [workspace.dependencies.soroban-env-host] -version = "=20.2.1" -# git = "https://github.com/stellar/rs-soroban-env" -# rev = "1bfc0f2a2ee134efc1e1b0d5270281d0cba61c2e" +version = "=20.2.2" +git = "https://github.com/stellar/rs-soroban-env" +rev = "8c9ab83c406bd86f56d52eae3e39dccf6b45b3da" # path = "../rs-soroban-env/soroban-env-host" [workspace.dependencies.soroban-simulation] -version = "=20.2.1" -# git = "https://github.com/stellar/rs-soroban-env" -# rev = "1bfc0f2a2ee134efc1e1b0d5270281d0cba61c2e" +version = "=20.2.2" +git = "https://github.com/stellar/rs-soroban-env" +rev = "8c9ab83c406bd86f56d52eae3e39dccf6b45b3da" # path = "../rs-soroban-env/soroban-simulation" [workspace.dependencies.soroban-spec] -version = "=20.3.1" -# git = "https://github.com/stellar/rs-soroban-sdk" -# rev = "4aef54ff9295c2fca4c5b9fbd2c92d0ff99f67de" +version = "=20.4.0" +git = "https://github.com/stellar/rs-soroban-sdk" +rev = "89efc3c211d41f1ab143bed0a09cd6af353bb098" # path = "../rs-soroban-sdk/soroban-spec" [workspace.dependencies.soroban-spec-rust] -version = "=20.3.1" +version = "=20.3.2" # git = "https://github.com/stellar/rs-soroban-sdk" # rev = "4aef54ff9295c2fca4c5b9fbd2c92d0ff99f67de" # path = "../rs-soroban-sdk/soroban-spec-rust" @@ -46,17 +46,17 @@ git = "https://github.com/stellar/soroban-tools" rev = "a59f5f421a27bab71472041fc619dd8b0d1cf902" [workspace.dependencies.soroban-sdk] -version = "=20.3.1" -# git = "https://github.com/stellar/rs-soroban-sdk" -# rev = "4aef54ff9295c2fca4c5b9fbd2c92d0ff99f67de" +version = "=20.4.0" +git = "https://github.com/stellar/rs-soroban-sdk" +rev = "89efc3c211d41f1ab143bed0a09cd6af353bb098" [workspace.dependencies.soroban-token-sdk] -version = "=20.3.1" +version = "=20.3.2" # git = "https://github.com/stellar/rs-soroban-sdk" # rev = "4aef54ff9295c2fca4c5b9fbd2c92d0ff99f67de" [workspace.dependencies.soroban-ledger-snapshot] -version = "=20.3.1" +version = "=20.3.2" # git = "https://github.com/stellar/rs-soroban-sdk" # rev = "4aef54ff9295c2fca4c5b9fbd2c92d0ff99f67de" diff --git a/cmd/crates/soroban-rpc/src/txn.rs b/cmd/crates/soroban-rpc/src/txn.rs index d713637b..4fc898b6 100644 --- a/cmd/crates/soroban-rpc/src/txn.rs +++ b/cmd/crates/soroban-rpc/src/txn.rs @@ -317,21 +317,10 @@ pub fn assemble( let classic_tx_fee: u64 = crate::DEFAULT_TRANSACTION_FEES.into(); // Choose larger of existing fee or inclusion + resource fee. - let base_tx_fee: u64 = tx - .fee - .max( - u32::try_from(classic_tx_fee + simulation.min_resource_fee) - .map_err(|_| Error::LargeFee(simulation.min_resource_fee + classic_tx_fee))?, - ) - .into(); // invariant: base_tx_fee <= u32::MAX - - // Pad the total fee by up to 15% for a bit of wiggle room. - // - // We know for a fact because of the .min() call that we will not exceed - // U32_MAX, but we can't get clippy to shut the fuck up for this single - // line, so we have a redundant error check anyway. - tx.fee = u32::try_from((base_tx_fee * 115 / 100).min(u64::from(u32::MAX))) - .map_err(|_| Error::LargeFee(base_tx_fee))?; + tx.fee = tx.fee.max( + u32::try_from(classic_tx_fee + simulation.min_resource_fee) + .map_err(|_| Error::LargeFee(simulation.min_resource_fee + classic_tx_fee))?, + ); tx.operations = vec![op].try_into()?; tx.ext = TransactionExt::V1(transaction_data); @@ -628,7 +617,7 @@ mod tests { // validate it auto updated the tx fees from sim response fees // since it was greater than tx.fee - assert_eq!(247, result.fee); + assert_eq!(215, result.fee); // validate it updated sorobantransactiondata block in the tx ext assert_eq!(TransactionExt::V1(transaction_data()), result.ext); @@ -735,20 +724,13 @@ mod tests { #[test] fn test_assemble_transaction_overflow_behavior() { // - // Test three separate cases: - // - // 1. Given a near-max (U32_MAX) resource fee, ensure the "wiggle room" - // doesn't cause an overflow due to correct math order. - // (Specifically, do U32_MAX - 15% - 100 - 1, so the final fee is - // U32_MAX-1.) - // - // 2. Given a large resource fee that WILL exceed on its own with the - // inclusion fee, ensure the overflow is caught with an error rather - // than silently ignored. - // - // 3. Given a max resource fee that exceeds U32_MAX *only with* wiggle - // room, ensure the overflow is ignored and we cap on the max. + // Test two separate cases: // + // 1. Given a near-max (u32::MAX - 100) resource fee make sure the tx + // fee does not overflow after adding the base inclusion fee (100). + // 2. Given a large resource fee that WILL exceed u32::MAX with the + // base inclusion fee, ensure the overflow is caught with an error + // rather than silently ignored. let txn = single_contract_fn_transaction(); let mut response = simulation_response(); @@ -756,41 +738,25 @@ mod tests { assert_eq!(txn.fee, 100, "modified txn.fee: update the math below"); // 1: wiggle room math overflows but result fits - let mut resource_fee = (u64::from(u32::MAX) * 85 / 100) - 100 - 1; - assert_eq!(resource_fee, 3_650_722_099); - response.min_resource_fee = resource_fee; + response.min_resource_fee = (u32::MAX - 100).into(); match assemble(&txn, &response) { Ok(asstxn) => { - let expected = (resource_fee + 100) * 115 / 100; - assert_eq!(asstxn.fee, u32::try_from(expected).unwrap()); + let expected = u32::MAX; + assert_eq!(asstxn.fee, expected); } r => panic!("expected success, got: {r:#?}"), } // 2: combo overflows, should throw - resource_fee = u64::from(u32::MAX); - assert_eq!(resource_fee, 4_294_967_295); - response.min_resource_fee = resource_fee; + response.min_resource_fee = (u32::MAX - 99).into(); match assemble(&txn, &response) { Err(Error::LargeFee(fee)) => { - let expected = resource_fee + 100; + let expected = u64::from(u32::MAX) + 1; assert_eq!(expected, fee, "expected {expected} != {fee} actual"); } r => panic!("expected LargeFee error, got: {r:#?}"), } - - // 2: combo works but wiggle room overflows, should cap - resource_fee = u64::from(u32::MAX) * 90 / 100; - assert_eq!(resource_fee, 3_865_470_565); - response.min_resource_fee = resource_fee; - - match assemble(&txn, &response) { - Ok(asstxn) => { - assert_eq!(asstxn.fee, u32::MAX); - } - r => panic!("expected success, got: {r:#?}"), - } } } diff --git a/cmd/soroban-rpc/internal/preflight/preflight.go b/cmd/soroban-rpc/internal/preflight/preflight.go index 584c47d6..59c15152 100644 --- a/cmd/soroban-rpc/internal/preflight/preflight.go +++ b/cmd/soroban-rpc/internal/preflight/preflight.go @@ -2,7 +2,6 @@ package preflight import ( "context" - "errors" "fmt" "runtime/cgo" "time" @@ -36,7 +35,7 @@ type snapshotSourceHandle struct { } const ( - defaultInstructionLeeway uint64 = 3000000 + defaultInstructionLeeway uint64 = 0 ) // SnapshotSourceGet takes a LedgerKey XDR in base64 string and returns its matching LedgerEntry XDR in base64 string @@ -150,6 +149,23 @@ func GetPreflight(ctx context.Context, params PreflightParameters) (Preflight, e } } +func getLedgerInfo(params PreflightParameters) (C.ledger_info_t, error) { + simulationLedgerSeq, err := getSimulationLedgerSeq(params.LedgerEntryReadTx) + if err != nil { + return C.ledger_info_t{}, err + } + + li := C.ledger_info_t{ + network_passphrase: C.CString(params.NetworkPassphrase), + sequence_number: C.uint32_t(simulationLedgerSeq), + protocol_version: 20, + timestamp: C.uint64_t(time.Now().Unix()), + // Current base reserve is 0.5XLM (in stroops) + base_reserve: 5_000_000, + } + return li, nil +} + func getFootprintTtlPreflight(params PreflightParameters) (Preflight, error) { opBodyXDR, err := params.OpBody.MarshalBinary() if err != nil { @@ -164,17 +180,16 @@ func getFootprintTtlPreflight(params PreflightParameters) (Preflight, error) { handle := cgo.NewHandle(snapshotSourceHandle{params.LedgerEntryReadTx, params.Logger}) defer handle.Delete() - simulationLedgerSeq, err := getSimulationLedgerSeq(params.LedgerEntryReadTx) + li, err := getLedgerInfo(params) if err != nil { return Preflight{}, err } res := C.preflight_footprint_ttl_op( C.uintptr_t(handle), - C.uint64_t(params.BucketListSize), opBodyCXDR, footprintCXDR, - C.uint32_t(simulationLedgerSeq), + li, ) FreeGoXDR(opBodyCXDR) @@ -206,38 +221,11 @@ func getInvokeHostFunctionPreflight(params PreflightParameters) (Preflight, erro return Preflight{}, err } sourceAccountCXDR := CXDR(sourceAccountXDR) - - hasConfig, stateArchivalConfig, _, err := db.GetLedgerEntry(params.LedgerEntryReadTx, xdr.LedgerKey{ - Type: xdr.LedgerEntryTypeConfigSetting, - ConfigSetting: &xdr.LedgerKeyConfigSetting{ - ConfigSettingId: xdr.ConfigSettingIdConfigSettingStateArchival, - }, - }) - if err != nil { - return Preflight{}, err - } - if !hasConfig { - return Preflight{}, errors.New("state archival config setting missing in ledger storage") - } - - simulationLedgerSeq, err := getSimulationLedgerSeq(params.LedgerEntryReadTx) + li, err := getLedgerInfo(params) if err != nil { return Preflight{}, err } - stateArchival := stateArchivalConfig.Data.MustConfigSetting().MustStateArchivalSettings() - li := C.ledger_info_t{ - network_passphrase: C.CString(params.NetworkPassphrase), - sequence_number: C.uint32_t(simulationLedgerSeq), - protocol_version: 20, - timestamp: C.uint64_t(time.Now().Unix()), - // Current base reserve is 0.5XLM (in stroops) - base_reserve: 5_000_000, - min_temp_entry_ttl: C.uint(stateArchival.MinTemporaryTtl), - min_persistent_entry_ttl: C.uint(stateArchival.MinPersistentTtl), - max_entry_ttl: C.uint(stateArchival.MaxEntryTtl), - } - handle := cgo.NewHandle(snapshotSourceHandle{params.LedgerEntryReadTx, params.Logger}) defer handle.Delete() resourceConfig := C.resource_config_t{ @@ -245,7 +233,6 @@ func getInvokeHostFunctionPreflight(params PreflightParameters) (Preflight, erro } res := C.preflight_invoke_hf_op( C.uintptr_t(handle), - C.uint64_t(params.BucketListSize), invokeHostFunctionCXDR, sourceAccountCXDR, li, diff --git a/cmd/soroban-rpc/internal/preflight/preflight_test.go b/cmd/soroban-rpc/internal/preflight/preflight_test.go index 926c2611..4b164212 100644 --- a/cmd/soroban-rpc/internal/preflight/preflight_test.go +++ b/cmd/soroban-rpc/internal/preflight/preflight_test.go @@ -24,8 +24,8 @@ var contractCostParams = func() *xdr.ContractCostParams { for i := 0; i < 23; i++ { result = append(result, xdr.ContractCostParamEntry{ Ext: xdr.ExtensionPoint{}, - ConstTerm: 0, - LinearTerm: 0, + ConstTerm: xdr.Int64((i + 1) * 10), + LinearTerm: xdr.Int64(i), }) } @@ -190,6 +190,16 @@ var mockLedgerEntriesWithoutTTLs = []xdr.LedgerEntry{ }, }, }, + { + LastModifiedLedgerSeq: 2, + Data: xdr.LedgerEntryData{ + Type: xdr.LedgerEntryTypeConfigSetting, + ConfigSetting: &xdr.ConfigSettingEntry{ + ConfigSettingId: xdr.ConfigSettingIdConfigSettingBucketlistSizeWindow, + BucketListSizeWindow: &[]xdr.Uint64{100, 200}, + }, + }, + }, } // Adds ttl entries to mockLedgerEntriesWithoutTTLs diff --git a/cmd/soroban-rpc/internal/test/simulate_transaction_test.go b/cmd/soroban-rpc/internal/test/simulate_transaction_test.go index 26866ed7..0b55c729 100644 --- a/cmd/soroban-rpc/internal/test/simulate_transaction_test.go +++ b/cmd/soroban-rpc/internal/test/simulate_transaction_test.go @@ -240,7 +240,7 @@ func TestSimulateTransactionSucceeds(t *testing.T) { // for test purposes, the most deterministic way to assert the resulting fee is expected value in test scope, is to capture // the resulting fee from current preflight output and re-plug it in here, rather than try to re-implement the cost-model algo // in the test. - ResourceFee: 132146, + ResourceFee: 118357, } // First, decode and compare the transaction data so we get a decent diff if it fails. @@ -546,7 +546,7 @@ func TestSimulateInvokeContractTransactionSucceeds(t *testing.T) { require.Contains(t, metrics, "soroban_rpc_json_rpc_request_duration_seconds_count{endpoint=\"simulateTransaction\",status=\"ok\"} 3") require.Contains(t, metrics, "soroban_rpc_preflight_pool_request_ledger_get_duration_seconds_count{status=\"ok\",type=\"db\"} 3") require.Contains(t, metrics, "soroban_rpc_preflight_pool_request_ledger_get_duration_seconds_count{status=\"ok\",type=\"all\"} 3") - require.Contains(t, metrics, "soroban_rpc_preflight_pool_request_ledger_entries_fetched_sum 67") + require.Contains(t, metrics, "soroban_rpc_preflight_pool_request_ledger_entries_fetched_sum 60") } func TestSimulateTransactionError(t *testing.T) { @@ -1130,7 +1130,7 @@ func TestSimulateSystemEvent(t *testing.T) { // for test purposes, the most deterministic way to assert the resulting fee is expected value in test scope, is to capture // the resulting fee from current preflight output and re-plug it in here, rather than try to re-implement the cost-model algo // in the test. - assert.InDelta(t, 100980, int64(transactionData.ResourceFee), 5000) + assert.InDelta(t, 85360, int64(transactionData.ResourceFee), 5000) assert.InDelta(t, 104, uint32(transactionData.Resources.WriteBytes), 15) require.GreaterOrEqual(t, len(response.Events), 3) } diff --git a/cmd/soroban-rpc/lib/preflight.h b/cmd/soroban-rpc/lib/preflight.h index 81db0c54..f52d56c1 100644 --- a/cmd/soroban-rpc/lib/preflight.h +++ b/cmd/soroban-rpc/lib/preflight.h @@ -10,9 +10,6 @@ typedef struct ledger_info_t { uint64_t timestamp; const char *network_passphrase; uint32_t base_reserve; - uint32_t min_temp_entry_ttl; - uint32_t min_persistent_entry_ttl; - uint32_t max_entry_ttl; } ledger_info_t; typedef struct xdr_t { @@ -43,7 +40,6 @@ typedef struct preflight_result_t { } preflight_result_t; preflight_result_t *preflight_invoke_hf_op(uintptr_t handle, // Go Handle to forward to SnapshotSourceGet - uint64_t bucket_list_size, // Bucket list size of current ledger const xdr_t invoke_hf_op, // InvokeHostFunctionOp XDR const xdr_t source_account, // AccountId XDR const ledger_info_t ledger_info, @@ -51,10 +47,9 @@ preflight_result_t *preflight_invoke_hf_op(uintptr_t handle, // Go Handle to for bool enable_debug); preflight_result_t *preflight_footprint_ttl_op(uintptr_t handle, // Go Handle to forward to SnapshotSourceGet - uint64_t bucket_list_size, // Bucket list size of current ledger const xdr_t op_body, // OperationBody XDR const xdr_t footprint, // LedgerFootprint XDR - uint32_t current_ledger_seq); // Current ledger sequence + const ledger_info_t ledger_info); // LedgerKey XDR to LedgerEntry XDR diff --git a/cmd/soroban-rpc/lib/preflight/Cargo.toml b/cmd/soroban-rpc/lib/preflight/Cargo.toml index 826c7cca..5c7d7648 100644 --- a/cmd/soroban-rpc/lib/preflight/Cargo.toml +++ b/cmd/soroban-rpc/lib/preflight/Cargo.toml @@ -12,5 +12,7 @@ libc = "0.2.147" sha2 = { workspace = true } # we need the testutils feature in order to get backtraces in the preflight library # when soroban rpc is configured to run with --preflight-enable-debug -soroban-env-host = { workspace = true, features = ["recording_mode", "testutils"]} +soroban-env-host = { workspace = true, features = ["recording_mode", "testutils", "unstable-next-api"]} soroban-simulation = { workspace = true } +anyhow = "1.0.75" +rand = { version = "0.8.5", features = [] } \ No newline at end of file diff --git a/cmd/soroban-rpc/lib/preflight/src/lib.rs b/cmd/soroban-rpc/lib/preflight/src/lib.rs index 25746e74..1beef0c9 100644 --- a/cmd/soroban-rpc/lib/preflight/src/lib.rs +++ b/cmd/soroban-rpc/lib/preflight/src/lib.rs @@ -1,23 +1,29 @@ +extern crate anyhow; extern crate base64; extern crate libc; extern crate sha2; extern crate soroban_env_host; extern crate soroban_simulation; +use anyhow::{anyhow, bail, Result}; use sha2::{Digest, Sha256}; +use soroban_env_host::storage::EntryWithLiveUntil; use soroban_env_host::xdr::{ - AccountId, Hash, InvokeHostFunctionOp, LedgerEntry, LedgerEntryData, LedgerFootprint, - LedgerKey, LedgerKeyTtl, Limits, OperationBody, ReadXdr, TtlEntry, WriteXdr, + AccountId, ExtendFootprintTtlOp, Hash, InvokeHostFunctionOp, LedgerEntry, LedgerEntryData, + LedgerFootprint, LedgerKey, LedgerKeyTtl, OperationBody, ReadXdr, ScErrorCode, ScErrorType, + SorobanTransactionData, TtlEntry, WriteXdr, }; -use soroban_env_host::LedgerInfo; -use soroban_simulation::{ledger_storage, ResourceConfig}; -use soroban_simulation::{ - simulate_footprint_ttl_op, simulate_invoke_hf_op, LedgerStorage, SimulationResult, +use soroban_env_host::{HostError, LedgerInfo, DEFAULT_XDR_RW_LIMITS}; +use soroban_simulation::simulation::{ + simulate_extend_ttl_op, simulate_invoke_host_function_op, simulate_restore_op, + InvokeHostFunctionSimulationResult, RestoreOpSimulationResult, SimulationAdjustmentConfig, }; -use std::error::Error; +use soroban_simulation::{AutoRestoringSnapshotSource, NetworkConfig, SnapshotSourceWithArchive}; +use std::cell::RefCell; use std::ffi::{CStr, CString}; use std::panic; use std::ptr::null_mut; +use std::rc::Rc; use std::{mem, slice}; #[repr(C)] @@ -28,25 +34,20 @@ pub struct CLedgerInfo { pub timestamp: u64, pub network_passphrase: *const libc::c_char, pub base_reserve: u32, - pub min_temp_entry_ttl: u32, - pub min_persistent_entry_ttl: u32, - pub max_entry_ttl: u32, } -impl From for LedgerInfo { - fn from(c: CLedgerInfo) -> Self { - let network_passphrase = from_c_string(c.network_passphrase); - Self { - protocol_version: c.protocol_version, - sequence_number: c.sequence_number, - timestamp: c.timestamp, - network_id: Sha256::digest(network_passphrase).into(), - base_reserve: c.base_reserve, - min_temp_entry_ttl: c.min_temp_entry_ttl, - min_persistent_entry_ttl: c.min_persistent_entry_ttl, - max_entry_ttl: c.max_entry_ttl, - } - } +fn fill_ledger_info(c_ledger_info: CLedgerInfo, network_config: &NetworkConfig) -> LedgerInfo { + let network_passphrase = from_c_string(c_ledger_info.network_passphrase); + let mut ledger_info = LedgerInfo { + protocol_version: c_ledger_info.protocol_version, + sequence_number: c_ledger_info.sequence_number, + timestamp: c_ledger_info.timestamp, + network_id: Sha256::digest(network_passphrase).into(), + base_reserve: c_ledger_info.base_reserve, + ..Default::default() + }; + network_config.fill_config_fields_in_ledger_info(&mut ledger_info); + ledger_info } #[repr(C)] @@ -85,14 +86,6 @@ pub struct CResourceConfig { pub instruction_leeway: u64, } -impl From for ResourceConfig { - fn from(r: CResourceConfig) -> Self { - return ResourceConfig { - instruction_leeway: r.instruction_leeway, - }; - } -} - #[repr(C)] #[derive(Copy, Clone)] pub struct CPreflightResult { @@ -116,22 +109,69 @@ pub struct CPreflightResult { pub pre_restore_min_fee: i64, } -impl From for CPreflightResult { - fn from(s: SimulationResult) -> Self { - let mut result = Self { - error: string_to_c(s.error), - auth: xdr_vec_to_c(s.auth), - result: option_xdr_to_c(s.result), - transaction_data: option_xdr_to_c(s.transaction_data), - min_fee: s.min_fee, - events: xdr_vec_to_c(s.events), - cpu_instructions: s.cpu_instructions, - memory_bytes: s.memory_bytes, +impl Default for CPreflightResult { + fn default() -> Self { + Self { + error: CString::new(String::new()).unwrap().into_raw(), + auth: get_default_c_xdr_vector(), + result: get_default_c_xdr(), + transaction_data: get_default_c_xdr(), + min_fee: 0, + events: get_default_c_xdr_vector(), + cpu_instructions: 0, + memory_bytes: 0, pre_restore_transaction_data: get_default_c_xdr(), pre_restore_min_fee: 0, + } + } +} + +impl CPreflightResult { + fn new_from_invoke_host_function( + invoke_hf_result: InvokeHostFunctionSimulationResult, + restore_preamble: Option, + error: String, + ) -> Self { + let mut result = Self { + error: string_to_c(error), + auth: xdr_vec_to_c(invoke_hf_result.auth), + result: option_xdr_to_c( + invoke_hf_result + .invoke_result + .map_or_else(|_| None, |v| Some(v)), + ), + min_fee: invoke_hf_result + .transaction_data + .as_ref() + .map_or_else(|| 0, |r| r.resource_fee), + transaction_data: option_xdr_to_c(invoke_hf_result.transaction_data), + // TODO: Diagnostic and contract events should be separated in the response + events: xdr_vec_to_c(invoke_hf_result.diagnostic_events), + cpu_instructions: invoke_hf_result.simulated_instructions as u64, + memory_bytes: invoke_hf_result.simulated_memory as u64, + ..Default::default() }; - if let Some(p) = s.restore_preamble { - result.pre_restore_min_fee = p.min_fee; + if let Some(p) = restore_preamble { + result.pre_restore_min_fee = p.transaction_data.resource_fee; + result.pre_restore_transaction_data = xdr_to_c(p.transaction_data); + }; + result + } + + fn new_from_transaction_data( + transaction_data: Option, + restore_preamble: Option, + error: String, + ) -> Self { + let min_fee = transaction_data.as_ref().map_or(0, |d| d.resource_fee); + let mut result = Self { + error: string_to_c(error), + transaction_data: option_xdr_to_c(transaction_data), + min_fee, + ..Default::default() + }; + if let Some(p) = restore_preamble { + result.pre_restore_min_fee = p.transaction_data.resource_fee; result.pre_restore_transaction_data = xdr_to_c(p.transaction_data); }; result @@ -141,7 +181,6 @@ impl From for CPreflightResult { #[no_mangle] pub extern "C" fn preflight_invoke_hf_op( handle: libc::uintptr_t, // Go Handle to forward to SnapshotSourceGet and SnapshotSourceHas - bucket_list_size: u64, // Bucket list size for current ledger invoke_hf_op: CXDR, // InvokeHostFunctionOp XDR in base64 source_account: CXDR, // AccountId XDR in base64 ledger_info: CLedgerInfo, @@ -151,7 +190,6 @@ pub extern "C" fn preflight_invoke_hf_op( catch_preflight_panic(Box::new(move || { preflight_invoke_hf_op_or_maybe_panic( handle, - bucket_list_size, invoke_hf_op, source_account, ledger_info, @@ -163,105 +201,173 @@ pub extern "C" fn preflight_invoke_hf_op( fn preflight_invoke_hf_op_or_maybe_panic( handle: libc::uintptr_t, - bucket_list_size: u64, // Go Handle to forward to SnapshotSourceGet and SnapshotSourceHas - invoke_hf_op: CXDR, // InvokeHostFunctionOp XDR in base64 - source_account: CXDR, // AccountId XDR in base64 - ledger_info: CLedgerInfo, + invoke_hf_op: CXDR, // InvokeHostFunctionOp XDR in base64 + source_account: CXDR, // AccountId XDR in base64 + c_ledger_info: CLedgerInfo, resource_config: CResourceConfig, enable_debug: bool, -) -> Result> { +) -> Result { let invoke_hf_op = - InvokeHostFunctionOp::from_xdr(from_c_xdr(invoke_hf_op), Limits::none()).unwrap(); - let source_account = AccountId::from_xdr(from_c_xdr(source_account), Limits::none()).unwrap(); - let go_storage = GoLedgerStorage { - golang_handle: handle, - current_ledger_sequence: ledger_info.sequence_number, + InvokeHostFunctionOp::from_xdr(from_c_xdr(invoke_hf_op), DEFAULT_XDR_RW_LIMITS).unwrap(); + let source_account = + AccountId::from_xdr(from_c_xdr(source_account), DEFAULT_XDR_RW_LIMITS).unwrap(); + let go_storage = Rc::new(GoLedgerStorage::new(handle)); + let network_config = NetworkConfig::load_from_snapshot(go_storage.as_ref())?; + let ledger_info = fill_ledger_info(c_ledger_info, &network_config); + let auto_restore_snapshot = Rc::new(AutoRestoringSnapshotSource::new( + go_storage.clone(), + &ledger_info, + )?); + + let mut adjustment_config = SimulationAdjustmentConfig::default_adjustment(); + // It would be reasonable to extend `resource_config` to be compatible with `adjustment_config` + // in order to let the users customize the resource/fee adjustments in a more granular fashion. + adjustment_config.instructions.additive_factor = adjustment_config + .instructions + .additive_factor + .max(resource_config.instruction_leeway.min(u32::MAX as u64) as u32); + // Here we assume that no input auth means that the user requests the recording auth. + let auth_entries = if invoke_hf_op.auth.is_empty() { + None + } else { + Some(invoke_hf_op.auth.to_vec()) }; - let ledger_storage = - LedgerStorage::with_restore_tracking(Box::new(go_storage), ledger_info.sequence_number)?; - let result = simulate_invoke_hf_op( - ledger_storage, - bucket_list_size, - invoke_hf_op, - source_account, - LedgerInfo::from(ledger_info), - resource_config.into(), + // Invoke the host function. The user errors should normally be captured in `invoke_hf_result.invoke_result` and + // this should return Err result for misconfigured ledger. + let invoke_hf_result = simulate_invoke_host_function_op( + auto_restore_snapshot.clone(), + &network_config, + &adjustment_config, + &ledger_info, + invoke_hf_op.host_function, + auth_entries, + &source_account, + rand::Rng::gen(&mut rand::thread_rng()), enable_debug, - ); - match result { - Ok(r) => Ok(r.into()), - Err(e) => Err(e), - } + )?; + let maybe_restore_result = match &invoke_hf_result.invoke_result { + Ok(_) => auto_restore_snapshot.simulate_restore_keys_op( + &network_config, + &SimulationAdjustmentConfig::default_adjustment(), + &ledger_info, + ), + Err(e) => Err(e.clone().into()), + }; + let error_str = extract_error_string(&maybe_restore_result, go_storage.as_ref()); + Ok(CPreflightResult::new_from_invoke_host_function( + invoke_hf_result, + maybe_restore_result.unwrap_or(None), + error_str, + )) } #[no_mangle] pub extern "C" fn preflight_footprint_ttl_op( handle: libc::uintptr_t, // Go Handle to forward to SnapshotSourceGet and SnapshotSourceHas - bucket_list_size: u64, // Bucket list size for current ledger op_body: CXDR, // OperationBody XDR footprint: CXDR, // LedgerFootprint XDR - current_ledger_seq: u32, + ledger_info: CLedgerInfo, ) -> *mut CPreflightResult { catch_preflight_panic(Box::new(move || { - preflight_footprint_ttl_op_or_maybe_panic( - handle, - bucket_list_size, - op_body, - footprint, - current_ledger_seq, - ) + preflight_footprint_ttl_op_or_maybe_panic(handle, op_body, footprint, ledger_info) })) } - fn preflight_footprint_ttl_op_or_maybe_panic( handle: libc::uintptr_t, - bucket_list_size: u64, op_body: CXDR, footprint: CXDR, - current_ledger_seq: u32, -) -> Result> { - let op_body = OperationBody::from_xdr(from_c_xdr(op_body), Limits::none()).unwrap(); - let footprint = LedgerFootprint::from_xdr(from_c_xdr(footprint), Limits::none()).unwrap(); - let go_storage = GoLedgerStorage { - golang_handle: handle, - current_ledger_sequence: current_ledger_seq, + c_ledger_info: CLedgerInfo, +) -> Result { + let op_body = OperationBody::from_xdr(from_c_xdr(op_body), DEFAULT_XDR_RW_LIMITS)?; + let footprint = LedgerFootprint::from_xdr(from_c_xdr(footprint), DEFAULT_XDR_RW_LIMITS)?; + let go_storage = Rc::new(GoLedgerStorage::new(handle)); + let network_config = NetworkConfig::load_from_snapshot(go_storage.as_ref())?; + let ledger_info = fill_ledger_info(c_ledger_info, &network_config); + // TODO: It would make for a better UX if the user passed only the neccesary fields for every operation. + // That would remove a possibility of providing bad operation body, or a possibility of filling wrong footprint + // field. + match op_body { + OperationBody::ExtendFootprintTtl(extend_op) => { + preflight_extend_ttl_op(extend_op, footprint.read_only.as_slice(), go_storage, &network_config, &ledger_info) + }, + OperationBody::RestoreFootprint(_) => { + preflight_restore_op(footprint.read_write.as_slice(), go_storage, &network_config, &ledger_info) + } + _ => Err(anyhow!("encountered unsupported operation type: '{:?}', instead of 'ExtendFootprintTtl' or 'RestoreFootprint' operations.", + op_body.discriminant()).into()) + } +} +fn preflight_extend_ttl_op( + extend_op: ExtendFootprintTtlOp, + keys_to_extend: &[LedgerKey], + go_storage: Rc, + network_config: &NetworkConfig, + ledger_info: &LedgerInfo, +) -> Result { + let auto_restore_snapshot = AutoRestoringSnapshotSource::new(go_storage.clone(), ledger_info)?; + let simulation_result = simulate_extend_ttl_op( + &auto_restore_snapshot, + &network_config, + &SimulationAdjustmentConfig::default_adjustment(), + &ledger_info, + keys_to_extend, + extend_op.extend_to, + ); + let (maybe_transaction_data, maybe_restore_result) = match simulation_result { + Ok(r) => ( + Some(r.transaction_data), + auto_restore_snapshot.simulate_restore_keys_op( + &network_config, + &SimulationAdjustmentConfig::default_adjustment(), + &ledger_info, + ), + ), + Err(e) => (None, Err(e)), }; - let ledger_storage = &LedgerStorage::new(Box::new(go_storage), current_ledger_seq); - let result = simulate_footprint_ttl_op( - ledger_storage, - bucket_list_size, - op_body, - footprint, - current_ledger_seq, + + let error_str = extract_error_string(&maybe_restore_result, go_storage.as_ref()); + Ok(CPreflightResult::new_from_transaction_data( + maybe_transaction_data, + maybe_restore_result.unwrap_or(None), + error_str, + )) +} + +fn preflight_restore_op( + keys_to_restore: &[LedgerKey], + go_storage: Rc, + network_config: &NetworkConfig, + ledger_info: &LedgerInfo, +) -> Result { + let simulation_result = simulate_restore_op( + go_storage.as_ref(), + &network_config, + &SimulationAdjustmentConfig::default_adjustment(), + &ledger_info, + keys_to_restore, ); - match result { - Ok(r) => Ok(r.into()), - Err(e) => Err(e), - } + let error_str = extract_error_string(&simulation_result, go_storage.as_ref()); + Ok(CPreflightResult::new_from_transaction_data( + simulation_result + .map(|r| Some(r.transaction_data)) + .unwrap_or(None), + None, + error_str, + )) } fn preflight_error(str: String) -> CPreflightResult { let c_str = CString::new(str).unwrap(); CPreflightResult { error: c_str.into_raw(), - auth: get_default_c_xdr_vector(), - result: get_default_c_xdr(), - transaction_data: get_default_c_xdr(), - min_fee: 0, - events: get_default_c_xdr_vector(), - cpu_instructions: 0, - memory_bytes: 0, - pre_restore_transaction_data: get_default_c_xdr(), - pre_restore_min_fee: 0, + ..Default::default() } } -fn catch_preflight_panic( - op: Box Result>>, -) -> *mut CPreflightResult { +fn catch_preflight_panic(op: Box Result>) -> *mut CPreflightResult { // catch panics before they reach foreign callers (which otherwise would result in // undefined behavior) - let res: std::thread::Result>> = + let res: std::thread::Result> = panic::catch_unwind(panic::AssertUnwindSafe(op)); let c_preflight_result = match res { Err(panic) => match panic.downcast::() { @@ -277,7 +383,7 @@ fn catch_preflight_panic( } fn xdr_to_c(v: impl WriteXdr) -> CXDR { - let (xdr, len) = vec_to_c_array(v.to_xdr(Limits::none()).unwrap()); + let (xdr, len) = vec_to_c_array(v.to_xdr(DEFAULT_XDR_RW_LIMITS).unwrap()); CXDR { xdr, len } } @@ -385,37 +491,40 @@ extern "C" { struct GoLedgerStorage { golang_handle: libc::uintptr_t, - current_ledger_sequence: u32, + internal_error: RefCell>, } impl GoLedgerStorage { + fn new(golang_handle: libc::uintptr_t) -> Self { + Self { + golang_handle, + internal_error: RefCell::new(None), + } + } + // Get the XDR, regardless of ttl - fn get_xdr_internal( - &self, - key_xdr: &mut Vec, - ) -> std::result::Result, ledger_storage::Error> { + fn get_xdr_internal(&self, key_xdr: &mut Vec) -> Option> { let key_c_xdr = CXDR { xdr: key_xdr.as_mut_ptr(), len: key_xdr.len(), }; let res = unsafe { SnapshotSourceGet(self.golang_handle, key_c_xdr) }; if res.xdr.is_null() { - return Err(ledger_storage::Error::NotFound); + return None; } let v = from_c_xdr(res); unsafe { FreeGoXDR(res) }; - Ok(v) + Some(v) } -} -impl ledger_storage::LedgerGetter for GoLedgerStorage { - fn get( - &self, - key: &LedgerKey, - include_not_live: bool, - ) -> std::result::Result<(LedgerEntry, Option), ledger_storage::Error> { - let mut key_xdr = key.to_xdr(Limits::none())?; - let xdr = self.get_xdr_internal(&mut key_xdr)?; + // Gets a ledger entry by key, including the archived/removed entries. + // The failures of this function are not recoverable and should only happen in case + // if the underlying storage is somehow corrupted. + fn get_fallible(&self, key: &LedgerKey) -> anyhow::Result> { + let mut key_xdr = key.to_xdr(DEFAULT_XDR_RW_LIMITS)?; + let Some(xdr) = self.get_xdr_internal(&mut key_xdr) else { + return Ok(None); + }; let live_until_ledger_seq = match key { // TODO: it would probably be more efficient to do all of this in the Go side @@ -425,36 +534,65 @@ impl ledger_storage::LedgerGetter for GoLedgerStorage { let ttl_key = LedgerKey::Ttl(LedgerKeyTtl { key_hash: Hash(key_hash), }); - let mut ttl_key_xdr = ttl_key.to_xdr(Limits::none())?; - let ttl_entry_xdr = self.get_xdr_internal(&mut ttl_key_xdr)?; - let ttl_entry = LedgerEntry::from_xdr(ttl_entry_xdr, Limits::none())?; - if let LedgerEntryData::Ttl(TtlEntry { + let mut ttl_key_xdr = ttl_key.to_xdr(DEFAULT_XDR_RW_LIMITS)?; + let ttl_entry_xdr = self.get_xdr_internal(&mut ttl_key_xdr).ok_or_else(|| { + anyhow!( + "TTL entry is missing for an entry that should have TTL with key: '{key:?}'" + ) + })?; + let ttl_entry = LedgerEntry::from_xdr(ttl_entry_xdr, DEFAULT_XDR_RW_LIMITS)?; + let LedgerEntryData::Ttl(TtlEntry { live_until_ledger_seq, .. }) = ttl_entry.data - { - Some(live_until_ledger_seq) - } else { - return Err(ledger_storage::Error::UnexpectedLedgerEntryTypeForTtlKey { - ledger_entry_type: ttl_entry.data.name().to_string(), - }); - } + else { + bail!( + "unexpected non-TTL entry '{:?}' has been fetched for TTL key '{:?}'", + ttl_entry, + ttl_key + ); + }; + Some(live_until_ledger_seq) } _ => None, }; - if !include_not_live - && live_until_ledger_seq.is_some() - && !is_live(live_until_ledger_seq.unwrap(), self.current_ledger_sequence) - { - return Err(ledger_storage::Error::NotLive); - } + let entry = LedgerEntry::from_xdr(xdr, DEFAULT_XDR_RW_LIMITS)?; + Ok(Some((Rc::new(entry), live_until_ledger_seq))) + } +} - let entry = LedgerEntry::from_xdr(xdr, Limits::none())?; - Ok((entry, live_until_ledger_seq)) +impl SnapshotSourceWithArchive for GoLedgerStorage { + fn get_including_archived( + &self, + key: &Rc, + ) -> std::result::Result, HostError> { + let res = self.get_fallible(key.as_ref()); + match res { + Ok(res) => Ok(res), + Err(e) => { + // Store the internal error in the storage as the info won't be propagated from simulation. + if let Ok(mut err) = self.internal_error.try_borrow_mut() { + *err = Some(e.into()); + } + // Errors that occur in storage are not recoverable, so we force host to halt by passing + // it an internal error. + return Err((ScErrorType::Storage, ScErrorCode::InternalError).into()); + } + } } } -pub(crate) fn is_live(live_until_ledger_seq: u32, current_ledger_seq: u32) -> bool { - live_until_ledger_seq >= current_ledger_seq +fn extract_error_string(simulation_result: &Result, go_storage: &GoLedgerStorage) -> String { + match simulation_result { + Ok(_) => String::new(), + Err(e) => { + // Override any simulation result with a storage error (if any). Simulation does not propagate the storage + // errors, but these provide more exact information on the root cause. + match go_storage.internal_error.borrow().as_ref() { + Some(e) => format!("{e:?}"), + None => format!("{e:?}"), + } + } + } } From 1f4b5d8dbaaa6ea48670dc0a94fb314f22a59f04 Mon Sep 17 00:00:00 2001 From: Dmytro Kozhevin Date: Wed, 6 Mar 2024 14:28:00 -0500 Subject: [PATCH 2/2] Update cmd/soroban-rpc/lib/preflight/src/lib.rs Co-authored-by: Alfonso Acosta --- cmd/soroban-rpc/lib/preflight/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/soroban-rpc/lib/preflight/src/lib.rs b/cmd/soroban-rpc/lib/preflight/src/lib.rs index 1beef0c9..fd80b018 100644 --- a/cmd/soroban-rpc/lib/preflight/src/lib.rs +++ b/cmd/soroban-rpc/lib/preflight/src/lib.rs @@ -518,8 +518,8 @@ impl GoLedgerStorage { } // Gets a ledger entry by key, including the archived/removed entries. - // The failures of this function are not recoverable and should only happen in case - // if the underlying storage is somehow corrupted. + // The failures of this function are not recoverable and should only happen when + // the underlying storage is somehow corrupted. fn get_fallible(&self, key: &LedgerKey) -> anyhow::Result> { let mut key_xdr = key.to_xdr(DEFAULT_XDR_RW_LIMITS)?; let Some(xdr) = self.get_xdr_internal(&mut key_xdr) else {