feat(contract-dev): edit upgrade contracts to fit styleguide#2110
feat(contract-dev): edit upgrade contracts to fit styleguide#2110
Conversation
📝 WalkthroughWalkthroughRewrote the contract upgradeability guide: clarified when address-stable upgrades are needed, separated code vs data upgrade timing, restructured basic/delayed/hot patterns, expanded migration risks and ethics links, updated examples and wording. Documentation-only changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 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 |
This comment has been minimized.
This comment has been minimized.
![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/upgrades.mdx`:
- Line 43: Remove the unnecessary comma in the sentence "An admin sends an
upgrade message to the contract, that contains new code, data, or both." by
deleting the comma after "contract" so the restrictive clause reads "that
contains new code, data, or both" directly following "contract"; update the
sentence in the upgrades.mdx content where that exact line appears.
- Line 413: Fix the missing verb in the sentence that currently reads "The
migration logic these steps:" by changing it to a complete phrase such as "The
migration logic follows these steps:" or "The migration logic includes these
steps:" so the sentence is grammatically correct (locate the phrase "The
migration logic these steps:" and replace with the chosen corrected version).
- Line 12: The sentence currently reads "(DEX), that are referenced by many
other contracts" with an incorrect comma; edit the line containing "The upgrade
pattern is also required for [vanity](/contract-dev/vanity) contracts and
protocols such as distributed exchanges (DEX), that are referenced by many other
contracts." to remove the comma after "(DEX)" so it reads "(DEX) that are
referenced by many other contracts."
- Line 32: Update the sentence "Use delayed upgrades to allow users to react to
compromised admin keys or unwanted update." by changing the singular noun
"update" to the plural "updates" so it reads "...or unwanted updates" to match
the plural "admin keys" and maintain parallel structure; locate the sentence in
the content of the techniques/upgrades section (the line starting with "Use
delayed upgrades to allow users...") and replace "update" with "updates".
- Line 415: Update the example abbreviation in the list item text that currently
reads "e.g, `oldStorage` with `adminAddress` and `counter`" to use the correct
punctuation "e.g.," so the phrase becomes "e.g., `oldStorage` with
`adminAddress` and `counter`" in the sentence starting "1. Loading the storage
using the old structure...".
- Line 106: Replace the article "an" with "a" before the symbol RequestUpgrade
in the sentence "An admin sends an `RequestUpgrade` message..." so it reads "A
admin sends a `RequestUpgrade` message..." — update the text around the
identifier RequestUpgrade accordingly to use the correct indefinite article.
🪄 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: 625f5522-98ab-4aba-a90e-430388e1f3fe
📒 Files selected for processing (1)
contract-dev/techniques/upgrades.mdx
There was a problem hiding this comment.
Pull request overview
Edits the “Upgrading contracts” technique guide to better match the repository’s documentation style guide, improving structure, clarity, and callout usage.
Changes:
- Rewrites the introduction and examples to improve readability and align phrasing with the style guide.
- Restructures the “basic”, “delayed”, and “hot” upgrade pattern sections with clearer step-by-step descriptions and headings.
- Standardizes/clarifies
<Aside>callouts (titles/types) and refines surrounding explanatory text.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Thanks for the thoughtful improvements to Per-comment submission: 10 posted, 1 failed. Unposted inline comments (raw text):
|
This comment has been minimized.
This comment has been minimized.
|
/review |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
/review |
|
/review |
|
/review |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
contract-dev/techniques/upgrades.mdx (2)
411-418:⚠️ Potential issue | 🟡 MinorClean up remaining style-guide polish items.
Line 411 is still wordy, Line 417 has an unnecessary comma, and Line 426 places the period inside a non-quoted parenthetical.
Proposed wording
-The new version of `hotUpgradeData()` function is what is called after the code was switched with `setTvmRegisterC3()` and performs the migration. +The new version of the `hotUpgradeData()` function runs after the code is switched with `setTvmRegisterC3()` and performs the migration.-1. Reorder fields, to move `counter` before `adminAddress`. +1. Reorder fields to move `counter` before `adminAddress`.-- The contract receives frequent state updates (DEX pools, oracles, lending protocols.) +- The contract receives frequent state updates (DEX pools, oracles, lending protocols).Also applies to: 426-426
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contract-dev/techniques/upgrades.mdx` around lines 411 - 418, Shorten and simplify the sentence describing hotUpgradeData() to be more concise (refer to the sentence mentioning hotUpgradeData() and setTvmRegisterC3()), remove the unnecessary trailing comma in the sentence that currently ends with a comma (the line referencing the migration steps), and move the period outside the parentheses where a sentence currently ends with the period inside a non-quoted parenthetical (the sentence around contract.setData()); keep the meaning identical but apply the proposed wording style polish to those three spots.
102-102:⚠️ Potential issue | 🟡 MinorReplace remaining contractions in technical prose.
Use “do not” and “That is” to keep the style formal and consistent.
Proposed wording
-The delay allows users to withdraw funds or exit positions if they don't trust the upgrade or if an admin account is compromised. +The delay allows users to withdraw funds or exit positions if they do not trust the upgrade or if an admin account is compromised.-The contract never executes the original `hotUpgradeData()` function because it is immediately replaced by the new code during the upgrade. The new code defines the actual migration logic. That's why the migration function must have a `method_id` that's stable across versions, so the runtime can call it after the upgrade. +The contract never executes the original `hotUpgradeData()` function because it is immediately replaced by the new code during the upgrade. The new code defines the actual migration logic. That is why the migration function must have a `method_id` that is stable across versions, so the runtime can call it after the upgrade.Also applies to: 315-315
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contract-dev/techniques/upgrades.mdx` at line 102, The paragraph describing the delayed upgrade pattern contains informal contractions; replace "don't" with "do not" and "that's" with "That is" (and scan the rest of upgrades.mdx for any remaining contractions to convert to their expanded forms) so the technical prose remains formal and consistent—update the sentence about users withdrawing funds or exiting positions and any other sentences using contractions accordingly.
🤖 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/upgrades.mdx`:
- Around line 111-112: Clarify the pending-request cleanup: state that when a
request is approved the contract calls setCodePostponed() and then either
removes CurrentRequest from storage (if no replacement data is provided) or, if
setData() is given a full replacement cell, that replacement cell must already
encode the new storage state without any pending CurrentRequest entry; update
the text to explicitly mention both branches and reference CurrentRequest,
setData(), and setCodePostponed() so readers know where cleanup must happen.
- Line 241: Update the inaccurate sentence about failed migrations in the
upgrades doc: replace the claim "If the migration function fails, the contract
becomes unusable" with a precise statement that if hotUpgradeData() (the
migration function) throws during compute, the entire transaction aborts and no
state changes are applied (the contract remains unchanged), and clarify the real
risk is a migration that succeeds but writes invalid storage, which can corrupt
the contract; modify the paragraph around the hotUpgradeData() explanation
accordingly so it mentions both the abort-on-throw behavior and the true danger
of successful-but-invalid migrations.
- Around line 47-48: Update the wording to state that set_code is applied during
the action phase as part of the sequential transaction phases rather than "after
the transaction completes"; specifically replace the sentence that says "apply
the new code during the action phase after the transaction completes" with a
clear line like: "apply the new code during the action phase — set_code is
processed as part of the transaction's action phase and the new code becomes
active only after that action phase succeeds." Reference the terms set_code and
action phase when making the edit.
---
Duplicate comments:
In `@contract-dev/techniques/upgrades.mdx`:
- Around line 411-418: Shorten and simplify the sentence describing
hotUpgradeData() to be more concise (refer to the sentence mentioning
hotUpgradeData() and setTvmRegisterC3()), remove the unnecessary trailing comma
in the sentence that currently ends with a comma (the line referencing the
migration steps), and move the period outside the parentheses where a sentence
currently ends with the period inside a non-quoted parenthetical (the sentence
around contract.setData()); keep the meaning identical but apply the proposed
wording style polish to those three spots.
- Line 102: The paragraph describing the delayed upgrade pattern contains
informal contractions; replace "don't" with "do not" and "that's" with "That is"
(and scan the rest of upgrades.mdx for any remaining contractions to convert to
their expanded forms) so the technical prose remains formal and
consistent—update the sentence about users withdrawing funds or exiting
positions and any other sentences using contractions accordingly.
🪄 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: 4ca949e7-0934-435b-a961-6be508440df9
📒 Files selected for processing (1)
contract-dev/techniques/upgrades.mdx
| 1. Process subsequent messages with the new code. | ||
|
|
||
| The upgrade happens immediately in a single transaction. The new code becomes active after the transaction completes. Any data replacement happens during the compute phase, so the new data is immediately available when the transaction ends. | ||
| The upgrade runs in a single transaction. New code becomes active after the transaction completes, and new data is available when the transaction ends. If the message does not provide enough Toncoin to run both the compute phase and the action phase, the entire transaction is aborted and no state changes from the upgrade are applied. Test the upgrade script to estimate gas requirements, and send enough Toncoin to execute the full upgrade transaction. |
There was a problem hiding this comment.
Wondering about this one technically. The stdlib reference says contract.setCodePostponed() creates an output action that takes effect only after successful termination, while contract.setData() writes storage during the compute phase. If the compute phase succeeds (so setData commits) but the action phase then runs out of Toncoin, I'd expect the storage change to persist while the SETCODE action does not - leaving the contract with the new data schema but still the old code.
If that's right, then "the entire transaction is aborted and no state changes from the upgrade are applied" is misleading, and this is actually a fairly sharp footgun worth calling out directly. Could be worth verifying against the actual TVM semantics and either correcting the claim or adding a warning.
There was a problem hiding this comment.
Good point!
As far as the docs explain it:
"If an action fails (for example, due to a lack of funds), the transaction may revert or skip the action, depending on its mode. For example, mode = 0, flag = 2 means that any errors arising while processing this message during the action phase are ignored."
Depending on these modes, it seems both can be true.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
contract-dev/techniques/upgrades.mdx (1)
108-108:⚠️ Potential issue | 🟡 MinorRemove unnecessary comma before "before".
The subordinate clause "before accepting approvals" is restrictive and shouldn't be preceded by a comma.
📝 Proposed fix
-1. The contract waits for the specified timeout, before accepting approvals. +1. The contract waits for the specified timeout before accepting approvals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contract-dev/techniques/upgrades.mdx` at line 108, Remove the unnecessary comma in the sentence "The contract waits for the specified timeout, before accepting approvals." by deleting the comma before "before" so it reads "The contract waits for the specified timeout before accepting approvals." Ensure the corrected sentence replaces the original in the file where that exact line appears.
🤖 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/upgrades.mdx`:
- Line 417: The sentence contains an unnecessary comma before the infinitive
phrase; remove the comma so it reads "Reorder fields to move `counter` before
`adminAddress`" (reference the words `counter` and `adminAddress` to locate the
sentence and delete the comma before "to").
- Line 315: The sentence contains contractions—change "That's" and "that's" to
their expanded forms ("That is" and "that is") in the paragraph explaining
hotUpgradeData() and method_id so the documentation follows the no-contractions
style; ensure references to hotUpgradeData() and method_id remain unchanged and
read naturally after expansion.
---
Duplicate comments:
In `@contract-dev/techniques/upgrades.mdx`:
- Line 108: Remove the unnecessary comma in the sentence "The contract waits for
the specified timeout, before accepting approvals." by deleting the comma before
"before" so it reads "The contract waits for the specified timeout before
accepting approvals." Ensure the corrected sentence replaces the original in the
file where that exact line appears.
🪄 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: 50973eb2-1908-4005-9539-b46d733124eb
📒 Files selected for processing (1)
contract-dev/techniques/upgrades.mdx
| 1. `contract.setCodePostponed(msg.code)` schedules the new code | ||
| 1. `setTvmRegisterC3()` switches register C3 to the new code immediately | ||
| 1. `hotUpgradeData(msg.additionalData)` is called and runs with the new code | ||
| The contract never executes the original `hotUpgradeData()` function because it is immediately replaced by the new code during the upgrade. The new code defines the actual migration logic. That's why the migration function must have a `method_id` that's stable across versions, so the runtime can call it after the upgrade. |
There was a problem hiding this comment.
Remove contractions.
Per the project style guide (no contractions in documentation prose), "That's" and "that's" should be expanded.
📝 Proposed fix
-The contract never executes the original `hotUpgradeData()` function because it is immediately replaced by the new code during the upgrade. The new code defines the actual migration logic. That's why the migration function must have a `method_id` that's stable across versions, so the runtime can call it after the upgrade.
+The contract never executes the original `hotUpgradeData()` function because it is immediately replaced by the new code during the upgrade. The new code defines the actual migration logic. For this reason, the migration function must have a `method_id` that is stable across versions, so the runtime can call it after the upgrade.📝 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.
| The contract never executes the original `hotUpgradeData()` function because it is immediately replaced by the new code during the upgrade. The new code defines the actual migration logic. That's why the migration function must have a `method_id` that's stable across versions, so the runtime can call it after the upgrade. | |
| The contract never executes the original `hotUpgradeData()` function because it is immediately replaced by the new code during the upgrade. The new code defines the actual migration logic. For this reason, the migration function must have a `method_id` that is stable across versions, so the runtime can call it after the upgrade. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contract-dev/techniques/upgrades.mdx` at line 315, The sentence contains
contractions—change "That's" and "that's" to their expanded forms ("That is" and
"that is") in the paragraph explaining hotUpgradeData() and method_id so the
documentation follows the no-contractions style; ensure references to
hotUpgradeData() and method_id remain unchanged and read naturally after
expansion.
| 1. Writes the migrated storage immediately with `contract.setData()` | ||
| 1. Load the storage using the old structure (e.g., `OldStorage` with `adminAddress` and `counter`). | ||
| 1. Create new storage with the additional `metadata` field from `additionalData`. | ||
| 1. Reorder fields, to move `counter` before `adminAddress`. |
There was a problem hiding this comment.
Remove the unnecessary comma before "to".
"to move counter before adminAddress" is an infinitive phrase modifying the imperative, not a non-restrictive clause.
📝 Proposed fix
-1. Reorder fields, to move `counter` before `adminAddress`.
+1. Reorder fields to move `counter` before `adminAddress`.📝 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.
| 1. Reorder fields, to move `counter` before `adminAddress`. | |
| 1. Reorder fields to move `counter` before `adminAddress`. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contract-dev/techniques/upgrades.mdx` at line 417, The sentence contains an
unnecessary comma before the infinitive phrase; remove the comma so it reads
"Reorder fields to move `counter` before `adminAddress`" (reference the words
`counter` and `adminAddress` to locate the sentence and delete the comma before
"to").
closes #2109
Summary by CodeRabbit