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

Fix/masquerade checksum #1455

Merged
merged 7 commits into from
Oct 16, 2023
Merged

Fix/masquerade checksum #1455

merged 7 commits into from
Oct 16, 2023

Conversation

bigboydiamonds
Copy link
Collaborator

@bigboydiamonds bigboydiamonds commented Oct 16, 2023

Pairing with @abtestingalpha to create fix for double checking queryable address is checksum'd prior to querying when in Masquerade Mode.

090131e56c58d4c5d6ca1caf15524e8fc5f45856: synapse-interface preview link

Summary by CodeRabbit

  • Refactor: Improved the address validation process in the search bar and transaction history. The system now uses a more robust method to validate and process addresses, enhancing the reliability of search results and transaction history retrieval.
  • New Feature: The search bar now accepts and processes checksummed addresses, providing more accurate portfolio balance information and transaction history for users.
  • Usability: The update ensures that only valid addresses are used when fetching user portfolio balances and transaction history, reducing potential errors and improving the overall user experience.

eb5b76602bbb85561de83722483acd26883f3325: synapse-interface preview link

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2023

Walkthrough

The changes primarily focus on improving the address validation process in the application. The getValidAddress function is introduced and used across different components to ensure that only valid addresses are used in API calls and other operations.

Changes

File Path Summary
.../components/Portfolio/SearchBar.tsx Replaced searchInputIsAddress with getValidAddress for better address validation. Updated dependencies in useEffect hook.
.../slices/transactions/updater.tsx Updated fetchUserHistoricalActivity and fetchUserPendingActivity to use getValidAddress for address validation before API calls.
.../utils/isValidAddress.tsx Introduced getValidAddress function that returns a validated address instead of a boolean. Enhanced error handling in isValidAddress.

"In the land of code, where logic is king, 🤴
A rabbit hopped in, making changes with a zing. 🐇💥
No more false addresses, only the valid shall pass, 🚫➡️✅
Making our application robust and truly first-class. 🏰🥇
So let's celebrate these changes, for they're not just any, 🎉
They're the work of CodeRabbit, wise and canny!" 🐇🧠


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
  • Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
    resume reviews, enter @coderabbitai resume as any issue or review comment.

@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (2ee5234) 50.69210% compared to head (cc1afd0) 50.69210%.

Additional details and impacted files
@@              Coverage Diff              @@
##              master       #1455   +/-   ##
=============================================
  Coverage   50.69210%   50.69210%           
=============================================
  Files            356         356           
  Lines          24274       24274           
  Branches         267         267           
=============================================
  Hits           12305       12305           
  Misses         10775       10775           
  Partials        1194        1194           
Flag Coverage Δ
packages 90.62003% <ø> (ø)

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.

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

Commits Files that changed from the base of the PR and between 2ee5234 and a113e3b.
Files selected for processing (3)
  • packages/synapse-interface/components/Portfolio/SearchBar.tsx (2 hunks)
  • packages/synapse-interface/slices/transactions/updater.tsx (2 hunks)
  • packages/synapse-interface/utils/isValidAddress.tsx (1 hunks)
Additional comments (Suppressed): 7
packages/synapse-interface/slices/transactions/updater.tsx (1)
  • 42-42: The getValidAddress function is imported from the isValidAddress module. This function is used to validate and checksum addresses before querying.
packages/synapse-interface/components/Portfolio/SearchBar.tsx (4)
  • 16-16: The getValidAddress function is imported but not used in the old hunk. Ensure that all calls to this function throughout the codebase have been updated to match the new usage.

  • 71-73: The getValidAddress function is used to validate the search input. This is a good practice as it ensures that the address is valid before it is used in subsequent operations.

  • 78-83: The fetchAndStoreSearchInputPortfolioBalances function now uses the validated address checkSearchInputIsAddress instead of the raw search input. This is a good practice as it ensures that the address is valid before it is used in the API call.

  • 86-88: The clearSearchInput function is now called when masqueradeActive is true and checkSearchInputIsAddress is not null. This is a good practice as it ensures that the search input is cleared when it is not a valid address.

