refactor(contract-dev): port gas article code samples to Tolk#2101
refactor(contract-dev): port gas article code samples to Tolk#2101delovoyhomie wants to merge 9 commits into
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe documentation migrated gas estimation examples from Tact to Tolk, adding a Value flow section and updating fee, forward-fee, storage-fee, and receiver validation examples to use Tolk APIs and assert-based errors. ChangesDocumentation Code Examples
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
contract-dev/gas.mdx (2)
144-152:⚠️ Potential issue | 🟡 MinorStale prose references
computeDataSize()after Tolk port.The Aside still talks about
computeDataSize()and its "second argument: maximum number of cells to visit", but the code at Lines 134-135 now usescell.calculateSizeStrict(8192). Similarly, Line 152's "In Tolk, both computations can be performed with a call tomsg.send()with mode 1024" is disconnected from the preceding snippet (nomsg.send()example is shown) and the old Tact-era phrasing leaks through.Please rewrite this Aside and the following paragraph to reference
calculateSizeStrict()/calculateForwardFee()by their Tolk names so readers can actually find them in the stdlib.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contract-dev/gas.mdx` around lines 144 - 152, The Aside and following paragraph still reference the old computeDataSize() API and Tact-era phrasing; update them to mention Tolk stdlib functions calculateSizeStrict(maxCells) and calculateForwardFee() instead (replace “computeDataSize” and its "second argument" wording with calculateSizeStrict(8192) and explain the maxCells parameter), and change the Tolk example text to reference using msg.send() with mode 1024 as a way to perform the same computations in Tolk while keeping the TVM reference to SENDRAWMSG; ensure the prose points readers to the stdlib names calculateSizeStrict and calculateForwardFee so links/searches work.
165-189:⚠️ Potential issue | 🟠 MajorFix function name and parameters to match Tolk stdlib signature.
Two issues here:
Line 179 calls
getSimpleForwardFee(...), but the prose at line 169 and the Tolk stdlib reference both confirm the canonical function iscalculateForwardFeeWithoutLumpPrice.The function call passes
isAccountInMasterchainas a parameter, but examining the parallel functioncalculateForwardFee(workchain, bits, cells)at lines 137–142 establishes the correct pattern: all parameters should be explicit, withworkchainas the first parameter, followed bybitsandcells.🔧 Suggested fix
- val additionalFwdFee = getSimpleForwardFee( - additionalFieldsSize.cells, - additionalFieldsSize.bits, - isAccountInMasterchain - ); + val additionalFwdFee = calculateForwardFeeWithoutLumpPrice( + workchain, + additionalFieldsSize.bits, + additionalFieldsSize.cells, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contract-dev/gas.mdx` around lines 165 - 189, The call to getSimpleForwardFee should be renamed to calculateForwardFeeWithoutLumpPrice and its arguments reordered to match the Tolk stdlib: pass the workchain identifier as the first argument, then bits, then cells (i.e., use the same signature pattern as calculateForwardFee(workchain, bits, cells)); replace the isAccountInMasterchain argument with the actual workchain value for the account/message and pass additionalFieldsSize.bits and additionalFieldsSize.cells in that order.
🧹 Nitpick comments (1)
contract-dev/gas.mdx (1)
234-258: Unusedmsgbinding and missinglazyin message-parse examples.Minor refactor consistency items across the ported
onInternalMessagesamples (Lines 215, 238, 279, 300, 331):
- The recommended Tolk idiom per
tolk/features/message-handling.mdx:7-62isval msg = lazy X.fromSlice(in.body);— readers copying from these docs miss the perf benefit without thelazykeyword.- In the samples on Lines 238 and 279 the parsed
msgis declared but never referenced. Either use it (e.g. inmsg.amountor similar) or drop the binding so the snippet doesn't read as dead code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contract-dev/gas.mdx` around lines 234 - 258, The sample onInternalMessage examples declare val msg = UserIn.fromSlice(in.body) but omit the recommended lazy and in some snippets the binding is unused; update the parsing to use the lazy idiom (e.g., make the binding lazy by changing the declaration to use lazy with UserIn.fromSlice) and either reference msg where needed (e.g., use msg fields in the fee/assert logic) or remove the unused val msg binding so there is no dead code; target the onInternalMessage examples and the val msg declarations to apply these changes.
🤖 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/gas.mdx`:
- Around line 298-308: Replace the invalid placeholder and the non-existent
function: in onInternalMessage (and any other places using getFreezeLimit), set
otherFees to a concrete value (e.g., 0) instead of `...`, and replace calls to
getFreezeLimit(workchain) with the actual freeze_due_limit configuration value
(100000000 nanotons) or a call to the real config accessor if one exists in your
codebase; ensure the assertion uses the concrete freeze_due_limit and the
concrete otherFees variable so the snippet compiles (references:
onInternalMessage, InMessage, Operation.fromSlice, otherFees, getFreezeLimit,
freeze_due_limit).
---
Outside diff comments:
In `@contract-dev/gas.mdx`:
- Around line 144-152: The Aside and following paragraph still reference the old
computeDataSize() API and Tact-era phrasing; update them to mention Tolk stdlib
functions calculateSizeStrict(maxCells) and calculateForwardFee() instead
(replace “computeDataSize” and its "second argument" wording with
calculateSizeStrict(8192) and explain the maxCells parameter), and change the
Tolk example text to reference using msg.send() with mode 1024 as a way to
perform the same computations in Tolk while keeping the TVM reference to
SENDRAWMSG; ensure the prose points readers to the stdlib names
calculateSizeStrict and calculateForwardFee so links/searches work.
- Around line 165-189: The call to getSimpleForwardFee should be renamed to
calculateForwardFeeWithoutLumpPrice and its arguments reordered to match the
Tolk stdlib: pass the workchain identifier as the first argument, then bits,
then cells (i.e., use the same signature pattern as
calculateForwardFee(workchain, bits, cells)); replace the isAccountInMasterchain
argument with the actual workchain value for the account/message and pass
additionalFieldsSize.bits and additionalFieldsSize.cells in that order.
---
Nitpick comments:
In `@contract-dev/gas.mdx`:
- Around line 234-258: The sample onInternalMessage examples declare val msg =
UserIn.fromSlice(in.body) but omit the recommended lazy and in some snippets the
binding is unused; update the parsing to use the lazy idiom (e.g., make the
binding lazy by changing the declaration to use lazy with UserIn.fromSlice) and
either reference msg where needed (e.g., use msg fields in the fee/assert logic)
or remove the unused val msg binding so there is no dead code; target the
onInternalMessage examples and the val msg declarations to apply these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contract-dev/techniques/gas.mdx (1)
179-183:⚠️ Potential issue | 🟠 MajorReplace the non-existent
getSimpleForwardFeeAPI with the Tolk stdlib function.The code at line 179 uses
getSimpleForwardFee(), which does not exist in Tolk's stdlib. The correct function iscalculateForwardFeeWithoutLumpPrice(workchain: int8, bits: int, cells: int), mentioned in the preceding paragraph. However, the function signatureonInternalMessage(in: InMessage)does not have access to aworkchainparameter. Determine the appropriate workchain value (or how to obtain it from the context) before applying the fix. See Tolk gas-payments stdlib for the correct API signature.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contract-dev/techniques/gas.mdx` around lines 179 - 183, Replace the nonexistent getSimpleForwardFee call in onInternalMessage with the Tolk stdlib calculateForwardFeeWithoutLumpPrice and pass a proper workchain value: derive workchain as int8 from isAccountInMasterchain (use -1 for masterchain, 0 for basic workchain) and call calculateForwardFeeWithoutLumpPrice(workchain, additionalFieldsSize.bits, additionalFieldsSize.cells) so the argument order matches the stdlib signature.
♻️ Duplicate comments (1)
contract-dev/techniques/gas.mdx (1)
302-303:⚠️ Potential issue | 🟡 MinorReplace the unresolved freeze-limit placeholder.
getFreezeLimit(workchain)is not a Tolk stdlib helper, and...is not valid Tolk syntax. Use a real config parser or a concretefreeze_due_limitconstant, and reuse it in the final validation snippet too. This duplicates a prior review comment but remains unresolved. Refs: Tolk stdlib docs, TON limits docs.🔧 Possible illustrative fix
+const FreezeDueLimit = ton("0.1"); + fun onInternalMessage(in: InMessage) { val msg = Operation.fromSlice(in.body); // The trace is still receiver -> A -> B - val freezeLimit = getFreezeLimit(workchain); - val otherFees = ...; + val freezeLimit = FreezeDueLimit; + val otherFees = 0; // compute + forward fees go here // n equals 3 because receiver -> A -> B assert (in.valueCoins >= freezeLimit * 3 + otherFees) throw ERR_NOT_ENOUGH_TONS; }- 3 * getFreezeLimit(workchain); + 3 * FreezeDueLimit;Verification:
#!/bin/bash set -euo pipefail gas="$(curl -fsSL https://raw.githubusercontent.com/ton-blockchain/ton/master/crypto/smartcont/tolk-stdlib/gas-payments.tolk)" if printf '%s\n' "$gas" | grep -n 'getFreezeLimit'; then echo "Unexpected getFreezeLimit API found in Tolk stdlib" >&2 exit 1 else echo "getFreezeLimit is not present in Tolk gas-payments stdlib" fi curl -fsSL https://docs.ton.org/foundations/limits | grep -E 'freeze_due_limit|100000000'Also applies to: 348-348
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contract-dev/techniques/gas.mdx` around lines 302 - 303, The snippet uses a non-existent helper getFreezeLimit(workchain) and an invalid '...' placeholder; replace them by declaring a concrete constant (e.g. freeze_due_limit = <value_from_TON_limits_or_config>) or parsing it from a real config parser, then assign val freezeLimit = freeze_due_limit and compute val otherFees explicitly instead of '...'; update the final validation snippet to reuse freeze_due_limit/freezeLimit (and remove getFreezeLimit and '...') so all references (freezeLimit, otherFees) are real values derived from the declared constant or parsed config.
🧹 Nitpick comments (1)
contract-dev/techniques/gas.mdx (1)
288-288: UseBounceMode.NoBounceinstead of deprecated boolean bounce.Current Tolk stdlib marks
falseas deprecated for the bounce field; documentation examples should use the enum form. The Tolk stdlib deprecation notice explicitly recommendsBounceMode.NoBounceinstead.♻️ Proposed refactor
- bounce: false, + bounce: BounceMode.NoBounce,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contract-dev/techniques/gas.mdx` at line 288, The example uses the deprecated boolean bounce flag; replace the boolean usage (bounce: false) with the enum form (bounce: BounceMode.NoBounce) and ensure the BounceMode symbol is in scope (import or reference the enum where the message/transaction object is constructed). Update the object property to use BounceMode.NoBounce and remove the boolean literal so the code uses the recommended enum constant.
🤖 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/gas.mdx`:
- Around line 107-109: The snippet uses the helper function calculateGasFee but
Tolk only auto-imports common.tolk, so add the explicit import for the fee
helpers by inserting import "@stdlib/gas-payments"; immediately before the
calculateGasFee usage (ensure the import appears at the top of the tolk code
block containing calculateGasFee so the fee helper symbols are available).
---
Outside diff comments:
In `@contract-dev/techniques/gas.mdx`:
- Around line 179-183: Replace the nonexistent getSimpleForwardFee call in
onInternalMessage with the Tolk stdlib calculateForwardFeeWithoutLumpPrice and
pass a proper workchain value: derive workchain as int8 from
isAccountInMasterchain (use -1 for masterchain, 0 for basic workchain) and call
calculateForwardFeeWithoutLumpPrice(workchain, additionalFieldsSize.bits,
additionalFieldsSize.cells) so the argument order matches the stdlib signature.
---
Duplicate comments:
In `@contract-dev/techniques/gas.mdx`:
- Around line 302-303: The snippet uses a non-existent helper
getFreezeLimit(workchain) and an invalid '...' placeholder; replace them by
declaring a concrete constant (e.g. freeze_due_limit =
<value_from_TON_limits_or_config>) or parsing it from a real config parser, then
assign val freezeLimit = freeze_due_limit and compute val otherFees explicitly
instead of '...'; update the final validation snippet to reuse
freeze_due_limit/freezeLimit (and remove getFreezeLimit and '...') so all
references (freezeLimit, otherFees) are real values derived from the declared
constant or parsed config.
---
Nitpick comments:
In `@contract-dev/techniques/gas.mdx`:
- Line 288: The example uses the deprecated boolean bounce flag; replace the
boolean usage (bounce: false) with the enum form (bounce: BounceMode.NoBounce)
and ensure the BounceMode symbol is in scope (import or reference the enum where
the message/transaction object is constructed). Update the object property to
use BounceMode.NoBounce and remove the boolean literal so the code uses the
recommended enum constant.
🪄 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: e08d6a54-9838-4ca1-9864-22043048c71e
📒 Files selected for processing (1)
contract-dev/techniques/gas.mdx
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| fun onInternalMessage(in: InMessage) { | ||
| val workchain = contract.getAddress().getWorkchain(); | ||
| val minTonsForStorage = calculateStorageFee( | ||
| workchain, | ||
| secondsInFiveYears, | ||
| isAccountInMasterchain | ||
| maxBits, | ||
| maxCells, | ||
| ); | ||
| nativeReserve( | ||
| val oldBalance = contract.getOriginalBalance() - in.valueCoins; | ||
| reserveToncoinsOnBalance( | ||
| max(oldBalance, minTonsForStorage), | ||
| ReserveAtMost | ||
| RESERVE_MODE_AT_MOST, | ||
| ); | ||
| // Process operation with remaining value... | ||
| } | ||
| // Also this contract probably will require some code, that will allow owner to withdraw TONs from this contract. | ||
| // This contract probably also requires some code allowing the owner to withdraw TONs from it. | ||
| ``` | ||
|
|
||
| In this approach, a receiver contract should calculate maximum possible storage fees for all contracts in trace. | ||
|
|
||
| ```tact | ||
| const secondsInFiveYears: Int = 5 * 365 * 24 * 60 * 60; | ||
| receive(msg: UserIn) { | ||
| // Suppose trace will be | ||
| ```tolk | ||
| import "@stdlib/gas-payments"; | ||
|
|
||
| const secondsInFiveYears = 5 * 365 * 24 * 60 * 60; | ||
|
|
||
| fun onInternalMessage(in: InMessage) { |
There was a problem hiding this comment.
[HIGH] Error identifiers in prose not formatted as code
Lines 243–268 describe Tolk error handling using identifiers such as ERR_NOT_ENOUGH_TONS and ERR_INSUFFICIENT_TON_ATTACHED in running text, but they are not consistently wrapped in code formatting. The style guide requires that error codes and identifiers be rendered in code font so they stand out visually, remain copy‑pasteable, and are clearly distinguished from surrounding prose. Leaving them as plain text decreases readability and can make it harder to spot exact names.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
| ```tolk | ||
| const FreezeDueLimit = ton("0.1"); // Current mainnet freeze_due_limit; adjust for other networks if needed. | ||
|
|
||
| ```tact | ||
| receive(msg: Operation) { | ||
| fun onInternalMessage(in: InMessage) { | ||
| // The trace is still receiver -> A -> B | ||
| let freezeLimit = getFreezeLimit(isAccountsInMasterchain); | ||
| let otherFees = ...; | ||
| val freezeLimit = FreezeDueLimit; | ||
| val otherFees = 0; // Compute + forward fees go here. | ||
| // n equals 3 because receiver -> A -> B | ||
| require( | ||
| messageValue() >= freezeLimit * 3 + otherFees, | ||
| "Not enough TONs" | ||
| ); | ||
| assert (in.valueCoins >= freezeLimit * 3 + otherFees) | ||
| throw ERR_NOT_ENOUGH_TONS; | ||
| } |
There was a problem hiding this comment.
[HIGH] Hardcoded FreezeDueLimit requires explicit network-scope safety callout
The example on lines 331–341 hardcodes const FreezeDueLimit = ton("0.1"); and comments that this reflects the “Current mainnet freeze_due_limit; adjust for other networks if needed.” Without an explicit safety callout, this may encourage readers to copy the value verbatim across networks, which can misprice storage reservation and affect funds, contrary to the style guidance for safety‑critical content. Because the code is illustrative, the primary risk is misunderstanding the network-specific nature of the constant, not the numeric value itself.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
| assert (in.valueCoins >= totalStorageFees + otherFees) | ||
| throw ERR_NOT_ENOUGH_TONS; | ||
| } |
There was a problem hiding this comment.
[MEDIUM] Receiver reserve check uses generic error identifier without quoted mapping
The example check on lines 287–289 throws ERR_NOT_ENOUGH_TONS without showing the corresponding user-facing error string, while similar examples later introduce identifiers like ERR_INSUFFICIENT_TON_ATTACHED. For consistency and grep‑ability, the style rules favor either using a clearly named identifier everywhere or showing how it maps to a literal error message in comments or surrounding text. Making this mapping explicit in the snippet avoids ambiguity when correlating documentation with runtime errors.
Please leave a reaction 👍/👎 to this suggestion to improve future reviews for everyone!
|
Thanks for the detailed work on Per-comment submission: 8 posted, 5 failed. Unposted inline comments (raw text):
|
|
do we have some tests for these contracts? |
no, should we? |
yes, it is open source |
delovoyhomie
requested review from
ijonele and
kay-is
and removed request for
ijonele and
skywardboundd
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contract-dev/techniques/gas.mdx`:
- Around line 373-387: The fee formula in minFees reuses the forward-fee
estimate (fwdFee / in.originalForwardFee) across hops but lacks the
same-workchain precondition, so add a guard before or alongside the minFees
calculation (near messageCount/minFees) that verifies all hops remain in the
same workchain as workchain (e.g., check that receiver, vault, pool and final
vault workchain IDs equal workchain); if they are not equal, fall back to a
conservative fee calculation or refuse to use the reused fwdFee estimate so you
don't under-estimate attached value.
🪄 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: 684d77de-feb6-4255-a50d-2f82d8077a4e
📒 Files selected for processing (1)
contract-dev/techniques/gas.mdx
| // Count all messages in the operation chain. | ||
| // IMPORTANT: we know that each of these messages is | ||
| // less than or equal to `SwapRequest`. | ||
| val messageCount = 3; // receiver -> vault -> pool -> vault | ||
|
|
||
| // Calculate minimum required | ||
| let minFees = | ||
| // Calculate the minimum required value | ||
| val minFees = | ||
| messageCount * fwdFee + | ||
| // Operation in first vault | ||
| getComputeFee(GasSwapRequest, isInMasterchain) + | ||
| // Operation in pool | ||
| getComputeFee(GasPoolSwap, isInMasterchain) + | ||
| // Operation in second vault | ||
| getComputeFee(GasVaultPayout, isInMasterchain) + | ||
| 3 * getFreezeLimit(); | ||
|
|
||
| require( | ||
| ctx.value >= msg.amount + minFees, | ||
| "Insufficient TON attached" | ||
| ); | ||
| // Operation in the first vault | ||
| calculateGasFee(workchain, GasSwapRequest) + | ||
| // Operation in the pool | ||
| calculateGasFee(workchain, GasPoolSwap) + | ||
| // Operation in the second vault | ||
| calculateGasFee(workchain, GasVaultPayout) + | ||
| 3 * FreezeDueLimit; |
There was a problem hiding this comment.
Add the missing same-workchain precondition next to the minFees formula.
This formula reuses in.originalForwardFee across hops, but that estimate is only valid when all hops stay in the same workchain (in addition to the message-size bound). Without that guard, this can under-estimate attached value.
Suggested doc-level fix
// Count all messages in the operation chain.
// IMPORTANT: we know that each of these messages is
// less than or equal to `SwapRequest`.
+ // IMPORTANT: all hops are in the same workchain.
val messageCount = 3; // receiver -> vault -> pool -> vault📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Count all messages in the operation chain. | |
| // IMPORTANT: we know that each of these messages is | |
| // less than or equal to `SwapRequest`. | |
| val messageCount = 3; // receiver -> vault -> pool -> vault | |
| // Calculate minimum required | |
| let minFees = | |
| // Calculate the minimum required value | |
| val minFees = | |
| messageCount * fwdFee + | |
| // Operation in first vault | |
| getComputeFee(GasSwapRequest, isInMasterchain) + | |
| // Operation in pool | |
| getComputeFee(GasPoolSwap, isInMasterchain) + | |
| // Operation in second vault | |
| getComputeFee(GasVaultPayout, isInMasterchain) + | |
| 3 * getFreezeLimit(); | |
| require( | |
| ctx.value >= msg.amount + minFees, | |
| "Insufficient TON attached" | |
| ); | |
| // Operation in the first vault | |
| calculateGasFee(workchain, GasSwapRequest) + | |
| // Operation in the pool | |
| calculateGasFee(workchain, GasPoolSwap) + | |
| // Operation in the second vault | |
| calculateGasFee(workchain, GasVaultPayout) + | |
| 3 * FreezeDueLimit; | |
| // Count all messages in the operation chain. | |
| // IMPORTANT: we know that each of these messages is | |
| // less than or equal to `SwapRequest`. | |
| // IMPORTANT: all hops are in the same workchain. | |
| val messageCount = 3; // receiver -> vault -> pool -> vault | |
| // Calculate the minimum required value | |
| val minFees = | |
| messageCount * fwdFee + | |
| // Operation in the first vault | |
| calculateGasFee(workchain, GasSwapRequest) + | |
| // Operation in the pool | |
| calculateGasFee(workchain, GasPoolSwap) + | |
| // Operation in the second vault | |
| calculateGasFee(workchain, GasVaultPayout) + | |
| 3 * FreezeDueLimit; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@contract-dev/techniques/gas.mdx` around lines 373 - 387, The fee formula in
minFees reuses the forward-fee estimate (fwdFee / in.originalForwardFee) across
hops but lacks the same-workchain precondition, so add a guard before or
alongside the minFees calculation (near messageCount/minFees) that verifies all
hops remain in the same workchain as workchain (e.g., check that receiver,
vault, pool and final vault workchain IDs equal workchain); if they are not
equal, fall back to a conservative fee calculation or refuse to use the reused
fwdFee estimate so you don't under-estimate attached value.
|
To fix the formatting issues:
Alternatively, a maintainer can comment /fmt in this PR to auto-apply fixes in a new commit from the bot. |
kay-is
left a comment
kay-is
left a comment
There was a problem hiding this comment.
Just added some suggestions to improve consistency or cosmetics.
|
|
||
| ```mermaid | ||
| flowchart LR | ||
| User1((User)) --> Receiver((Receiver)) --> A((A)) --> B((B)) --> User2((User)) |
There was a problem hiding this comment.
I added arrows to the flow chart.
| User1((User)) -->> Receiver((Receiver)) -->> A((A)) -->> B((B)) -->> User2((User)) |
|
|
||
| ```mermaid | ||
| flowchart LR | ||
| User1((User)) --> A((A)) --> B((B)) --> User2((User)) |
There was a problem hiding this comment.
I added arrows to the flow chart.
| User1((User)) -->> A((A)) -->> B((B)) -->> User2((User)) |
|
|
||
| ```tact | ||
| const GasSwapRequest: Int = 0; | ||
| ```tolk |
There was a problem hiding this comment.
| ```tolk | |
| ```tolk title="Tolk" |
| - Extract resource consumption from the `send()` method's return value. The sections below describe ways to compute consumption of different kinds of resources. | ||
| - Use `expect(extractedValue).toBeLessThanOrEqual(hardcodedConstant)` to verify that the hardcoded limit was not exceeded. | ||
|
|
||
| ```ts |
There was a problem hiding this comment.
| ```ts title="TypeScript" |
|
|
||
| ```tact | ||
| const GasSwapRequest: Int = 12000; | ||
| ```tolk |
There was a problem hiding this comment.
| ```tolk | |
| ```tolk title="Tolk" |
| 1. receiver; | ||
| 1. internal contract A; | ||
| 1. internal contract B. |
There was a problem hiding this comment.
| 1. receiver; | |
| 1. internal contract A; | |
| 1. internal contract B. | |
| 1. Receiver contract | |
| 1. Internal contract A | |
| 1. Internal contract B |
|
|
||
| ### Receiver requirements | ||
|
|
||
| Receiver contracts must verify that the attached Toncoin is sufficient to cover fees for all contracts in the subsequent trace. If an entry contract accepts a user message it must guarantee that the message will not later fail due to insufficient attached TON. "Accept" doesn't mean the call to `accept_message()`, but semantic acceptance, i.e., no throw and no asset returns. |
There was a problem hiding this comment.
| Receiver contracts must verify that the attached Toncoin is sufficient to cover fees for all contracts in the subsequent trace. If an entry contract accepts a user message it must guarantee that the message will not later fail due to insufficient attached TON. "Accept" doesn't mean the call to `accept_message()`, but semantic acceptance, i.e., no throw and no asset returns. | |
| Receiver contracts must verify that the attached Toncoin is sufficient to cover fees for all contracts in the subsequent trace. If an entry contract accepts a user message it must guarantee that the message will not later fail due to insufficient attached Toncoin. "Accept" doesn't mean the call to `accept_message()`, but semantic acceptance, i.e., no throw and no asset returns. |
| // Process operation with remaining value... | ||
| } | ||
| // Also this contract probably will require some code, that will allow owner to withdraw TONs from this contract. | ||
| // This contract probably also requires some code allowing the owner to withdraw TON from it. |
There was a problem hiding this comment.
| // This contract probably also requires some code allowing the owner to withdraw TON from it. | |
| // This contract probably also requires some code allowing the owner to withdraw Toncoins from it. |
| }); | ||
| ``` | ||
|
|
||
| This confirms that all incoming value was consumed or forwarded, with none left behind. It helps identify any bugs that cause accumulation of TON on any contract. |
There was a problem hiding this comment.
| This confirms that all incoming value was consumed or forwarded, with none left behind. It helps identify any bugs that cause accumulation of Toncoins on any contract. |
| // It may also be necessary to handle fees on this exact | ||
| // contract if it is not supposed to hold users' TONs. | ||
| // That can be done in either of the two approaches | ||
| // contract if it is not supposed to hold users' TON. |
There was a problem hiding this comment.
| // contract if it is not supposed to hold users' TON. | |
| // contract if it is not supposed to hold users' Toncoin. |
|
Warning There are quite a few suggested changes, which is great, but I must warn us not to apply them via buttons in GitHub's interface. Instead, treat them as nicely presented steps that could be taken, and apply changes locally only! That is because GitHub's suggestions are completely broken and, when applied, hurt the PR — actually committed lines and changes differ from their representation in the UI! |
|
@novusnota regarding to your last comment, how should we merge this PR? |
@delovoyhomie As usual — the comment was only a warning to not apply GitHub suggestions in the interface, and instead make changes directly, locally :) |
6 participants
Closes #1003
Summary by CodeRabbit