Skip to content

Feat/init defindex - #73

Merged
armandocodecr merged 2 commits into
Trustless-Work:defindex-escrowfrom
aguilar1x:feat/init-defindex
Sep 16, 2025
Merged

armandocodecr merged 2 commits into
Trustless-Work:defindex-escrowfrom
aguilar1x:feat/init-defindex

Conversation

@aguilar1x

@aguilar1x aguilar1x commented Sep 16, 2025 •

Copy link
Copy Markdown
Contributor

CLR-S (2)

Pull Request | Trustless Work

1. Issue Link

  • Closes #(issue number)

2. Brief Description of the Issue


3. Changes Made

  • Change 1
  • Change 2
  • Change 3

4. Evidence After Solution

Loom Video - After Solution


5. Important Notes

  • Note 1
  • Note 2
  • Note 3

If you don't use this template, you'd be ignored

Summary by CodeRabbit

  • New Features
    • Added vault integration to enable direct deposits from escrow into a configured vault.
    • Introduced vault address management, including setting, retrieving, and using a stored address.
    • Exposed a way to transfer escrow-held token balances into the vault.
    • Provided access to the vault’s share token address.
    • Expanded authorization so a designated vault operator can manage dispute status.
  • Tests
    • Temporarily removed existing escrow contract tests.

@coderabbitai

coderabbitai Bot commented Sep 16, 2025 •

Copy link
Copy Markdown

Caution

Review failed

Failed to post review comments.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d4669d and cd4a664.

📒 Files selected for processing (5)
  • contracts/escrow/src/contract.rs (2 hunks)
  • contracts/escrow/src/core/escrow.rs (3 hunks)
  • contracts/escrow/src/core/validators/dispute.rs (1 hunks)
  • contracts/escrow/src/storage/types.rs (2 hunks)
  • contracts/escrow/src/tests/test.rs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
contracts/escrow/src/contract.rs (1)
contracts/escrow/src/core/escrow.rs (4)
  • send_to_vault (137-161)
  • e (127-127)
  • e (159-159)
  • get_escrow (130-135)
contracts/escrow/src/core/escrow.rs (1)
contracts/escrow/src/contract.rs (2)
  • send_to_vault (168-170)
  • get_escrow (96-98)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build

Walkthrough

Introduces vault integration to the escrow contract: new public endpoints for vault deposits, address management, and a stored-address convenience; a manager method to send escrow-held tokens to a vault via cross-contract call; expanded roles and storage keys to include a vault operator and vault address; updates dispute authorization; removes tests.

Changes

Cohort / File(s) Summary of Changes
Escrow contract API: vault endpoints
contracts/escrow/src/contract.rs
Added public methods: send_to_vault, deposit_via_vault, set_vault_address, get_vault_address, get_vault_share_token_address, deposit_via_vault_stored. Imported IntoVal. Implements operator/platform auth checks, vault address storage, and event emissions.
Core manager: vault sending
contracts/escrow/src/core/escrow.rs
Added EscrowManager::send_to_vault: reads escrow/token balance, prepares vectors/args, and performs cross-contract deposit call to vault. Expanded imports for cross-contract calls and conversions.
Validation: dispute role expansion
contracts/escrow/src/core/validators/dispute.rs
Extended authorization to include vault_operator for dispute flag changes. No signature changes.
Storage types: roles and keys
contracts/escrow/src/storage/types.rs
Added vault_operator to Roles. Added DataKey::VaultAddr for storing vault address.
Tests
contracts/escrow/src/tests/test.rs
Replaced entire test suite with a placeholder comment, removing previous tests.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Operator
  participant EscrowContract
  participant EscrowManager
  participant Token as TokenClient
  participant Vault as VaultContract

  Operator->>EscrowContract: deposit_via_vault(operator, user, vault_addr, amounts_desired, amounts_min, invest)
  note right of EscrowContract: Auth check: operator == roles.vault_operator<br/>User & operator auth
  EscrowContract->>Vault: deposit(user, amounts_desired, amounts_min, invest)
  Vault-->>EscrowContract: result

  Operator->>EscrowContract: send_to_vault()
  EscrowContract->>EscrowManager: send_to_vault(env)
  EscrowManager->>Token: balance(escrow_trustline)
  Token-->>EscrowManager: balance
  EscrowManager->>Vault: deposit(escrow_addr, [balance], [0], invest=false)
  Vault-->>EscrowManager: result
Loading
sequenceDiagram
  autonumber
  actor Platform
  participant EscrowContract
  participant Storage

  Platform->>EscrowContract: set_vault_address(platform_addr, vault_addr)
  note right of EscrowContract: Auth check: platform_addr == roles.platform_address
  EscrowContract->>Storage: write(DataKey::VaultAddr, vault_addr)
  Storage-->>EscrowContract: ok

  Operator->>EscrowContract: deposit_via_vault_stored(operator, user, amounts_desired, amounts_min, invest)
  EscrowContract->>Storage: read(DataKey::VaultAddr)
  Storage-->>EscrowContract: vault_addr
  EscrowContract->>EscrowContract: deposit_via_vault(..., vault_addr, ...)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

I thump my paws—new vaults appear,
With keys and roles now crystal-clear.
We hop some tokens, swift and spry,
Cross-contract calls go vault-ward, fly!
Tests now silent, fields enhanced—
A rabbit’s ledger, neatly balanced. 🐇💼

Pre-merge checks and finishing touches and finishing touches

❌ Failed checks (3 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title "Feat/init defindex" is vague and does not describe the primary changes in this PR; the changeset primarily adds vault integration to the escrow contract (new methods like send_to_vault and deposit_via_vault), introduces a vault_operator role and VaultAddr storage, and removes the test suite. Because the title neither references the escrow/vault work nor summarizes the main change, it will mislead reviewers and make history harder to scan. Update the title to clearly reflect the main functional change. Rename the PR to a concise, specific title such as "feat(escrow): add vault integration and send_to_vault" or "feat(escrow): add vault_operator, vault address management, and deposit_via_vault" so reviewers immediately understand the primary change; avoid branch names or vague words.
Description Check ⚠️ Warning The PR body uses the repository template but remains mostly placeholder text: the issue link is "Closes #(issue number)", the brief description is empty, "Changes Made" lists "Change 1/2/3" rather than concrete modifications, the Loom link is a placeholder, and Important Notes are generic placeholders. The provided raw_summary shows substantive code changes (vault integration, role/storage additions, and deletion of tests) that are not documented in the description, so reviewers lack necessary context. Populate the template with concrete information: add the related issue number or remove the placeholder, write a concise problem statement and summary of intent, replace "Change 1/2/3" with the actual file-level changes and a short description of each (e.g., added send_to_vault, deposit_via_vault, vault_operator role, VaultAddr storage; tests removed), include test evidence or a working Loom/video link and any migration/compatibility notes, and restore or add tests covering the new functionality before merging.
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@aguilar1x
aguilar1x changed the base branch from develop to defindex-escrow September 16, 2025 09:26
@armandocodecr armandocodecr reopened this Sep 16, 2025
@armandocodecr
armandocodecr merged commit a6f3fef into Trustless-Work:defindex-escrow Sep 16, 2025
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants