-
Notifications
You must be signed in to change notification settings - Fork 90
feat(sdk): setup SVM package #4189
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
Conversation
sdk/packages/svm/README.md
Outdated
|
||
The Omni SDK contains APIs for interfacing with Omni SolverNet, your gateway to any transaction, on any chain. | ||
|
||
Note - given we're in alpha, we're still making improvements, particularly on e2e type safety, testing, and documentation. If you have any feedback or requests, please reach out to us (telegram below). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can remove this now, and we should update the copy here to be SVM specific
unless we want the SVM to be in alpha? in which case we should use an alpha version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's going to be alpha at first, this is just copied from the core package README, I think we can edit when we're closer to a release and decide on alpha/beta or other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no worries, mind updating the copy though? something like this is easy to forget and quicker than deferring
or just remove everything and put TODO xD
sdk/packages/svm/package.json
Outdated
{ | ||
"name": "@omni-network/svm", | ||
"description": "SVM support for Omni Solvernet", | ||
"version": "0.2.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
version doesn't match the version.ts file, and should we start SVM package at 0.0.1 or alpha?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want core and react packages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the version.
At this point I don't think we'll have core or react packages specific to SVM, more that this package will need to be added alongside the others to add SVM support, but that's what I'll try to figure out for the ADR.
"bn.js": "^5.2.2", | ||
"bs58": "^6.0.0" | ||
}, | ||
"devDependencies": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some of these can be taken from catalog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems we are not putting typescript in the catalog so it's not clear to me what should or shouldn't go there, any more details please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no specific instructions, up to us really so feel free to hoist things, it just makes life easier to share common deps
@@ -0,0 +1,684 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these json files are pretty large, do we need all of this stuff? I don't think bundlers tree shake JSON files so have been following the practice of only pulling in what's necessary
also, do we need the JSON abi if we have the ts version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the instructions other than open
so we can add them back as needed, maybe there are further changes we can make but until all the flows are implemented it seems premature to try to optimise this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool thanks
why do we need json and ts abi ?
expense: EvmTokenExpense | ||
} | ||
|
||
export async function createOpenParams( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feels like a util, unless you disagree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather have this separate as it's specific to creating the open instruction, these modules and APIs will likely change a lot as more functionalities as added anyways.
Setup for SVM package, mainly logic to open an order
issue: none