-
Notifications
You must be signed in to change notification settings - Fork 181
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
feat: add base support #6933
Conversation
packages/unchained-client/src/evm/arbitrum/parser/__tests__/arbitrum.test.ts
Show resolved
Hide resolved
scripts/generateAssetData/generateRelatedAssetIndex/generateRelatedAssetIndex.ts
Show resolved
Hide resolved
There was a problem hiding this 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!
There was a problem hiding this 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.
Been debugging this too @gomesalexandre - some additional info that might help:
|
There was a problem hiding this 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
Derp, good catch and will work on getting #6889 in first with this to follow. |
There was a problem hiding this 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 🎉
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#672Pull Request Type
Issue (if applicable)
closes #6833
Risk
Low - does not change any existing behavior and is behind a feature flag
Testing
Engineering
Operations
Screenshots (if applicable)