fix(ergo-sigma): pre-v3 trees auto-upcast mixed-kind numeric operands

[arkadianet]

The last coal — and a reversed verdict

This vector was previously classified as mis-blessed ("JVM coerces" looked like a compiler artifact). The harness audit traced the bless to genuine consensus behavior, and the Scala source confirms it:

DeserializationSigmaBuilder.applyUpcast (SigmaBuilder.scala:741-756) auto-inserts an Upcast node on the narrower operand of a mixed-kind numeric two-operand op at deserialization when the tree version is < 3:

// since v3 trees, Upcast nodes are not inserted automatically
// ... the reason to remove auto-upcast is to make deserialization
// always producing tree which was serialized
override protected def applyUpcast[T <: SType](left: Value[T], right: Value[T]) =
  if (VersionContext.current.isV3OrLaterErgoTreeVersion) (left, right)
  else super.applyUpcast(left, right)

Exactly three builder families route through it (SigmaBuilder.scala:678-705): arithOp (Plus/Minus/Multiply/Divide/Modulo/Min/Max), comparisonOp (GT/GE/LT/LE), equalityOp (EQ/NEQ). BitOp constructs directly — not upcast. upcastTo is a no-op on the already-max operand (syntax.scala:174), so exactly one node per op. The blessed cost reconciles: 35 = Const 5 + Upcast 10 + Const 5 + Plus 15. sigma-rust ships the same coercion.

We rejected such trees ("matching numeric types" TypeError) — a false-reject on the mainnet-reachable v0–v2 surface: a crafted pre-v3 tree with Int + Long validates in Scala and would split us off the chain.

Fix

apply_pre_v3_auto_upcast in cast.rs, applied at eval time (parse-time node insertion would break our byte-identical reserialization invariant): after both operand evals, the narrower operand widens to the other's kind, charging the Upcast NumericCastCostKind (10; 30 for a BigInt target — it's a TypeBasedCost, not flat) exactly once, gated on !is_v3_ergo_tree(). Wired into all 13 affected eval fns; the upcast runs before add_arith_cost so the BigInt arith rate keys on the upcast kind, matching the Scala node type after builder rewriting. UnsignedBigInt never participates (v6-only carrier, unreachable pre-v3).

Vectors

