-
Notifications
You must be signed in to change notification settings - Fork 7
feat(NFT): add NFT how to transfer #708
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
…fy-ton-docs into add-NFT-how-to-transfer
|
Thanks for the updates to the NFT transfer guide. A few high‑severity fixes are needed before this can merge. Findings (5)High (5)[HIGH] Missing safety callout for keys and fund transferDescription: Suggestion: @@
-Or you can do it manually. Below there is an illustrative example:
+Or you can do it manually. Below there is an illustrative example:
+
+import { Aside } from '/snippets/aside.jsx';
+
+<Aside type="danger" title="Funds and keys at risk">
+ Risk: This example uses a private mnemonic and sends TON value.
+ Scope: Affects your wallet and transferred assets.
+ Rollback: Transfers are irreversible. If a mnemonic is exposed, rotate it and move funds immediately.
+ Environment: Use TON Testnet and a disposable test wallet. Do not use real mnemonics on mainnet.
+</Aside>[HIGH] Inline mnemonic encourages unsafe secret handlingDescription: Suggestion: - const your_mnemonic = "put your mnemonic here, ...";
- const keyPair = await mnemonicToPrivateKey(your_mnemonic.split(" "));
+ const MNEMONIC = process.env.MNEMONIC as string; // set MNEMONIC in your environment (testnet only)
+ const keyPair = await mnemonicToPrivateKey(MNEMONIC.split(" "));[HIGH] Invalid placeholder format and undefined placeholders in codeDescription: Suggestion: - const NFT_ADDRESS = Address.parse("put your NFT item address");
+ const NFT_ADDRESS = Address.parse("<NFT_ITEM_ADDR>");
@@
- const RECEIVER_ADDRESS = Address.parse("put receiver address");
+ const RECEIVER_ADDRESS = Address.parse("<RECEIVER_ADDR>");Define placeholders (first use) below the code:
[HIGH] Incorrect field name for NFT transfer conditionDescription: Suggestion: -If `forward_ton_amount > 0`, then `forward_payload` must comply with one of the [standard formats](https://github.com/ton-blockchain/TEPs/blob/master/text/0062-nft-standard.md#forward_payload-format).
+If `forward_amount > 0`, then `forward_payload` must comply with one of the [standard formats](https://github.com/ton-blockchain/TEPs/blob/master/text/0062-nft-standard.md#forward_payload-format).[HIGH] TEP links use moving “master” branch instead of stable permalinksDescription: Suggestion: - NFT transfer is an operation specified in [TEP 0062](https://github.com/ton-blockchain/TEPs/blob/master/text/0062-nft-standard.md). Its implementation must comply with this standard,
+ NFT transfer is an operation specified in [TEP‑62](https://github.com/ton-blockchain/TEPs/blob/1fbc23cac69723c53251f686ec90d81bf0e83443/text/0062-nft-standard.md). Its implementation must comply with this standard,
@@
- If `forward_ton_amount > 0`, then `forward_payload` must comply with one of the [standard formats](https://github.com/ton-blockchain/TEPs/blob/master/text/0062-nft-standard.md#forward_payload-format).
+ If `forward_ton_amount > 0`, then `forward_payload` must comply with one of the [standard formats](https://github.com/ton-blockchain/TEPs/blob/1fbc23cac69723c53251f686ec90d81bf0e83443/text/0062-nft-standard.md#forward_payload-format). |
…fy-ton-docs into add-NFT-how-to-transfer
anton-trunov
left a comment
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.
Please fix the merge conflicts
This manually creation was there! But then Philip decided to exclude that from the arcticle for some reasons. |
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 update. I left several suggestions in standard/tokens/nft/how-to-transfer.mdx:; please apply the inline suggestions.
| NFT transfer is an operation specified in [TEP 0062](https://github.com/ton-blockchain/TEPs/blob/master/text/0062-nft-standard.md), see also [here](/standard/tokens/nft/how-it-works#transfer-nft-item). Its implementation must comply with this standard, and NFT item contracts must support the logic described there. The NFT transfer message is also specified and must comply with the corresponding TL-B scheme. This scheme involves, among other things, | ||
|
|
||
| <Stub issue="668" /> | ||
| - `forward_amount`: the amount of nanotons to be sent to the destination address; | ||
| - `forward_payload`: optional custom data that should be sent to the destination address. | ||
|
|
||
| `forward_payload` must comply with one of the [standard formats](https://github.com/ton-blockchain/TEPs/blob/master/text/0062-nft-standard.md#forward_payload-format). |
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] External spec links use moving target instead of a stable permalink
The references to the NFT TEP use links pinned to the repository’s master branch (L6 and L11). Normative references must use a versioned or permanent URL (e.g., a specific commit) to avoid drift and preserve reproducibility.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
|
add Tolk and tl-b after 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.
No documentation issues detected.
There will be a separate issue for that. Please, remove the Request changes. |
anton-trunov
left a comment
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.
Please resolve the merge conflicts and AI review issues
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 update. In standard/tokens/nft/transfer.mdx I left several suggestions; please apply the inline suggestions.
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 update—there’s one inline suggestion in standard/tokens/nft/transfer.mdx: please apply it.
Closes #668