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

Update client to support Preview 10 #103

Merged
merged 19 commits into from
Jul 3, 2023
Merged

Update client to support Preview 10 #103

merged 19 commits into from
Jul 3, 2023

Conversation

Shaptic
Copy link
Contributor

@Shaptic Shaptic commented Jun 28, 2023

NEEDS RPC CHANGES MERGED FIRST.

Done:

  • yarn upgrade-interactive --latest on dev dependencies
  • upgrade stellar-base to preview 10
  • add durability ('persistent' | 'temporary') support to ledger keys
  • move to single-function invokes
  • move to single-result simulations
  • move to new auth xdr structure

Remaining:

The major change is to getContractData(), adding an optional parameter:

  /**
   * @param {Durability} [durability] - The "durability keyspace" that this
   *    ledger key belongs to, which is either 'temporary' or 'persistent' (the
   *    default), see {@link Durability}.
   * 
   * @warning If the data entry in question is a 'temporary' entry, it's
   *    entirely possible that it has expired out of existence. Future versions
   *    of this client may provide abstractions to handle that.
   **/

@Shaptic Shaptic marked this pull request as ready for review June 28, 2023 21:37
@Shaptic Shaptic changed the title [DRAFT] Update client to support Preview 10 Update client to support Preview 10 Jun 28, 2023
src/transaction.ts Outdated Show resolved Hide resolved
src/server.ts Outdated Show resolved Hide resolved
src/server.ts Outdated
*/
public async getContractData(
contractId: string,
key: xdr.ScVal,
expirationType: 'temporary' | 'persistent' = 'temporary'
Copy link
Contributor

Choose a reason for hiding this comment

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

3 suggestions:

  • Let's call this durability to match the API elsewhere, and in the xdr.
  • wdyt of making persistent the default? That is what we are pushing people towards.
  • Maybe extract the Durability to a type or enum?
Suggested change
expirationType: 'temporary' | 'persistent' = 'temporary'
durability: Durability = 'persistent'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh really? I thought temporary was actually preferred (i.e. if you want to pay for and worry about persistence, you should know its purpose & understand it better).

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this dialog suggests that not having a default might be the preferred way to go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess it begs the question of "how much do we want people to care about storage durability?" is the answer "when it's important", "usually", or "always"?

src/server.ts Outdated Show resolved Hide resolved
src/server.ts Outdated Show resolved Hide resolved
src/soroban_rpc.ts Outdated Show resolved Hide resolved
@Shaptic Shaptic merged commit 8554b7a into main Jul 3, 2023
@Shaptic Shaptic deleted the v0.9.0 branch July 3, 2023 17:54
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.

3 participants