-
Notifications
You must be signed in to change notification settings - Fork 76
Feature/lit 3549 js sdk add datiltest support #523
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
Feature/lit 3549 js sdk add datiltest support #523
Conversation
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.
I've shipped the update to wrapped-keys backend so it supports datil-test
now 👍
Looking at how 'give me an RPC URL for network X' logic is become more complex, and it spans across so many packages, I wonder if it would make sense to create a map of network names to RPC urls in constants
and reference it there instead of using || statements checking specific network names -- basically the same pattern I used here:
js-sdk/packages/wrapped-keys/src/lib/service-client/constants.ts
Lines 5 to 17 in 88cf00e
type NETWORK_TYPES = 'TestNetworks' | 'Production'; | |
const SERVICE_URL_BY_NETWORKTYPE: Record<NETWORK_TYPES, string> = { | |
TestNetworks: 'https://test.wrapped.litprotocol.com/encrypted', | |
Production: 'https://wrapped.litprotocol.com/encrypted', | |
}; | |
export const SERVICE_URL_BY_LIT_NETWORK: Record<SupportedNetworks, string> = { | |
[LitNetwork.DatilDev]: SERVICE_URL_BY_NETWORKTYPE.TestNetworks, | |
[LitNetwork.DatilTest]: SERVICE_URL_BY_NETWORKTYPE.TestNetworks, | |
[LitNetwork.Cayenne]: SERVICE_URL_BY_NETWORKTYPE.TestNetworks, | |
[LitNetwork.Manzano]: SERVICE_URL_BY_NETWORKTYPE.TestNetworks, | |
[LitNetwork.Habanero]: SERVICE_URL_BY_NETWORKTYPE.Production, | |
}; |
WDYT?
I just added one question about the staging.apis
domain vs. lit-general-worker
-- otherwise this looks good to me.
100% Update 1: Update 2: Update 3: |
…-a-map-of-network-names-to-rpc-url Feature/lit 3593 js sdk add a map of network names to rpc url
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.
I think our symbol
is backwards from what I see on chainlist -- otherwise this is GTG. Nice work on the constant maps <3 :)
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.
Almost there I think -- tstLPX/tstLIT is a bit of a mindbender. Just one other comment re: relayerUrl
handling & types
Co-authored-by: Daryl Collins <daryl@litprotocol.com> Signed-off-by: Anson <ansonox@gmail.com>
Co-authored-by: Daryl Collins <daryl@litprotocol.com> Signed-off-by: Anson <ansonox@gmail.com>
…://github.com/LIT-Protocol/js-sdk into feature/lit-3549-js-sdk-add-datiltest-support
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.
2 last changes I think, and a nit. We're almost there :D
Co-authored-by: Daryl Collins <daryl@litprotocol.com> Signed-off-by: Anson <ansonox@gmail.com>
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.
👍 LGTM, I think this is GTG 💖
Nice work <3
Signed-off-by: Anson <ansonox@gmail.com>
Description
Add support for
datil-test
npm tag
Test
❗️ All tests passed except wrapped keys tests, we need to adddatil-test
support in the following services:✅ (11 Jul 24) All tests related to these services now works!