From d46c0acb6844151763777e6a497f93a832c49a80 Mon Sep 17 00:00:00 2001 From: Shawn Reuland Date: Mon, 11 Dec 2023 15:39:29 -0800 Subject: [PATCH 1/4] increased preflight instruction fee padding to 3 million --- cmd/soroban-rpc/lib/preflight/src/fees.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/soroban-rpc/lib/preflight/src/fees.rs b/cmd/soroban-rpc/lib/preflight/src/fees.rs index 1cfc16b16..3e08a7120 100644 --- a/cmd/soroban-rpc/lib/preflight/src/fees.rs +++ b/cmd/soroban-rpc/lib/preflight/src/fees.rs @@ -138,12 +138,12 @@ fn calculate_host_function_soroban_resources( .map(|c| c.encoded_new_value.as_ref().map_or(0, Vec::len) as u32) .sum(); - // Add a 20% leeway with a minimum of 1 million instructions + // Add a 20% leeway with a minimum of 3 million instructions let budget_instructions = budget .get_cpu_insns_consumed() .context("cannot get instructions consumed")?; let instructions = max( - budget_instructions + 1000000, + budget_instructions + 3000000, budget_instructions * 120 / 100, ); Ok(SorobanResources { From 8c70a13db6c4d950a835bb4504c81a01eaf7699f Mon Sep 17 00:00:00 2001 From: Shawn Reuland Date: Mon, 11 Dec 2023 18:49:52 -0800 Subject: [PATCH 2/4] updated test threshold interval for larger inst padding on fees --- cmd/soroban-rpc/internal/test/simulate_transaction_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/soroban-rpc/internal/test/simulate_transaction_test.go b/cmd/soroban-rpc/internal/test/simulate_transaction_test.go index 2ec2b414e..745c85b27 100644 --- a/cmd/soroban-rpc/internal/test/simulate_transaction_test.go +++ b/cmd/soroban-rpc/internal/test/simulate_transaction_test.go @@ -244,7 +244,7 @@ func TestSimulateTransactionSucceeds(t *testing.T) { err := xdr.SafeUnmarshalBase64(result.TransactionData, &transactionData) assert.NoError(t, err) assert.Equal(t, expectedTransactionData.Resources.Footprint, transactionData.Resources.Footprint) - assert.InDelta(t, uint32(expectedTransactionData.Resources.Instructions), uint32(transactionData.Resources.Instructions), 200000) + assert.InDelta(t, uint32(expectedTransactionData.Resources.Instructions), uint32(transactionData.Resources.Instructions), 3200000) assert.InDelta(t, uint32(expectedTransactionData.Resources.ReadBytes), uint32(transactionData.Resources.ReadBytes), 10) assert.InDelta(t, uint32(expectedTransactionData.Resources.WriteBytes), uint32(transactionData.Resources.WriteBytes), 300) assert.InDelta(t, int64(expectedTransactionData.ResourceFee), int64(transactionData.ResourceFee), 3000) From df4d88c9ab0dadea7a96dcd64ba588806e9a6300 Mon Sep 17 00:00:00 2001 From: Shawn Reuland Date: Mon, 11 Dec 2023 22:00:59 -0800 Subject: [PATCH 3/4] fixed simulation fee test threshold assert to be minimally deterministic on resulting fee, it fluctuated alot after increasing the instr budget to 3M --- cmd/soroban-rpc/internal/test/simulate_transaction_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/soroban-rpc/internal/test/simulate_transaction_test.go b/cmd/soroban-rpc/internal/test/simulate_transaction_test.go index 745c85b27..6dead0f07 100644 --- a/cmd/soroban-rpc/internal/test/simulate_transaction_test.go +++ b/cmd/soroban-rpc/internal/test/simulate_transaction_test.go @@ -236,7 +236,6 @@ func TestSimulateTransactionSucceeds(t *testing.T) { ReadBytes: 0, WriteBytes: 7048, }, - ResourceFee: 113910, } // First, decode and compare the transaction data so we get a decent diff if it fails. @@ -247,7 +246,7 @@ func TestSimulateTransactionSucceeds(t *testing.T) { assert.InDelta(t, uint32(expectedTransactionData.Resources.Instructions), uint32(transactionData.Resources.Instructions), 3200000) assert.InDelta(t, uint32(expectedTransactionData.Resources.ReadBytes), uint32(transactionData.Resources.ReadBytes), 10) assert.InDelta(t, uint32(expectedTransactionData.Resources.WriteBytes), uint32(transactionData.Resources.WriteBytes), 300) - assert.InDelta(t, int64(expectedTransactionData.ResourceFee), int64(transactionData.ResourceFee), 3000) + assert.Greater(t, int64(transactionData.ResourceFee), int64(0)) // Then decode and check the result xdr, separately so we get a decent diff if it fails. assert.Len(t, result.Results, 1) @@ -1122,7 +1121,7 @@ func TestSimulateSystemEvent(t *testing.T) { require.NoError(t, err) assert.InDelta(t, 7464, uint32(transactionData.Resources.ReadBytes), 200) - assert.InDelta(t, 80980, int64(transactionData.ResourceFee), 5000) + assert.Greater(t, int64(transactionData.ResourceFee), int64(0)) assert.InDelta(t, 104, uint32(transactionData.Resources.WriteBytes), 15) require.GreaterOrEqual(t, len(response.Events), 3) } From a27cf1197be6dbb817471ba6148e113a7d4e065e Mon Sep 17 00:00:00 2001 From: Shawn Reuland Date: Tue, 12 Dec 2023 10:22:38 -0800 Subject: [PATCH 4/4] adjusted the resulting fee assertion with latest preflight results --- .../internal/test/simulate_transaction_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/cmd/soroban-rpc/internal/test/simulate_transaction_test.go b/cmd/soroban-rpc/internal/test/simulate_transaction_test.go index 6dead0f07..340f68085 100644 --- a/cmd/soroban-rpc/internal/test/simulate_transaction_test.go +++ b/cmd/soroban-rpc/internal/test/simulate_transaction_test.go @@ -236,6 +236,11 @@ func TestSimulateTransactionSucceeds(t *testing.T) { ReadBytes: 0, WriteBytes: 7048, }, + // the resulting fee is derived from the compute factors and a default padding is applied to instructions by preflight + // 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: 135910, } // First, decode and compare the transaction data so we get a decent diff if it fails. @@ -246,7 +251,7 @@ func TestSimulateTransactionSucceeds(t *testing.T) { assert.InDelta(t, uint32(expectedTransactionData.Resources.Instructions), uint32(transactionData.Resources.Instructions), 3200000) assert.InDelta(t, uint32(expectedTransactionData.Resources.ReadBytes), uint32(transactionData.Resources.ReadBytes), 10) assert.InDelta(t, uint32(expectedTransactionData.Resources.WriteBytes), uint32(transactionData.Resources.WriteBytes), 300) - assert.Greater(t, int64(transactionData.ResourceFee), int64(0)) + assert.InDelta(t, int64(expectedTransactionData.ResourceFee), int64(transactionData.ResourceFee), 3000) // Then decode and check the result xdr, separately so we get a decent diff if it fails. assert.Len(t, result.Results, 1) @@ -1121,7 +1126,11 @@ func TestSimulateSystemEvent(t *testing.T) { require.NoError(t, err) assert.InDelta(t, 7464, uint32(transactionData.Resources.ReadBytes), 200) - assert.Greater(t, int64(transactionData.ResourceFee), int64(0)) + // the resulting fee is derived from compute factors and a default padding is applied to instructions by preflight + // 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, 104, uint32(transactionData.Resources.WriteBytes), 15) require.GreaterOrEqual(t, len(response.Events), 3) }