-
Notifications
You must be signed in to change notification settings - Fork 41
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
ink chain support #1785
ink chain support #1785
Conversation
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected] |
aed3012
to
caecf31
Compare
fix: splice apechain curtis, ink sepolia testnet rpcs into default rpcs
add ink, inkSepolia chain migrations fix: apechain, apechainCurtis migrations in rainbowChains
caecf31
to
67cbd3f
Compare
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.
Ink sends: ✅
Ink swaps: ✅
Ink swaps on DYORSwap: Swap failed: Unknown error: "Cannot read properties of null (reading 'gasLimit')". Try increasing your slippage tolerance.
(tried swapping 0.001 eth for Kraken @ 25% slippage)
Ink signing / sending on dapp: ✅
@brody This one went through for me; maybe was a temporary issue on their side. Can you try again? Otherwise might be a state issue on your client https://inkchain-temp.cloud.blockscout.com/tx/0xe5d533b4a3f2982f0f8c2909af5603e669f92c639aa6ca9bd2614ac142107410 |
only happens when i try to swap Kraken. weth works fine. |
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.
The code looks good to me. I was able to transact on dapps using the bx. I experienced a similar issue to Brody but i don't believe it's a blocker.
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.
dapp issue is preexisting
Fixes BX-####
Figma link (if any):
What changed (plus any additional context for devs)
[email protected]
ink
andinkSepolia
hardcoded referencesInk ETH
; awaiting larger listinkSepolia
faucet URLuseSearchCurrencyLists
search supportNotable changes
avalancheFuji
andapechainCurtis
implementation issues and migrationsrainbowChains
, so included a newLOCAL_CHAINS
concept inreferences/chains
so that we can hardcode chains before backend rolls out chain support (mainly missing testnets). The testnet labels in Network settings required we added hardcoded references forchainsLabel
andchainsName
as well. This should be refactored in the future so we have one shared local chains reference, or expand backend results more easily.v1.5.53
release included a version bump for bothrainbowChains
anduserChains
persisted states, but lacked adequate migrations. This caused a loss of data for preferences for custom networks, network ordering and toggles, custom assets, RPCs, and more in late October.userChains
occured before this version as well, so we likely have had multiple data loss events.rainbowChains
I documented various state deltas and modified thev10
migration to properly merge theapechain
chain id. This will only be useful for users that still havev9
or below state; most users will have lost data here. I bumped to version 11 and addedv11
migration to mergeapechainCurtis
,ink
, andinkSepolia
. I added curtis in this 2nd migration because references to it were missing frominitialChains
that was used inv10
because it wasn't included as a backend network and the local chains concept didn't exist, so didn't wind up in state for users (despite other codebase references).userChains
issues were a bit more involved. TheuserChainsOrder
that was used for bootstrapping chain ordering state was inadvertently re-ordering networks, so new chains would appear in an ordering that didn't reflect backend's returned networks.userChains
was missingpersistOptions
entirely, so each time the version was bumped, we lost most of the data here. I created migrations forv1
throughv5
that naively reset state as was the previous behavior; it will be too challenging to roll the clock back on that to understand what the expected states should have been with each version bump.v5
migration foruserChains
follows the goal behavior ofrainbowChains
and tries to cleanly appendapechain
for users that haven't yet migrated.v6
does the same and appendsapechainCurtis
,ink
, andinkSepolia
. I successfully tested this flow from a local build ofv1.5.64
to these changes.Next steps
rainbowChains
anduserChains
as they are both error prone. We should replacerainbowChains
with an omnidirectional static import from thenetworks.json
file that is generated from backend. That way, we always know that the networks available to a build are bundled in that build, and can never diverge in state. We'll be able to adopt dynamic networks at the same time this way. We also userainbowChains
to add/remove/set the alternative RPC. This feature should be its own store; we should track the delta or RPC overrides without being destructive to the underlying backend data. This would allow us to change metadata from backend (like an RPC or native asset) without overriding user choice. If backend changes and user still relies on defaults (i.e. no reference to the network in preference overrides), then they inherit the new defaults; otherwise we take preference to their existing setting.userChains
has many of the same problems and really doesn't need to exist. We should capturecustomChains
in its own store, andchainOrdering
in its own store. There is no need to intermix mainnet chains with custom chains or user preferences. From the divergent, easier to maintain stores, we can create bootstrap a data model that can be relied upon in the codebase in the same way, without making tradeoffs for the underlying state integrity.Screen recordings / screenshots
What to test