chore: update mappings in create transaction flow #67
Conversation
WalkthroughThe recent modifications encompass significant updates to the escrow dispute handling, including the restructuring of dispute templates and data mappings, removal of certain fields from the transaction structure, and updates to various associated files. These changes aim to enhance the processing of transaction disputes by streamlining the data structure and improving the integration of dispute templates. Changes
Possibly related issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
✅ Deploy Preview for kleros-escrow-v2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (1)
120-137: Review and OptimizedataMappingsConfigurationThe
dataMappingsconfiguration includes GraphQL and IPFS fetch types, which are crucial for fetching and populating data in the escrow transaction. Ensure that:
- The GraphQL endpoint and IPFS URI are securely configured and not susceptible to injection attacks.
- The
populateandseekfields are correctly implemented to map the necessary data without redundancy or errors.Consider adding error handling for the data fetch operations to manage cases where the external data sources might not be available or return errors.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (2 hunks)
Additional context used
Learnings (1)
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (2)
User: kemuru PR: kleros/escrow-v2#60 File: web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx:180-180 Timestamp: 2024-06-11T17:11:32.906Z Learning: The `refetchAllowance` function call is necessary in the `DepositPaymentButton` component to ensure the frontend updates correctly after an approval action.User: kemuru PR: kleros/escrow-v2#60 File: web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx:63-64 Timestamp: 2024-06-11T17:14:13.327Z Learning: Errors in the `useContractRead` hook for ERC20 token allowance checks in the `DepositPaymentButton` component are managed by the `wrapWithToast()` function, which logs errors in a notification popup. Additionally, the quantity for the approval is pre-validated and stored in the context of the NewTransactionContext.
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (1)
120-137: Review and OptimizedataMappingsConfigurationThe
dataMappingsconfiguration includes GraphQL and IPFS fetch types, which are crucial for fetching and populating data in the escrow transaction. Ensure that:
- The GraphQL endpoint and IPFS URI are securely configured and not susceptible to injection attacks.
- The
populateandseekfields are correctly implemented to map the necessary data without redundancy or errors.Consider adding error handling for the data fetch operations to manage cases where the external data sources might not be available or return errors.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (2 hunks)
Additional context used
Learnings (1)
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (2)
User: kemuru PR: kleros/escrow-v2#60 File: web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx:180-180 Timestamp: 2024-06-11T17:11:32.906Z Learning: The `refetchAllowance` function call is necessary in the `DepositPaymentButton` component to ensure the frontend updates correctly after an approval action.User: kemuru PR: kleros/escrow-v2#60 File: web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx:63-64 Timestamp: 2024-06-11T17:14:13.327Z Learning: Errors in the `useContractRead` hook for ERC20 token allowance checks in the `DepositPaymentButton` component are managed by the `wrapWithToast()` function, which logs errors in a notification popup. Additionally, the quantity for the approval is pre-validated and stored in the context of the NewTransactionContext.
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- contracts/deploy/00-escrow.ts (4 hunks)
- web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (2 hunks)
Files not reviewed due to errors (1)
- web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (no review received)
Files skipped from review due to trivial changes (1)
- contracts/deploy/00-escrow.ts
Additional context used
Learnings (1)
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (2)
User: kemuru PR: kleros/escrow-v2#60 File: web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx:180-180 Timestamp: 2024-06-11T17:11:32.906Z Learning: The `refetchAllowance` function call is necessary in the `DepositPaymentButton` component to ensure the frontend updates correctly after an approval action.User: kemuru PR: kleros/escrow-v2#60 File: web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx:63-64 Timestamp: 2024-06-11T17:14:13.327Z Learning: Errors in the `useContractRead` hook for ERC20 token allowance checks in the `DepositPaymentButton` component are managed by the `wrapWithToast()` function, which logs errors in a notification popup. Additionally, the quantity for the approval is pre-validated and stored in the context of the NewTransactionContext.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
contracts/src/EscrowUniversal.sol (3)
Line range hint
56-71: Review of Constructor Initialization and Event EmissionThe constructor correctly initializes contract state variables and sets the initial dispute template. However, the event
ParameterUpdatedis emitted with potentially uninitialized values. Consider ensuring all parameters are initialized before emitting this event to provide accurate and meaningful logs.+ emit ParameterUpdated(_feeTimeout, _settlementTimeout, _arbitratorExtraData); - emit ParameterUpdated(feeTimeout, settlementTimeout, arbitratorExtraData);
Line range hint
156-178: Review of Native Currency Transaction Creation:createNativeTransactionThis method is well-implemented with clear logic for creating a native transaction. It correctly handles the initialization and emits relevant events. However, consider checking for reentrancy attacks as native currency transfers are involved.
Consider using the nonReentrant modifier from OpenZeppelin's contracts to guard against reentrancy vulnerabilities.
Line range hint
270-298: Critical Review of Transaction Execution Logic:executeTransactionThe method
executeTransactioncorrectly checks transaction deadlines and dispute status before proceeding. This is crucial for avoiding execution in invalid states. However, the direct use of.sendfor ether transfers is risky and might lead to unexpected failures.Consider using
.transferfor safer ETH transfers or better yet, thecallmethod with proper checks as recommended by recent Solidity patterns to prevent issues related to gas stipends.- transaction.seller.send(amount); + (bool sent, ) = transaction.seller.call{value: amount}(""); + require(sent, "Failed to send Ether");
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- contracts/README.md (1 hunks)
- contracts/src/EscrowUniversal.sol (2 hunks)
- contracts/src/interfaces/Types.sol (1 hunks)
- subgraph/mappings/escrow.ts (1 hunks)
- subgraph/package.json (1 hunks)
- subgraph/schema.graphql (1 hunks)
- subgraph/subgraph.yaml (1 hunks)
- web/src/hooks/queries/useTransactionsQuery.ts (1 hunks)
- web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (2 hunks)
Files not reviewed due to errors (1)
- subgraph/mappings/escrow.ts (no review received)
Files skipped from review due to trivial changes (3)
- subgraph/package.json
- subgraph/schema.graphql
- web/src/hooks/queries/useTransactionsQuery.ts
Additional context used
Learnings (1)
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (2)
User: kemuru PR: kleros/escrow-v2#60 File: web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx:180-180 Timestamp: 2024-06-11T17:11:32.906Z Learning: The `refetchAllowance` function call is necessary in the `DepositPaymentButton` component to ensure the frontend updates correctly after an approval action.User: kemuru PR: kleros/escrow-v2#60 File: web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx:63-64 Timestamp: 2024-06-11T17:14:13.327Z Learning: Errors in the `useContractRead` hook for ERC20 token allowance checks in the `DepositPaymentButton` component are managed by the `wrapWithToast()` function, which logs errors in a notification popup. Additionally, the quantity for the approval is pre-validated and stored in the context of the NewTransactionContext.
LanguageTool
contracts/README.md
[uncategorized] ~69-~69: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ... env vars are required: -PRIVATE_KEY: the private key of the deployer account...
[uncategorized] ~70-~70: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...or the testnets. -MAINNET_PRIVATE_KEY: the private key of the deployer account...
[uncategorized] ~71-~71: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...unt used for Mainnet. -INFURA_API_KEY: the API key for infura. The ones below...
[uncategorized] ~75-~75: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...low are optional: -ETHERSCAN_API_KEY: to verify the source of the newly deplo...
[uncategorized] ~76-~76: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...s on Etherscan. -ARBISCAN_API_KEY: to verify the source of the newly deplo...
[uncategorized] ~77-~77: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ... on Arbitrum. -GNOSISSCAN_API_KEY: to verify the source of the newly deplo...
[uncategorized] ~81-~81: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE_2)
Context: ...ed for the kleros-v2 contracts currently so this would fail. For now consider deplo...
[typographical] ~81-~81: Consider adding a comma here. (FOR_NOW_COMMA)
Context: ...contracts currently so this would fail. For now consider deploying to a testnet fork (n...
[grammar] ~89-~89: The word ‘deploy’ is a verb. Did you mean the noun “deployment” (= release, placement)? (PREPOSITION_VERB)
Context: ...node --tags nothing**Shell 2: the deploy script**bash yarn deploy --network...
[grammar] ~105-~105: The word ‘deploy’ is a verb. Did you mean the noun “deployment” (= release, placement)? (PREPOSITION_VERB)
Context: ... start-testnet-fork**Shell 2: the deploy script**bash yarn deploy-devnet-fo...
Markdownlint
contracts/README.md
12-12: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
15-15: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
9-9: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
10-10: Expected: 1; Actual: 0; Above (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
16-16: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
17-17: Expected: 1; Actual: 0; Above (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
83-83: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
89-89: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
97-97: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
105-105: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
127-127: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
133-133: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
Additional comments not posted (1)
contracts/src/EscrowUniversal.sol (1)
Line range hint
74-76: Governance Method Review:changeGovernorThe implementation of
changeGovernoris secure and follows best practices by using theonlyByGovernormodifier to restrict access.
PR-Codex overview
The PR updates the Escrow Universal contracts and subgraph for Arbitrum Sepolia.
Detailed summary
templateDataandtemplateDataMappingsfrom contractsSummary by CodeRabbit
New Features
Bug Fixes
EscrowUniversalcontract in the Arbitrum Sepolia section.Documentation
Refactor
templateDataandtemplateDataMappingsfields from multiple components and contracts.Chores
@kleros/escrow-v2-subgraphpackage version from 2.0.3 to 2.0.4.