Skip to content

Conversation

@kronosapiens
Copy link

@kronosapiens kronosapiens commented Dec 3, 2025

  • Add Controller to the list of valid wallets
  • Make the downloads property of WalletProvider optional

@kronosapiens kronosapiens force-pushed the maint/add-controller branch 3 times, most recently from 7507876 to faedf90 Compare December 3, 2025 19:10
@fracek
Copy link
Collaborator

fracek commented Dec 4, 2025

It's not clear how people benefit from this change. The discovery package is used for dapps to display a list of wallets users can download, so removing that property defeats the purpose (it's also a breaking change).

I think you should open an issue and we can discuss how to best server non-downloadable wallets in the next version of get-starknet?

@kronosapiens
Copy link
Author

kronosapiens commented Dec 4, 2025

Hi @fracek,

I made this change in response to Ohad's advice on how to integrate Controller into the Starkgate bridge. See his full message below:

Our "Connect wallet" functionality on StarkGate is built by Dynamic on top of get-starknet (https://github.com/starknet-io/get-starknet).

To get the Cartridge wallet displayed as a connectivity option it needs to:

Conform to the Starknet Wallet API (https://github.com/starknet-io/get-starknet/blob/master/packages/core/documentation/walletAPIdocumentation.md#wallet-api-version-).

Be listed in the get-starknet core discovery file (https://github.com/starknet-io/get-starknet/blob/master/packages/core/src/discovery.ts)

Be integrated by Dynamic. For this we can get you in contact with them and also push from our side, and it's expected to take 2-4 weeks from the time it's supported in get-starknet.

@fracek
Copy link
Collaborator

fracek commented Dec 8, 2025

Looks good to me. We need the following changes:

  • fix the tests/ci
  • generate a changefile. I think we should have a minor bump since the api changed

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