packages/synapse-interface/utils/isValidAddress.tsx (2)
  • 1-10: The isValidAddress function has been updated to catch and log both InvalidAddressError and any other exceptions. This is a good practice as it helps in debugging and understanding the nature of the error. However, the validatedAddress variable is not used after being assigned. If it's not needed, consider removing it to clean up the code.
   try {
     const validatedAddress: string = getAddress(address)
     getAddress(address)
     return true
  } catch (e: InvalidAddressError | any) {
    console.error('isValidAddress error: ', e)
     return false
   }
  • 13-21: The getValidAddress function is introduced. It is similar to isValidAddress but returns the validated address instead of a boolean. This function can be useful when the validated address is needed for further processing. However, the function returns null when an exception is caught. This could potentially lead to null pointer exceptions in the calling code. Consider throwing the exception to the caller to handle it appropriately.
export const getValidAddress = (address: string): Address | any => {
  try {
    const validatedAddress: Address = getAddress(address)
    return validatedAddress
  } catch (e: InvalidAddressError | any) {
    console.error('getValidAddress error: ', e)
    return null
    throw e
  }
}

Comment on lines 99 to 108
searchedBalancesAndAllowances
)[0] as Address
fetchUserHistoricalActivity({
address: queriedAddress,
address: getValidAddress(queriedAddress),
startTime: queryHistoricalTime,
})
fetchUserPendingActivity({
address: queriedAddress,
address: getValidAddress(queriedAddress),
startTime: queryPendingTime,
})
Copy link
Contributor

Choose a reason for hiding this comment

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

The getValidAddress function is used to validate the queriedAddress before it is used in the fetchUserHistoricalActivity and fetchUserPendingActivity functions. This ensures that the address is valid and checksummed before querying, improving the reliability of the system.

-        fetchUserHistoricalActivity({
-         address: queriedAddress,
-         startTime: queryHistoricalTime,
-       })
-       fetchUserPendingActivity({
-         address: queriedAddress,
-         startTime: queryPendingTime,
-       })
+        fetchUserHistoricalActivity({
+         address: getValidAddress(queriedAddress),
+         startTime: queryHistoricalTime,
+       })
+       fetchUserPendingActivity({
+         address: getValidAddress(queriedAddress),
+         startTime: queryPendingTime,
+       })

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

Commits Files that changed from the base of the PR and between a113e3b and cc1afd0.
Files selected for processing (2)
  • packages/synapse-interface/components/Portfolio/SearchBar.tsx (2 hunks)
  • packages/synapse-interface/slices/transactions/updater.tsx (2 hunks)
Additional comments (Suppressed): 5
packages/synapse-interface/components/Portfolio/SearchBar.tsx (3)
  • 16-16: The getValidAddress function is imported from the isValidAddress module. This function is used to validate and checksum the address before querying.

  • 71-73: The searchInputIsAddress variable is replaced with checksumValidAddress. This new variable stores the validated and checksummed address from getValidAddress.

  • 76-89: The fetchAndStoreSearchInputPortfolioBalances function now uses checksumValidAddress instead of searchInput. This ensures that the address is validated and checksummed before querying. The dependencies of the useEffect hook are updated accordingly.

packages/synapse-interface/slices/transactions/updater.tsx (2)
  • 42-42: The getValidAddress function is imported from the isValidAddress module. This function is used to validate and checksum the address before querying. This is a good practice as it ensures the reliability of the system in Masquerade Mode.

  • 98-108: The getValidAddress function is used to validate the queried address before making API calls in fetchUserHistoricalActivity and fetchUserPendingActivity functions. This is a good practice as it ensures that the address is valid and checksummed before querying, reducing the chances of errors and improving the reliability of the system.

-        fetchUserHistoricalActivity({
-         address: queriedAddress,
-         startTime: queryHistoricalTime,
-       })
-       fetchUserPendingActivity({
-        address: queriedAddress,
-         startTime: queryPendingTime,
-       })
+        fetchUserHistoricalActivity({
+         address: getValidAddress(queriedAddress),
+         startTime: queryHistoricalTime,
+       })
+       fetchUserPendingActivity({
+         address: getValidAddress(queriedAddress),
+         startTime: queryPendingTime,
+       })

@bigboydiamonds bigboydiamonds merged commit 4416f0a into master Oct 16, 2023
42 checks passed
@bigboydiamonds bigboydiamonds deleted the fix/masquerade-checksum branch October 16, 2023 23:17
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