feat(sharding): put in agreement with styleguide and port code samples to Tolk#2131
feat(sharding): put in agreement with styleguide and port code samples to Tolk#2131
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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 for contract sharding is reorganized and expanded. The narrative replaces an earlier discussion of StateInit handling with an explanation of deriving child contract addresses from parent address and key. An "NFT and jetton examples" section is added, and an unbounded sharded structures example is rewritten from Tact to multi-file Tolk code with parent/child validation logic. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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. Review rate limit: 0/1 reviews remaining, refill in 56 minutes and 6 seconds.Comment |
This comment has been minimized.
This comment has been minimized.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| self.numChildren = 0; | ||
| } | ||
| fun onInternalMessage(in: InMessage) { | ||
| val msg = lazy TodoChildMessage.fromSlice(in.body); |
There was a problem hiding this comment.
I'm not sure, but I think the else branch in the match below may not be reachable as written. Since TodoChildMessage = Identify is a single-variant union, lazy TodoChildMessage.fromSlice(in.body) should throw on any body that doesn't parse as Identify - including an empty body, since the opcode parse fails first
If the intent is to ignore empty top-up messages, the empty-body check probably needs to happen before the lazy fromSlice call for instance
fun onInternalMessage(in: InMessage) {
+ if (in.body.isEmpty()) {
+ return;
+ }
val msg = lazy TodoChildMessage.fromSlice(in.body);it definitely worths double-checking against actual Tolk semantics
This comment has been minimized.
This comment has been minimized.
| struct (0x5b6f1392) DeployAnother {} | ||
|
|
||
| // Build StateInit for a TodoChild instance with the given seqno. | ||
| fun calcDeployedTodoChild( |
There was a problem hiding this comment.
maybe we should move it to parent.tolk (I am not sure)
There was a problem hiding this comment.
Addressed by moving calcDeployedTodoChild to parent.tolk. But this still needs to be validated
![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
♻️ Duplicate comments (1)
contract-dev/techniques/contract-sharding.mdx (1)
90-102:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
child.tolk: empty top-up fallback likely won’t run after upfront parse.On Line 91,
lazy TodoChildMessage.fromSlice(in.body)runs before the Line 100 empty-body fallback. IffromSlicerejects empty/unknown bodies first, the intended “ignore empty top-up” path is unreachable.Suggested doc snippet fix
fun onInternalMessage(in: InMessage) { + if (in.body.isEmpty()) { + return; + } val msg = lazy TodoChildMessage.fromSlice(in.body); match (msg) { Identify => { val storage = lazy TodoChildStorage.load(); assert (in.senderAddress == storage.parentAddress) throw 0xFFFF; debug.print(storage.seqno); } - else => { - // Ignore empty top-up messages, reject everything else. - assert (in.body.isEmpty()) throw 0xFFFF; - } + else => { throw 0xFFFF; } } }In Tolk, does `lazy <UnionType>.fromSlice(in.body)` throw on an empty body or unknown opcode before `match` fallback branches can execute?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contract-dev/techniques/contract-sharding.mdx` around lines 90 - 102, The current code eagerly calls TodoChildMessage.fromSlice(in.body) in onInternalMessage which can throw on empty/unknown payloads and prevents the intended empty-body fallback; fix by deferring parsing: first check in.body.isEmpty() (or use a non-throwing/optional parse helper) and handle the empty top-up branch before calling TodoChildMessage.fromSlice, or move the fromSlice call inside the Identify match arm so parsing only happens when the message is non-empty and intended to be parsed.
🤖 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/contract-sharding.mdx`:
- Around line 42-43: The sample is missing the Poll/Voter example and
bounced-message handling from Issue `#500`: add two new contract examples named
Poll and Voter (mirroring the parent/child pattern) where Poll deploys Voter
instances and assigns sequence numbers, extend the shared storage definitions to
include the Poll/Voter message types and a bounced-message envelope, and
implement explicit bounce handling in the Voter (e.g., an onBounce or
handle_bounced_message function and examples of sending messages that may bounce
and how Poll reacts); update the narrative to show deployment, message flow, and
bounced-message test cases and link to Issue `#500` for scope reference.
---
Duplicate comments:
In `@contract-dev/techniques/contract-sharding.mdx`:
- Around line 90-102: The current code eagerly calls
TodoChildMessage.fromSlice(in.body) in onInternalMessage which can throw on
empty/unknown payloads and prevents the intended empty-body fallback; fix by
deferring parsing: first check in.body.isEmpty() (or use a non-throwing/optional
parse helper) and handle the empty top-up branch before calling
TodoChildMessage.fromSlice, or move the fromSlice call inside the Identify match
arm so parsing only happens when the message is non-empty and intended to be
parsed.
🪄 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: 7403c291-289a-4bf7-97a2-c4bb1517b988
📒 Files selected for processing (1)
contract-dev/techniques/contract-sharding.mdx
| The following example shows a parent contract that deploys child contracts and assigns each child a sequence number. The shared storage file defines the data layouts and message types used by both contracts. | ||
|
|
There was a problem hiding this comment.
Issue #500 scope is still incomplete (missing Poll/Voter + bounced-message logic).
The page now has a solid parent/child sample, but it still does not include (or link to) the requested Poll/Voter Tolk example demonstrating bounced-message handling from the linked issue scope.
Also applies to: 85-110, 114-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contract-dev/techniques/contract-sharding.mdx` around lines 42 - 43, The
sample is missing the Poll/Voter example and bounced-message handling from Issue
`#500`: add two new contract examples named Poll and Voter (mirroring the
parent/child pattern) where Poll deploys Voter instances and assigns sequence
numbers, extend the shared storage definitions to include the Poll/Voter message
types and a bounced-message envelope, and implement explicit bounce handling in
the Voter (e.g., an onBounce or handle_bounced_message function and examples of
sending messages that may bounce and how Poll reacts); update the narrative to
show deployment, message flow, and bounced-message test cases and link to Issue
`#500` for scope reference.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
|
||
| Consider NFTs: the collection acts as the parent contract, and each NFT item is a child contract. The key in this case is the item index, and only the collection can set the initial owner. | ||
|
|
||
| For jettons, the parent contract is the minter, and the child contracts are user wallets. The key is the wallet's smart contract address. |
There was a problem hiding this comment.
small accuracy thing: "The key is the wallet's smart contract address" is ambiguous - and probably wrong as it reads.the previous sentence says the children are user wallets, so "the wallet's smart contract address" naturally refers to the child's own address, which is the output of address derivation, not the input/key
the key for a jetton wallet is the address of the wallet's owner (the user's main wallet contract) which gets fed into address derivation along with the minter address and the wallet code
-For jettons, the parent contract is the minter, and the child contracts are user wallets. The key is the wallet's smart contract address.
+For jettons, the parent contract is the minter, and the child contracts are jetton wallets. The key is the address of the jetton wallet's owner, and the value is the owner's jetton balance.| match (msg) { | ||
| Identify => { | ||
| val storage = lazy TodoChildStorage.load(); | ||
| assert (in.senderAddress == storage.parentAddress) throw 0xFFFF; |
There was a problem hiding this comment.
throw 0xFFFF is reused for three different conditions: "sender is not the parent" (here), "unexpected message body" (line 101), and "sender is not the admin" (line 144). same code for three distinct failure modes makes a tx-dump in an explorer harder to triage
worth picking distinct codes (any value in 32-65535 outside reserved ranges), e.g. 100 / 101 / 200, but I am not sure
3 participants
closes #500
Summary by CodeRabbit