-
Notifications
You must be signed in to change notification settings - Fork 22
feat(walletkit): how to work with Toncoin #1526
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
base: main
Are you sure you want to change the base?
Conversation
| } | ||
| ``` | ||
|
|
||
| Since the kit helps to manage initialized TON wallets, all outgoing transfers are controlled and will not happen on their own. |
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 not sure this sentence should be included. User can have wallet imported in different app for example.
Or you can send tx outside of wallet kit
|
|
||
| ### Confirm transaction delivery | ||
|
|
||
| TON achieves transaction [finality](https://en.wikipedia.org/wiki/Blockchain#Finality) after a single masterchain block confirmation, where new blocks are produced approximately every 5 seconds. Once a transaction appears in a masterchain block, it becomes irreversible. |
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.
Every 3 seconds
| "ecosystem/tma/telegram-ui/platform-and-palette", | ||
| { | ||
| "group": "Reference", | ||
| "pages": ["ecosystem/tma/telegram-ui/reference/avatar"] |
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.
Eh, JSON formatting — we should probably add Prettier for everything non-md and non-mdx. As of now, these changes will be reverted
| function formatTransfers( | ||
| transfers: Array<{ amount: bigint; asset: string }>, | ||
| ): string { | ||
| return transfers | ||
| .map((t) => `${Number(t.amount) / 1e9} ${t.asset}`) | ||
| .join(', '); | ||
| } |
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.
Overflows are not taken into account. And the function is just bad, rewrite it.
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 the updates in ecosystem/ton-connect/walletkit: I’ve left several suggestions to tighten tone, correct a reference, and add required safety context—please apply the inline suggestions.
| <Aside> | ||
| [Initialize the WalletKit](/ecosystem/ton-connect/walletkit/web/init), [set up at least one TON wallet](/ecosystem/ton-connect/walletkit/web/wallets), handle [connection requests](/ecosystem/ton-connect/walletkit/web/connections) and [transaction requests](/ecosystem/ton-connect/walletkit/web/events) before using examples on this page. | ||
| </Aside> | ||
|
|
||
| To work with Toncoin, the wallet service needs to handle [contract balances](#balances) and perform transfers initiated [from dApps](#transfers-from-dapps) and [from within the wallet service itself](#transfers-from-the-wallet-service). |
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.
[HIGH] Missing safety callout for funds‑moving Toncoin examples
The Toncoin guide contains runnable examples that construct and send Toncoin transfers via sendToncoin and createTransferTonTransaction, but the only page‑top <Aside> describes setup prerequisites and does not call out funds risk, environment (testnet vs mainnet), or mitigation. Per the safety‑critical content rules, any page that moves funds or changes fee/withdrawal behavior must include a visible Caution/Warning <Aside> summarizing risk, scope, environment, and safer defaults (see contribute/style-guide-extended.mdx §11.1–11.2). Without this, readers may run mainnet‑impacting transfer code without explicit risk framing. The primary location above covers where the missing page‑level safety callout should be introduced; the transfer examples themselves appear later on the page (for example, sendToncoin and createTransferTonTransaction at lines 261–302) and serve as evidence of funds movement.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
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.
Once again, I'm not sure callouts are needed for those examples given the context of implementing a wallet service that manages wallet contracts (and funds) :)
| - [Discrete one-off checks](#on-demand-balance-check) have almost no value on their own — the state might change immediately after the query completes, invalidating its results. Thus, such checks are only practical when handling `transaction` requests. | ||
| - [Continuous monitoring](#continuous-balance-monitoring) is useful for UI display, showing the most recent balance to users, but should not be used for transaction confirmations. | ||
|
|
||
| Notice that both cases require querying the blockchain data via the API client set during the [WalletKit initialization](/ecosystem/ton-connect/walletkit/web/init#param-api-client). Obtain and provide the key from the selected client to access higher requests-per-second limits. |
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.
[HIGH] Broken anchor to apiClient configuration parameter
Line 21 links to [WalletKit initialization](/ecosystem/ton-connect/walletkit/web/init#param-api-client) to reference the apiClient configuration. In web/init.mdx, the parameter is defined via <ParamField path="apiClient">, and the existing example for storage uses the #param-<path> slug pattern (for example, path="storage" referenced as #param-storage). There is no definition or heading that would generate an element with id="param-api-client" anywhere in the repo, so this anchor is almost certainly broken. The same incorrect anchor is also used later on this page (around L191–L193) and in web/events.mdx, so readers following these links will not land on the intended configuration parameter, violating the HIGH-severity rule for broken anchors.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
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.
That's not true — the header is generated by the <ParamField> component, but that's a rather implicit behavior.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Closes #366
Page preview: https://companyname-a7d5b98e-closes-366-walletkit-toncoin.mintlify.app/ecosystem/ton-connect/walletkit/web/toncoin