Make TTL getters exclude the current ledger. (#1269)

### What

Make TTL getters exclude the current ledger.

### Why

`extend_ttl` methods always extend the TTL without excluding the current
ledger (because the entry has to be alive right now in order to be
extensible). However, `max_ttl` getter currently returns the TTL value
that includes the current ledger, thus extending entries by `max_ttl` is
not possible. I've updated `max_ttl` getter, as well as the newly
introduced `get_ttl` utils to always return TTL without counting the
current ledger, so their behavior is more consistent with `extend_ttl`
behavior.

### Known limitations

N/A

soroban-sdk/src/deploy.rs

@@ -270,7 +270,6 @@ mod testutils {
                 .unwrap()
                 .checked_sub(self.env.ledger().sequence())
                 .unwrap()
-                + 1
         }
 
         fn get_contract_code_ttl(&self, contract: &Address) -> u32 {
@@ -280,7 +279,6 @@ mod testutils {
                 .unwrap()
                 .checked_sub(self.env.ledger().sequence())
                 .unwrap()
-                + 1
         }
     }
 }

soroban-sdk/src/ledger.rs

@@ -155,7 +155,11 @@ impl testutils::Ledger for Ledger {
 
     fn set_max_entry_ttl(&self, max_entry_ttl: u32) {
         self.with_mut(|ledger_info| {
-            ledger_info.max_entry_ttl = max_entry_ttl;
+            // For the sake of consistency across SDK methods,
+            // we always make  TTL values to not include the current ledger.
+            // The actual network setting in env expects this to include
+            // the current ledger, so we need to add 1 here.
+            ledger_info.max_entry_ttl = max_entry_ttl.saturating_add(1);
         });
     }
 

soroban-sdk/src/storage.rs

@@ -139,19 +139,15 @@ impl Storage {
         }
     }
 
-    /// Returns the maximum TTL (number of ledgers that an entry can have rent paid
-    /// for it in one moment).
-    ///
-    /// When counting the number of ledgers an entry is active for, the current
-    /// ledger is included. If an entry is created in the current ledger, its
-    /// maximum live_until ledger will be the TTL (value returned from
-    /// the function) plus the current ledger. This means the last ledger
-    /// that the entry will be accessible will be the current ledger sequence
-    /// plus the max TTL minus one.
+    /// Returns the maximum time-to-live (TTL) for all the Soroban ledger entries.
+    ///
+    /// TTL is the number of ledgers left until the instance entry is considered
+    /// expired, excluding the current ledger. Maximum TTL represents the maximum
+    /// possible TTL of an entry and maximum extension via `extend_ttl` methods.
     pub fn max_ttl(&self) -> u32 {
         let seq = self.env.ledger().sequence();
         let max = self.env.ledger().max_live_until_ledger();
-        max - seq + 1
+        max - seq
     }
 
     /// Returns if there is a value stored for the given key in the currently
@@ -625,7 +621,6 @@ mod testutils {
                 .unwrap()
                 .checked_sub(env.ledger().sequence())
                 .unwrap()
-                + 1
         }
     }
 
@@ -641,7 +636,6 @@ mod testutils {
                 .unwrap()
                 .checked_sub(env.ledger().sequence())
                 .unwrap()
-                + 1
         }
     }
 
@@ -657,7 +651,6 @@ mod testutils {
                 .unwrap()
                 .checked_sub(env.ledger().sequence())
                 .unwrap()
-                + 1
         }
     }
 

soroban-sdk/src/tests/contract_store.rs

@@ -1,3 +1,5 @@
+use crate::testutils::storage::{Instance, Persistent, Temporary};
+use crate::testutils::Ledger;
 use crate::{self as soroban_sdk};
 use soroban_sdk::{contract, contractimpl, contracttype, Env};
 
@@ -35,31 +37,39 @@ impl Contract {
         env.storage().instance().set(&DataKey::Key(k), &v);
     }
 
