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

assess impact on RPC of upcoming protocol 21 changes #106

Closed
mollykarcher opened this issue Mar 19, 2024 · 2 comments
Closed

assess impact on RPC of upcoming protocol 21 changes #106

mollykarcher opened this issue Mar 19, 2024 · 2 comments
Assignees

Comments

@mollykarcher
Copy link
Contributor

mollykarcher commented Mar 19, 2024

For potential inclusion in protocol 21, the core team is working on a series of CAPs. We should look through these to identify if there is any significant downstream impact to RPC that it is possible to get ahead of, prior to the actual builds being available.

Improvements to host VM instantiation, which (at a minimum) has the potential to increase overall transaction volume on the network, as well as affect any RPC tests that assert on costs. Detail captured in the following CAPs:

New host functions, details in the following CAPs:

XDR change not included in CAPs:

Output of this should be the implementation tickets for any work on our side that falls out of this

@mollykarcher mollykarcher added this to the Sprint 44 milestone Mar 19, 2024
@mollykarcher mollykarcher moved this from Backlog to Next Sprint Proposal in Platform Scrum Mar 19, 2024
@2opremio
Copy link
Contributor

2opremio commented Mar 27, 2024

I don't think there is any considerable impact. We will have to

  1. Update the XDR in stellar/go and update soroban-rpc accordingly (but I don't expect any unautomated code changes)
  2. Update Core and rs-soroban-env/sdk. There may be changes on how the VM is instantiated after the new cost model (@dmkozh can you confirm?), but ever since we moved the simulation code to rs-soroban-env I don't expect a high impact due to internal API changes.
  3. Since the cost model changes, we should add a simulateTransaction (or rather GetPreflight) which exercises simulating transactions with the old cost model (to make sure we have no regressions)
  4. I don't really expect any changes due to the new host functions (maybe the VM needs to be instantated differently just like we need for pnrg? @dmkozh can you confirm?). I am not even sure it's worth adding integration tests for the new functions.
  5. Release new RPC docker image with the new Core.

@mollykarcher mollykarcher moved this from Next Sprint Proposal to Current Sprint in Platform Scrum Mar 27, 2024
@dmkozh
Copy link
Contributor

dmkozh commented Mar 27, 2024

There may be changes on how the VM is instantiated after the new cost model (@dmkozh can you confirm?), b

There shouldn't be any non-test changes necessary. There could be changes in hardcoded expected instructions and resources fee when you're testing with protocol 21 enabled.

Since the cost model changes, we should add a simulateTransaction (or rather GetPreflight) which exercises simulating transactions with the old cost model (to make sure we have no regressions)

I'm not sure if this is something you can meaningfully test (there is an unbounded number of possible cost model configurations, so testing with any one of them is as good as testing with multiple). That said, what I think should be covered with tests is preflight both for protocol 20 and protocol 21 (or more generally protocol vCurr and protocol vNext). The reason is that there will be a period of time where a new version of RPC is being used, but the network hasn't been upgraded yet. That will cover structural changes in the cost model (as more cost types have been added).

maybe the VM needs to be instantated differently just like we need for pnrg?

Not really, VM instantiation happens internally in the host code, so you just need to pick up a fresh env release to get it working. There are no changes in public API.

I am not even sure it's worth adding integration tests for the new functions.

I agree, I think 99% of the time RPC shouldn't worry about the new host functions.

@mollykarcher mollykarcher moved this from To Do to In Progress in Platform Scrum Apr 9, 2024
@2opremio 2opremio closed this as completed May 7, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Platform Scrum May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

3 participants