Skip to content

Conversation

@0xshitake
Copy link
Contributor

@0xshitake 0xshitake commented Oct 15, 2025

transactions and disputeIDtoTransactionID are state variables, and the compilation output of IEscrow.sol doesn’t include the default getters for public variables, so I’m adding two functions to this interface.

This is useful for anyone using frontend SDKs like wagmi, which generate TS types based on the ABI file.


PR-Codex overview

This PR focuses on enhancing the IEscrow interface in the contracts/src/interfaces/IEscrow.sol file by adding detailed transaction retrieval and dispute mapping functionalities, and it updates related imports in the EscrowUniversal and EscrowView contracts.

Detailed summary

  • Added import of NATIVE, Status, Party, Transaction, and Resolution from ./interfaces/Types.sol in EscrowUniversal.sol.
  • Updated imports in EscrowView.sol to separate EscrowUniversal and other types.
  • Updated import in IEscrow.sol to include IERC20, Party, Status, Resolution, and Transaction from ./Types.sol.
  • Added transactions function in IEscrow for detailed transaction information retrieval.
  • Added disputeIDtoTransactionID function in IEscrow to map dispute IDs to transaction IDs.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Summary by CodeRabbit

  • New Features

    • Added read-only accessors to fetch full transaction details by ID and to map dispute IDs to transaction IDs, improving visibility into escrow state.
  • Refactor

    • Reorganized internal type imports to centralize shared types and streamline references without changing behavior.
  • Style

    • Minor formatting adjustments to tuple unpacking for clarity; no functional changes.

@netlify
Copy link

netlify bot commented Oct 15, 2025

Deploy Preview for kleros-escrow-v2 ready!

Name Link
🔨 Latest commit f8354ba
🔍 Latest deploy log https://app.netlify.com/projects/kleros-escrow-v2/deploys/68f7a441d78658000835b319
😎 Deploy Preview https://deploy-preview-128--kleros-escrow-v2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 15, 2025

Walkthrough

This change centralizes type imports to ./interfaces/Types.sol, reformats a local tuple destructuring in EscrowView without changing behavior, and expands the IEscrow interface with two new view getters (transactions and disputeIDtoTransactionID). No business logic alterations in EscrowUniversal or EscrowView.

Changes

Cohort / File(s) Summary of changes
Imports consolidation
contracts/src/EscrowUniversal.sol, contracts/src/EscrowView.sol, contracts/src/interfaces/IEscrow.sol
Centralizes and reorganizes imports to ./interfaces/Types.sol (NATIVE, Status, Party, Transaction, Resolution, IERC20). EscrowView now imports EscrowUniversal from its local path.
View tuple destructuring style
contracts/src/EscrowView.sol
Reformats tuple unpacking in getPayoutMessages to use explicit leading commas/aligned destructuring; semantics and downstream usage unchanged.
Interface expansion (new getters)
contracts/src/interfaces/IEscrow.sol
Adds view getters: transactions(uint256) returning detailed transaction fields and disputeIDtoTransactionID(uint256) returning the mapped transaction ID. No removals or other signature changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor DApp as DApp/Client
  participant IEscrow as IEscrow (contract)

  rect rgb(240,248,255)
    note over DApp,IEscrow: Query transaction details
    DApp->>IEscrow: transactions(transactionID)
    IEscrow-->>DApp: (buyer, seller, amounts, deadlines, disputeID, fees, timestamp, status, token)
  end

  rect rgb(245,255,250)
    note over DApp,IEscrow: Resolve dispute -> transaction
    DApp->>IEscrow: disputeIDtoTransactionID(disputeID)
    IEscrow-->>DApp: transactionID
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

Type: Feature🗿, Package: Contracts

Suggested reviewers

  • kemuru

Poem

I twitch my whiskers at tidy imports,
Two getters hop in with measured starts. 🥕
Tuples reformed, no logic shaken,
Small changes made, no code forsaken.
— a rabbit’s nod to neat refactors.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "feat: add getters to interface" accurately reflects the primary objective stated in the PR description: adding explicit getter functions to the IEscrow.sol interface to support frontend SDKs. The changeset's main substantive changes are the addition of two new getter functions—transactions(uint256) and disputeIDtoTransactionID(uint256)—to the interface, with supporting import reorganizations in related files. The title is concise, specific, and clearly communicates the core change without vague language or misleading framing.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18dae17 and f8354ba.

📒 Files selected for processing (1)
  • contracts/src/EscrowView.sol (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • contracts/src/EscrowView.sol
⏰ 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). (6)
  • GitHub Check: Redirect rules - kleros-escrow-v2
  • GitHub Check: Redirect rules - kleros-escrow-v2
  • GitHub Check: Header rules - kleros-escrow-v2
  • GitHub Check: Header rules - kleros-escrow-v2
  • GitHub Check: Pages changed - kleros-escrow-v2
  • GitHub Check: Pages changed - kleros-escrow-v2

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
contracts/src/interfaces/IEscrow.sol (1)

160-195: Transaction struct order matches IEscrow.transactions() getter

  • Definition in contracts/src/interfaces/Types.sol aligns exactly with the interface return values.
  • Optional: add a comment by the Transaction struct reminding maintainers to update this interface if fields change.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7dd88a4 and 18dae17.

📒 Files selected for processing (3)
  • contracts/src/EscrowUniversal.sol (1 hunks)
  • contracts/src/EscrowView.sol (2 hunks)
  • contracts/src/interfaces/IEscrow.sol (2 hunks)
🔇 Additional comments (4)
contracts/src/interfaces/IEscrow.sol (1)

5-5: LGTM - Import reorganization improves code clarity.

Using named imports from Types.sol makes the dependencies explicit and improves code readability.

contracts/src/EscrowUniversal.sol (1)

13-13: LGTM - Import consolidation aligns with interface changes.

The named imports from Types.sol maintain consistency with the interface definition and make type dependencies explicit.

contracts/src/EscrowView.sol (2)

7-8: LGTM - Import organization aligns with codebase changes.

The reorganized imports maintain consistency with the changes in IEscrow.sol and EscrowUniversal.sol, ensuring all files use the centralized type definitions from Types.sol.


30-43: LGTM - Tuple destructuring reformatted without semantic changes.

The reformatting uses explicit leading comma-destructuring for skipped values, which is a common Solidity pattern. The semantic meaning remains unchanged—positions 3, 8, 9, and 12 still correctly extract amount, buyerFee, sellerFee, and token from the transactions getter.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 15, 2025
@kemuru kemuru self-requested a review October 15, 2025 17:45
kemuru
kemuru previously approved these changes Oct 15, 2025
@jaybuidl jaybuidl dismissed stale reviews from kemuru and coderabbitai[bot] via f8354ba October 21, 2025 15:18
@unknownunknown1 unknownunknown1 marked this pull request as ready for review October 21, 2025 15:18
@jaybuidl jaybuidl merged commit c382dc8 into kleros:dev Oct 21, 2025
5 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.

3 participants