Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Initial implementation of the Armory SDK #241
Initial implementation of the Armory SDK #241
Changes from 9 commits
cc37b76
22bd57c
5decf21
fb51d97
861595c
5341487
bbd17c9
fadfd04
db094cb
fcaa9a7
b1303cd
ca0391a
050cff4
56bb8cc
be3eb5c
250a5f0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
Thanks for adding an example project.
I'm thinking about a better name that describes what it's properly. What do you think
armory-sdk-nodejs
? I'm adding the runtime in the name of the example because it doesn't solve the problem of using it on the browser.Remember that the directory structure must reflect the project name (e.g.
examples/armory-sdk-nodejs/project.json
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.
Yeah that is actually a very good naming. I tried multiple ones, and gave up with this one as nothing was satisfying.
I did 'basic' because I'm expecting other examples will complexify with time, and this example felt like the most straightforward way to use our system
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.
Why have you changed the naming?
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'm expecting that we will have an armory client later, that will be instantiated and extends viem clients. It will have other capabilities.
Also, as we integrate with other tools in web3, when they have a client its almost always something that was instantiated with a transport and that has actual broadcasting capabilities. Let's stay on that path, and keep this naming for when we have something with these capabilities
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.
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.
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.
signRequest
is a confusing/misleading name because of the action of signing the request to send it to the API.Why haven't you implement a specific function like
signTransaction
that takes aSignTransactionAction
?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.
What's this?
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.
Generated by Nx when I generated Armory-sdk as a publishable lib
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.
Can we have a proper README?