Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Soroban: Add extendTtl Builtin Method #1709

Merged

Conversation

tareknaser
Copy link
Contributor

This PR adds support for extendPersistentTtl() method in Soroban, along with a test and documentation. Also, the Soroban testing infrastructure has been refactored to allow more flexible environment manipulation.

Documentation for extend_ttl
Fixes #1669

Changes

  • Added support for extendPersistentTtl as a method on uint64 for the Soroban target.
    • In the Soroban SDK, extend_ttl is a generic function (IntoVal<Env, Val>)
pub fn extend_ttl<K>(&self, key: &K, threshold: u32, extend_to: u32)
where
    K: IntoVal<Env, Val>,

but Solidity does not support that, so it's implemented as a method instead.

  • One assertion in the different_storage_types test is affected due to changes in diagnostic capture. A follow-up PR will address this.

@tareknaser tareknaser force-pushed the soroban_builtin_extend_ttl branch 2 times, most recently from a8de760 to 384437c Compare February 15, 2025 23:10
@tareknaser
Copy link
Contributor Author

Update: extendPersistentTtl is now renamed to extendTtl for both Persistent and Temporary storage types. For Instance storage type, a new builtin called extendInstanceTtl has been added as a standalone global function (not a method).

Tests for all storage types have been added in tests/soroban_testcases/ttl.rs.

@tareknaser tareknaser changed the title Soroban: Add extendPersistentTtl Builtin Method Soroban: Add extendTtl Builtin Method Feb 15, 2025
Copy link
Contributor

@salaheldinsoliman salaheldinsoliman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!
Looks good but could you please:
1- Sign your commits for the DCO to pass
2- Add a test under integration/soroban?

@tareknaser tareknaser force-pushed the soroban_builtin_extend_ttl branch from 384437c to 1542bfd Compare February 16, 2025 17:39
@tareknaser tareknaser force-pushed the soroban_builtin_extend_ttl branch from 1542bfd to 0020820 Compare February 16, 2025 17:40
@tareknaser
Copy link
Contributor Author

1- Sign your commits for the DCO to pass

Sure, should be fixed now

2- Add a test under integration/soroban?

This is a bit tricky. Writing integration tests for the time-to-live functionality would require a way to access the current ledger state during tests (like we do now in tests/soroban_testcases/ttl.rs using soroban_sdk::testutils). We’d need to do something similar in JavaScript.

There’s a package @stellar/stellar-sdk that we currently use for Soroban integration tests, but I’m not sure if it has all the testing utilities we’d need. I haven’t looked into it deeply yet, but the general idea is that we’d have to update the testing setup to make it work (if possible).

For now, I’d like to get this PR through, and in a follow-up PR, I’ll add support for other variable types in extendTtl() method with tests for each type and explore adding integration tests.

@salaheldinsoliman salaheldinsoliman merged commit fe96d0d into hyperledger-solang:main Feb 22, 2025
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add builtin to extend TTL for Soroban storage variables
2 participants