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(wallet): allow marking any NFT as junk or not junk #21290

Merged
merged 18 commits into from
Mar 5, 2024

Conversation

josheleonard
Copy link
Contributor

@josheleonard josheleonard commented Dec 7, 2023

Resolves brave/brave-browser#36399
Resolves brave/brave-browser#36400
Resolves brave/brave-browser#34763

Cleans up the token management logic and allows spam NFTs to be marked as "not junk"

Submitter Checklist:

  • I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally:
    • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
    • npm run lint, npm run presubmit wiki, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

See STR in issues

@josheleonard josheleonard self-assigned this Dec 7, 2023
@josheleonard josheleonard requested a review from a team as a code owner December 7, 2023 14:31
@github-actions github-actions bot added CI/storybook-url Deploy storybook and provide a unique URL for each build feature/web3/wallet labels Dec 7, 2023
@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from 2d953c5 to 846f76a Compare December 7, 2023 14:32
@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch 2 times, most recently from 7055b73 to 7eeb60f Compare December 7, 2023 14:59
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from 7eeb60f to 5823666 Compare December 7, 2023 17:39
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@brave brave deleted a comment from github-actions bot Dec 8, 2023
@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from 5823666 to 97becd7 Compare December 11, 2023 12:12
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from 97becd7 to d43e4e8 Compare December 11, 2023 17:52
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from d43e4e8 to 51a9171 Compare December 11, 2023 19:19
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from 51a9171 to 1991d12 Compare December 12, 2023 10:28
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from 1991d12 to 8a78707 Compare December 13, 2023 11:02
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from 8a78707 to bb24a63 Compare December 18, 2023 14:00
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from bb24a63 to 214bc10 Compare January 5, 2024 14:47
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from 13e85ce to efcc70e Compare January 8, 2024 11:39
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from d1c35e3 to 5f8e80f Compare January 8, 2024 13:42
@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from a3d056b to bcf537c Compare February 28, 2024 22:35
Copy link
Contributor

@Douglashdaniel Douglashdaniel left a comment

Choose a reason for hiding this comment

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

++

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from c55a8c4 to 6af0156 Compare March 2, 2024 15:06
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard force-pushed the chore--cleanup-token-management-code branch from 6af0156 to 14c189a Compare March 5, 2024 13:02
Copy link
Contributor

github-actions bot commented Mar 5, 2024

[puLL-Merge] - brave/brave-core@21290

Description

This PR makes several major changes to the Brave Wallet UI components, focusing on refactoring actions and hooks related to asset management. The primary motivation appears to be code simplification and the removal of deprecated or unnecessary functionalities.

Changes

Changes

General Changes Across Multiple Files

  • Removed unnecessary and deprecated code related to asset management, including specific actions for adding, updating, and removing user assets.
  • Introduced new hooks and updated existing ones to use the API slice directly for mutations related to user asset visibility, adding, and removing tokens.
  • Refactored components to utilize these new patterns, simplifying their logic and making them more maintainable.

Specific File Changes

  • wallet_actions.ts & handlers.ts: Significant refactoring around asset management actions. Removed outdated actions and handlers, including those for adding and removing assets.
  • lib.ts, action_types.ts, local-storage-keys.ts: Updated to reflect changes in how assets are handled, particularly around visibility and storage.
  • assets-management.test.tsx & assets-management.ts: Removed these files as their functionalities have been refactored into new patterns.
  • wallet-selectors.ts: Updated selectors to remove references to removed state properties.
  • nfts.endpoints.ts & token.endpoints.ts: Adjusted API endpoints and handlers to align with the new asset management approach.
  • blockchain-token.entity.ts & wallet.slice.ts: Updated to streamline state management and remove deprecated functionalities.
  • UI components (nft-view.tsx, nft-grid-view.tsx, etc.): Updated to use new hooks for asset visibility and management. Simplified logic by removing deprecated hooks and actions.

Security Hotspots

  1. Use of Local Storage (High Risk): Changes in local-storage-keys.ts and related files could potentially expose sensitive information if not handled correctly. Ensure that only non-sensitive data is stored in local storage and that proper access controls are in place.
  2. API Mutations (Medium Risk): The refactoring introduces several new mutation hooks (useAddUserTokenMutation, useRemoveUserTokenMutation, useUpdateUserAssetVisibleMutation). It's crucial to ensure these mutations are securely handled, especially regarding authentication and data validation, to prevent unauthorized access or manipulation.
  3. Asset Visibility Changes (Low Risk): Changes to asset visibility handling might inadvertently expose hidden assets if not properly managed. Review the logic surrounding these changes to ensure that asset visibility states are correctly updated and persisted.

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@josheleonard josheleonard merged commit fee7615 into master Mar 5, 2024
19 checks passed
@josheleonard josheleonard deleted the chore--cleanup-token-management-code branch March 5, 2024 15:08
@github-actions github-actions bot added this to the 1.65.x - Nightly milestone Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/storybook-url Deploy storybook and provide a unique URL for each build feature/web3/wallet puLL-Merge
Projects
None yet
4 participants