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

Pausable #2

Merged
merged 9 commits into from
Jan 10, 2025
Merged

Pausable #2

merged 9 commits into from
Jan 10, 2025

Conversation

brozorec
Copy link
Collaborator

@brozorec brozorec commented Jan 7, 2025

Handles #1

This PR proposes an example implementation of "Pausable" as the first module of the Soroban Contracts Library. It includes a contract module and a simple example that demonstrates its practical usage.

Here some questions and issues that emerged:

  1. The contract module has a file "clients.rs" where the "Pausable" trait is defined which is supposed to be implemented by clients as it's the case here in "examples". Do we have a better idea of naming this file?
  2. The instance storage entries have Time-to-Live and must be bumped (as it's the case for temporary and persistent). One of the differences between persistent and instance is that there's only one instance entry containing all the instance variables while potentially many persistent ones. That means that one bump for an instance variable extends the TTL for all other instance ones. In most of the cases, instance bumping is done is the setter functions. Do we take care of bumping the instance in the module functions or we leave it to the client? The risk if we decide to bump in the module functions is that this operation will be hidden away from the client who might adopt their own bumping strategy which eventually results in a double-bumping (the consequences of that need to be examined). A possible solution could be to leave bumping to clients and develop another module that exposes all the necessary utils for them to bump as they like.
  3. Do we prefix test functions with test_?
  4. Errors are defined in enums by assigning to each one an id. Figured out ids can be overwritten as here which can cause issues when debugging, check tests cases. What approach can we adopt to attribute ids that have lower chances to be accidentally overwritten?

PR Checklist

  • Tests
  • Documentation

@brozorec brozorec marked this pull request as ready for review January 8, 2025 14:52
@brozorec brozorec requested a review from ozgunozerk January 8, 2025 14:53
@ozgunozerk
Copy link
Collaborator

Great work 💯
will approve after we resolve the above conversations, which are nit-picks

@brozorec brozorec requested a review from ozgunozerk January 9, 2025 13:37
Copy link
Collaborator

@ozgunozerk ozgunozerk left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@brozorec brozorec merged commit 0075f68 into main Jan 10, 2025
4 checks passed
@brozorec brozorec deleted the pausable branch January 10, 2025 09:08
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.

2 participants