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

Đang mở
  • Có gì không ổn?
#16 8 bình luận 0 reaction 1 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

@KunmiWrites-bit đang làm issue này rồi.

Từ ngày 30/8/2026.

  • #235 của @KunmiWrites-bit — đang mở

Đánh giá

Issue này chưa được đánh giá.

Mô tả

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.

Ngôn ngữ chính
Rust
Star
1
Fork
102
Merge trung bình
4 giờ 12 phút
Pull request đã merge (30 ngày)
30

Chuẩn bị môi trường

  • Không có Dockerfile hay tệp Docker Compose
  • Có mẫu pull request
  • Không có hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của MergeFi/contracts

Tất cả issue của MergeFi/contracts

Issue tương tự

Thêm issue về Rust

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.