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

feat: add base support #6933

Merged
merged 26 commits into from
May 20, 2024
Merged

feat: add base support #6933

merged 26 commits into from
May 20, 2024

Conversation

kaladinlight
Copy link
Contributor

@kaladinlight kaladinlight commented May 16, 2024

Description

Add basic support for base blockchain. (Send/Receive, Balances, Transaction History, Balance History).

This does not include swap support, nft support, or on ramps

Pending: shapeshift/hdwallet#672

Pull Request Type

  • 🐛 Bug fix (Non-breaking Change: Fixes an issue)
  • 🛠️ Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
  • 💅 New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

closes #6833

Risk

High Risk PRs Require 2 approvals

Low - does not change any existing behavior and is behind a feature flag

What protocols, transaction types or contract interactions might be affected by this PR?

Testing

  • Ensure you can send transactions (native and token - supported wallets)
  • Ensure new transactions are detected over websocket (pending and confirmed)
  • Ensure balances and transaction history are accurate
  • Ensure network/assets are displayed correctly
  • Ensure market data is accurate

Engineering

Operations

Screenshots (if applicable)

image
image
image
image
image
image
image
image
image

@kaladinlight kaladinlight requested a review from a team as a code owner May 16, 2024 22:18
package.json Outdated Show resolved Hide resolved
@0xApotheosis 0xApotheosis self-assigned this May 20, 2024
@gomesalexandre gomesalexandre self-requested a review May 20, 2024 08:43
@gomesalexandre gomesalexandre mentioned this pull request May 20, 2024
3 tasks
Copy link
Contributor

@0xApotheosis 0xApotheosis left a comment

Choose a reason for hiding this comment

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

Code-wise looking strong, though a few possible unrelated changes in this PR.

Will do a functional test once I acquire some tokens on Base!

packages/caip/src/constants.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

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

First pass - noticed accounts are in an infinite fetching loop after a full cache clear, which isn't the case on develop
https://jam.dev/c/785810ab-12f6-4b28-b424-1bb344a48595

This is caused by shapeshift/hdwallet#671 (which isn't present in develop currently since we didn't merge its web fren #6827) which this PR includes. Its web fren actually fixes the infinite loop issue - when the multichain snap isn't installed, trying to derive an account 0+ always derive the same account 0 pubkey (which has activity) meaning we fetch the "next" account number (really 0th) again ad vitam aeternam.

@kaladinlight, ideally #6889 needs to go in before but alternatively, if we don't want this to go first, we can also cherry-pick the <AppContext /> bits separately here.

All of the second pass testing below has been done after merging #6889 locally.

@0xApotheosis
Copy link
Contributor

0xApotheosis commented May 20, 2024

Been debugging this too @gomesalexandre - some additional info that might help:

  • I'm getting the infinite fetching loop only on MetaMask (no Snap). Native is working fine.
  • The state seems to get corrupted - e.g. if I switch back to develop I'm still in an infinite loop (though a fresh cache gets it working again)

Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

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

Tested locally, LGTM

https://jam.dev/c/39c0bb88-da60-4925-a533-ee1c6edc421e

Happy to get this in after the snap bits go in as part of/before this

@kaladinlight
Copy link
Contributor Author

First pass - noticed accounts are in an infinite fetching loop after a full cache clear, which isn't the case on develop https://jam.dev/c/785810ab-12f6-4b28-b424-1bb344a48595

This is caused by shapeshift/hdwallet#671 (which isn't present in develop currently since we didn't merge its web fren #6827) which this PR includes. Its web fren actually fixes the infinite loop issue - when the multichain snap isn't installed, trying to derive an account 0+ always derive the same account 0 pubkey (which has activity) meaning we fetch the "next" account number (really 0th) again ad vitam aeternam.

@kaladinlight, ideally #6889 needs to go in before but alternatively, if we don't want this to go first, we can also cherry-pick the <AppContext /> bits separately here.

All of the second pass testing below has been done after merging #6889 locally.

Derp, good catch and will work on getting #6889 in first with this to follow.

Copy link
Contributor

@gomesalexandre gomesalexandre left a comment

Choose a reason for hiding this comment

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

Retested locally, confirmed the previously spotted issue has been fixed and things are still happy on the BASE side of things 🎉

https://jam.dev/c/20ef45c5-b602-4e9c-b41d-17b2015a180b

@kaladinlight kaladinlight merged commit 35df7ee into develop May 20, 2024
3 checks passed
@kaladinlight kaladinlight deleted the add-base-support branch May 20, 2024 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Base "base" support
3 participants