feat(core): implement multi-op transaction decoder - #390
Conversation
…oolbox-Lab#379) - Add multi_op_decoder.rs module with per-operation decoding - Refactor core decoder loop to iterate over operations array in TransactionEnvelope - Map each operation's results and events from TransactionResultMeta correctly - Aggregate or isolate diagnostics per operation in the final report - Add decode_transaction_with_op_filter function for fetching and decoding - Update TransactionContext with operation_index and operation_count fields - Update DiagnosticReport with operation tracking fields - Export MultiOpDecoder and decode_transaction_with_op_filter from decode module Closes Toolbox-Lab#377 Closes Toolbox-Lab#379
|
@JhayJ22 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! 🚀 |
📝 WalkthroughWalkthroughAdds ChangesMulti-operation transaction decoding
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant decode_transaction_with_op_filter
participant SorobanRpcClient
participant MultiOpDecoder
participant DiagnosticReport
Caller->>decode_transaction_with_op_filter: provide transaction hash and optional operation index
decode_transaction_with_op_filter->>SorobanRpcClient: fetch transaction JSON
SorobanRpcClient-->>decode_transaction_with_op_filter: return transaction data
decode_transaction_with_op_filter->>MultiOpDecoder: decode transaction
MultiOpDecoder->>DiagnosticReport: construct per-operation reports
MultiOpDecoder-->>decode_transaction_with_op_filter: return report collection
decode_transaction_with_op_filter-->>Caller: return all reports or selected report
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 9
🧹 Nitpick comments (5)
crates/core/src/lib.rs (1)
17-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
decode_transaction_with_op_filterisn't lifted to the crate root alongsideMultiOpDecoder.The type is re-exported here but its primary async entry point is only reachable via
decode::decode_transaction_with_op_filter. Worth exporting both together for a consistent surface, assuming the CLI consumes the crate root.🤖 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/lib.rs` around lines 17 - 18, Update the crate-root re-exports in lib.rs to include decode_transaction_with_op_filter alongside MultiOpDecoder, preserving its existing async API and making it reachable directly from the crate root.crates/core/src/decode/multi_op_decoder.rs (4)
25-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
DefaultforMultiOpDecoder.A unit struct with a public
new()and noDefaultimpl tripsclippy::new_without_default, which the sibling decoders in this module (ReturnValueDecoder,FunctionCallDecoder) also expose — worth checking whether the crate denies clippy warnings in CI.♻️ Proposed change
+#[derive(Default)] pub struct MultiOpDecoder;🤖 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/multi_op_decoder.rs` around lines 25 - 30, Add a public Default implementation for the unit struct MultiOpDecoder, delegating to or matching its existing new() behavior; keep the constructor and decoding behavior unchanged.
181-202: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEnrichment failures are swallowed by
.ok().Both
enrich_diagnostic_reportandenrich_resource_reportreturnGratResult<()>, and discarding it means a decoding regression produces silently thinner reports with no trace. Log atwarn/debugwith the operation index at minimum.🤖 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/multi_op_decoder.rs` around lines 181 - 202, Handle the results of enrich_diagnostic_report and enrich_resource_report instead of discarding them with .ok(). When either enrichment fails, log the error at warn or debug level and include the operation index; preserve the existing report-generation flow rather than propagating the enrichment failure.
384-390: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo tests for the partitioning heuristic.
The event-to-operation attribution here is the riskiest logic in the module (depth tracking, unbalanced
fn_call/fn_return, empty-event early return producing a 1-element vec for an N-op transaction) and has no unit coverage. Table-driven tests over syntheticDiagnosticEventsequences would pin the boundaries down cheaply. Want me to draft them?🤖 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/multi_op_decoder.rs` around lines 384 - 390, Add table-driven unit tests covering partition_events_by_operation, using synthetic DiagnosticEvent sequences to verify depth-based attribution, unbalanced fn_call/fn_return handling, and empty events returning a single-element vector even when num_operations is greater than one. Include boundary cases for one operation and empty input while preserving the function’s current behavior.
436-486: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
partition_contract_events_by_operationduplicatespartition_events_by_operationverbatim.The two functions are identical apart from the event type and the
bodyaccess path (event.event.bodyvsevent.body). Extract one generic helper parameterized by afn(&T) -> Option<&ContractEventV0>accessor; thefn_call/fn_returnsymbol matching should also be pulled into sharedis_call_topic/is_return_topichelpers so the two implementations cannot drift.🤖 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/multi_op_decoder.rs` around lines 436 - 486, Refactor partition_contract_events_by_operation and the corresponding partition_events_by_operation into one generic partitioning helper parameterized by a fn(&T) -> Option<&ContractEventV0> accessor, preserving the existing partition behavior and event cloning. Extract the duplicated topic checks into shared is_call_topic and is_return_topic helpers, and update both callers to use the shared implementation so symbol matching remains consistent.
🤖 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/multi_op_decoder.rs`:
- Around line 396-431: Update crates/core/src/decode/multi_op_decoder.rs:396-431
in partition_events_by_operation so events before the first recognized fn_call
are assigned to operation 0 instead of dropped, then extract the shared loop
into a generic helper parameterized by a ContractEventV0 accessor. Replace the
duplicated implementation at crates/core/src/decode/multi_op_decoder.rs:436-486
in partition_contract_events_by_operation with delegation to that helper,
ensuring both paths use the same leading-event behavior.
- Around line 514-518: Update the op_index filtering logic in the
multi-operation decoder so an index greater than or equal to reports.len()
returns an appropriate error instead of falling through to the all-reports
result. Preserve the existing single-report clone for valid indices and the
current behavior when op_index is absent.
- Around line 138-155: Update the report construction in the multi-operation
decoder so successful operations use an explicit success category and name, and
set their severity to Severity::Info instead of relying on
DiagnosticReport::new’s error default. Preserve error severity and metadata for
failed operations, and consolidate the duplicated success/failure branches where
only the summary differs.
- Around line 157-169: Update the TransactionContext literal in the report
construction flow to populate the required operation_index and operation_count
fields, using the corresponding per-operation metadata available in the decoder,
or add the established default tail if appropriate. Also assign
report.operation_index and report.operation_count during each operation’s
processing so the exposed report metadata is populated consistently.
- Around line 239-248: Update the fallback in the operation-result lookup around
op_results.get(i) so missing entries are represented explicitly rather than
fabricated as InvokeHostFunctionResult::Success with a zero hash. Preserve
actual decoded results unchanged and propagate the absent-result state through
the surrounding multi-operation decode flow.
- Around line 374-382: Update get_operation to handle TransactionEnvelope::TxV0
consistently with num_ops by reading and cloning the requested operation from
its tx.operations collection, or explicitly reject TxV0 by changing the
corresponding operation-count behavior so no misleading report is produced.
- Around line 250-365: Update the InvokeHostFunction handling in the
operation-result construction so each op.as_ref().and_then closure returns
Some(...) or None rather than bare tuples, preserving the existing fallback
values. Convert all string literals assigned to OperationResultInfo.error_name,
including HostError and NonInvokeHostFunctionOperation, into String values to
match Option<String>.
- Around line 64-75: Update the transaction-result match in the multi-operation
decoder so TxFeeBumpInnerSuccess unwraps its inner transaction result and
extracts its TxSuccess or TxFailed operations into op_results, allowing them to
use the existing per-operation decoding path. Preserve NotSorobanTransaction for
unsupported result variants and remove the aggregated build_report
short-circuit.
- Around line 204-221: Update the FailureAttribution construction in the
multi-operation decoder to render contract_id with the existing hash_to_strkey
conversion, propagating encoding failures instead of defaulting to an empty
string. Replace the hardcoded call_depth: 0 with the nesting depth already
tracked by the partitioner, while preserving the existing event and function
attribution flow.
---
Nitpick comments:
In `@crates/core/src/decode/multi_op_decoder.rs`:
- Around line 25-30: Add a public Default implementation for the unit struct
MultiOpDecoder, delegating to or matching its existing new() behavior; keep the
constructor and decoding behavior unchanged.
- Around line 181-202: Handle the results of enrich_diagnostic_report and
enrich_resource_report instead of discarding them with .ok(). When either
enrichment fails, log the error at warn or debug level and include the operation
index; preserve the existing report-generation flow rather than propagating the
enrichment failure.
- Around line 384-390: Add table-driven unit tests covering
partition_events_by_operation, using synthetic DiagnosticEvent sequences to
verify depth-based attribution, unbalanced fn_call/fn_return handling, and empty
events returning a single-element vector even when num_operations is greater
than one. Include boundary cases for one operation and empty input while
preserving the function’s current behavior.
- Around line 436-486: Refactor partition_contract_events_by_operation and the
corresponding partition_events_by_operation into one generic partitioning helper
parameterized by a fn(&T) -> Option<&ContractEventV0> accessor, preserving the
existing partition behavior and event cloning. Extract the duplicated topic
checks into shared is_call_topic and is_return_topic helpers, and update both
callers to use the shared implementation so symbol matching remains consistent.
In `@crates/core/src/lib.rs`:
- Around line 17-18: Update the crate-root re-exports in lib.rs to include
decode_transaction_with_op_filter alongside MultiOpDecoder, preserving its
existing async API and making it reachable directly from the crate root.
🪄 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: cc47ac1f-b6eb-459f-9daa-a41b2f1a7f6f
📒 Files selected for processing (4)
crates/core/src/decode/mod.rscrates/core/src/decode/multi_op_decoder.rscrates/core/src/lib.rscrates/core/src/types/report.rs
| let op_results = match tx_result.result { | ||
| TransactionResultResult::TxSuccess(ops) => ops, | ||
| TransactionResultResult::TxFailed(ops) => ops, | ||
| TransactionResultResult::TxFeeBumpInnerSuccess(_) => { | ||
| return Ok(vec![build_report(&classify_error(tx_data)?).map_err(|e| { | ||
| crate::error::GratError::Internal(format!("{}", e)) | ||
| })?]) | ||
| } | ||
| _ => { | ||
| return Err(crate::error::GratError::NotSorobanTransaction.into()); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -e rs . crates/core/src --exec rg -n 'TxFeeBumpInner|InnerTransactionResultPair' {} +Repository: Toolbox-Lab/Grat
Length of output: 3867
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== multi_op_decoder.rs ==\n'
sed -n '1,180p' crates/core/src/decode/multi_op_decoder.rs
printf '\n== host_error.rs ==\n'
sed -n '140,230p' crates/core/src/decode/host_error.rs
printf '\n== fee-bump-related definitions ==\n'
rg -n 'TxFeeBumpInnerSuccess|TxFeeBumpInnerFailed|InnerTransactionResultPair|TransactionResultResult' crates/core/src -g '*.rs'Repository: Toolbox-Lab/Grat
Length of output: 12943
Unwrap fee-bump inner results before decoding operations.
TxFeeBumpInnerSuccess short-circuits to a single aggregated report, and any other fee-bump result falls into NotSorobanTransaction, so fee-bump envelopes never reach the per-operation path. Unwrap the inner result pair and feed its TxSuccess/TxFailed ops through the same decoder instead.
🤖 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/multi_op_decoder.rs` around lines 64 - 75, Update the
transaction-result match in the multi-operation decoder so TxFeeBumpInnerSuccess
unwraps its inner transaction result and extracts its TxSuccess or TxFailed
operations into op_results, allowing them to use the existing per-operation
decoding path. Preserve NotSorobanTransaction for unsupported result variants
and remove the aggregated build_report short-circuit.
| let error_category = op_info.error_category.clone().unwrap_or_else(|| "unknown".to_string()); | ||
| let error_name = op_info.error_name.clone().unwrap_or_else(|| "Unknown".to_string()); | ||
|
|
||
| let mut report = if op_info.is_success { | ||
| DiagnosticReport::new( | ||
| &error_category, | ||
| 0, | ||
| &error_name, | ||
| &format!("Operation {} succeeded", i + 1), | ||
| ) | ||
| } else { | ||
| DiagnosticReport::new( | ||
| &error_category, | ||
| 0, | ||
| &error_name, | ||
| &format!("Operation {} failed", i + 1), | ||
| ) | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Successful operations are reported as errors.
For a successful op, error_category/error_name fall back to "unknown"/"Unknown" and DiagnosticReport::new hard-codes severity: Severity::Error (see crates/core/src/types/report.rs line 167). Every success report therefore renders as an error with a bogus category. Set an explicit success category and downgrade severity (e.g. Severity::Info) on the success path; the two branches also differ only by the summary string and can be collapsed.
🐛 Proposed fix
- let error_category = op_info.error_category.clone().unwrap_or_else(|| "unknown".to_string());
- let error_name = op_info.error_name.clone().unwrap_or_else(|| "Unknown".to_string());
-
- let mut report = if op_info.is_success {
- DiagnosticReport::new(
- &error_category,
- 0,
- &error_name,
- &format!("Operation {} succeeded", i + 1),
- )
- } else {
- DiagnosticReport::new(
- &error_category,
- 0,
- &error_name,
- &format!("Operation {} failed", i + 1),
- )
- };
+ let (default_category, default_name) = if op_info.is_success {
+ ("success", "Success")
+ } else {
+ ("unknown", "Unknown")
+ };
+ let error_category = op_info.error_category.clone().unwrap_or_else(|| default_category.to_string());
+ let error_name = op_info.error_name.clone().unwrap_or_else(|| default_name.to_string());
+
+ let mut report = DiagnosticReport::new(
+ &error_category,
+ 0,
+ &error_name,
+ &format!(
+ "Operation {} {}",
+ i + 1,
+ if op_info.is_success { "succeeded" } else { "failed" }
+ ),
+ );
+ if op_info.is_success {
+ report.severity = crate::types::report::Severity::Info;
+ }📝 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.
| let error_category = op_info.error_category.clone().unwrap_or_else(|| "unknown".to_string()); | |
| let error_name = op_info.error_name.clone().unwrap_or_else(|| "Unknown".to_string()); | |
| let mut report = if op_info.is_success { | |
| DiagnosticReport::new( | |
| &error_category, | |
| 0, | |
| &error_name, | |
| &format!("Operation {} succeeded", i + 1), | |
| ) | |
| } else { | |
| DiagnosticReport::new( | |
| &error_category, | |
| 0, | |
| &error_name, | |
| &format!("Operation {} failed", i + 1), | |
| ) | |
| }; | |
| let (default_category, default_name) = if op_info.is_success { | |
| ("success", "Success") | |
| } else { | |
| ("unknown", "Unknown") | |
| }; | |
| let error_category = op_info.error_category.clone().unwrap_or_else(|| default_category.to_string()); | |
| let error_name = op_info.error_name.clone().unwrap_or_else(|| default_name.to_string()); | |
| let mut report = DiagnosticReport::new( | |
| &error_category, | |
| 0, | |
| &error_name, | |
| &format!( | |
| "Operation {} {}", | |
| i + 1, | |
| if op_info.is_success { "succeeded" } else { "failed" } | |
| ), | |
| ); | |
| if op_info.is_success { | |
| report.severity = crate::types::report::Severity::Info; | |
| } |
🤖 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/multi_op_decoder.rs` around lines 138 - 155, Update
the report construction in the multi-operation decoder so successful operations
use an explicit success category and name, and set their severity to
Severity::Info instead of relying on DiagnosticReport::new’s error default.
Preserve error severity and metadata for failed operations, and consolidate the
duplicated success/failure branches where only the summary differs.
| report.transaction_context = Some(crate::types::report::TransactionContext { | ||
| tx_hash: tx_data | ||
| .get("hash") | ||
| .and_then(|h| h.as_str()) | ||
| .unwrap_or("unknown") | ||
| .to_string(), | ||
| ledger_sequence: tx_data.get("ledger").and_then(|v| v.as_u64()).unwrap_or(0) as u32, | ||
| function_name: op_info.function_name.clone(), | ||
| arguments: op_info.arguments.clone(), | ||
| return_value: op_info.return_value.clone(), | ||
| fee: overall_fee.clone(), | ||
| resources: overall_resource_summary.clone(), | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Struct literal omits the new TransactionContext fields, and operation metadata is never populated.
TransactionContext in crates/core/src/types/report.rs (lines 71-76) now has operation_index and operation_count; this literal has no ..Default::default() tail, so it won't compile (E0063). Also report.operation_index / report.operation_count are never set anywhere in this file, so the per-operation metadata the PR is meant to expose stays None.
🐛 Proposed fix
fee: overall_fee.clone(),
resources: overall_resource_summary.clone(),
+ operation_index: Some(i),
+ operation_count: Some(num_ops),
});
+
+ report.operation_index = Some(i);
+ report.operation_count = Some(num_ops);📝 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.
| report.transaction_context = Some(crate::types::report::TransactionContext { | |
| tx_hash: tx_data | |
| .get("hash") | |
| .and_then(|h| h.as_str()) | |
| .unwrap_or("unknown") | |
| .to_string(), | |
| ledger_sequence: tx_data.get("ledger").and_then(|v| v.as_u64()).unwrap_or(0) as u32, | |
| function_name: op_info.function_name.clone(), | |
| arguments: op_info.arguments.clone(), | |
| return_value: op_info.return_value.clone(), | |
| fee: overall_fee.clone(), | |
| resources: overall_resource_summary.clone(), | |
| }); | |
| report.transaction_context = Some(crate::types::report::TransactionContext { | |
| tx_hash: tx_data | |
| .get("hash") | |
| .and_then(|h| h.as_str()) | |
| .unwrap_or("unknown") | |
| .to_string(), | |
| ledger_sequence: tx_data.get("ledger").and_then(|v| v.as_u64()).unwrap_or(0) as u32, | |
| function_name: op_info.function_name.clone(), | |
| arguments: op_info.arguments.clone(), | |
| return_value: op_info.return_value.clone(), | |
| fee: overall_fee.clone(), | |
| resources: overall_resource_summary.clone(), | |
| operation_index: Some(i), | |
| operation_count: Some(num_ops), | |
| }); | |
| report.operation_index = Some(i); | |
| report.operation_count = Some(num_ops); |
🤖 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/multi_op_decoder.rs` around lines 157 - 169, Update
the TransactionContext literal in the report construction flow to populate the
required operation_index and operation_count fields, using the corresponding
per-operation metadata available in the decoder, or add the established default
tail if appropriate. Also assign report.operation_index and
report.operation_count during each operation’s processing so the exposed report
metadata is populated consistently.
| report.cross_contract_attribution = if !op_contract_events.is_empty() { | ||
| Some(crate::types::report::FailureAttribution { | ||
| contract_address: op_contract_events.iter().filter_map(|e| { | ||
| if let ContractEventBody::V0(v0) = &e.body { | ||
| v0.contract_id.as_ref().map(|h| { | ||
| crate::xdr::codec::XdrCodec::to_xdr_base64(h).unwrap_or_default() | ||
| }) | ||
| } else { | ||
| None | ||
| } | ||
| }).next().unwrap_or_default(), | ||
| function_name: op_info.function_name.clone(), | ||
| call_depth: 0, | ||
| origin_description: format!("Operation {}", i + 1), | ||
| }) | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the existing contract-id -> strkey conversion used elsewhere in the crate.
rg -n -C3 'Strkey|strkey|to_contract_address|Contract\(' crates/core/src --type=rust -g '!**/tests/**' | head -60Repository: Toolbox-Lab/Grat
Length of output: 4306
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== multi_op_decoder excerpt =="
sed -n '150,260p' crates/core/src/decode/multi_op_decoder.rs
echo
echo "== address helper =="
sed -n '1,120p' crates/core/src/types/address.rs
echo
echo "== codec helpers around contract/address encoding =="
sed -n '1,140p' crates/core/src/xdr/codec.rs
echo
echo "== search for depth tracking / partitioning =="
rg -n -C3 'call_depth|depth|partition' crates/core/src/decode crates/core/src -g '!**/tests/**'Repository: Toolbox-Lab/Grat
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== multi_op_decoder relevant lines =="
nl -ba crates/core/src/decode/multi_op_decoder.rs | sed -n '180,235p'
echo
echo "== address helper relevant lines =="
nl -ba crates/core/src/types/address.rs | sed -n '1,80p'
echo
echo "== codec contract/address helpers =="
nl -ba crates/core/src/xdr/codec.rs | sed -n '1,120p'
echo
echo "== call_depth usage =="
rg -n -C2 'call_depth' crates/core/src -g '!**/tests/**'
echo
echo "== partition/depth tracking =="
rg -n -C2 'partition|depth' crates/core/src/decode/multi_op_decoder.rs crates/core/src/decode -g '!**/tests/**'Repository: Toolbox-Lab/Grat
Length of output: 232
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the surrounding decoder structure and any depth variables.
nl -ba crates/core/src/decode/multi_op_decoder.rs | sed -n '1,260p'Repository: Toolbox-Lab/Grat
Length of output: 194
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== FailureAttribution definition =="
sed -n '1,220p' crates/core/src/types/report.rs | grep -n -A12 -B3 'FailureAttribution'
echo
echo "== contract address rendering helper =="
rg -n -C2 'scaddress_to_strkey|to_strkey|from_contract_id|contract_address' crates/core/src -g '!**/tests/**' | sed -n '1,120p'
echo
echo "== multi_op_decoder call_depth context =="
sed -n '1,260p' crates/core/src/decode/multi_op_decoder.rs | grep -n -A8 -B8 'call_depth'Repository: Toolbox-Lab/Grat
Length of output: 9797
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '384,486p' crates/core/src/decode/multi_op_decoder.rsRepository: Toolbox-Lab/Grat
Length of output: 3410
Render contract_address as a contract strkey and propagate the real depth. to_xdr_base64(h) turns the hash into opaque XDR instead of the C... contract ID used elsewhere (hash_to_strkey), and unwrap_or_default() hides encoding failures. call_depth: 0 also drops the nesting info the partitioner already tracks.
🤖 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/multi_op_decoder.rs` around lines 204 - 221, Update
the FailureAttribution construction in the multi-operation decoder to render
contract_id with the existing hash_to_strkey conversion, propagating encoding
failures instead of defaulting to an empty string. Replace the hardcoded
call_depth: 0 with the nesting depth already tracked by the partitioner, while
preserving the existing event and function attribution flow.
| let op_result = op_results.get(i).cloned().unwrap_or_else(|| { | ||
| OperationResult { | ||
| ext: stellar_xdr::curr::ExtensionPoint::V0, | ||
| tr: OperationResultTr::InvokeHostFunction( | ||
| stellar_xdr::curr::InvokeHostFunctionResult::Success( | ||
| stellar_xdr::curr::Hash([0; 32]), | ||
| ), | ||
| ), | ||
| } | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing operation result is fabricated as Success.
When op_results is shorter than num_ops (truncated/absent result XDR), this synthesizes an InvokeHostFunctionResult::Success with a zero hash, so a failed or unknown operation is reported as succeeded. Represent the gap explicitly instead.
🐛 Proposed fix
- let op_result = op_results.get(i).cloned().unwrap_or_else(|| {
- OperationResult {
- ext: stellar_xdr::curr::ExtensionPoint::V0,
- tr: OperationResultTr::InvokeHostFunction(
- stellar_xdr::curr::InvokeHostFunctionResult::Success(
- stellar_xdr::curr::Hash([0; 32]),
- ),
- ),
- }
- });
-
- let info = match &op_result.tr {
+ let Some(op_result) = op_results.get(i) else {
+ results.push(OperationResultInfo {
+ function_name: invoke_fn_name(op.as_ref()),
+ arguments: vec![],
+ return_value: None,
+ is_success: false,
+ error_category: Some("Unknown".to_string()),
+ error_name: Some("MissingOperationResult".to_string()),
+ });
+ continue;
+ };
+
+ let info = match &op_result.tr {📝 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.
| let op_result = op_results.get(i).cloned().unwrap_or_else(|| { | |
| OperationResult { | |
| ext: stellar_xdr::curr::ExtensionPoint::V0, | |
| tr: OperationResultTr::InvokeHostFunction( | |
| stellar_xdr::curr::InvokeHostFunctionResult::Success( | |
| stellar_xdr::curr::Hash([0; 32]), | |
| ), | |
| ), | |
| } | |
| }); | |
| let Some(op_result) = op_results.get(i) else { | |
| results.push(OperationResultInfo { | |
| function_name: invoke_fn_name(op.as_ref()), | |
| arguments: vec![], | |
| return_value: None, | |
| is_success: false, | |
| error_category: Some("Unknown".to_string()), | |
| error_name: Some("MissingOperationResult".to_string()), | |
| }); | |
| continue; | |
| }; | |
| let info = match &op_result.tr { |
🤖 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/multi_op_decoder.rs` around lines 239 - 248, Update
the fallback in the operation-result lookup around op_results.get(i) so missing
entries are represented explicitly rather than fabricated as
InvokeHostFunctionResult::Success with a zero hash. Preserve actual decoded
results unchanged and propagate the absent-result state through the surrounding
multi-operation decode flow.
| let info = match &op_result.tr { | ||
| OperationResultTr::InvokeHostFunction(inv_result) => { | ||
| match inv_result { | ||
| stellar_xdr::curr::InvokeHostFunctionResult::Success(hash) => { | ||
| let (fname, args, ret_val) = op.as_ref().and_then(|o| { | ||
| if let OperationBody::InvokeHostFunction(invoke) = &o.body { | ||
| let fname = invoke.function_name.to_string(); | ||
| let args: Vec<String> = invoke.args.iter().map(|a| format!("{a:?}")).collect(); | ||
| (Some(fname), args, None) | ||
| } else { | ||
| (None, vec![], None) | ||
| } | ||
| }).unwrap_or((None, vec![], None)); | ||
|
|
||
| OperationResultInfo { | ||
| function_name: fname, | ||
| arguments: args, | ||
| return_value: ret_val, | ||
| is_success: true, | ||
| error_category: None, | ||
| error_name: None, | ||
| } | ||
| } | ||
| stellar_xdr::curr::InvokeHostFunctionResult::Trapped => { | ||
| let (fname, _) = op.as_ref().and_then(|o| { | ||
| if let OperationBody::InvokeHostFunction(invoke) = &o.body { | ||
| (Some(invoke.function_name.to_string()), ()) | ||
| } else { | ||
| (None, ()) | ||
| } | ||
| }).unwrap_or((None, ())); | ||
|
|
||
| OperationResultInfo { | ||
| function_name: fname, | ||
| arguments: vec![], | ||
| return_value: None, | ||
| is_success: false, | ||
| error_category: Some("Contract".to_string()), | ||
| error_name: Some("HostError"), | ||
| } | ||
| } | ||
| stellar_xdr::curr::InvokeHostFunctionResult::ResourceLimitExceeded => { | ||
| let (fname, _) = op.as_ref().and_then(|o| { | ||
| if let OperationBody::InvokeHostFunction(invoke) = &o.body { | ||
| (Some(invoke.function_name.to_string()), ()) | ||
| } else { | ||
| (None, ()) | ||
| } | ||
| }).unwrap_or((None, ())); | ||
|
|
||
| OperationResultInfo { | ||
| function_name: fname, | ||
| arguments: vec![], | ||
| return_value: None, | ||
| is_success: false, | ||
| error_category: Some("Budget".to_string()), | ||
| error_name: Some("HostError"), | ||
| } | ||
| } | ||
| stellar_xdr::curr::InvokeHostFunctionResult::EntryArchived => { | ||
| let (fname, _) = op.as_ref().and_then(|o| { | ||
| if let OperationBody::InvokeHostFunction(invoke) = &o.body { | ||
| (Some(invoke.function_name.to_string()), ()) | ||
| } else { | ||
| (None, ()) | ||
| } | ||
| }).unwrap_or((None, ())); | ||
|
|
||
| OperationResultInfo { | ||
| function_name: fname, | ||
| arguments: vec![], | ||
| return_value: None, | ||
| is_success: false, | ||
| error_category: Some("Storage".to_string()), | ||
| error_name: Some("HostError"), | ||
| } | ||
| } | ||
| stellar_xdr::curr::InvokeHostFunctionResult::Malformed | ||
| | stellar_xdr::curr::InvokeHostFunctionResult::InsufficientRefundableFee => { | ||
| let (fname, _) = op.as_ref().and_then(|o| { | ||
| if let OperationBody::InvokeHostFunction(invoke) = &o.body { | ||
| (Some(invoke.function_name.to_string()), ()) | ||
| } else { | ||
| (None, ()) | ||
| } | ||
| }).unwrap_or((None, ())); | ||
|
|
||
| OperationResultInfo { | ||
| function_name: fname, | ||
| arguments: vec![], | ||
| return_value: None, | ||
| is_success: false, | ||
| error_category: Some("Context".to_string()), | ||
| error_name: Some("HostError"), | ||
| } | ||
| } | ||
| } | ||
| } | ||
| _ => { | ||
| let fname = op.as_ref().and_then(|o| { | ||
| if let OperationBody::InvokeHostFunction(invoke) = &o.body { | ||
| Some(invoke.function_name.to_string()) | ||
| } else { | ||
| None | ||
| } | ||
| }).unwrap_or_default(); | ||
|
|
||
| OperationResultInfo { | ||
| function_name: if fname.is_empty() { None } else { Some(fname) }, | ||
| arguments: vec![], | ||
| return_value: None, | ||
| is_success: false, | ||
| error_category: Some("Unknown".to_string()), | ||
| error_name: Some("NonInvokeHostFunctionOperation"), | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the field type and the InvokeHostFunctionResult variants in use.
rg -n -A8 'struct OperationResultInfo' crates/core/src/decode/multi_op_decoder.rs
fd -t f 'Cargo.toml' --exec rg -n 'stellar-xdr' {} +Repository: Toolbox-Lab/Grat
Length of output: 892
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '220,380p' crates/core/src/decode/multi_op_decoder.rs | cat -nRepository: Toolbox-Lab/Grat
Length of output: 8253
Fix the and_then and error_name type mismatches
- Every
op.as_ref().and_then(|o| { ... })branch returns a tuple, butand_thenneeds anOption<_>. error_nameisOption<String>, soSome("HostError")/Some("NonInvokeHostFunctionOperation")needs.to_string().
🤖 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/multi_op_decoder.rs` around lines 250 - 365, Update
the InvokeHostFunction handling in the operation-result construction so each
op.as_ref().and_then closure returns Some(...) or None rather than bare tuples,
preserving the existing fallback values. Convert all string literals assigned to
OperationResultInfo.error_name, including HostError and
NonInvokeHostFunctionOperation, into String values to match Option<String>.
| fn get_operation(envelope: &TransactionEnvelope, index: usize) -> Option<Operation> { | ||
| match envelope { | ||
| TransactionEnvelope::Tx(TxV1Envelope { tx, .. }) => tx.operations.get(index).cloned(), | ||
| TransactionEnvelope::TxFeeBump(fb) => match &fb.tx.inner_tx { | ||
| FeeBumpTransactionInnerTx::Tx(TxV1Envelope { tx, .. }) => tx.operations.get(index).cloned(), | ||
| }, | ||
| TransactionEnvelope::TxV0(_) => None, | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
stellar-xdr rust TransactionV0Envelope tx operations field
💡 Result:
In the Stellar XDR definitions, the TransactionV0Envelope struct does not contain an operations field directly [1][2]. Instead, the TransactionV0Envelope holds a tx field of type TransactionV0 [1][2], and the operations are contained within that TransactionV0 struct [3][4]. Specifically, the structures are defined as follows: 1. TransactionV0Envelope contains: - tx: TransactionV0 [1][2] - signatures: VecM<DecoratedSignature, 20> [1][2] 2. TransactionV0 contains: - source_account_ed25519: Uint256 [3][4] - fee: u32 [3][4] - seq_num: SequenceNumber [3][4] - time_bounds: Option [3][4] - memo: Memo [3][4] - operations: VecM<Operation, 100> [3][4] - ext: TransactionV0Ext [3][4] Therefore, to access the operations in Rust, you must access them through the tx field: envelope.tx.operations [3][4].
Citations:
- 1: https://docs.rs/stellar-base/latest/stellar_base/xdr/struct.TransactionV0Envelope.html
- 2: https://docs.rs/stellar-xdr/latest/aarch64-apple-darwin/stellar_xdr/curr/struct.TransactionV0Envelope.html
- 3: https://docs.rs/stellar-xdr/latest/stellar_xdr/struct.TransactionV0.html
- 4: https://docs.rs/stellar-base/latest/stellar_base/xdr/struct.TransactionV0.html
Handle TxV0 operations consistently. num_ops hard-codes TxV0 to 1, but get_operation returns None even though TransactionV0 stores operations in tx.operations. That yields one misleading report for every TxV0 envelope; either read tx.operations here too or reject TxV0 explicitly.
🤖 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/multi_op_decoder.rs` around lines 374 - 382, Update
get_operation to handle TransactionEnvelope::TxV0 consistently with num_ops by
reading and cloning the requested operation from its tx.operations collection,
or explicitly reject TxV0 by changing the corresponding operation-count behavior
so no misleading report is produced.
| for event in diagnostic_events { | ||
| if let ContractEventBody::V0(v0) = &event.event.body { | ||
| let is_call = v0.topics.iter().any(|t| { | ||
| if let ScVal::Symbol(s) = t { | ||
| let s_low = s.to_string().to_lowercase(); | ||
| s_low == "fn_call" || s_low == "function_call" || s_low == "call" | ||
| } else { | ||
| false | ||
| } | ||
| }); | ||
|
|
||
| if is_call { | ||
| if depth == 0 && current_op < num_operations { | ||
| current_op += 1; | ||
| } | ||
| depth += 1; | ||
| } | ||
|
|
||
| if current_op > 0 && current_op <= num_operations { | ||
| partitions[current_op - 1].push(event.clone()); | ||
| } | ||
|
|
||
| let is_return = v0.topics.iter().any(|t| { | ||
| if let ScVal::Symbol(s) = t { | ||
| let s_low = s.to_string().to_lowercase(); | ||
| s_low == "fn_return" || s_low == "function_return" || s_low == "return" | ||
| } else { | ||
| false | ||
| } | ||
| }); | ||
|
|
||
| if is_return { | ||
| depth = depth.saturating_sub(1); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Duplicated partitioning logic means the dropped-leading-events bug exists twice. partition_events_by_operation and partition_contract_events_by_operation are copies of each other, so the current_op > 0 guard that discards every event preceding the first recognized fn_call has to be fixed in both places — the shared root cause is the missing generic abstraction.
crates/core/src/decode/multi_op_decoder.rs#L396-L431: attribute leading events to operation 0 instead of dropping them, then extract the loop into a generic helper parameterized by aContractEventV0accessor.crates/core/src/decode/multi_op_decoder.rs#L436-L486: delete the duplicated body and delegate to that shared helper so the fix cannot drift.
📍 Affects 1 file
crates/core/src/decode/multi_op_decoder.rs#L396-L431(this comment)crates/core/src/decode/multi_op_decoder.rs#L436-L486
🤖 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/multi_op_decoder.rs` around lines 396 - 431, Update
crates/core/src/decode/multi_op_decoder.rs:396-431 in
partition_events_by_operation so events before the first recognized fn_call are
assigned to operation 0 instead of dropped, then extract the shared loop into a
generic helper parameterized by a ContractEventV0 accessor. Replace the
duplicated implementation at crates/core/src/decode/multi_op_decoder.rs:436-486
in partition_contract_events_by_operation with delegation to that helper,
ensuring both paths use the same leading-event behavior.
| if let Some(idx) = op_index { | ||
| if idx < reports.len() { | ||
| return Ok(vec![reports[idx].clone()]); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Out-of-range op_index silently returns every report.
When idx >= reports.len() the guard falls through and all operation reports are returned, so a caller asking for operation 7 of a 2-op transaction gets both reports with no indication the filter was ignored. Return an error instead.
🐛 Proposed fix
if let Some(idx) = op_index {
- if idx < reports.len() {
- return Ok(vec![reports[idx].clone()]);
- }
+ let report = reports.get(idx).cloned().ok_or_else(|| {
+ crate::error::GratError::Internal(format!(
+ "Operation index {} out of range; transaction has {} operation(s)",
+ idx,
+ reports.len()
+ ))
+ })?;
+ return Ok(vec![report]);
}📝 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 let Some(idx) = op_index { | |
| if idx < reports.len() { | |
| return Ok(vec![reports[idx].clone()]); | |
| } | |
| } | |
| if let Some(idx) = op_index { | |
| let report = reports.get(idx).cloned().ok_or_else(|| { | |
| crate::error::GratError::Internal(format!( | |
| "Operation index {} out of range; transaction has {} operation(s)", | |
| idx, | |
| reports.len() | |
| )) | |
| })?; | |
| return Ok(vec![report]); | |
| } |
🤖 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/multi_op_decoder.rs` around lines 514 - 518, Update
the op_index filtering logic in the multi-operation decoder so an index greater
than or equal to reports.len() returns an appropriate error instead of falling
through to the all-reports result. Preserve the existing single-report clone for
valid indices and the current behavior when op_index is absent.
1 participant
Implements multi-operation transaction decoding (#377, #379).
Changes
Files changed
Closes #377
Closes #379
Summary by CodeRabbit