-
Couldn't load subscription status.
- Fork 43
create with-iota example #906
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit b6e989b:
|
|
will need to resolve lockfile conflicts before merging. probably best to wait for #917 to go through and either rebase or cherrypick the commit. but should work as is for testing and approval. |
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.
Overall looks great! Just some minor comments on organization
examples/with-iota/src/index.ts
Outdated
| const provider = new IotaClient({ url: getFullnodeUrl('testnet') }); | ||
| const publicKey = new Ed25519PublicKey(Buffer.from(IOTA_PUBLIC_KEY!, 'hex')); | ||
|
|
||
| if (publicKey.toIotaAddress() !== IOTA_ADDRESS) { |
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.
Nit: Maybe group the public key check above with the iota address + public key undefined check for organization
examples/with-iota/src/index.ts
Outdated
| digest: coins.data[0]!.digest, | ||
| }, | ||
| ]); | ||
| const coin = tx.splitCoins(tx.gas, [tx.pure('u64', amount)]); |
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.
Maybe a quick comment here to explain what splitCoins is doing and how that affects the tx building that seems to be happening above and below this
| const signature = Buffer.from(r + s, 'hex'); | ||
| const serialized = toSerializedSignature({ signature, pubKey: publicKey }); | ||
|
|
||
| const result = await provider.executeTransactionBlock({ |
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.
Also generally grouping together and commenting for "environment checking", "transaction building" and "execution" would be helpful!
Summary & Motivation
Iota are a new partner and should have an example integration in our sdk (and docs).
also changed
with-suiexample to match the more up to date one from our docs.How I Tested These Changes
not very successfully, @ reviewer please test
Did you add a changeset?
If updating one of our packages, you'll likely need to add a changeset to your PR. To do so, run
pnpm changeset.pnpm changesetwill generate a file where you should write a human friendly message about the changes. Note how this (example) includes the package name (should be auto added by the command) along with the type of semver change (major.minor.patch) (which you should set).These changes will be used at release time to determine what packages to publish and how to bump their version. For more context see this comment.