Bumping crates and version #629
Conversation
Benchmark report
|
kubaplas
force-pushed
the
maintenance/bumping-crates
branch
2 times, most recently
from
0822baf to
d53356f
Compare
kubaplas
force-pushed
the
maintenance/bumping-crates
branch
from
d53356f to
181fbd4
Compare
Benchmark report
|
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Code Review by LlamaPReview
🎯 TL;DR & Recommendation
Recommendation: Request Changes
This PR bumps dependencies and versions but introduces critical breaking changes to factory contract schemas, which will cause runtime execution failures for existing callers, alongside a risk of broken schema generation from a dependency source change.
🌟 Strengths
- Updates multiple dependencies to newer versions, improving compatibility and security.
- Maintains consistent versioning across the workspace.
| Priority | File | Category | Impact Summary (≤12 words) | Anchors |
|---|---|---|---|---|
| P0 | examples/.../better_counter_factory_schema.json | Architecture | Breaking API change causes runtime failures for batch_upgrade_child_contract. | path:examples/src/factory/counter.rs, method:batch_upgrade_child_contract |
| P0 | examples/.../counter_factory_schema.json | Architecture | Same breaking change as above. | |
| P0 | examples/.../f_token_factory_schema.json | Architecture | Same breaking change as above. | |
| P1 | Cargo.toml | Architecture | Dependency source change risks breaking schema generation. | path:odra-macros/src/ast/factory/impl_item.rs |
| P2 | Cargo.toml | Maintainability | Strict version pinning may cause dependency conflicts. | path:odra-wasm-client/Cargo.toml, path:odra-wasm-client-builder/Cargo.toml |
| P2 | examples/.cargo/config.toml | Documentation | Formatting change suggests auto-generation, hiding functional issues. |
🔍 Notable Themes
- The same breaking schema change appears across multiple factory contracts, indicating a systematic issue in the schema update process.
- Multiple dependency updates are bundled with functional changes, increasing review complexity.
📈 Risk Diagram
This diagram illustrates the risk of runtime failure due to breaking schema changes in factory contracts.
sequenceDiagram
participant U as User
participant FC as Factory Contract
participant SR as Schema Registry
U->>FC: Call batch_upgrade_child_contract<br/>with structured arguments
FC->>SR: Validate against schema
SR->>FC: Schema expects single args (List<U8>)
note over FC: R1(P0): Schema mismatch causes<br/>runtime execution failure
FC-->>U: Execution fails
💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.
| thiserror = "1.0" | ||
| tokio = "1.48" | ||
| toml = "0.9" | ||
| wasm-bindgen = "=0.2.100" |
There was a problem hiding this comment.
P2 | Confidence: Medium
The dependency wasm-bindgen is pinned to an exact version (=0.2.100) in the workspace dependencies. This strict pinning can cause dependency resolution conflicts in downstream projects and makes transitive dependency updates difficult. While pinning might be necessary for WASM compatibility, using a caret requirement (^0.2.100) would be more standard, allowing patch version updates while maintaining major/minor compatibility. This should be evaluated against the project's actual compatibility requirements.
| wasm-bindgen = "=0.2.100" | |
| wasm-bindgen = "0.2.100" |
Evidence: path:odra-wasm-client/Cargo.toml, path:odra-wasm-client-builder/Cargo.toml
|
|
||
| [alias] | ||
| rl = "run --features=livenet" | ||
| rl = "run --features=livenet" |
There was a problem hiding this comment.
P2 | Confidence: Low
The change appears to be only a whitespace/formatting change (adding a newline at end of file). This suggests the entire PR may have been auto-formatted or generated by a tool. While not functionally harmful, it indicates the changes weren't manually curated. Reviewers should be aware that subtle functional changes (like the schema breakages) might be hidden among formatting changes.
| { | ||
| "name": "batch_upgrade_child_contract", | ||
| "description": "", | ||
| "is_mutable": true, |
There was a problem hiding this comment.
[Contextual Comment]
This comment refers to code near real line 85. Anchored to nearest_changed(87) line 87.
P0 | Confidence: High
The schema for the batch_upgrade_child_contract entry point was significantly simplified from three distinct arguments (default_args, names_to_upgrade, specific_args) to a single args argument. This is a breaking public API change. The Evidence Anchor shows a test in examples/src/factory/counter.rs that calls this method with a BTreeMap<String, BetterCounterUpgradeArgs>. The current schema change would make this call incompatible, as the contract's Wasm entry point now expects a single serialized bytes argument instead of the previous structured arguments. This will cause runtime execution failures for any existing callers. The change appears to be an unintended side effect of an automated schema regeneration.
| { | ||
| "name": "batch_upgrade_child_contract", | ||
| "description": "", | ||
| "is_mutable": true, |
There was a problem hiding this comment.
[Contextual Comment]
This comment refers to code near real line 79. Anchored to nearest_changed(81) line 81.
P0 | Confidence: High
Same breaking API change as identified in better_counter_factory_schema.json. The CounterFactory contract's batch_upgrade_child_contract entry point schema has been incorrectly updated from a three-argument structure to a single args argument. This will break any existing integrations or tests that call this method with the expected argument structure. The change is inconsistent with the actual contract implementation and will cause runtime failures.
| { | ||
| "name": "batch_upgrade_child_contract", | ||
| "description": "", | ||
| "is_mutable": true, |
There was a problem hiding this comment.
[Contextual Comment]
This comment refers to code near real line 97. Anchored to nearest_changed(99) line 99.
P0 | Confidence: High
Third instance of the same breaking schema change for factory contract entry points. The FTokenFactory contract's batch_upgrade_child_contract schema has been incorrectly modified. This pattern across multiple factory contract schemas indicates a systematic issue with schema regeneration logic, not an isolated error.
Benchmark report
|
No description provided.