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

💉 Inject metadata into signer #3872

Merged
merged 23 commits into from
May 30, 2023
Merged

Conversation

WRadoslaw
Copy link
Contributor

@WRadoslaw WRadoslaw commented Nov 22, 2022

Warning
FOR THE QA TEAM
I broke my polkadot wallet a few times while working on this PR. I think this is fixed but when the time comes make sure to backup your accounts before testing it. Also for this reason this PR should be thoroughly tested with all supported browsers and wallets.

@vercel
Copy link

vercel bot commented Nov 22, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
dao ✅ Ready (Inspect) Visit Preview May 19, 2023 8:11am
pioneer-2 ✅ Ready (Inspect) Visit Preview May 19, 2023 8:11am
pioneer-2-storybook ✅ Ready (Inspect) Visit Preview May 19, 2023 8:11am

const sign = useCallback(() => send('SIGN'), [service])
const sign = useCallback(() => {
if (wallet && api) {
return wallet.updateMetadata(api.chainInfo).then(() => send('SIGN'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for clarification: will this ask the user to update metadata every time a tx is signed or does the signer silently disregard the request if it is the same already?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it will be silenced if the version of the specification for given rpc is the same as in the extension

Copy link
Member

@thesan thesan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WRadoslaw sorry it took me a while to get back to this one.

Nice work! I just made a few changes to still be able to fallback to the original ApiRx in one line change.

Hopefully the CI will pass 🤞

Copy link
Contributor

@traumschule traumschule left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested here 👍

@ivanturlakov
Copy link

Tested on https://dao-git-feature-inject-metadata-into-signer-joystream.vercel.app/
Mainnet

@thesan All is works as expected. I am not encountered any problems with updating metadata

Chrome

✅ Polkadot(successfully updated metadata/wallet is ok)
✅ SubWallet(successfully updated metadata/wallet is ok)
✅ Talisman(metadata up to date/app don’t ask to update)

Brave

✅ Polkadot(successfully updated metadata/wallet is ok)
✅ SubWallet(successfully updated metadata/wallet is ok)
✅ Talisman(successfully updated metadata/wallet is ok)

Firefox

✅ Polkadot(successfully updated metadata/wallet is ok)
✅ SubWallet(successfully updated metadata/wallet is ok)
✅ Talisman(successfully updated metadata/wallet is ok)

@thesan
Copy link
Member

thesan commented May 30, 2023

Great work @ivanturlakov 🙌

I wasn't expecting it to work with Talisman everywhere. I was just hopping it would not break the wallet.
Oh right! it doesn't update on Chrome because it's already up to date...

@thesan thesan merged commit 7c4ffc2 into dev May 30, 2023
@thesan thesan deleted the feature/inject-metadata-into-signer branch May 30, 2023 10:53
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.

4 participants