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

WIP: Starknet Snap docs #1509

Merged
merged 100 commits into from
Oct 18, 2024
Merged

WIP: Starknet Snap docs #1509

merged 100 commits into from
Oct 18, 2024

Conversation

joaniefromtheblock
Copy link
Contributor

@joaniefromtheblock joaniefromtheblock commented Aug 28, 2024

Description

Draft of Starknet Snap docs.

Issue(s) fixed

Fixes #1511
Fixes #1512
Fixes #1513
Fixes #1514
Fixes #1515
Fixes #1516
Fixes #1526

Preview

Preview: https://metamask-docs-git-wip-starknet-snap-metamask-web.vercel.app/wallet/how-to/use-non-evm-networks/starknet/sign-starknet-data/
(If out of date, see the Vercel link at the bottom of the PR activity for the latest preview.)

Checklist

Complete this checklist before merging your PR:

  • If this PR contains a major change to the documentation content, I have added an entry to the top of the "What's new?" page.
  • The proposed changes have been reviewed and approved by a member of the documentation team.

Copy link

vercel bot commented Aug 28, 2024

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

Name Status Preview Comments Updated (UTC)
metamask-docs ✅ Ready (Inspect) Visit Preview 18 resolved Oct 18, 2024 5:58pm

@alexandratran alexandratran marked this pull request as draft August 28, 2024 19:42
@alexandratran alexandratran changed the title WIP: Starknet Snap Rough Skeleton WIP: Starknet Snap docs Sep 5, 2024
@Montoya
Copy link
Collaborator

Montoya commented Sep 6, 2024

Some notes on connect vs install:

  • We are moving away from using the term "install" with Snaps. We prefer "add" such as "add to MetaMask."
  • For the dapp, the primary action to initiate interacting with Starknet accounts is to connect to the Snap, just like a dapp connects with MetaMask to interact with Ethereum accounts. Whether the user has the Starknet Snap installed already is not important. If the user needs to install the Snap, they will be prompted to do so.
    • Despite this, we do need to explain that the user can reject the prompt to add the Snap to MetaMask and document what to expect (the response that the dapp will receive if the user rejects the request) and encourage the dapp to do something in that instance, like display a message to the user that they need to add the Snap to MetaMask in order to proceed.
    • Also, in terms of working with Starknet specifically, we may need to explain that a user will need to take some steps to set up a Starknet account before they can actually use it with the dapp, so the dapp should thoughtfully design that onboarding flow. Whether the user needs to add the Snap (and thus they will be completely new to Starknet) or they already have it but their account is not funded or deployed, the dapp should handle those scenarios.

Copy link
Contributor

@khanti42 khanti42 left a comment

Choose a reason for hiding this comment

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

We still need some changes on api commented with "needs check" because the api changed. I will do a separate review for them.

wallet/reference/non-evm-apis/starknet-snap-api.md Outdated Show resolved Hide resolved
wallet/reference/non-evm-apis/starknet-snap-api.md Outdated Show resolved Hide resolved
wallet/reference/non-evm-apis/starknet-snap-api.md Outdated Show resolved Hide resolved
wallet/reference/non-evm-apis/starknet-snap-api.md Outdated Show resolved Hide resolved
wallet/reference/non-evm-apis/starknet-snap-api.md Outdated Show resolved Hide resolved
wallet/reference/non-evm-apis/starknet-snap-api.md Outdated Show resolved Hide resolved
wallet/reference/non-evm-apis/starknet-snap-api.md Outdated Show resolved Hide resolved
wallet/reference/non-evm-apis/starknet-snap-api.md Outdated Show resolved Hide resolved
wallet/reference/non-evm-apis/starknet-snap-api.md Outdated Show resolved Hide resolved
@khanti42 khanti42 mentioned this pull request Sep 16, 2024
Copy link
Contributor

@alexandratran alexandratran left a comment

Choose a reason for hiding this comment

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

Some suggestions on the intro pages.

wallet/how-to/use-non-evm-networks/index.md Outdated Show resolved Hide resolved
wallet/how-to/use-non-evm-networks/index.md Outdated Show resolved Hide resolved
wallet/how-to/use-non-evm-networks/index.md Show resolved Hide resolved
wallet/how-to/use-non-evm-networks/index.md Show resolved Hide resolved
wallet/how-to/use-non-evm-networks/starknet/index.md Outdated Show resolved Hide resolved
wallet/how-to/use-non-evm-networks/starknet/index.md Outdated Show resolved Hide resolved
wallet/how-to/use-non-evm-networks/starknet/index.md Outdated Show resolved Hide resolved
wallet/how-to/use-non-evm-networks/starknet/index.md Outdated Show resolved Hide resolved
wallet/how-to/use-non-evm-networks/starknet/index.md Outdated Show resolved Hide resolved
Copy link
Contributor

@khanti42 khanti42 left a comment

Choose a reason for hiding this comment

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

LGTM

const sendStarknetTransaction = async (contractAddress, contractFuncName, contractCallData, address, chainId, maxFee = null) => {
const provider = await getEip6963Provider()
// Or window.ethereum.isMetaMask if you don't support EIP-6963.
const sendStarknetTransaction = async (address, chainId, contractAddress, entrypoint, calldata, maxFee = null) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Updated these params

Copy link
Contributor

@alexandratran alexandratran left a comment

Choose a reason for hiding this comment

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

LGTM if my edits are fine.

@joaniefromtheblock joaniefromtheblock merged commit 59f03b4 into main Oct 18, 2024
17 checks passed
@joaniefromtheblock joaniefromtheblock deleted the wip-starknet-snap branch October 18, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants