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

[SDK+UI+REACT-UI+PROTOCOL]: Improve network support in TC2 #32

Closed
shaharyakir opened this issue Feb 16, 2023 · 4 comments
Closed

[SDK+UI+REACT-UI+PROTOCOL]: Improve network support in TC2 #32

shaharyakir opened this issue Feb 16, 2023 · 4 comments

Comments

@shaharyakir
Copy link

Improve network support in TC2

Current state

Upon connection, TC2 reports back the network ID.
However, the user could be using a dapp that "thinks" it's in testnet mode, not paying attention that the wallet reported back is actually connected to a mainnet wallet.

This is a source for UX confusion, and a boilerplate effort for dapp owners to handle network mismatches.

image
In this example, the dapp could easily ignore the network mismatch.

Proposed solution

  1. Add a network parameter (default: mainnet) to the TonConnect (and TonConnectUIProvider etc.) constructor
  2. Persist different storage keys per network, so that if the app "thinks" it's in testnet, it cannot restore a mainnet connection and vice-versa
  3. Provide the network parameter to the wallet via the bridge, so wallets are encouraged to prompt the user to switch between networks.
  4. Throw an error from TonConnect in case a different network was reported back from the wallet than the one requested.
@siandreev
Copy link
Contributor

Hi @shaharyakir , thanks for your suggestion! If I got you right, there is no big difference for dApp where to declare/check chain_id. In the current protocol implementation dApp should check chain_id like this

connector.onStatusChange(wallet => {
	if (wallet?.account.chain === CHAIN.TESTNET) {
	    // handle wrong chain connection, show corresponding UI
		alert('Please change selected network to the Mainnet in the wallet to use the dApp')
	}
})

Your suggestion:

	connector.connect(connectionSource, { chain: CHAIN.MAINNET })

Thus, in both cases, the dApp needs to explicitly declare the required chain. That's why I think the suggestion won't make a valuable improvement.

@shaharyakir
Copy link
Author

Consider that a dapp will never want to receive a mainnet connection when it was looking for testnet, so having TonConnect throw an error in that case is valuable on its own.

Second, passing the chain id to the constructor is easier and more explicit in its intent than listening.

Third, passing the chain id via the bridge lets the wallet handle this case and prompt the user to change networks in the wallet.

All these remove unnecessary burden from the dapp developers.

We have a handful of wallets and only one TC2 spec, but there would be hundreds or thousands of dapps.

And if we don't implement this, at least some dapps are bound to make the mistake of not handling these nuances correctly ( + the duplicated effort of doing so).

@shaharyakir
Copy link
Author

@siandreev

@siandreev
Copy link
Contributor

In the current model, the wallet simply tells the dapp which accounts the user has. In the future, this will allow us to make a convenient system for changing accounts in the dapp. SDK will automatically add account and network information to action requests. Therefore, the suggestion to specify the network in the connection does not seem necessary to us.

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

No branches or pull requests

2 participants