-
Notifications
You must be signed in to change notification settings - Fork 14
applying rules from styleguide on your first smart contract #1233
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
|
To fix the formatting issues:
npx remark -o --silent --silently-ignore contract-dev/first-smart-contract.mdx |
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—I've proposed several targeted edits in contract-dev/first-smart-contract.mdx. Please apply the inline suggestions to move this forward.
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—nice progress. In contract-dev/first-smart-contract.mdx: several suggestions are ready; please apply the inline suggestions.
| <Step | ||
| title="Run the script" | ||
| > |
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 transfer
This section triggers an on-chain transaction that sends Toncoin, but there is no visible safety callout warning readers about risk, scope, rollback/mitigation, and environment. The deployment script sends value via toNano('0.05') when calling sendDeploy (evidence:
docs/contract-dev/first-smart-contract.mdx
Line 362 in 0e83352
| await firstContract.sendDeploy(provider.sender(), toNano('0.05')); |
<Aside> component. See https://github.com/ton-org/docs/blob/main/contribute/style-guide-extended.mdx?plain=1#L541-L563 for requirements.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
| Create a script `./scripts/sendIncrease.ts` that increases the counter: | ||
|
|
||
| const contractAddress = Address.parse('<CONTRACT_ADDRESS>'); | ||
|
|
||
| export async function run(provider: NetworkProvider) { | ||
| const firstContract = provider.open(new FirstContract(contractAddress)); | ||
| await firstContract.sendIncrease(provider.sender(), { value: toNano('0.05'), increaseBy: 42 }); | ||
| await provider.waitForLastTransaction(); | ||
| } | ||
| ``` | ||
|
|
||
| Do not forget to replace `<CONTRACT_ADDRESS>` with your actual contract address from Step 5! | ||
|
|
||
| #### Understanding the script breakdown: | ||
|
|
||
| - **Address parsing**: `Address.parse()` converts the string address to a TON Address object | ||
| - **Contract opening**: `provider.open()` creates a connection to the deployed contract | ||
| - **Value attachment**: `toNano('0.05')` converts 0.05 TON to nanotons (the smallest TON unit) | ||
| - **Message parameters**: `increaseBy: 42` tells the contract to increase the counter by 42 | ||
| - **Transaction waiting**: `waitForLastTransaction()` waits for the transaction to be processed on-chain | ||
|
|
||
| To run this script: | ||
|
|
||
| ```bash | ||
| npx blueprint run sendIncrease --testnet --tonconnect --tonviewer | ||
| ``` | ||
| ```typescript title="./scripts/sendIncrease.ts" |
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] Placeholder <CONTRACT_ADDRESS> not defined at first use
<CONTRACT_ADDRESS> first appears inside the code block, and its definition appears afterward. Per the placeholder rule, definitions must appear at first use to prevent incorrect copying (see https://github.com/ton-org/docs/blob/main/contribute/style-guide-extended.mdx?plain=1#L473-L477). Move the definition to precede the code block.
| Create a script `./scripts/sendIncrease.ts` that increases the counter: | |
| const contractAddress = Address.parse('<CONTRACT_ADDRESS>'); | |
| export async function run(provider: NetworkProvider) { | |
| const firstContract = provider.open(new FirstContract(contractAddress)); | |
| await firstContract.sendIncrease(provider.sender(), { value: toNano('0.05'), increaseBy: 42 }); | |
| await provider.waitForLastTransaction(); | |
| } | |
| ``` | |
| Do not forget to replace `<CONTRACT_ADDRESS>` with your actual contract address from Step 5! | |
| #### Understanding the script breakdown: | |
| - **Address parsing**: `Address.parse()` converts the string address to a TON Address object | |
| - **Contract opening**: `provider.open()` creates a connection to the deployed contract | |
| - **Value attachment**: `toNano('0.05')` converts 0.05 TON to nanotons (the smallest TON unit) | |
| - **Message parameters**: `increaseBy: 42` tells the contract to increase the counter by 42 | |
| - **Transaction waiting**: `waitForLastTransaction()` waits for the transaction to be processed on-chain | |
| To run this script: | |
| ```bash | |
| npx blueprint run sendIncrease --testnet --tonconnect --tonviewer | |
| ``` | |
| ```typescript title="./scripts/sendIncrease.ts" | |
| Create a script `./scripts/sendIncrease.ts` that increases the counter. | |
| Define `<CONTRACT_ADDRESS>` — the deployed contract address obtained in the deployment step: | |
| ```typescript title="./scripts/sendIncrease.ts" |
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
| 1. **Direct contract call**: The `getCounter()` method directly calls the contract's `currentCounter` getter | ||
| 1. **Instant response**: The result is returned immediately without blockchain confirmation | ||
| 1. **Data parsing**: Our wrapper automatically converts the returned stack value to a JavaScript number | ||
| To call a get method, use `provider.get(<GET_METHOD_NAME>)`: |
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] Placeholder not defined on first use (<GET_METHOD_NAME>)
The placeholder <GET_METHOD_NAME> is introduced in “use provider.get(<GET_METHOD_NAME>)” without an immediate definition. Placeholders must be defined at first use to prevent copy/paste mistakes (see https://github.com/ton-org/docs/blob/main/contribute/style-guide-extended.mdx?plain=1#L473-L477). Define it in the text where it first appears.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
/review |
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 contract-dev/first-smart-contract.mdx, I left several suggestions—please apply the inline suggestions.
verytactical
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.
AI review
|
To fix the formatting issues:
npx remark -o --silent --silently-ignore contract-dev/first-smart-contract.mdx |
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 contract-dev/first-smart-contract.mdx: I’ve left several suggestions; please apply the inline suggestions.
closes #934