-    pub fn extend_ttl_persistent(env: Env, k: i32) {
+    pub fn extend_ttl_persistent(env: Env, k: i32, extend_to: u32) {
         env.storage()
             .persistent()
-            .extend_ttl(&DataKey::Key(k), 100, 100);
+            .extend_ttl(&DataKey::Key(k), extend_to, extend_to);
     }
 
-    pub fn extend_ttl_temporary(env: Env, k: i32) {
+    pub fn extend_ttl_temporary(env: Env, k: i32, extend_to: u32) {
         env.storage()
             .temporary()
-            .extend_ttl(&DataKey::Key(k), 100, 100);
+            .extend_ttl(&DataKey::Key(k), extend_to, extend_to);
     }
 
-    pub fn extend_ttl_instance(env: Env) {
-        env.storage().instance().extend_ttl(100, 100);
+    pub fn extend_ttl_instance(env: Env, extend_to: u32) {
+        env.storage().instance().extend_ttl(extend_to, extend_to);
     }
 }
 
 #[test]
 fn test_storage() {
     let e = Env::default();
+    e.ledger().set_min_persistent_entry_ttl(100);
+    e.ledger().set_min_temp_entry_ttl(50);
+    e.ledger().set_max_entry_ttl(20_000);
+
     let contract_id = e.register_contract(None, Contract);
     let client = ContractClient::new(&e, &contract_id);
 
     // Smoke test instance bump before putting any data into it.
-    client.extend_ttl_instance();
+    client.extend_ttl_instance(&1000);
+    assert_eq!(
+        e.as_contract(&contract_id, || e.storage().instance().get_ttl()),
+        1000
+    );
 
     assert!(client.get_persistent(&11).is_none());
     assert!(client.get_temporary(&11).is_none());
@@ -105,8 +115,81 @@ fn test_storage() {
         Some(333_i32)
     );
 
-    // Smoke test temp/persistent bumps. This can be enhanced when we provided
-    // expiration ledger getter for tests.
-    client.extend_ttl_persistent(&11);
-    client.extend_ttl_temporary(&11);
+    client.extend_ttl_persistent(&11, &10_000);
+    assert_eq!(
+        e.as_contract(&contract_id, || e
+            .storage()
+            .persistent()
+            .get_ttl(&DataKey::Key(11))),
+        10_000
+    );
+
+    client.extend_ttl_persistent(&11, &e.storage().max_ttl());
+    assert_eq!(
+        e.as_contract(&contract_id, || e
+            .storage()
+            .persistent()
+            .get_ttl(&DataKey::Key(11))),
+        e.storage().max_ttl()
+    );
+    // Persistent entry bumps past max TTL are clamped to be just max TTL.
+    client.extend_ttl_persistent(&11, &(e.storage().max_ttl() + 1000));
+    assert_eq!(
+        e.as_contract(&contract_id, || e
+            .storage()
+            .persistent()
+            .get_ttl(&DataKey::Key(11))),
+        e.storage().max_ttl()
+    );
+
+    client.extend_ttl_temporary(&11, &5_000);
+    assert_eq!(
+        e.as_contract(&contract_id, || e
+            .storage()
+            .temporary()
+            .get_ttl(&DataKey::Key(11))),
+        5_000
+    );
+    client.extend_ttl_temporary(&11, &e.storage().max_ttl());
+    assert_eq!(
+        e.as_contract(&contract_id, || e
+            .storage()
+            .temporary()
+            .get_ttl(&DataKey::Key(11))),
+        e.storage().max_ttl()
+    );
+    // Extending temp entry TTL past max will panic, so that's not covered in this test.
+
+    client.extend_ttl_instance(&2000);
+    assert_eq!(
+        e.as_contract(&contract_id, || e.storage().instance().get_ttl()),
+        2000
+    );
+
+    client.extend_ttl_instance(&e.storage().max_ttl());
+    assert_eq!(
+        e.as_contract(&contract_id, || e.storage().instance().get_ttl()),
+        e.storage().max_ttl()
+    );
+    // Persistent entry bumps past max TTL are clamped to be just max TTL, and
+    // the instance storage is just a persistent entry.
+    client.extend_ttl_instance(&(e.storage().max_ttl() + 1000));
+    assert_eq!(
+        e.as_contract(&contract_id, || e.storage().instance().get_ttl()),
+        e.storage().max_ttl()
+    );
+}
+
+#[test]
+#[should_panic(expected = "trying to extend past max live_until ledger")]
+fn test_temp_storage_extension_past_max_ttl_panics() {
+    let e = Env::default();
+    e.ledger().set_min_temp_entry_ttl(50);
+    e.ledger().set_max_entry_ttl(20_000);
+    let contract_id = e.register_contract(None, Contract);
+    let client = ContractClient::new(&e, &contract_id);
+    e.as_contract(&contract_id, || {
+        e.storage().temporary().set(&DataKey::Key(11), &2222_i32);
+    });
+    client.extend_ttl_temporary(&11, &(e.storage().max_ttl() + 1));
 }

soroban-sdk/src/tests/storage_testutils.rs

@@ -60,21 +60,24 @@ fn ttl_getters() {
 
     // Initial TTLs are defined by min persistent/temp entry TTL settings for the
     // persistent/temp entries respectively.
+    // `get_ttl` methods don't count the current ledger, while the initial entry
+    // TTL includes the current ledger, thus the expected values are one less
+    // than the respective min_persistent_entry_ttl/min_temp_entry_ttl settings.
     let test_initial_storage_ttls = || {
-        assert_eq!(e.storage().instance().get_ttl(), 100);
-        assert_eq!(e.storage().persistent().get_ttl(&1), 100);
-        assert_eq!(e.storage().temporary().get_ttl(&2), 10);
+        assert_eq!(e.storage().instance().get_ttl(), 99);
+        assert_eq!(e.storage().persistent().get_ttl(&1), 99);
+        assert_eq!(e.storage().temporary().get_ttl(&2), 9);
     };
     e.as_contract(&contract_a, test_initial_storage_ttls);
     e.as_contract(&contract_b, test_initial_storage_ttls);
 
     // Instance and code have the same initial TTL as any other persistent entry.
     for from_contract in [&contract_a, &contract_b] {
         e.as_contract(from_contract, || {
-            assert_eq!(e.deployer().get_contract_instance_ttl(&contract_a), 100);
-            assert_eq!(e.deployer().get_contract_code_ttl(&contract_a), 100);
-            assert_eq!(e.deployer().get_contract_instance_ttl(&contract_b), 100);
-            assert_eq!(e.deployer().get_contract_code_ttl(&contract_b), 100);
+            assert_eq!(e.deployer().get_contract_instance_ttl(&contract_a), 99);
+            assert_eq!(e.deployer().get_contract_code_ttl(&contract_a), 99);
+            assert_eq!(e.deployer().get_contract_instance_ttl(&contract_b), 99);
+            assert_eq!(e.deployer().get_contract_code_ttl(&contract_b), 99);
         });
     }
 
@@ -89,27 +92,20 @@ fn ttl_getters() {
     });
 
     // Contract A has TTL extended for its entries.
-    // When TTL is extended, the current ledger is not included in `extend_to`
-    // parameter, so e.g. extending an entry to live for 1000 ledgers from now
-    // means that the TTL becomes 1001 (current ledger + 1000 ledgers of extension).
     e.as_contract(&contract_a, || {
-        assert_eq!(e.storage().instance().get_ttl(), 1001);
-        assert_eq!(e.storage().persistent().get_ttl(&1), 501);
-        assert_eq!(e.storage().temporary().get_ttl(&2), 301);
+        assert_eq!(e.storage().instance().get_ttl(), 1000);
+        assert_eq!(e.storage().persistent().get_ttl(&1), 500);
+        assert_eq!(e.storage().temporary().get_ttl(&2), 300);
     });
     // Contract B has no TTLs extended for its own storage.
     e.as_contract(&contract_b, test_initial_storage_ttls);
 
-    for from_contract in [&contract_a, &contract_b] {
-        e.as_contract(from_contract, || {
-            assert_eq!(e.deployer().get_contract_instance_ttl(&contract_a), 1001);
-            assert_eq!(e.deployer().get_contract_code_ttl(&contract_a), 2001);
-            // Instance hasn't been extended for B.
-            assert_eq!(e.deployer().get_contract_instance_ttl(&contract_b), 100);
-            // Code has been extended for B.
-            assert_eq!(e.deployer().get_contract_code_ttl(&contract_b), 2001);
-        });
-    }
+    assert_eq!(e.deployer().get_contract_instance_ttl(&contract_a), 1000);
+    assert_eq!(e.deployer().get_contract_code_ttl(&contract_a), 2000);
+    // Instance hasn't been extended for B.
+    assert_eq!(e.deployer().get_contract_instance_ttl(&contract_b), 99);
+    // Code has been extended for B.
+    assert_eq!(e.deployer().get_contract_code_ttl(&contract_b), 2000);
 }
 
 #[test]
@@ -138,7 +134,7 @@ fn temp_entry_expiration() {
         // The entry is written and the new TTL is set based on min temp entry TTL
         // setting.
         assert_eq!(e.storage().temporary().get(&1), Some(3));
-        assert_eq!(e.storage().temporary().get_ttl(&1), 100);
+        assert_eq!(e.storage().temporary().get_ttl(&1), 99);
     });
 }
 

