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

オープン
  • 何か問題がありますか？
#16 コメント 8 件 リアクション 0 件 担当者 1 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

@KunmiWrites-bit がすでに取り組んでいます。

2026年8月30日 から。

  • #235 @KunmiWrites-bit による — オープン

評価

この issue はまだ評価されていません。

説明

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.

主要言語
Rust
スター
1
フォーク
102
平均マージ
4時間 12分
マージ済み PR（30日）
30

環境構築

  • Dockerfile・Docker Compose ファイルなし
  • プルリクエストのテンプレートあり
  • コントリビューションガイドなし

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

MergeFi/contracts のほかの issue

MergeFi/contracts の issue をすべて見る

似ている issue

Rust の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。