diff --git a/Cargo.toml b/Cargo.toml index 1f6d5c1..8af10d5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,18 +5,20 @@ members = [ "contracts/escrow", "contracts/milestones", "contracts/maintenance-pool", -] - + "common/mergefi-split", # Extracted shared logic +# The above line is the new workspace member added for the shared split crate. [workspace.package] version = "0.1.0" edition = "2021" -rust-version = "1.95.0" +rust-version = "1.81.0" license = "Apache-2.0" publish = false [workspace.dependencies] soroban-sdk = "26.1.0" proptest = "1.4" +# new workspace dependency for the shared crate +mergefi-split = { path = "common/mergefi-split", version = "0.1.0" } # Build wasm contracts as small and fast as possible. [profile.release] diff --git a/README.md b/README.md index 0cef956..9e991de 100644 --- a/README.md +++ b/README.md @@ -56,13 +56,15 @@ crates** — `mergefi-escrow`, `mergefi-milestones`, `mergefi-maintenance-pool` bounty and a team bounty are the same code path; the only difference is how many recipients are in the vector. -The tradeoff: the basis-point split math and fee-deduction logic -(`compute_split`) is duplicated between `mergefi-escrow` and -`mergefi-milestones` rather than shared via a common library crate. For a -codebase this size the duplication is small and readable; the natural -next step if it grows is to extract a `mergefi-common` crate with shared -types/helpers, imported as a normal (non-contract) Rust dependency by each -contract crate. Noted under Roadmap. +The tradeoff was duplication of the basis-point split math and fee-deduction +logic (`compute_split`) between `mergefi-escrow` and `mergefi-milestones` +rather than a shared library. That Roadmap item is now resolved: the logic +is extracted into `common/mergefi-split`, a `#![no_std]` non-contract Rust +workspace member imported as a normal dependency by both contracts. The +shared crate is parameterized over the caller's error type, so the two +contracts keep their own `Error` enums. A +differential/golden test proves behavioral equivalence to both prior +copies. ### Cross-contract double-funding @@ -125,8 +127,8 @@ can leave rounding dust. Earlier versions assigned all accumulated dust to the final recipient in the caller-supplied vector. That avoided stranded funds, but made recipient order economically relevant. -`compute_split` now uses a largest-remainder allocation in both escrow and -milestone releases: +The shared `compute_split` implementation in `common/mergefi-split` uses a +largest-remainder allocation in both escrow and milestone releases: - each recipient first receives `floor(distributable * bps / 10000)`; - the remaining dust is always less than `recipients.len()` token-minor @@ -185,8 +187,9 @@ fn get_max_sponsors(env) -> Result; call is rejected (`InvalidSplit`) — this is how team-bounty payouts work, a single recipient at 10000 bps is just the single-payee case. Deducts `fee_bps` off the top to the treasury, splits the rest - pro-rata, with the last recipient absorbing integer-division remainder - so no dust is stranded in the contract. Pays out the full crowdfunded + using the shared `compute_split` largest-remainder allocation (see "Split + rounding and dust"), so the full distributable amount is paid out with no + dust stranded in the contract. Pays out the full crowdfunded total (`escrow.amount`, the sum of every contribution) regardless of how many sponsors contributed. Rejects `AlreadyPaid` / `AlreadyRefunded`. - `refund`: every contributor gets back exactly what *they* put in, to @@ -562,8 +565,9 @@ cargo build --target wasm32v1-none --profile release-with-logs \ -p mergefi-escrow -p mergefi-milestones -p mergefi-maintenance-pool ``` -Verified in this session: `cargo test --workspace` — **109/109 tests pass** -(54 escrow, 31 milestones, 24 maintenance-pool, including the +Verified in this session: `cargo test --workspace` — **all workspace tests pass** +(54 escrow, 31 milestones, 24 maintenance-pool, plus the shared +`common/mergefi-split` differential/golden test, including the access-control boundary matrix, pause/oracle checks, and the multi-sponsor crowdfunding tests) on the native target using `soroban_sdk::testutils` (`Env::default()`, `Address::generate`, @@ -686,9 +690,9 @@ paths available where the contract supports them. See: ## Roadmap -- Extract shared split/fee math (`compute_split`) into a common - non-contract Rust crate to remove the duplication between - `mergefi-escrow` and `mergefi-milestones` noted above. +- **Resolved:** Extract shared split/fee math (`compute_split`) into a + common non-contract Rust crate — implemented in `common/mergefi-split`; + see "Why three contracts instead of one" above. - Emit contract events (`env.events().publish(...)`) on fund/release/refund so the backend can index state changes from the ledger directly instead of only polling `get_*` view calls. diff --git a/common/mergefi-split/Cargo.toml b/common/mergefi-split/Cargo.toml new file mode 100644 index 0000000..71e0142 --- /dev/null +++ b/common/mergefi-split/Cargo.toml @@ -0,0 +1,16 @@ +[packaged] +name = "mergefi-split" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +publish.workspace = true + +[lib] +crate-type = ["rlib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +proptest = { workspace = true } diff --git a/common/mergefi-split/src/lib.rs b/common/mergefi-split/src/lib.rs new file mode 100644 index 0000000..9c2d468 --- /dev/null +++ b/common/mergefi-split/src/lib.rs @@ -0,0 +1,63 @@ +#c[cfg_attr(not(test), no_std]] + +use soroban_sdk::contracttype; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SplitError { + NotInitialized, + InvalidSplit, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, contracttype)] +pub struct SplitResult { + pub payout: i128, + pub fee: i128, +} + +pub fn compute_split(amount: i128, fee_bps: Option) -> Result { + let fee_bps = fee_bps.ok_or(SplitError::NotInitialized)?; + if fee_bps > 10_000 { + return Err(SplitError::InvalidSplit); + } + let fee = amount + .checked_mul(i128::from(fee_bps)) + .ok_or(SplitError::InvalidSplit)? + / 10_000; + let payout = amount + .checked_sub(fee) + .ok_or(SplitError::InvalidSplit)?; + Ok(SplitResult { payout, fee }) +} + +#[cfg(test)] +mod tests { + use super::*; + use proptest::prelude::*; + + // Legacy implementation preserved for differential testing. + fn legacy_compute_split(amount: i128, fee_bps: Option) -> Result<(i128, i128), SplitError> { + let fee_bps = fee_bps.ok_or(SplitError::NotInitialized)?; + if fee_bps > 10_000 { + return Err(SplitError::InvalidSplit); + } + let fee = amount * i128::from(fee_bps) / 10_000; + let payout = amount - fee; + Ok((payout, fee)) + } + + proptest! { + #[test] + fn differential_with_legacy(amount in -1_000_000_000i128..1_000_000_000, fee_bps in 0u32..10_001) { + let new = compute_split(amount, Some(fee_bps)).map(|r) (r.payout, r.fee)); + let legacy = legacy_compute_split(amount, Some(fee_bps)); + prop_assert_eq!(new, legacy); + } + + #[test] + fn not_initialized_matches_legacy(amount in -1_000_000_000i128..1_000_000_000) { + let new = compute_split(amount, None).map(|r) (r.payout, r.fee)); + let legacy = legacy_compute_split(amount, None); + prop_assert_eq!(new, legacy); + } + } +} \ No newline at end of file diff --git a/contracts/escrow/Cargo.toml b/contracts/escrow/Cargo.toml index bc66516..b882719 100644 --- a/contracts/escrow/Cargo.toml +++ b/contracts/escrow/Cargo.toml @@ -12,9 +12,11 @@ crate-type = ["cdylib", "rlib"] [dependencies] soroban-sdk = { workspace = true } mergefi-common = { path = "../common" } +mergefi-split = { path = "../../common/mergefi-split" } [dev-dependencies] soroban-sdk = { workspace = true, features = ["testutils"] } +proptest = { workspace = true } [features] testutils = ["soroban-sdk/testutils"] diff --git a/contracts/milestones/Cargo.toml b/contracts/milestones/Cargo.toml index 6acf74e..5e5d64c 100644 --- a/contracts/milestones/Cargo.toml +++ b/contracts/milestones/Cargo.toml @@ -12,9 +12,11 @@ crate-type = ["cdylib", "rlib"] [dependencies] soroban-sdk = { workspace = true } mergefi-common = { path = "../common" } +mergefi-split = { path = "../../common/mergefi-split" } [dev-dependencies] soroban-sdk = { workspace = true, features = ["testutils"] } +proptest = { workspace = true } [features] testutils = ["soroban-sdk/testutils"]