-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Absolute tx deadline #60
Conversation
WalkthroughThe updates primarily involve renaming the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant SmartContract
User->>Frontend: Initiate Transaction
Frontend->>Frontend: Calculate Deadline using BigInt
Frontend->>SmartContract: createNativeTransaction(_deadline, ...)
SmartContract-->>Frontend: Transaction Created
Frontend-->>User: Transaction Confirmation
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. |
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- contracts/README.md (1 hunks)
- contracts/src/Escrow.sol (2 hunks)
- contracts/src/EscrowToken.sol (2 hunks)
- contracts/src/EscrowUniversal.sol (4 hunks)
- contracts/src/interfaces/IEscrow.sol (3 hunks)
- subgraph/package.json (1 hunks)
- subgraph/subgraph.yaml (1 hunks)
Files skipped from review due to trivial changes (1)
- subgraph/package.json
Additional context used
LanguageTool
contracts/README.md
[uncategorized] ~71-~71: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ... env vars are required: -PRIVATE_KEY
: the private key of the deployer account...
[uncategorized] ~72-~72: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...or the testnets. -MAINNET_PRIVATE_KEY
: the private key of the deployer account...
[uncategorized] ~73-~73: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...unt used for Mainnet. -INFURA_API_KEY
: the API key for infura. The ones below...
[uncategorized] ~77-~77: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...low are optional: -ETHERSCAN_API_KEY
: to verify the source of the newly deplo...
[uncategorized] ~78-~78: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...s on Etherscan. -ARBISCAN_API_KEY
: to verify the source of the newly deplo...
[uncategorized] ~79-~79: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ... on Arbitrum. -GNOSISSCAN_API_KEY
: to verify the source of the newly deplo...
[uncategorized] ~83-~83: 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] ~83-~83: 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] ~91-~91: 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] ~107-~107: 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
85-85: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
91-91: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
99-99: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
107-107: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
129-129: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
135-135: null (MD036, no-emphasis-as-heading)
Emphasis used instead of a heading
Additional comments not posted (9)
subgraph/subgraph.yaml (2)
9-9
: Ensure the new contract address is correct and intended.Verification successful
The new contract address '0xBa2cC03e2e34C5DEDaEbC481b2531e06946353De' appears consistently in the repository, including deployment files and documentation. This suggests that the address is correct and intended for the EscrowUniversal contract.
subgraph/subgraph.yaml
contracts/README.md
contracts/deployments/arbitrumSepoliaDevnet/EscrowUniversal.json
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new contract address for EscrowUniversal. # Test: Search for the address in the deployment logs or contract management system. echo "Verify the address '0xBa2cC03e2e34C5DEDaEbC481b2531e06946353De' is the correct and intended for EscrowUniversal."Length of output: 240
Script:
#!/bin/bash # Description: Search for the new contract address in the repository to verify its usage. # Search for the address '0xBa2cC03e2e34C5DEDaEbC481b2531e06946353De' in the repository. rg '0xBa2cC03e2e34C5DEDaEbC481b2531e06946353De'Length of output: 638
11-11
: Verify the new startBlock aligns with the deployment block of the new contract.contracts/README.md (1)
21-21
: Ensure the updated link points to the correct and intended address for EscrowUniversal.Verification successful
To verify the updated link for EscrowUniversal, we need to check the deployment history or contract details to ensure the address is correct.
The verification script confirms that the link in the README points to the correct and intended address for the EscrowUniversal contract.
- The address
0xBa2cC03e2e34C5DEDaEbC481b2531e06946353De
is associated with the EscrowUniversal contract on Sepolia Arbiscan.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the updated link for EscrowUniversal in the README. # Test: Check if the link 'https://sepolia.arbiscan.io/address/0xBa2cC03e2e34C5DEDaEbC481b2531e06946353De' points to the correct and intended address. echo "Verify the link points to the correct and intended address for EscrowUniversal."Length of output: 168
Script:
#!/bin/bash # Description: Verify the updated link for EscrowUniversal in the README. # Fetch the contract details from the blockchain explorer to verify the address. curl -s https://sepolia.arbiscan.io/address/0xBa2cC03e2e34C5DEDaEbC481b2531e06946353De | grep -i 'EscrowUniversal'Length of output: 4822
contracts/src/interfaces/IEscrow.sol (2)
77-84
: The parameter change from_timeoutPayment
to_deadline
aligns with the PR objectives. Ensure this change is integrated correctly across all components.
Line range hint
94-103
: The parameter change from_timeoutPayment
to_deadline
increateERC20Transaction
is consistent with the PR objectives. Ensure this change is integrated correctly across all components.Verification successful
The search results indicate that the
createERC20Transaction
function is used in multiple files, including Solidity contracts and a TypeScript file. To ensure the integration of the_deadline
parameter is correct, we need to verify that all instances ofcreateERC20Transaction
in these files have been updated to use_deadline
.Here are the files where
createERC20Transaction
is used:
contracts/src/Escrow.sol
contracts/src/EscrowUniversal.sol
contracts/src/EscrowToken.sol
contracts/src/interfaces/IEscrow.sol
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx
We need to inspect these files to ensure the
_deadline
parameter is correctly integrated.
The search results confirm that the
createERC20Transaction
function is present in the specified Solidity and TypeScript files. However, the output does not provide enough detail to verify if the_deadline
parameter is correctly integrated in all instances.To ensure thorough verification, we need to inspect the actual function definitions and usages in these files to confirm the correct integration of the
_deadline
parameter.
The search results show that the
createERC20Transaction
function is defined and used in the relevant Solidity and TypeScript files. The context provided confirms that the_deadline
parameter is correctly integrated in the function definitions and usages.Here are the key points:
- The
_deadline
parameter is present in the function definitions in the Solidity files (contracts/src/Escrow.sol
,contracts/src/EscrowUniversal.sol
,contracts/src/EscrowToken.sol
,contracts/src/interfaces/IEscrow.sol
).- The TypeScript file (
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx
) also references thecreateERC20Transaction
function, ensuring the integration is consistent.Based on this information, the integration of the
_deadline
parameter appears to be correctly implemented across the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify integration of the `_deadline` parameter in `createERC20Transaction`. # Test: Search for usages of `createERC20Transaction` to ensure they now use `_deadline`. rg --type solidity $'createERC20Transaction'Length of output: 76
Script:
#!/bin/bash # Description: Verify integration of the `_deadline` parameter in `createERC20Transaction`. # Test: Search for usages of `createERC20Transaction` to ensure they now use `_deadline`. rg 'createERC20Transaction'Length of output: 75972
Script:
#!/bin/bash # Description: Verify the integration of the `_deadline` parameter in `createERC20Transaction` across all relevant files. # Check the Solidity files for the correct usage of `_deadline`. rg 'createERC20Transaction' contracts/src/Escrow.sol contracts/src/EscrowUniversal.sol contracts/src/EscrowToken.sol contracts/src/interfaces/IEscrow.sol # Check the TypeScript file for the correct usage of `_deadline`. rg 'createERC20Transaction' web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsxLength of output: 851
Script:
#!/bin/bash # Description: Verify the integration of the `_deadline` parameter in `createERC20Transaction` across all relevant files. # Extract the function definitions and usages of `createERC20Transaction` in Solidity files. rg --context 5 'createERC20Transaction' contracts/src/Escrow.sol contracts/src/EscrowUniversal.sol contracts/src/EscrowToken.sol contracts/src/interfaces/IEscrow.sol # Extract the function definitions and usages of `createERC20Transaction` in the TypeScript file. rg --context 5 'createERC20Transaction' web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsxLength of output: 4291
contracts/src/Escrow.sol (1)
127-127
: The change in the assignment oftransaction.deadline
aligns with the PR objectives. Ensure this change does not introduce any logical errors or unexpected behaviors.contracts/src/EscrowToken.sol (1)
Line range hint
138-152
: The update from_timeoutPayment
to_deadline
in thecreateERC20Transaction
function aligns with the PR objectives. Ensure that the new_deadline
parameter is correctly documented and tested.contracts/src/EscrowUniversal.sol (2)
Line range hint
126-137
: The update from_timeoutPayment
to_deadline
in thecreateNativeTransaction
function aligns with the PR objectives. Ensure that the new_deadline
parameter is correctly documented and tested.
Line range hint
157-170
: The update from_timeoutPayment
to_deadline
in thecreateERC20Transaction
function aligns with the PR objectives. Ensure that the new_deadline
parameter is correctly documented and tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (8 hunks)
Additional comments not posted (2)
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (2)
52-52
: Refactor to handle potential exceptions in date parsing.
[REFACTOR_SUGGESTion]
Consider wrapping thenew Date(deadline).getTime()
operation in a try-catch block to handle cases wheredeadline
might not be a valid date string, which would throw a runtime error.+ try { + const deadlineTimestamp = BigInt(Math.floor(new Date(deadline).getTime() / 1000)); + } catch (error) { + console.error("Invalid deadline:", error); + return; // or handle error appropriately + }
109-109
: EnsuredeadlineTimestamp
is correctly calculated and used.Verify that the
deadlineTimestamp
calculation correctly handles all expected date formats and time zones. This is crucial as incorrect timestamp calculations can lead to transaction errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
PR-Codex overview
This PR updates the version, addresses, and deadlines in the Escrow Universal subgraph and contract functions.
Detailed summary
_timeoutPayment
with_deadline
in contract functionsSummary by CodeRabbit
New Features
DepositPaymentButton
to handle deadline calculations and navigation based on deadline status.Bug Fixes
Refactor
_timeoutPayment
with_deadline
in transaction functions across multiple contracts for consistency.Chores
2.0.1
insubgraph/package.json
.EscrowUniversal
data source address and start block insubgraph.yaml
.