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
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.
Background / Context
compute_splitis implemented twice, nearly identically, once incontracts/escrow/src/lib.rs:211-254(pub(crate), returningcrate::error::Error) and once incontracts/milestones/src/lib.rs:234-275(private, returning its ownErrortype). 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_sdktypes likeAddress/Vec/Env, and each contract's distinctErrorenum).Requirements
common/mergefi-splitor similar, added toCargo.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 sharedErrorenum both contract-specific errors can convert into/from) so bothescrowandmilestonescan use it without forcing identical#[contracterror]enums (which would be a larger, riskier breaking change to each contract's public error API).contracts/escrow/src/lib.rsandcontracts/milestones/src/lib.rsto call the shared implementation, removing the duplicatedcompute_split/Payoutsdefinitions.Cargo.tomlworkspace members and the README's "Why three contracts instead of one" section to reflect the resolved Roadmap item.Acceptance Criteria
#![no_std], no#[contract]attribute (confirmed it does not itself become a deployable contract)escrowandmilestonescontracts consume the shared crate, duplicated code removedcargo test --workspaceandmake build(wasm) both green for all three contractsTechnical Notes / Hints
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 readingDataKey::FeeBpsitself (sinceDataKeyalso differs per contract) — this is a real API design decision, not a copy-paste.opt-level = "z",lto = trueare 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
Errortypes andDataKeystorage schemas without forcing an awkward common error/storage abstraction, carefulno_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.