Fix Envio init failure with long contract names - #905
Conversation
Entity names are formed as {contract_name}_{event_name} and must be <= 63 chars.
Long contract names like InitializableImmutableAdminUpgradeabilityProxy were
causing envio init to fail with "Entity name is too long" errors.
Added truncation logic to normalize_contract_name() to cap contract names at
30 characters, leaving 32 chars for event names plus 1 for the separator.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 5 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthrough
ChangesContract Name Normalization
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Contract names are now capped before entity names are generated, preventing initialization failures from names exceeding the 63-character limit. The change is narrowly scoped and has coverage for truncation boundaries. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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
🤖 Fix all issues with AI agents
In `@codegenerator/cli/src/config_parsing/contract_import/converters.rs`:
- Around line 14-15: The truncation uses byte-based slicing on variable
normalized which can panic for multi-byte UTF-8 characters; change the logic to
truncate by Unicode scalar values instead (use
normalized.chars().take(MAX_CONTRACT_NAME_LENGTH).collect::<String>() or an
equivalent char-aware approach) so you measure and cut by characters rather than
bytes, and return that safe String when normalized exceeds
MAX_CONTRACT_NAME_LENGTH.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
codegenerator/cli/src/config_parsing/contract_import/converters.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
🔇 Additional comments (2)
codegenerator/cli/src/config_parsing/contract_import/converters.rs (2)
5-8: LGTM!The constant is well-documented with clear rationale. The math correctly accounts for the 63-character limit (30 + 1 + 32 = 63).
176-191: LGTM!Good test coverage for truncation behavior. Using the actual problematic contract name from the issue ("InitializableImmutableAdminUpgradeabilityProxy") validates the real-world fix.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| if normalized.len() > MAX_CONTRACT_NAME_LENGTH { | ||
| normalized[..MAX_CONTRACT_NAME_LENGTH].to_string() |
There was a problem hiding this comment.
Potential panic if contract name contains multi-byte UTF-8 characters.
String::len() returns byte count and slicing operates on byte indices. If truncation occurs mid-character on a multi-byte UTF-8 string, this will panic. While contract names are typically ASCII, using character-based truncation would be safer.
Suggested safer truncation
- if normalized.len() > MAX_CONTRACT_NAME_LENGTH {
- normalized[..MAX_CONTRACT_NAME_LENGTH].to_string()
+ if normalized.chars().count() > MAX_CONTRACT_NAME_LENGTH {
+ normalized.chars().take(MAX_CONTRACT_NAME_LENGTH).collect()
} else {
normalized
}📝 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.
| if normalized.len() > MAX_CONTRACT_NAME_LENGTH { | |
| normalized[..MAX_CONTRACT_NAME_LENGTH].to_string() | |
| if normalized.chars().count() > MAX_CONTRACT_NAME_LENGTH { | |
| normalized.chars().take(MAX_CONTRACT_NAME_LENGTH).collect() | |
| } else { | |
| normalized | |
| } |
🤖 Prompt for AI Agents
In `@codegenerator/cli/src/config_parsing/contract_import/converters.rs` around
lines 14 - 15, The truncation uses byte-based slicing on variable normalized
which can panic for multi-byte UTF-8 characters; change the logic to truncate by
Unicode scalar values instead (use
normalized.chars().take(MAX_CONTRACT_NAME_LENGTH).collect::<String>() or an
equivalent char-aware approach) so you measure and cut by characters rather than
bytes, and return that safe String when normalized exceeds
MAX_CONTRACT_NAME_LENGTH.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
3 participants
Entity names are formed as {contract_name}_{event_name} and must be <= 63 chars. Long contract names like InitializableImmutableAdminUpgradeabilityProxy were causing envio init to fail with "Entity name is too long" errors.
Added truncation logic to normalize_contract_name() to cap contract names at 30 characters, leaving 32 chars for event names plus 1 for the separator.
Closes #794
Summary by CodeRabbit
New Features
Tests