feat(contract-dev): edit jetton processing guide to fit style guide.#2125
feat(contract-dev): edit jetton processing guide to fit style guide.#2125
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRewrote the on-chain jetton processing doc into a three-approach comparison: manual wallet management, automatic wallet discovery (TEP-89), and on-chain get-method emulation. Intro and threat diagram updated to emphasize verifying the jetton wallet; multiple sequence diagrams and callouts added or refactored. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contract-dev/techniques/on-chain-jetton-processing.mdx`:
- Line 12: The sentence contains a typo "a contracts must verify" in the
JettonNotify warning; update the wording to be grammatically correct and clear
(e.g., change to "a contract must verify" or "contracts must verify") so the
security note about verifying that a JettonNotify comes from the correct jetton
wallet address reads cleanly; ensure the terms JettonNotify and jetton wallet
address remain unchanged.
- Line 101: The phrase saying a jetton wallet "shares an owner" with the
receiving contract is inaccurate; update the wording around
SetTrustedJettonWallet to state that the trusted jetton wallet must be the
jetton wallet derived for the receiving contract's address (i.e., the jetton
wallet whose owner is the receiving contract address), not the contract's
admin/owner, and replace the ambiguous language with this precise explanation
and a link/reference to the TON Docs find jetton wallet procedure.
- Around line 223-224: The wording is inaccurate: TEP-89 defines internal
messages named provide_wallet_address and take_wallet_address (not get-methods),
so update the sentence to say "TEP-89 defines the internal messages
provide_wallet_address and take_wallet_address" and change any "get-methods"
reference to "internal messages"; also correct agreement from "platform such as
DeDust only allows them" to plural "platforms such as DeDust only allow them" to
match plural "platforms".
- Around line 119-121: The InitContract flow must attach enough TON to fund the
TEP-89 response so the minter can call take_wallet_address; update the
docs/examples around InitContract and the ProvideWalletAddress call (which uses
SEND_MODE_CARRY_ALL_REMAINING_MESSAGE_VALUE) to explicitly state and show that
sufficient value is included, otherwise take_wallet_address will fail and
jettonWalletAddress remains unset; reference InitContract, ProvideWalletAddress,
take_wallet_address, jettonWalletAddress and TEP-89 in the clarification so
readers know where to add the funds.
- Around line 22-29: The table row labeled "TEP-89 compatibility" should be
renamed to something like "TEP-89 required" or "Requires TEP-89" to avoid
implying incompatibility, and update the three column values so they indicate
whether the approach requires TEP-89 (e.g., Manual management: No, Automatic
discovery: Yes, Get-method emulation: No); update any adjacent explanatory text
in the sections "Manual management", "Automatic discovery", and "Get-method
emulation" to clarify that TEP-89-compatible jettons can still be used with the
first and third approaches even though they don’t require TEP-89.
- Around line 279-288: Update the wording around validating StateInit in the
JettonNotify flow to clarify that hashing a supplied StateInit only proves the
initial state (not necessarily the current code/data used by
get_wallet_address/calculateJettonWallet) and therefore this low-trust path must
require a current-state proof (or on-chain proof of code/data) unless the guide
explicitly states the minter is immutable; mention JettonNotify, StateInit,
jettonMinter, get_wallet_address and calculateJettonWallet by name and add a
short note linking to TON upgrade/address derivation concepts so readers know to
require a current state proof when they don’t control the minter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7aa9e7a2-bad2-490c-8389-cdd2cdbfcc95
📒 Files selected for processing (1)
contract-dev/techniques/on-chain-jetton-processing.mdx
There was a problem hiding this comment.
Pull request overview
Updates the “On-chain jetton processing” technique doc to better match the documentation style guide (closes #2117), including callout formatting, restructured sections, and refreshed diagrams.
Changes:
- Standardizes callouts using
<Aside type="...">and rewrites the intro for clearer guidance. - Reorganizes the content into three verification approaches with a comparison table.
- Replaces/updates diagrams and code block titles to improve readability.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --- | ||
| title: "On-chain Jetton processing" | ||
| title: "On-chain jetton processing" | ||
| sidebarTitle: "Jetton processing" |
There was a problem hiding this comment.
[HIGH] Page title lowercases proper noun “Jetton”
In the frontmatter, the updated title uses “On-chain jetton processing”, which lowercases “Jetton” even though it is treated as a canonical proper noun in the docs and terminology. This violates the style guide’s requirement to preserve capitalization for named standards and branded terms while otherwise using sentence case for headings. Keeping “Jetton” capitalized in the page title preserves consistency with the terminology section and with other occurrences in this document.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
|
Thanks for the updates in Per-comment submission: 1 posted, 1 failed. Unposted inline comments (raw text):
|
Closes #2117.
Summary by CodeRabbit