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

feat: Generate TS from XDR #871

Merged
merged 70 commits into from
Jun 12, 2023
Merged

Conversation

willemneal
Copy link
Member

What

Generate a TS file from XDR to allow for building TS interfaces from contracts.

Why

Allow Dapps to create custom interfaces for the contracts they interact with. This will be integrated with the CLI to be part of workflow.

Known limitations

[TODO or N/A]

@willemneal willemneal force-pushed the feat/gen_ts branch 3 times, most recently from 32c25f7 to 806986f Compare May 20, 2023 15:05
- No need to include files that don't have variables
- Those backticks were silly
this removes the need for the `@signme` comments in the contract
I asked Dmitry: "if preflight returns an `auth` array, should we sign
the whole transaction?"

He answered:

> not necessarily. `ContractAuth` entries do need a signature
> to be filled in. if `addressWithNonce` is missing, then it's invoker auth
> and can be signed as a part of tx. otherwise, the respective `Address` has
> to sign the payload corresponding to `ContractAuth` (you can find more
> details in this doc:
> https://soroban.stellar.org/docs/how-to-guides/invoking-contracts-with-transactions#authorization-data)

This seems to cover those alternative cases, just throwing
NotImplementedErrors for now, but at least it's not quietly or
confusingly failing.
@willemneal
Copy link
Member Author

I updated the branch and it even shows it on GH, but it is not showing up here for me.

@willemneal
Copy link
Member Author

Just changed the target branch to itself and that seemed to fix it.

willemneal and others added 10 commits June 9, 2023 11:28
- use a build script to create both ES Modules and CommonJS
- specify `exports` in `package.json` so that consumers that use this
  library with `import` statements get ESM and those that use it with
  `require` get CJS.
- add `.js` extensions to file paths. This should be inserted
  automatically by TypeScript during the build, but it's not, and for
  some reason NextJS can't consume ESM that doesn't have it.
- remove environment variables. NextJS needs to have them with
  `process.env`, Astro needs to have them with `import.meta.env`, and
  there's no good way to have both. We'll be modifying this code soon
  anyhow.
Copy link
Contributor

@paulbellamy paulbellamy left a comment

Choose a reason for hiding this comment

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

A couple optional formatting/text suggestions, but LGTM.

soroban-spec/fixtures/ts/src/index.ts Outdated Show resolved Hide resolved
soroban-spec/fixtures/ts/src/invoke.ts Outdated Show resolved Hide resolved
soroban-spec/fixtures/ts/src/invoke.ts Outdated Show resolved Hide resolved
@leighmcculloch leighmcculloch merged commit 7d143e4 into stellar:release/v0.8.5 Jun 12, 2023
@leighmcculloch
Copy link
Member

leighmcculloch commented Jun 12, 2023

Approved and merged into release/v0.8.5, but I think we should plan a couple follow up tasks:

  • This change needs opening against main. We won't commit a merge commit of release/v0.8.5 into main because we don't use merge commits in this repository. Someone can open a merge as a PR then we'll squash and merge that commit into main. Or the below items could be visited and the final product merged into main.

  • Break up the soroban-spec crate into soroban-spec, soroban-spec-rust, soroban-spec-json, soroban-spec-typescript. Why: Having typescript dependencies in the soroban-sdk repo is going to make development here pretty complex, and ultimately the typescript generation code needs a unique set of reviewers/owners who are different to those of the rest of the code here. We can house the soroban-spec-typescript crate in the soroban-tools repo.

  • Remove the dependence on the JSON types so that conversion from XDR spec to typescript is ScSpecEntry -> TypeScript, and not ScSpecEntry -> JSON -> TypeScript. Why: For the reasons mentioned at feat: Generate TS from XDR #871 (comment), feat: Generate TS from XDR #871 (comment), and feat: Generate TS from XDR #871 (review).

cc @paulbellamy @tsachiherman

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.

5 participants