Extract `compute_split` into a shared crate with proof of behavioral equivalence

Abierto
  • ¿Algo no está bien?
#16 8 comentarios 0 reacciones 1 asignado Ver en GitHub

Los mantenedores suelen responder en 1 día

@KunmiWrites-bit ya está trabajando en esto.

Desde el 30/8/2026.

  • #235 de @KunmiWrites-bit — abierto

Evaluación

Este issue todavía no se ha evaluado.

Descripción

architecture enhancement GrantFox OSS Maybe Rewarded Official Campaign | FWC26 Stellar Wave testing very hard

Background / Context

compute_split is implemented twice, nearly identically, once in contracts/escrow/src/lib.rs:211-254 (pub(crate), returning crate::error::Error) and once in contracts/milestones/src/lib.rs:234-275 (private, returning its own Error type). The README's "Why three contracts instead of one" section explicitly acknowledges this duplication as a known tradeoff and names the fix in its Roadmap: "Extract shared split/fee math (compute_split) into a common non-contract Rust crate to remove the duplication... imported as a normal (non-contract) Rust dependency by each contract crate."

Problem Statement

Beyond the maintenance-burden argument the README already makes, duplicated financial-math logic is a genuine correctness risk: any fix to one copy (e.g., from the dust-fairness issue or overflow-checked-arithmetic issue tracked elsewhere in this bounty set) must be manually and perfectly replicated in the other copy, and the two implementations could silently drift — different rounding behavior, different error conditions — without either contract's own test suite catching it, since each only tests against itself. This task is to actually perform the extraction the README calls for, correctly, as a genuine "non-contract Rust dependency" (a Soroban contract can depend on ordinary Rust crates for logic as long as the shared crate itself has no #[contract] — this needs careful handling of #![no_std], soroban_sdk types like Address/Vec/Env, and each contract's distinct Error enum).

Requirements

  • Create a new workspace member (e.g., common/mergefi-split or similar, added to Cargo.toml's [workspace] members) containing the shared split/fee computation logic, generic over the caller's error type (e.g., via a trait or a shared Error enum both contract-specific errors can convert into/from) so both escrow and milestones can use it without forcing identical #[contracterror] enums (which would be a larger, riskier breaking change to each contract's public error API).
  • Migrate both contracts/escrow/src/lib.rs and contracts/milestones/src/lib.rs to call the shared implementation, removing the duplicated compute_split/Payouts definitions.
  • Write a differential test (in the shared crate or a new integration-test crate) that runs both the pre-refactor and post-refactor logic (or, if the pre-refactor code is deleted, a saved golden-output fixture generated before deletion) against a large randomized/property-based input set to prove zero behavioral drift was introduced by the refactor itself, independent of any of the other fairness/overflow fixes that might also be landing around the same time.
  • Update Cargo.toml workspace members and the README's "Why three contracts instead of one" section to reflect the resolved Roadmap item.

Acceptance Criteria

  • New shared crate added to the workspace, #![no_std], no #[contract] attribute (confirmed it does not itself become a deployable contract)
  • Both escrow and milestones contracts consume the shared crate, duplicated code removed
  • Differential/golden test proving behavioral equivalence pre/post refactor
  • cargo test --workspace and make build (wasm) both green for all three contracts
  • README updated, Roadmap item resolved

Technical Notes / Hints

  • Each contract currently returns its own Error::InvalidSplit/Error::NotInitialized — decide whether the shared crate returns a generic error the caller maps, or takes the fee lookup as a parameter rather than reading DataKey::FeeBps itself (since DataKey also differs per contract) — this is a real API design decision, not a copy-paste.
  • Watch wasm binary size impact (opt-level = "z", lto = true are already set) — verify the extraction doesn't regress compiled wasm size meaningfully.

Difficulty Justification

Despite sounding like simple deduplication, this requires nontrivial Rust API design to share logic across two crates with genuinely different Error types and DataKey storage schemas without forcing an awkward common error/storage abstraction, careful no_std-compatible crate structuring within a Soroban workspace, and rigorous differential testing to prove the refactor is behavior-preserving rather than trusting it by inspection.

Lenguaje dominante
Rust
Estrellas
1
Forks
102
Merge medio
4 h 12 min
PR fusionados (30 d)
30

Preparar el entorno

  • Sin Dockerfile ni archivo de Docker Compose
  • Tiene una plantilla de pull request
  • Sin guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de MergeFi/contracts

Todos los issues de MergeFi/contracts

Issues similares

Más issues de Rust

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.