soroban-sdk/src/testutils.rs

@@ -401,7 +401,7 @@ pub trait Deployer {
     /// Gets the TTL of the given contract's instance.
     ///
     /// TTL is the number of ledgers left until the instance entry is considered
-    /// expired including the current ledger.
+    /// expired, excluding the current ledger.
     ///
     /// Panics if there is no instance corresponding to the provided address,
     /// or if the instance has expired.
@@ -410,7 +410,7 @@ pub trait Deployer {
     /// Gets the TTL of the given contract's Wasm code entry.
     ///
     /// TTL is the number of ledgers left until the contract code entry
-    /// is considered expired, including the current ledger.
+    /// is considered expired, excluding the current ledger.
     ///
     /// Panics if there is no contract instance/code corresponding to
     /// the provided address, or if the instance/code has expired.

soroban-sdk/src/testutils/mock_auth.rs

@@ -2,8 +2,6 @@
 
 use crate::{contract, contractimpl, xdr, Address, Env, Symbol, TryFromVal, Val, Vec};
 
-use super::Ledger;
-
 #[doc(hidden)]
 #[contract(crate_path = "crate")]
 pub struct MockAuthContract;
@@ -32,13 +30,13 @@ impl<'a> From<&MockAuth<'a>> for xdr::SorobanAuthorizationEntry {
     fn from(value: &MockAuth) -> Self {
         let env = value.address.env();
         let curr_ledger = env.ledger().sequence();
-        let max_entry_ttl = env.ledger().get().max_entry_ttl;
+        let max_entry_ttl = env.storage().max_ttl();
         Self {
             root_invocation: value.invoke.into(),
             credentials: xdr::SorobanCredentials::Address(xdr::SorobanAddressCredentials {
                 address: value.address.try_into().unwrap(),
                 nonce: env.with_generator(|mut g| g.nonce()),
-                signature_expiration_ledger: curr_ledger + max_entry_ttl - 1,
+                signature_expiration_ledger: curr_ledger + max_entry_ttl,
                 signature: xdr::ScVal::Void,
             }),
         }

soroban-sdk/src/testutils/storage.rs

@@ -8,7 +8,7 @@ pub trait Persistent {
     /// Gets the TTL for the persistent storage entry corresponding to the provided key.
     ///
     /// TTL is the number of ledgers left until the persistent entry is considered
-    /// expired, including the current ledger.
+    /// expired, excluding the current ledger.
     ///
     /// Panics if there is no entry corresponding to the key, or if the entry has expired.
     fn get_ttl<K: IntoVal<Env, Val>>(&self, key: &K) -> u32;
@@ -22,7 +22,7 @@ pub trait Temporary {
     /// Gets the TTL for the temporary storage entry corresponding to the provided key.
     ///
     /// TTL is the number of ledgers left until the temporary entry is considered
-    /// non-existent, including the current ledger.
+    /// non-existent, excluding the current ledger.
     ///
     /// Panics if there is no entry corresponding to the key.
     fn get_ttl<K: IntoVal<Env, Val>>(&self, key: &K) -> u32;
@@ -36,6 +36,6 @@ pub trait Instance {
     /// Gets the TTL for the current contract's instance entry.
     ///
     /// TTL is the number of ledgers left until the instance entry is considered
-    /// expired, including the current ledger.
+    /// expired, excluding the current ledger.
     fn get_ttl(&self) -> u32;
 }