Skip to content
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

fe/updated time remaining pending transaction #1541

Merged
merged 10 commits into from
Nov 7, 2023

Conversation

bigboydiamonds
Copy link
Collaborator

@bigboydiamonds bigboydiamonds commented Nov 7, 2023

  • Implement retries for resolving transactions via viem waitForTransaction()
  • Ensure elapsed time / time remaining calculations are accurate for all pending transactions

Summary by CodeRabbit

Summary by CodeRabbit

  • Bug Fixes
    • Improved the calculation and tracking of elapsed time and time remaining for pending transactions.
    • Added a mechanism to reset elapsed time for unique transactions.
  • Refactor
    • Refactored the timeRemaining calculation in the PendingTransaction component.
  • Chores
    • Added console logs for resolved transactions.

d70b108ddf1353afea3b97b6f6ed55ea7933a08a: synapse-interface preview link
23ceffe90d581a3b805929bc4b80caba4fe7d826: synapse-interface preview link
ccc0c4c7f2da3fde7c9fbfd03c9b6673049ad48d: synapse-interface preview link
95c511d04bbb7266f1840ad38698f775da25163c: synapse-interface preview link

Copy link
Contributor

coderabbitai bot commented Nov 7, 2023

Walkthrough

The changes primarily focus on improving the tracking and calculation of elapsed time and time remaining for pending transactions in the PendingTransaction component. The update interval for the MostRecentTransaction component has also been reduced for more frequent updates.

Changes

File Path Change Summary
.../MostRecentTransaction.tsx The update interval for the current time has been reduced from 60 seconds to 30 seconds.
.../PendingTransaction.tsx Introduced new state variables and updated the elapsed time tracking mechanism. Refactored the timeRemaining calculation and added a new useEffect hook to reset elapsed time for unique transactions. Added a console log for resolved transactions and modified the timeRemaining value based on the transaction submission status.

Poem

🍂 As the leaves fall this November day, 🍁

Code changes dance and sway. ⌨️💃

Time tracking now more precise, ⏱️

For pending transactions, oh so nice! 💼

In '63, the first touch-tone phone, ☎️

Now we track transactions on our own. 📲

Celebrate the progress, hop and twirl, 🎉🐇

In this ever-changing tech world. 🌐🔄


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai help to get help.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

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.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 3864694 and 1066f75.
Files selected for processing (2)
  • packages/synapse-interface/components/Portfolio/Transaction/MostRecentTransaction.tsx (1 hunks)
  • packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (3 hunks)
Files skipped from review due to trivial changes (1)
  • packages/synapse-interface/components/Portfolio/Transaction/MostRecentTransaction.tsx
Additional comments: 3
packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (3)
  • 99-159: The logic for tracking elapsed time and calculating remaining time seems sound. However, it's important to ensure that the startedTimestamp is always accurate and updated correctly when a transaction starts. Also, the updatedElapsedTime state variable should be updated at regular intervals as long as the transaction is in progress.

  • 213-217: Logging the resolved transaction is a good practice for debugging and tracking purposes. However, ensure that this does not expose any sensitive information in a production environment.

  • 248-256: The Transaction component is being passed the correct props. Ensure that the Transaction component handles these props correctly.

Copy link

codecov bot commented Nov 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (413a921) 51.23324% compared to head (6ab3239) 51.23324%.
Report is 5 commits behind head on master.

Additional details and impacted files
@@              Coverage Diff              @@
##              master       #1541   +/-   ##
=============================================
  Coverage   51.23324%   51.23324%           
=============================================
  Files            362         362           
  Lines          24691       24691           
  Branches         283         283           
=============================================
  Hits           12650       12650           
  Misses         10813       10813           
  Partials        1228        1228           
Flag Coverage Δ
packages 91.00610% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bigboydiamonds
Copy link
Collaborator Author

@coderabbitai pause

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.

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 1066f75 and 934492e.
Files selected for processing (1)
  • packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (4 hunks)
Additional comments: 2
packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (2)
  • 212-216: Logging the resolved transaction is a good practice for debugging purposes. However, ensure that this does not expose sensitive information in a production environment.

  • 247-255: The props passed to the Transaction component are correctly derived from the state and props of the PendingTransaction component. However, ensure that all these values are validated and sanitized before being passed to the Transaction component.

Copy link

cloudflare-workers-and-pages bot commented Nov 7, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6ab3239
Status: ✅  Deploy successful!
Preview URL: https://285713b5.sanguine.pages.dev
Branch Preview URL: https://fe-fallback-states.sanguine.pages.dev

View logs

@bigboydiamonds bigboydiamonds changed the title Fe/fallback states fe/updated-time-remaining-pending-transaction Nov 7, 2023
@bigboydiamonds bigboydiamonds changed the title fe/updated-time-remaining-pending-transaction fe/updated time remaining pending transaction Nov 7, 2023
@bigboydiamonds
Copy link
Collaborator Author

@coderabbitai resume

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.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 934492e and 94121b6.
Files selected for processing (1)
  • packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (3 hunks)
Additional comments: 3
packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (3)
  • 99-159: The logic for tracking elapsed time and calculating remaining time seems sound. However, it's worth noting that the setInterval function can lead to potential issues if the component unmounts before the interval is cleared. This can be mitigated by checking if the component is still mounted before calling setUpdatedElapsedTime.

  • 196-241: The retry logic in updateResolvedTransaction is a good practice for handling network requests. However, it's important to ensure that the maximum number of retries and the delay between retries are set to reasonable values to avoid unnecessary network traffic and potential rate limiting issues.

  • 267-275: The conditional assignment of timeRemaining based on isSubmitted is a good practice to ensure that the correct value is passed to the child component. However, it's important to ensure that isSubmitted is always correctly set to avoid potential issues.

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.

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 94121b6 and 6ab3239.
Files selected for processing (1)
  • packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (3 hunks)
Additional comments: 2
packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (2)
  • 95-160: The logic for calculating the elapsed time and remaining time seems to be correct. However, it's important to ensure that the startedTimestamp is always accurate and updated correctly when a transaction is initiated. Also, the isSubmitted flag should be set to true only when the transaction is actually submitted. This is to ensure that the elapsed time and remaining time are calculated correctly.

  • 268-276: The timeRemaining prop passed to the Transaction component is conditionally set based on the isSubmitted flag. This is a good practice as it ensures that the remaining time is only calculated and displayed when the transaction is actually submitted.

@bigboydiamonds bigboydiamonds merged commit 9fd9acf into master Nov 7, 2023
43 checks passed
@bigboydiamonds bigboydiamonds deleted the fe/fallback-states branch November 7, 2023 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants