feat(core): implement ArgumentDecoder using function parameter specs … - #389
Conversation
|
@uche102 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughThe PR adds spec-aware argument decoding, integrates it into function-call decoding, exposes the new decoder types, and updates contract-spec union parsing and related test fixtures. ChangesContract call decoding
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant FunctionCallDecoder
participant ArgumentDecoder
participant ReturnValueDecoder
Caller->>FunctionCallDecoder: function call inputs and optional specifications
FunctionCallDecoder->>ArgumentDecoder: decode typed or dynamic arguments
ArgumentDecoder->>ReturnValueDecoder: decode each ScVal
ReturnValueDecoder-->>ArgumentDecoder: JSON values
ArgumentDecoder-->>FunctionCallDecoder: decoded arguments
FunctionCallDecoder->>ReturnValueDecoder: decode optional return value
ReturnValueDecoder-->>FunctionCallDecoder: decoded return
FunctionCallDecoder-->>Caller: DecodedFunctionCall
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/core/src/decode/function_call_decoder.rs (1)
61-71: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid decoding the return value twice.
decode_to_stringinternally re-runs the samedecode_valuetraversal thatdecodejust performed onval/type_def/contract_spec, doubling recursive decode work (structs/vecs/UDTs) for every decoded return value.♻️ Proposed fix: derive the formatted string from the already-decoded value
let (return_value, formatted_return_value) = match return_val { Some(val) => { let type_def = func_spec.and_then(|f| f.return_type_def.as_ref()); let json_val = self.return_decoder.decode(val, type_def, contract_spec); - let formatted = self - .return_decoder - .decode_to_string(val, type_def, contract_spec); + let formatted = match &json_val { + Value::String(s) => s.clone(), + other => serde_json::to_string(other).unwrap_or_else(|_| other.to_string()), + }; (Some(json_val), Some(formatted)) } None => (None, None), };🤖 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 `@crates/core/src/decode/function_call_decoder.rs` around lines 61 - 71, Update the return-value handling in the match around return_decoder.decode and decode_to_string to decode val only once. Derive formatted_return_value from the already-produced json_val using the existing JSON-to-string formatting mechanism, while preserving None handling and the current return values.
🤖 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 `@crates/core/src/decode/argument_decoder.rs`:
- Around line 116-118: Run cargo fmt formatting on the import block in
argument_decoder.rs and the params vector literal near the referenced later
section, ensuring both match cargo fmt --all -- --check output without changing
behavior.
---
Nitpick comments:
In `@crates/core/src/decode/function_call_decoder.rs`:
- Around line 61-71: Update the return-value handling in the match around
return_decoder.decode and decode_to_string to decode val only once. Derive
formatted_return_value from the already-produced json_val using the existing
JSON-to-string formatting mechanism, while preserving None handling and the
current return values.
🪄 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 Plus
Run ID: 43a0b963-8be5-45c9-8988-f2ec44df28cc
📒 Files selected for processing (5)
crates/core/src/decode/argument_decoder.rscrates/core/src/decode/function_call_decoder.rscrates/core/src/decode/mod.rscrates/core/src/lib.rscrates/core/src/spec/decoder.rs
| use stellar_xdr::curr::{ | ||
| Hash, ScAddress, ScMapEntry, ScString, ScSymbol, ScVal, | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix cargo fmt failures reported by CI.
Pipeline logs flag this file's import block and params: vec![...] literal as failing cargo fmt --all -- --check.
🎨 Proposed rustfmt-compliant formatting
- use stellar_xdr::curr::{
- Hash, ScAddress, ScMapEntry, ScString, ScSymbol, ScVal,
- };
+ use stellar_xdr::curr::{Hash, ScAddress, ScMapEntry, ScString, ScSymbol, ScVal};- params: vec![("to".to_string(), "Address".to_string()), ("amount".to_string(), "U128".to_string())],
+ params: vec![
+ ("to".to_string(), "Address".to_string()),
+ ("amount".to_string(), "U128".to_string()),
+ ],Also applies to: 270-282
🤖 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 `@crates/core/src/decode/argument_decoder.rs` around lines 116 - 118, Run cargo
fmt formatting on the import block in argument_decoder.rs and the params vector
literal near the referenced later section, ensuring both match cargo fmt --all
-- --check output without changing behavior.
Source: Pipeline failures
Closes #375
Summary by CodeRabbit
New Features
Bug Fixes