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

Changes to support the API changes upcoming in rpc v22 #16

Open
leighmcculloch opened this issue Oct 3, 2024 · 6 comments · May be fixed by #12
Open

Changes to support the API changes upcoming in rpc v22 #16

leighmcculloch opened this issue Oct 3, 2024 · 6 comments · May be fixed by #12
Assignees

Comments

@leighmcculloch
Copy link
Member

The rpc client needs updating to support the following changes that are upcoming in rpc v22:

  • createdAt field in getTransaction response is now encoded as string instead of int64
  • The fields in getVersionInfo response were changed to Camel-case naming:
    • commitHash
    • buildTimestamp
    • captiveCoreVersion
    • protocolVersion
  • The legacy cost object has been removed from simulateTransaction response.
    • The correct resource costs can now be retrieved from the transaction XDR instead.
  • Remove pagingToken from getEvents response and replace it with Cursor.
    • Instead of being present in all event objects, the Cursor will be at the top level of the response similar to getTransactions. This makes it easier to use and brings it in sync with how Cursor is used in other RPC endpoints
  • Deprecated getLedgerEntry endpoint is now removed.
    • Users can use the more powerful getLedgerEntries endpoint to get the same result.
  • Add transaction hash to getTransactions response.
    • Each transaction object in the response now also includes a hex-encoded transaction hash string.

Ref: https://gist.github.com/aditya1702/8a0e3e05689217009692517d744c7f10

Related discussion:

The above list of changes include breaking changes. Some of the breaking changes are changes to the response fields, and there is not time period for where both old and new respond fields will be present to allow clients to adopt just one or the other at any one time, so when updating the client to support the new fields the client must be able to support both the old and new fields.

This may require rewriting the way the client parses the JSON, introducing custom parsers, or parsing some fields into dynamic json values, and then parsing them manually to one of the types we expect.

This change is a dependency of releasing the stellar-cli for protocol 22.

@willemneal
Copy link
Member

See ##12

@willemneal
Copy link
Member

willemneal commented Oct 3, 2024

there is not time period for where both old and new respond fields will be present to allow clients to adopt just one or the other at any one time

I'm confused why need backwards compatibility. When pointing at an RPC endpoint that supports v22, why would we need to support the old fields? If needed couldn't the old client be used with an older RPC endpoint?

@janewang
Copy link
Collaborator

janewang commented Oct 3, 2024

@willemneal Spoke with Molly and that platform team is unavailable next week as the whole team is onsite. I think given you already opened the PR, I assigned this to you. Thank you!

@leighmcculloch
Copy link
Member Author

I'm confused why need backwards compatibility. When pointing at an RPC endpoint that supports v22, why would we need to support the old fields?

A developer might be building for mainnet and testnet at the same time. They'll have a CLI installed, and new CLI's should be backwards compatible with the current network in use. Otherwise we end up with a poor developer experience where developers need to have multiple versions installed, actively switch between them.

Also, operators will need to deploy the new RPC ahead of network upgrades, and operators of those RPC shouldn't need to coordinate with users of the CLI. Users should be able to grab the new CLI, then at some point operators upgrade the RPC and the user is largely unaffected or doesn't care.

If needed couldn't the old client be used with an older RPC endpoint?

Most of the breaking changes are changes only in the response, which means the CLI can't know which client to use until it gets a response back and need to parse it.

@Shaptic
Copy link
Contributor

Shaptic commented Oct 7, 2024

A developer might be building for mainnet and testnet at the same time.

That's like saying prod is running Python 3 but staging is running Python 2 but you want the same python binary to support both 🧐 It's pretty standard to use different binaries for different targets - expecting a v22 and v21 CLI if there are different protocols on each one seems okay to me.

operators of those RPC shouldn't need to coordinate with users of the CLI

Except we expect this for every protocol upgrade? If we (SDF) deploy a v22 RPC, it's well-understood that everyone needs to update their SDKs according. Why is the CLI different?

new CLI's should be backwards compatible with the current network in use

Wouldn't this logic apply to all clients and SDKs, which certainly isn't the case?

@willemneal
Copy link
Member

I think for the CLI it does make more sense. If you are using it with mainnet and the new protocol release is just for testnet, you don't want to have to switch between two different binaries to interact with a different RPC. I'm almost done making it backwards compatible. It would be possible to choose a different client passed on the network, the CLI already does a verify network passphrase initially so we could use this info to create two different clients.

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 a pull request may close this issue.

4 participants