Skip to content

fix: detect parameter reordering as a breaking change in diff.rs - #2

Open
CodedTricks wants to merge 1 commit into
mainfrom
fix/256-detect-parameter-reordering
Open

fix: detect parameter reordering as a breaking change in diff.rs#2
CodedTricks wants to merge 1 commit into
mainfrom
fix/256-detect-parameter-reordering

Conversation

@CodedTricks

Copy link
Copy Markdown
Owner

Summary

crates/lumenqraph-core/src/diff.rs computes interface diffs by rendering function signatures as strings and comparing them. When a function's parameters are reordered (e.g. transfer(from, to, amount)transfer(amount, from, to)), the string changed and the diff did mark it breaking — but only via the generic changed bucket. The summary line said "changed function transfer: …" with no indication that only the order changed.

Since Soroban encoding is positional, a reorder breaks every caller that passes arguments by position. It deserves its own reordered change type so operators and webhook subscribers can immediately tell what kind of breaking change occurred.

What was implemented

  • ReorderedItem struct — mirrors ChangedItem (name, from, to)
  • SectionDiff.reordered — new Vec<ReorderedItem> field
  • is_empty / has_breaking — updated to include reordered
  • function_sigs() — now returns (BTreeMap<name, sig>, BTreeMap<name, params>) so the diff logic has the ordered parameter list per function
  • diff_section_with_params() — new helper that, when two signatures differ, checks whether the param set is the same but the order differs (is_param_reorder helper); if so, pushes to reordered instead of changed
  • build_summary() — emits "reordered function <name>: <from> became <to>" lines
  • crates/lumenqraph-core/Cargo.toml — adds missing url and tokio/net dependencies required by url_validation.rs (pre-existing omission that prevented the crate from building with the stable toolchain)

Tests

Two new tests added:

  • reordered_parameters_are_breaking_and_flagged_as_reordered — verifies the reordered bucket is used, changed is empty, breaking is true, and the summary line starts with "reordered function transfer"
  • renamed_parameter_is_changed_not_reordered — regression guard ensuring a rename still goes to changed, not reordered

All 21 diff tests pass.

Closes Lumen-Scribe#256

…-Scribe#256)

Soroban encoding is positional: a caller that passes arguments in order
(from, to, amount) will send 'from' where 'amount' is now expected if
the contract upgrades and reorders parameters. The diff module must flag
this explicitly so operators and webhook subscribers know that a reorder
is a breaking API change, not just a cosmetic rename.

Previously a reorder fell into the generic 'changed' bucket via string
comparison. It was already marked breaking, but the summary line said
'changed function transfer: ...' with no indication that only the order
changed, making it harder to triage the upgrade impact.

Changes:
- diff.rs: add ReorderedItem struct (name, from, to) mirroring ChangedItem
- SectionDiff: add 'reordered' Vec<ReorderedItem> field
- SectionDiff::is_empty / has_breaking: include reordered in both checks
- function_sigs(): return (BTreeMap<String,String>, BTreeMap<String,Vec<String>>)
  so callers have the ordered parameter list per function name
- diff_section_with_params(): when signatures differ, check whether the
  param set is identical but order differs (is_param_reorder helper);
  if so, push to reordered instead of changed
- build_summary(): emit 'reordered function <name>: ...' lines
- Two new tests:
  * reordered_parameters_are_breaking_and_flagged_as_reordered
  * renamed_parameter_is_changed_not_reordered (regression guard)
- crates/lumenqraph-core/Cargo.toml: add missing url and tokio/net deps
  required by url_validation.rs (pre-existing omission)

All 21 diff tests pass.

Closes Lumen-Scribe#256
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.

The diff module computes SpecDiff by rendering signatures to strings, which does not detect parameter reordering within a function

1 participant