Skip to content

feat(core): implement ArgumentDecoder using function parameter specs … - #389

Merged
codeZe-us merged 1 commit into
Toolbox-Lab:mainfrom
uche102:fix/argument-decoder-parameter-specs
Jul 30, 2026
Merged

feat(core): implement ArgumentDecoder using function parameter specs …#389
codeZe-us merged 1 commit into
Toolbox-Lab:mainfrom
uche102:fix/argument-decoder-parameter-specs

Conversation

@uche102

@uche102 uche102 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Closes #375

Summary by CodeRabbit

  • New Features

    • Added decoding for contract function arguments, including names, type-aware values, and readable formatting.
    • Added dynamic argument decoding when contract type information is unavailable.
    • Enhanced function call results with formatted return values.
    • Exposed argument decoding types for broader integration.
  • Bug Fixes

    • Improved handling of missing, extra, or mismatched argument definitions with safe fallback behavior.
    • Improved decoding of user-defined contract data types.

@drips-wave

drips-wave Bot commented Jul 29, 2026

Copy link
Copy Markdown

@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! 🚀

Learn more about application limits

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The 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.

Changes

Contract call decoding

Layer / File(s) Summary
Contract spec union parsing
crates/core/src/spec/decoder.rs
Union cases are mapped into contract-spec definitions, XDR parsing loop control is adjusted, and struct test fixtures initialize the lib field.
Typed argument decoding
crates/core/src/decode/argument_decoder.rs
ArgumentDecoder supports typed, function-spec, single-argument, and dynamic decoding with formatted JSON output and coverage for mismatches and UDTs.
Function-call integration and exports
crates/core/src/decode/function_call_decoder.rs, crates/core/src/decode/mod.rs, crates/core/src/lib.rs
FunctionCallDecoder delegates argument and return decoding, supports formatted returns, updates decoded argument traits, and publicly re-exports the new types.

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
Loading

Possibly related PRs

Suggested reviewers: emrys02, nazteeemba

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR also changes crates/core/src/spec/decoder.rs, which is outside the linked issue scope and not part of ArgumentDecoder. Move the spec decoder and test-helper changes to a separate PR unless they are required for #375 and added to the issue scope.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding an ArgumentDecoder based on function parameter specs.
Linked Issues check ✅ Passed The changes implement spec-based argument decoding, handle mismatches and type conflicts, and emit typed human-readable argument values.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
crates/core/src/decode/function_call_decoder.rs (1)

61-71: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid decoding the return value twice.

decode_to_string internally re-runs the same decode_value traversal that decode just performed on val/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

📥 Commits

Reviewing files that changed from the base of the PR and between 130c7de and 221c6bc.

📒 Files selected for processing (5)
  • crates/core/src/decode/argument_decoder.rs
  • crates/core/src/decode/function_call_decoder.rs
  • crates/core/src/decode/mod.rs
  • crates/core/src/lib.rs
  • crates/core/src/spec/decoder.rs

Comment on lines +116 to +118
use stellar_xdr::curr::{
Hash, ScAddress, ScMapEntry, ScString, ScSymbol, ScVal,
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 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

@codeZe-us
codeZe-us self-requested a review July 30, 2026 00:40
@codeZe-us
codeZe-us merged commit 6f30976 into Toolbox-Lab:main Jul 30, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ArgumentDecoder using function parameter specs

2 participants