• Before: 2500 nice / 1 coal (main at fix(ergo-sigma,ergo-ser): HOF function values + hasDeserialize reduction fork #52)
• After: 2501 nice / 0 coal / 2501 entries — the suite is fully closed, zero regression

Tests (411 total, +5)

• pre_v3_plus_mixed_kinds_auto_upcasts — Long(3) at v0; TypeError at v3 (gate both directions)
• pre_v3_auto_upcast_charges_numeric_cast_cost — twin-tree deltas: +10 fixed-width target, +30 BigInt target
• pre_v3_comparison_mixed_kinds_auto_upcasts — GT with the right operand narrower (covers the r-side widening branch)
• pre_v3_equality_mixed_kinds_auto_upcasts — EQ/NEQ (without upcast the carriers differ and PartialEq would yield a wrong false, not an error)
• pre_v3_min_mixed_kinds_auto_upcasts — result at the wider kind

Review notes

codex raised a charge-ordering point anchored on our eval_upcast's charge-first order — but Scala's Upcast.eval evaluates its input then charges (trees.scala:402-407), so the anchor was backwards. The genuine narrow residual (left-operand upcast charge lands between operand evals in Scala, after both here) is documented in the helper: totals are identical on every success path, a budget breach fires in both implementations, and exact charge-point parity is structurally unavailable at eval time (Scala places the node from static types at deserialization). Known pre-existing residual (no vector): at v3+, mixed-kind EQ/NEQ returns false here while Scala rejects at deserialization (SameTypeConstrain).

cargo test -p ergo-sigma --lib (411) and cargo test -p ergo-ser --lib (324) green; cargo fmt --all applied.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Pre-v3 ErgoTree evaluation now automatically handles mixed numeric types in binary operations by upcasting to the wider type, ensuring compatibility with expected Scala behavior.
  • Numeric cost calculations properly applied when automatic type upcasting occurs.
  • Tree version 3 maintains strict type checking for mixed numeric operations as intended.

• Tests

  • Added comprehensive test coverage for automatic numeric upcasting in pre-v3 evaluation scenarios, including arithmetic and comparison operations.

[coderabbitai]

Warning

Review limit reached

@arkadianet, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 43 minutes and 58 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b5ccfd86-c1a8-48d5-aabf-fa9e45175f9c

📥 Commits

Reviewing files that changed from the base of the PR and between 8cc67f9 and c881649.

📒 Files selected for processing (4)

• ergo-sigma/src/evaluator/opcodes/arithmetic.rs
• ergo-sigma/src/evaluator/opcodes/cast.rs
• ergo-sigma/src/evaluator/opcodes/comparison.rs
• ergo-sigma/src/evaluator/tests.rs

📝 Walkthrough

Walkthrough

This PR adds automatic implicit numeric upcasting to ErgoTree evaluation for versions before v3. When evaluating mixed-kind numeric operands (e.g., Int + Long), the system ranks the numeric types and widens the lower-ranked operand to match the higher-ranked one before executing arithmetic or comparison operations, charging cast cost in the process.

Changes

Pre-v3 numeric auto-upcasting

Layer / File(s)	Summary
Core auto-upcast mechanism ergo-sigma/src/evaluator/opcodes/cast.rs	apply_pre_v3_auto_upcast ranks numeric Values by kind (Byte < Short < Int < Long < BigInt) and widens the lower-ranked operand to the higher rank for non-v3 trees; widen_to_kind_of helper performs the actual conversion and charges NumericCastCostKind cost.
Arithmetic operator integration ergo-sigma/src/evaluator/opcodes/arithmetic.rs	Binary arithmetic (Plus, Minus, Multiply, Division, Modulo) and reduction (Min, Max) evaluators now call apply_pre_v3_auto_upcast on evaluated operands before cost charging and type-directed operation dispatch.
Comparison operator integration ergo-sigma/src/evaluator/opcodes/comparison.rs	Ordering (LT, LE, GT, GE) and equality (EQ, NEQ) evaluators apply apply_pre_v3_auto_upcast to operands before comparison cost or eq_with_cost evaluation.
Pre-v3 auto-upcast test suite ergo-sigma/src/evaluator/tests.rs	Tests confirm pre-v3 trees auto-upcast mixed-kind operands (e.g., Int + Long) and produce widened results, v3+ trees reject with TypeError, and numeric cast cost is charged exactly once per auto-upcast operation (with higher cost for BigInt targets).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• arkadianet/ergo#17: Modifies arithmetic opcode overflow and divide-by-zero semantics for the same numeric opcodes (Plus, Minus, Multiply, Division, Modulo, Min, Max) that are updated in this PR for auto-upcasting.
• arkadianet/ergo#30: Refactors EQ/NEQ opcode evaluation to use eq_with_cost, affecting the exact same equality comparison code path where this PR inserts auto-upcast preprocessing.
• arkadianet/ergo#21: Changes concrete BigInt/Byte/Short arithmetic and bounds/wrap/modulo rules for overlapping numeric operations (Minus, Plus, Multiply, Division, Modulo) modified by this PR.

Poem

🐰 In pre-v3 forests, numbers dance with grace,
Mixed kinds upcast to a wider place,
Cost is charged but once, oh so fair,
While v3's strictness keeps them separate.
Scala's spirit flows through this weaving care!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 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: implementing pre-v3 tree auto-upcasting of mixed-kind numeric operands, which is the core feature added across multiple files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/pre-v3-numeric-auto-upcast

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[arkadianet]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@ergo-sigma/src/evaluator/opcodes/comparison.rs`:
- Around line 121-123: The EQ/NEQ path still allows post-v3 mixed numeric kinds
to reach require_comparable/eq_with_cost; add a guard after
apply_pre_v3_auto_upcast(l,r, cx) that detects sigma/version >= 3 and differing
numeric kinds and returns an evaluation error (same rejection behavior as Scala)
instead of proceeding to require_comparable. Implement the check in
comparison.rs within the EQ/NEQ handling: inspect the resolved Values (l, r),
use the existing context version flag on cx.ctx (or equivalent) to detect v3+,
detect mixed numeric kinds (e.g., Int vs Long/BigInt or type-kind mismatch), and
return an Err that mirrors deserialization rejection so mixed-kind numeric
equality is rejected before calling require_comparable or eq_with_cost.

🪄 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

Run ID: e23e688a-7990-4a33-a24f-f1b31274efd9

📥 Commits

Reviewing files that changed from the base of the PR and between fa97cfc and 8cc67f9.

📒 Files selected for processing (4)

• ergo-sigma/src/evaluator/opcodes/arithmetic.rs
• ergo-sigma/src/evaluator/opcodes/cast.rs
• ergo-sigma/src/evaluator/opcodes/comparison.rs
• ergo-sigma/src/evaluator/tests.rs

[arkadianet]

Addressed in c881649 — verified against the Scala source first:

Valid, with two refinements over the suggestion. Scala's equalityOp (SigmaBuilder.scala:678-686) runs SameTypeConstrain after the (pre-v3) upcast and fails the tree at deserialization (ConstraintFailed). Without a guard our v3+ mixed-kind EQ fell through to PartialEq's catch-all — EQ(Int 1, Long 1) → false and NEQ → true, i.e. a crafted v3 tree would validate here while Scala rejects it.

Refinements:

1. No version flag — post-upcast mixed numeric kinds can only exist on v3+ trees (pre-v3 the upcast just equalized them), so reject_mixed_numeric_equality is self-gating.
2. Numeric kinds only, deliberately — SameTypeConstrain covers all types in Scala, but our collection/tuple carriers are not 1:1 with static types; a runtime-kind guard there could falsely reject valid spends. Numeric carriers map exactly (incl. BigInt vs UnsignedBigInt as distinct kinds). The non-numeric mismatch stays a documented residual.

Pinned by v3_equality_mixed_numeric_kinds_rejects (EQ and NEQ reject, matched kinds still work). 412 tests green, conformance suite still 2501/0.