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

ABI 0.2 (part 1/2) #3

Open
wants to merge 21 commits into
base: wrap-0.2-dev
Choose a base branch
from
Open

ABI 0.2 (part 1/2) #3

wants to merge 21 commits into from

Conversation

namesty
Copy link

@namesty namesty commented Jan 18, 2023

This PR includes:

  • New ABI types
  • Usage examples
  • New JSONSchema
  • Documentation on introduced concepts, decisions made, future plans

@dOrgJelli
Copy link
Member

Do we want to target a different branch that will act as more of a "staging" branch for wrap 0.2? I think wrap-0-2 is currently pretty heavily under development with the work @nerfZael and @Niraj-Kamdar are doing at the moment. Maybe we should:

  1. create a wrap-0.2-dev branch from master.
  2. rename the wrap-0-2 branch you all are working in to something more specific to the standards you all are working on.

@Niraj-Kamdar Niraj-Kamdar self-requested a review February 3, 2023 11:48
Copy link
Contributor

@Niraj-Kamdar Niraj-Kamdar left a comment

Choose a reason for hiding this comment

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

I have left some comments.

abi/0.2.json Show resolved Hide resolved
abi/abi-0.2.ts Show resolved Hide resolved
abi/abi-0.2.ts Show resolved Hide resolved
{
uri: "lib2",
type: "wasm",
id: "0",
Copy link
Contributor

Choose a reason for hiding this comment

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

What if we use IPFS hash of the abi as id?
It's guaranteed to be unique and since content addressable no need to embed the imported abi.

Copy link
Member

Choose a reason for hiding this comment

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

The goal of the "id" is to be a (1) small in size & (2) efficient to lookup (i.e. strcmp). The URI does not satisfy these things in my opinion.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it has its own benefits as I discussed and we should compare both and see which is a better way going forward.

@Niraj-Kamdar Niraj-Kamdar changed the base branch from wrap-0-2 to wrap-0.2-dev February 17, 2023 16:40
@dOrgJelli
Copy link
Member

Responded @Niraj-Kamdar, would you feel comfortable approving in its current state?

@dOrgJelli dOrgJelli changed the title ABI 0.2 ABI 0.2 (part 1/2) Feb 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

Successfully merging this pull request may close these issues.

4 participants