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

Add keplr to InjectedConnector #141

Closed
wants to merge 1 commit into from

Conversation

Thunnini
Copy link

Issue / feature description

keplr now supports starknet. Therefore, I would like starknetkit to support keplr.

Changes

  • Add keplr to defaultConnectors
  • Add keplr logo

Checklist

  • Rebased to the last commit of the target branch (or merged)
  • Code self-reviewed
  • Code self-tested
  • Tests updated (if needed)
  • All tests are passing locally

Please keep your pull request as small as possible. If you need to make multiple changes, please create separate pull requests for each change. Create a draft pull request if you need early feedback. This will mark the pull request as a work in progress and prevent it from being reviewed or merged until you are ready.

Please move drafts to the ready for review (regular PR) when you are ready to start the review process and other developers will pick it up from there.

@bluecco
Copy link
Contributor

bluecco commented Oct 17, 2024

Did some tests and leaving some comments:

  • this pr won't be merged at this moment, because it will change the default connectors list
  • keplr can be already used by initializing the injected connector
    • new InjectedConnector({ options: { id: "keplr" } })
  • using starknetkit connect method, it needs to be passed to the connectors option
const res = await connect({
      connectors: [
        new InjectedConnector({ options: { id: "argentX" } }),
        new InjectedConnector({ options: { id: "braavos" } }),
        new InjectedConnector({ options: { id: "keplr" } }),
        new ArgentMobileBaseConnector({
          dappName: "Starknetkit example dapp",
          url: window.location.hostname,
          chainId: CHAIN_ID,
          icons: [],
        }),
        new WebWalletConnector({ url: ARGENT_WEBWALLET_URL }),
      ],
      modalMode: "alwaysAsk",
    })

    const { wallet, connectorData, connector } = res
  • discovery is not working for keplr at the moment, we need to wait for @starknet-io/get-starknet-core to be updated for this

@Thunnini
Copy link
Author

@bluecco Then, can you leave out the default connectors list part and just add the icon part?

@bluecco
Copy link
Contributor

bluecco commented Oct 17, 2024

@Thunnini yes, adding in #142
will do some tests on the pre-release first

@bluecco
Copy link
Contributor

bluecco commented Oct 17, 2024

closing this in favour of #142
published new version of starknetkit
can be installed with starknetkit@latest (v2.3.3)

  • can connect with keplr
  • can send a transaction if the account is deployed

there are 2 issues on wallet side:

  • cannot sign a message
  • cannot perform a transaction if the account is undeployed

@bluecco bluecco closed this Oct 17, 2024
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.

2 participants