-
Notifications
You must be signed in to change notification settings - Fork 28
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
fix: improve TON
signer and MyTonWallet
provider
#914
fix: improve TON
signer and MyTonWallet
provider
#914
Conversation
TON
signer and MyTonWallet
providerTON
signer and MyTonWallet
provider
901efcb
to
17ad1fa
Compare
export const TON_CONNECT_MANIFEST_URL = | ||
'https://raw.githubusercontent.com/rango-exchange/rango-types/main/assets/manifests/tonconnect-manifest.json'; |
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.
Please move default manifest to assets repo instead.
17ad1fa
to
8b3f804
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.
Thanks for your effort on this. I've put some comments to address.
Please also make sure you will let QA know to check playground and exporting config as well.
widget/app/package.json
Outdated
@@ -10,7 +10,7 @@ | |||
}, | |||
"browserslist": "> 0.5%, last 2 versions, not dead", | |||
"scripts": { | |||
"dev": "parcel -p 3002 --cache-dir=.parcel-cache --no-cache", | |||
"dev": "parcel -p 3000 --cache-dir=.parcel-cache --no-cache", |
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.
Why this needed to be changed?
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.
Any changes to the API base URL, port, or API key are necessary for testing the staging environment. However, these changes must be reverted before merging this pull request.
export function myTonWallet() { | ||
return window.mytonwallet?.tonconnect ?? null; | ||
} | ||
|
||
export function parseAddress(rawAddress: string): string { | ||
return Address.parse(rawAddress).toString({ bounceable: false }); |
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 guess it is a backend requirement but can explain why it should be non-bouncable? for the record.
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.
No, that's not exactly the reason. I checked with the backend team, and they confirmed it doesn't make a difference for them. However, based on the documentation, I updated the address to a non-bounceable format. Additionally, if you connect using the MyTonWallet
provider, you'll notice that the address shown in the provider extension now matches the one displayed in our app.
https://docs.ton.org/v3/documentation/smart-contracts/addresses#bounceable-vs-non-bounceable-addresses
This comment was marked as resolved.
This comment was marked as resolved.
59a893c
to
25f4b7a
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.
lgtm
3730062
to
575d599
Compare
Summary
TON
tokens are not being displayed correctly in the token listnon-bounceable
address for theMytonWallet
providerBOC
after signing theTON
transactionHow did you test this change?
TON
tokens in the token list. Ensure that if a token has an address or name, it is displayed correctly. You can verify this by checking the meta response from the network tab and comparing it with the UI.MytonWallet
provider. The address displayed in the confirmation wallet modal must match the address selected and shown in theMytonWallet
UI.TON
blockchain as the source. Sign the transaction and check the network tab to ensure that the hash of the message is sent to the server with thecheckStatus
request.Note
There are some considerations for using the provider API instead of the
tonconnect/sdk
. Since the main pull request was made over a year ago, I might not recall everything, but here are a few points:tonconnect/sdk
uses callbacks to handle wallet connection events, which added extra effort for implementation.Checklist: