brave / brave-ios

Brave iOS Browser
https://brave.com
Mozilla Public License 2.0
1.7k stars 441 forks source link

Fix #8114: Support CoW Swap 🐮 orders in Safer Sign #8533

Closed StephenHeaps closed 9 months ago

StephenHeaps commented 9 months ago

Summary of Changes

This pull request fixes #8114

Submitter Checklist:

Test Plan:

Direct link to CoW Swap for WETH token -> COW token on Goerli Test Network (I used Goerli, any supported network sufficient). https://swap.cow.fi/#/5/swap/WETH/COW

  1. Test a CoW Swap trade using any non-native token.
  2. Verify:
    1. Safer Sign Swap UI is shown.
    2. Tapping Details shows the sign transaction details.
    3. Tapping link button beside tokens links out to explorer website for the token.
    4. Gas fee is free!
  3. Test a CoW Swap trade with a custom recipient (tap the gear, enable Custom Recipient toggle).
  4. Verify:
    1. Safer Sign Swap UI is shown
    2. Verify custom recipient is listed in the to token section (tap & hold to see full address).
    3. Links + gas fee verified above in Step 2.
  5. Test a CoW Swap trade using native token (ex. ETH on Goerli).
    • ⚠️ These trades cost gas, since they are real transactions. Also, we do not have the ability to display custom recipients in this case, for the time being.
  6. Verify Safer Sign Swap UI is shown as expected (similar to Swap in native Brave Wallet). Gas Fee shouldn't be 0, as this is a transaction instead of a signature request.
  7. Test a CoW swap with unknown tokens. If you used COW token for any of the above, this has been verified by the COW symbol displaying. Any token not in search assets should suffice!

Screenshots:

CoW Swap CoW Swap - details _
CoW Swap - custom recipient CoW Swap - custom recipient address CoW Swap Details - custom recipient address

Reviewer Checklist:

kdenhartog commented 9 months ago

These trades cost gas, since they are real transactions. Also, we do not have the ability to display custom recipients in this case, for the time being.

What do we display in this case? I'd assume the details are proper and we could see it there,but I don't expect many users will think to do that. I could see this being used as a tool to drain wallets.

StephenHeaps commented 9 months ago

These trades cost gas, since they are real transactions. Also, we do not have the ability to display custom recipients in this case, for the time being.

What do we display in this case? I'd assume the details are proper and we could see it there,but I don't expect many users will think to do that. I could see this being used as a tool to drain wallets.

In this case they are provided to us from BraveCore as TransactionInfo models (instead of SignMessageRequest models) display using the existing transaction confirmation (not sign message panel), with gas fees:

https://github.com/brave/brave-ios/assets/5314553/ac66f64f-3297-462c-990f-7d2826cb238a

kdenhartog commented 9 months ago

@StephenHeaps sorry, I misquoted what I was originally trying to comment on. I was referencing the custom recipients. I see on desktop as well as iOS that we'll want to fix. I'm opening an issue to get this fixed on Desktop. Do we need to do something to get this fixed on iOS in this PR or would it be easier to ship it in a follow up PR?

StephenHeaps commented 9 months ago

@StephenHeaps sorry, I misquoted what I was originally trying to comment on. I was referencing the custom recipients. I see on desktop as well as iOS that we'll want to fix. I'm opening an issue to get this fixed on Desktop. Do we need to do something to get this fixed on iOS in this PR or would it be easier to ship it in a follow up PR?

In this PR, iOS supports displaying CoW Swap custom recipients for safer sign signature requests (gas-less / free network fee, shown with Sign button), but we don't have ability to parse out the recipient in the ETHSwap transaction type yet (requires gas, shown with Confirm button).

github-actions[bot] commented 9 months ago

[puLL-Merge] - brave/brave-ios@8533

Description

This pull request introduces multiple improvements and refactoring to the Brave iOS wallet codebase, primarily focusing on the signing of messages and signatures for wallet transactions. It introduces a new store for managing sign message requests and a new view for handling CoW swap order requests. It also replaces the previous implementation of the transaction confirmation view with a new container view that better handles different types of transactions.

Changes ### Changes #### `Sources/BraveWallet/Crypto/Stores/CryptoStore.swift` - Added `signMessageRequestStore` property and related functions to manage the sign message requests. #### `Sources/BraveWallet/Crypto/Stores/SignMessageRequestStore.swift` - Implemented a new file to house the `SignMessageRequestStore` class, which manages sign message requests including updates and navigation between multiple requests. #### `Sources/BraveWallet/Crypto/Transaction Confirmations/PendingTransactionView.swift` - Refactored to use the new `SaferSignTransactionContainerView` instead of the removed `SwapTransactionConfirmationView`. #### `Sources/BraveWallet/Crypto/Transaction Confirmations/SaferSignTransactionContainerView.swift` - Created a new file containing `SaferSignTransactionContainerView`, a view for confirming more secure swaps and transactions. #### `Sources/BraveWallet/Crypto/Transaction Confirmations/SaferSignTransactionView.swift` - Introduced `SaferSignTransactionView`, which provides a detailed view of the transaction during the confirmation process. #### `Sources/BraveWallet/Crypto/Transaction Confirmations/SwapTransactionConfirmationView.swift` - Removed the `SwapTransactionConfirmationView` file as it appears to have been replaced by the newer container and view files for signing and transactions. #### `Sources/BraveWallet/Extensions/BraveWalletExtensions.swift` - Extensions added to `BraveWallet.NetworkInfo` for generating token block explorer URLs. #### `Sources/BraveWallet/Extensions/RpcServiceExtensions.swift` - The `ContractAddressChainIdPair` struct now conforms to `Hashable`. #### `Sources/BraveWallet/Panels/RequestContainerView.swift` - Updated to integrate the new `signMessageRequestStore` when displaying sign message requests. #### `Sources/BraveWallet/Panels/Signature Request/SaferSignMessageRequestContainerView.swift` - Created a new file for `SaferSignMessageRequestContainerView`, which handles sign message requests with additional security layers like CoW swaps. #### `Sources/BraveWallet/Panels/Signature Request/SignMessageRequestContainerView.swift` - Modified to use `SignMessageRequestStore` and simplified the logic to use the store's data directly. #### `Sources/BraveWallet/Panels/Signature Request/SignMessageRequestContentView.swift` - Introduced a view that specializes in displaying the content of a sign message request, including warnings when unknown Unicode characters are detected or when the message contains consecutive new lines. #### `Sources/BraveWallet/Panels/Signature Request/SignMessageRequestView.swift` - Updated the view to implement `SignMessageRequestContentView` for displaying request content. #### `Sources/BraveWallet/WalletStrings.swift` - Added a new localized string for the "Hide Details" button title.

Security Hotspots