Add m009 Service Provision Escrow mechanism spec#21
Add m009 Service Provision Escrow mechanism spec#21CShear wants to merge 2 commits intoregen-network:mainfrom
Conversation
Adds full mechanism spec for m009 following the m010 pattern: - SPEC.md with 13 sections covering milestone-based escrow, dispute resolution via Arbiter DAO, cancellation guards, and agent milestone review scoring - JSON schemas for agreement lifecycle, milestone review score, and KPI metrics - Reference implementation (m009_score.js, m009_kpi.js) with deterministic self-test - Test vectors and replay dataset fixtures including dispute scenarios (CLIENT_WINS, PROVIDER_WINS, SPLIT) - 4-factor scoring: deliverable quality (0.40), evidence completeness (0.25), milestone consistency (0.20), provider reputation (0.15) - Updates MECHANISM_CONSUMERS.md with m009 consumer mappings Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello @CShear, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new core mechanism, m009 Service Provision Escrow, designed to facilitate trustless, milestone-based service agreements within the ecosystem. It provides a robust framework for managing service engagements from proposal to completion, including agent-assisted deliverable review, defined token flows, and a structured dispute resolution process. The addition aims to enhance transparency and reliability for both clients and providers of ecosystem services. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive specification for the m009 Service Provision Escrow mechanism, including detailed documentation, JSON schemas, and a reference implementation. However, a critical security vulnerability has been identified: the milestone review scoring's confidence calculation incorrectly treats missing or undefined signals as available, potentially leading to inflated scores and incorrect approval recommendations for escrowed funds. Additionally, there are critical issues within the specification's token flow logic for dispute resolution and inconsistencies between the specification, reference implementation, and test data regarding state transitions, data type definitions, and KPI calculations. Several comments have been enhanced with references to established repository rules regarding transition logic, state definitions, and data structure alignment.
| | Outcome | Client receives | Provider receives | Community pool | | ||
| |---------|----------------|-------------------|----------------| | ||
| | **CLIENT_WINS** | remaining escrow − arbiter_fee | 0 (bond slashed: 50% client, 50% pool) | 50% of bond + arbiter_fee | | ||
| | **PROVIDER_WINS** | 0 | remaining escrow + bond − arbiter_fee | arbiter_fee | |
There was a problem hiding this comment.
The token flow for a PROVIDER_WINS dispute outcome appears to be incorrect and could lead to significant fund mismanagement. It states the provider receives remaining escrow + bond - arbiter_fee.
The term "remaining escrow" is ambiguous. If it refers to the entire remaining balance in the escrow contract (including funds for future, uncompleted milestones), this is a critical flaw. A provider winning a dispute on a single milestone should only be entitled to that milestone's payment, not the entire remaining contract value.
Please clarify that the payment is limited to the disputed milestone's amount. The disposition of funds for future milestones and the provider bond should also be specified based on whether the agreement continues or is terminated.
References
- When specifying transition logic between different mechanisms (e.g., multipliers), explicitly define the function (e.g.,
max()) and the state-based conditions (phase-gating) that govern the transition to prevent implementation ambiguity.
| |---------|----------------|-------------------|----------------| | ||
| | **CLIENT_WINS** | remaining escrow − arbiter_fee | 0 (bond slashed: 50% client, 50% pool) | 50% of bond + arbiter_fee | | ||
| | **PROVIDER_WINS** | 0 | remaining escrow + bond − arbiter_fee | arbiter_fee | | ||
| | **SPLIT(X%)** | X% of remaining − arbiter_fee share | (100−X)% of remaining + bond − arbiter_fee share | arbiter_fee | |
There was a problem hiding this comment.
Similar to the PROVIDER_WINS case, the token flow for a SPLIT(X%) resolution is ambiguous and potentially incorrect. It refers to splitting the "remaining" escrow, which is problematic.
The split should apply to the value of the disputed milestone, not the entire remaining escrow for the contract. Please clarify the specification to prevent incorrect implementation where a split on one milestone could incorrectly distribute funds for future, un-started work.
References
- When specifying transition logic between different mechanisms (e.g., multipliers), explicitly define the function (e.g.,
max()) and the state-based conditions (phase-gating) that govern the transition to prevent implementation ambiguity.
| "review_score": null | ||
| } | ||
| ], | ||
| "status": "RESOLVED", |
There was a problem hiding this comment.
The status of agreement agr-d03 is RESOLVED, which appears to be inconsistent with the state machine defined in SPEC.md.
According to the spec (section 6), a RESOLVED state where the provider wins (PROVIDER_WINS) and there are remaining milestones should transition to IN_PROGRESS. Since this agreement has a pending milestone, its status should likely be IN_PROGRESS rather than RESOLVED, which is a transient state.
Please update the test data to align with the specified state machine transitions.
References
- To improve document clarity, co-locate all state definitions and transitions within the primary state machine documentation, including definitions for edge-case flows like escalations.
| const cancelled_funded = agrs.filter(a => a.status === "CANCELLED" && a.escrow_amount) | ||
| .map(a => parseInt(a.escrow_amount?.amount ?? "0", 10)); | ||
| const completion_fees = Math.round(total_released * 0.01); | ||
| const cancellation_fees = Math.round(cancelled_funded.reduce((s, e) => s + e, 0) * 0.02); | ||
| const total_fees = completion_fees + cancellation_fees; |
There was a problem hiding this comment.
The logic for calculating cancellation fees is based on a flawed assumption. It identifies agreements for fee calculation using a.status === "CANCELLED" && a.escrow_amount. However, the specification allows for fee-free cancellations from the PROPOSED state, and the sample data (v0_sample.json, agr-005) shows that PROPOSED agreements can have an escrow_amount.
This implementation cannot distinguish between a fee-incurring cancellation from FUNDED and a fee-free cancellation from PROPOSED, which will lead to incorrect KPI calculations.
The underlying data model should be enhanced to provide the necessary state history to make this distinction. Without changes to the data model, this calculation will remain inaccurate.
References
- When specifying transition logic between different mechanisms (e.g., multipliers), explicitly define the function (e.g.,
max()) and the state-based conditions (phase-gating) that govern the transition to prevent implementation ambiguity.
| if (factors.iri_resolvable !== false) available++; | ||
| if (factors.has_prior_milestones) available++; | ||
| if (factors.spec_available !== false) available++; |
There was a problem hiding this comment.
The computeConfidence function contains a logic flaw where iri_resolvable and spec_available are checked using !== false. This means that if these properties are missing or undefined in the factors object, they will be incorrectly counted as 'available'. This vulnerability can lead to an inflated confidence score and potentially result in an APPROVE recommendation for a milestone lacking critical verification data, leading to the improper release of escrowed funds. It is recommended to use explicit true checks for these factors to ensure consistent and secure signal availability assessment.
if (factors.iri_resolvable === true) available++;
if (factors.has_prior_milestones) available++;
if (factors.spec_available === true) available++;| | Condition | Recommendation | | ||
| |-----------|---------------| | ||
| | `score >= 700 AND confidence >= 750` | APPROVE | | ||
| | `score >= 400 AND score < 700` | NEEDS_REVISION | | ||
| | `score < 400 OR confidence < 250` | FLAG_FOR_CLIENT | |
There was a problem hiding this comment.
The recommendation logic defined in this table is incomplete. There is a gap in the conditions. For example, a case where score = 800 and confidence = 500 does not match any of the specified conditions, leading to an undefined recommendation.
The reference implementation (m009_score.js) handles this by defaulting to NEEDS_REVISION, which is a reasonable approach. The specification should be updated to reflect this behavior for clarity and completeness. I suggest restructuring the conditions to be exhaustive, for example:
| Condition | Recommendation |
|---|---|
score >= 700 AND confidence >= 750 |
APPROVE |
score < 400 OR confidence < 250 |
FLAG_FOR_CLIENT |
| Otherwise | NEEDS_REVISION |
References
- When specifying transition logic between different mechanisms (e.g., multipliers), explicitly define the function (e.g.,
max()) and the state-based conditions (phase-gating) that govern the transition to prevent implementation ambiguity.
| "enum": ["PENDING", "IN_PROGRESS", "SUBMITTED", "APPROVED", "DISPUTED", "REVISED"] | ||
| }, | ||
| "deliverable_iri": { "type": ["string", "null"] }, | ||
| "review_score": { "type": ["number", "null"] } |
There was a problem hiding this comment.
The schema defines review_score as type number, which allows for floating-point values. However, the SPEC.md and the reference implementation both define and calculate the score as an integer (via Math.round).
To maintain consistency and enforce the correct data type, please change the type to integer.
| "review_score": { "type": ["number", "null"] } | |
| "review_score": { "type": ["integer", "null"] } |
References
- When defining data structures or type definitions, ensure they are aligned with the project's central data standards repository (
regen-data-standards) to maintain coherence.
…fixture state, KPI fees - computeConfidence: use explicit === true checks to prevent undefined being treated as available (security fix) - SPEC.md dispute resolutions: clarify "remaining" means disputed milestone amount, not entire contract escrow - SPEC.md recommendation table: make conditions exhaustive with "Otherwise → NEEDS_REVISION" fallthrough - v0_dispute_sample.json: agr-d03 status RESOLVED → IN_PROGRESS (provider wins with remaining milestones continues agreement) - m009_kpi.js: respect cancelled_from field to distinguish PROPOSED vs FUNDED cancellations for fee calculation - schema.json: review_score type number → integer (matches Math.round) Fixes Gemini review on PR regen-network#21. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
Test plan
node mechanisms/m009-service-escrow/reference-impl/m009_score.js— self-test PASS (5 test vectors)npm run verify— PASSnpm run check:index— mechanism index up to date🤖 Generated with Claude Code