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

Reset partial host state between top-level invocations #735

Closed
jayz22 opened this issue Mar 21, 2023 · 7 comments
Closed

Reset partial host state between top-level invocations #735

jayz22 opened this issue Mar 21, 2023 · 7 comments
Assignees

Comments

@jayz22
Copy link
Contributor

jayz22 commented Mar 21, 2023

No description provided.

@jayz22 jayz22 self-assigned this Mar 21, 2023
@dmkozh
Copy link
Contributor

dmkozh commented Jul 24, 2023

To expand on this a bit: we should probably reset budget after (or before) every contract invocation. It's probably also a good idea to reset the footprint as well. This way the users will be able to get both metering data and footprint data concerning only a single interesting invocation vs getting whatever has been accumulated before.

@jayz22
Copy link
Contributor Author

jayz22 commented Oct 3, 2023

Add the original PR comment for a bit more context. #704 (comment)

@leighmcculloch
Copy link
Member

every contract invocation

Every top-level contract invocation. For sub-invocations we shouldn't fiddle with the state I don't think.

@dmkozh
Copy link
Contributor

dmkozh commented Oct 3, 2023

Every top-level contract invocation. For sub-invocations we shouldn't fiddle with the state I don't think.

Yeah, definitely. Currently we only reset the auth manager when we pop the last frame. Instead, we should probably reset auth, budget and events when we push the first frame into stack. This will make sure that everything is clean when the contract is called (unlike auth budget is affected by pretty much any Env calls outside of any contracts/frames).

@jayz22 jayz22 changed the title Add proper ways to reset budget for auth testing Reset partial host state between top-level invocations Dec 18, 2023
@jayz22
Copy link
Contributor Author

jayz22 commented Dec 18, 2023

We (cc @dmkozh @graydon) discussed this issue in our planning meeting, here is a summary.
Resetting a subset host state (e.g. auth, budget, events) is necessary for user tests, i.e. users setting up multiple contract invocations using the SDK, because users want better metrics and state-reporting that are isolated for different invocations.
The benefits of doing that for internal tests, which is what this issue is originally about, is less clear. Automatically resetting partial host state based on the host's stack size may produce unwanted side effects and confusions (the budget is supposed to outlive the host in real invocation).
So we think the better fix is on the SDK side, making the internal::Env more granular and allowing automatic reset of certain components between top-invocations. @leighmcculloch does this approach make sense? @dmkozh is there a similar SDK issue already or do we need to create one?

@dmkozh
Copy link
Contributor

dmkozh commented Dec 19, 2023

stellar/rs-soroban-sdk#1113 is the related issue.

@jayz22
Copy link
Contributor Author

jayz22 commented Dec 19, 2023

Closing as per above.

@jayz22 jayz22 closed this as completed Dec 19, 2023
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

No branches or pull requests

3 participants