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

Open
wants to merge 40 commits into
base: main
Choose a base branch
from
Open

WIP: Starknet Snap docs #1509

wants to merge 40 commits into from

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 💬 9 unresolved
✅ 2 resolved
Oct 4, 2024 10:31pm

@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
</TabItem>
</Tabs>

## `starkNet_estimateAccountDeployFee`
Copy link
Contributor

Choose a reason for hiding this comment

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

needs check

Copy link
Contributor Author

@joaniefromtheblock joaniefromtheblock Sep 17, 2024

Choose a reason for hiding this comment

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

@khanti42 working on this time

</TabItem>
</Tabs>

## `starkNet_estimateFee`
Copy link
Contributor

Choose a reason for hiding this comment

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

needs check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@khanti42 working on this time

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

### Returns

The list of the stored user accounts.
Copy link
Contributor

Choose a reason for hiding this comment

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

There are two new optional return parameters in the account: upgradeRequired and deployRequired.

User accounts type is as follow :

export type AccContract = {
  addressSalt: string;
  publicKey: string; // in hex
  address: string; // in hex
  addressIndex: number;
  derivationPath: string;
  deployTxnHash: string; // in hex
  chainId: string; // in hex
  upgradeRequired?: boolean; // whether the account requires an upgrade to Cairo 1
  deployRequired?: boolean; // whether the account requires to be deployed
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@khanti42 I have questions about this one (raised in chat)

Choose a reason for hiding this comment

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

@khanti42 i think we dont need to mention this method, as it is a method to return the deployed + stored accounts for a single network, consider some edge case for Cairo Contract, it is better not have any starkNet_getStoredXXXX method mentioned in the page

cc @joaniefromtheblock

</TabItem>
</Tabs>

## `starkNet_signMessage`
Copy link
Contributor

Choose a reason for hiding this comment

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

needs check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@khanti42 working on this time

</TabItem>
</Tabs>

## `starkNet_verifyMessage`
Copy link
Contributor

Choose a reason for hiding this comment

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

needs check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@khanti42 working on this time

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
@joaniefromtheblock
Copy link
Contributor Author

joaniefromtheblock commented Sep 30, 2024

Still to do

Hi @khanti42, these are some action items (outstanding tasks) raised from our sync on Friday ( I might have missed a few, so please add if I forgot something):

  • Review wallet_invokeSnap code samples added to guide pages
  • Follow up on API methods that were flagged
  • Confirm tutorial steps
  • Investigation or decision on incorporating EIP-6963 (You mentioned this may be a Phase II activity)

So things @joaniefromtheblock needs to work on in the meanwhile:

  • Add screenshot for Snap connection
  • Adding screenshots for tutorial (if the build works)
  • Add note related to EIP-6963
  • Add token content to send transaction guide
  • Add wallet_invokeSnap alternative for all guides
  • Resolve all Vercel comments
  • Make draft timeline for docs release

Comment on lines +164 to +165
"unit": "wei",
"includeDeploy": true
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"unit": "wei",
"includeDeploy": true
"unit": "wei"

Comment on lines 130 to 132
### Returns

The response from Starknet's `feeder_gateway/estimate_fee` API endpoint.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### Returns
The response from Starknet's `feeder_gateway/estimate_fee` API endpoint.
### Returns
An object with the following properties (where values are originally `bigint` but converted to base-10 `string` format using `.toString(10)`):
- **`suggestedMaxFee`**: `string` - The maximum suggested fee for deploying the contract in wei. This value is originally a `bigint` and is converted to a base-10 `string` using `.toString(10)`, which ensures the number is expressed in decimal format.
- **`overallFee`**: `string` - The overall fee for the deployment transaction in wei. This value is originally a `bigint` and is converted to a base-10 `string` using `.toString(10)`, ensuring it is in decimal format.
- **`gasConsumed`**: `string` - The amount of gas consumed during the transaction. If not available, the default value is `'0'`. This value is originally a `bigint` and is converted to a base-10 `string` using `.toString(10)`.
- **`gasPrice`**: `string` - The gas price used for the transaction in wei. If not available, the default value is `'0'`. This value is originally a `bigint` and is converted to a base-10 `string` using `.toString(10)`.
- **`unit`**: `string` - The unit of the fees and gas values, which is `'wei'`.

Comment on lines 176 to 187
### Parameters

- `contractAddress`: `string` - Address of the target contract.
- `contractFuncName`: `string` - Target function name of the contract.
- `contractCallData`: `string` - (Optional) Call data for the target function with `,` as a separator.
- `senderAddress`: `string` - Address of the sender.
- `chainId`: `string` - (Optional) ID of the target Starknet network.
The default is the Starknet Goerli testnet.

### Returns

The response from Starknet's `feeder_gateway/estimate_fee` API endpoint.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### Parameters
- `contractAddress`: `string` - Address of the target contract.
- `contractFuncName`: `string` - Target function name of the contract.
- `contractCallData`: `string` - (Optional) Call data for the target function with `,` as a separator.
- `senderAddress`: `string` - Address of the sender.
- `chainId`: `string` - (Optional) ID of the target Starknet network.
The default is the Starknet Goerli testnet.
### Returns
The response from Starknet's `feeder_gateway/estimate_fee` API endpoint.
### Parameters
- `address`: `string` - The account address from which the transaction is being made.
- `invocations`: `array` - The invocations to estimate the fee for. Each invocation represents a contract call. Reference: [Invocations](https://starknetjs.com/docs/API/namespaces/types#invocations).
- `chainId`: `string` - The chain ID of the target Starknet network. If not provided, the default is the Starknet Goerli testnet.
- `details`: `object` - optional - The universal details associated with the invocations, such as nonce and version. Reference: [EstimateFeeDetails](https://starknetjs.com/docs/API/interfaces/types.EstimateFeeDetails).
### Returns
A promise that resolves to an `EstimateFeeResponse` object, which contains the following properties:
- `suggestedMaxFee`: `string` - The maximum suggested fee for the transaction in wei. This value is originally a `bigint` and is converted to a base-10 `string`.
- `overallFee`: `string` - The overall fee for the transaction in wei. This value is originally a `bigint` and is converted to a base-10 `string`.
- `unit`: `string` - The unit of the fees, typically `'wei'`.
- `includeDeploy`: `boolean` - Whether the transaction includes an account deployment step.

Comment on lines 195 to 210
await window.ethereum.request({
method: "wallet_invokeSnap",
params: {
snapId: "npm:@consensys/starknet-snap",
request: {
method: "starkNet_estimateFee",
params: {
contractAddress: "0x5B38Da6a701c568545dCfcB03FcB875f56beddC4",
contractFuncName: "transfer",
contractCallData: "0x456...,0x789...,100",
senderAddress: "0xb60e8dd61c5d32be8058bb8eb970870f07233155",
chainId: "0x1"
},
},
},
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
await window.ethereum.request({
method: "wallet_invokeSnap",
params: {
snapId: "npm:@consensys/starknet-snap",
request: {
method: "starkNet_estimateFee",
params: {
contractAddress: "0x5B38Da6a701c568545dCfcB03FcB875f56beddC4",
contractFuncName: "transfer",
contractCallData: "0x456...,0x789...,100",
senderAddress: "0xb60e8dd61c5d32be8058bb8eb970870f07233155",
chainId: "0x1"
},
},
},
})
const invocations: [
{
type: TransactionType.INVOKE,
payload: {
contractAddress:
'0x00b28a089e7fb83debee4607b6334d687918644796b47d9e9e38ea8213833137',
entrypoint: 'functionName',
calldata: ['1', '1'],
},
},
];
await window.ethereum.request({
method: "wallet_invokeSnap",
params: {
snapId: "npm:@consensys/starknet-snap",
request: {
method: "starkNet_estimateFee",
params: {
invocations,
address: "0xb60e8dd61c5d32be8058bb8eb970870f07233155",
chainId: "0x1",
},
},
},
})

<TabItem value="Request">

```js
await window.ethereum.request({
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
await window.ethereum.request({
const typedDataMessage = {
"types": {
"StarkNetDomain": [
{ "name": "name", "type": "felt" },
{ "name": "version", "type": "felt" },
{ "name": "chainId", "type": "felt" }
],
"Person": [
{ "name": "name", "type": "felt" },
{ "name": "wallet", "type": "felt" }
],
"Mail": [
{ "name": "from", "type": "Person" },
{ "name": "to", "type": "Person" },
{ "name": "contents", "type": "felt" }
]
},
"primaryType": "Mail",
"domain": {
"name": "Starknet Mail",
"version": "1",
"chainId": 1
},
"message": {
"from": {
"name": "Cow",
"wallet": "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826"
},
"to": {
"name": "Bob",
"wallet": "0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB"
},
"contents": "Hello, Bob!"
}
}
await window.ethereum.request({

Comment on lines +677 to +693
await window.ethereum.request({
method: "wallet_invokeSnap",
params: {
snapId: "npm:@consensys/starknet-snap",
request: {
method: "starkNet_sendTransaction",
params: {
contractAddress: "0x5B38Da6a701c568545dCfcB03FcB875f56beddC4",
contractFuncName: "transfer",
contractCallData: "0x456...,0x789...,100",
senderAddress: "0xb60e8dd61c5d32be8058bb8eb970870f07233155",
maxFee: "1000000000000000",
"chainId": "0x534e5f5345504f4c4941"
},
},
},
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
await window.ethereum.request({
method: "wallet_invokeSnap",
params: {
snapId: "npm:@consensys/starknet-snap",
request: {
method: "starkNet_sendTransaction",
params: {
contractAddress: "0x5B38Da6a701c568545dCfcB03FcB875f56beddC4",
contractFuncName: "transfer",
contractCallData: "0x456...,0x789...,100",
senderAddress: "0xb60e8dd61c5d32be8058bb8eb970870f07233155",
maxFee: "1000000000000000",
"chainId": "0x534e5f5345504f4c4941"
},
},
},
})
await window.ethereum.request({
method: "wallet_invokeSnap",
params: {
snapId: "npm:@consensys/starknet-snap",
request: {
method: "starkNet_executeTxn",
params: {
address: "0xb60e8dd61c5d32be8058bb8eb970870f07233155", // Sender's address
calls: [
{
contractAddress: "0x5B38Da6a701c568545dCfcB03FcB875f56beddC4", // Contract address
entrypoint: "transfer", // Function name
calldata: ["0x456...", "0x789...", "100"], // Function call data
}
],
details: {
maxFee: "1000000000000000", // Optional maximum fee
},
chainId: "0x534e5f5345504f4c4941", // Starknet Sepolia testnet (chain ID)
},
},
},
});

- Manages wallet connections and Starknet interactions.
- Provides results in more readable code.

The [`wallet_invokeSnap`](/snaps/reference/wallet-api-for-snaps/#wallet_invokesnap) method:
Copy link
Contributor

Choose a reason for hiding this comment

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

We can add a bullet point here that the wallet detection following the eip6963 standard needs to be implemented manually using for example this link : https://metamask.io/news/developers/how-to-implement-eip-6963-support-in-your-web-3-dapp/

Then in all the examples below we assume there is a provider object available which corresponds to the metamask object.


### Detect MetaMask

When using `wallet_invokeSnap`, use the following function to detect if MetaMask is installed:
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we should not describe how to handle eip6963 but instead tell the user that if they have connection issue with multiple wallet, this is most likely because they don't use eip6963 to handle the metamask detection. In that case they should refer to :
https://metamask.io/news/developers/how-to-implement-eip-6963-support-in-your-web-3-dapp/

Copy link
Contributor

Choose a reason for hiding this comment

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

This means we remove the helper function section.


### Verify Snap support

After detecting MetaMask, verify if it supports Snaps:
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we could specify in which circumstances there could be an issue. E.g. here its if the end users is trying to use the dApp with an old version of metamask that potentially does not support snaps. This is a way to test snap support in that case and prompt user to upgrade metamask.

Copy link

@stanleyyconsensys stanleyyconsensys left a comment

Choose a reason for hiding this comment

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

1st batch of the comment

Copy link

@stanleyyconsensys stanleyyconsensys left a comment

Choose a reason for hiding this comment

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

review comment batch 2

Comment on lines +40 to +69
const checkCurrentNetwork = async () => {
if (typeof window.ethereum !== "undefined" && window.ethereum.isMetaMask) {
try {
const response = await window.ethereum.request({
method: "wallet_invokeSnap",
params: {
snapId: "npm:@consensys/starknet-snap",
request: {
method: "starknet_getChainId"
}
}
});

let networkName;
switch (response) {
case "0x534e5f4d41494e":
networkName = "Mainnet";
break;
case "0x534e5f5345504f4c4941":
networkName = "Sepolia Testnet";
break;
default:
networkName = "Unknown Network";
}

console.log("Currently connected to:", networkName);
return response; // Returns the chain ID.
} catch (error) {
console.error("Error getting current Starknet network:", error);
throw error;

Choose a reason for hiding this comment

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

Suggested change
const checkCurrentNetwork = async () => {
if (typeof window.ethereum !== "undefined" && window.ethereum.isMetaMask) {
try {
const response = await window.ethereum.request({
method: "wallet_invokeSnap",
params: {
snapId: "npm:@consensys/starknet-snap",
request: {
method: "starknet_getChainId"
}
}
});
let networkName;
switch (response) {
case "0x534e5f4d41494e":
networkName = "Mainnet";
break;
case "0x534e5f5345504f4c4941":
networkName = "Sepolia Testnet";
break;
default:
networkName = "Unknown Network";
}
console.log("Currently connected to:", networkName);
return response; // Returns the chain ID.
} catch (error) {
console.error("Error getting current Starknet network:", error);
throw error;
const checkCurrentNetwork = async () => {
const provider = await getEip6963Provider()
if (provider) {
try {
const response = await provider.request({
method: "wallet_invokeSnap",
params: {
snapId: "npm:@consensys/starknet-snap",
request: {
method: "starkNet_getCurrentNetwork"
}
}
});
console.log("Currently connected to:", response.name);
return response.chainId; // Returns the chain ID.
} catch (error) {
console.error("Error getting current Starknet network:", error);
throw error;

we should always guide user to use eip6963 to get the provider, instead of using window.ethereum
hence i create a mock implementation const provider = await getEip6963Provider()

Copy link
Contributor Author

@joaniefromtheblock joaniefromtheblock Oct 3, 2024

Choose a reason for hiding this comment

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

This is a bit confusing @stanleyyconsensys. We can add a note to use the recommended EIP-6963, but for many of the other methods we are using window.ethereum. For consistency could we provide both, or maybe I can include a note to flag this,

console.log("Starknet Snap connected");

// Use the wallet_invokeSnap method to sign the transaction.
const response = await window.ethereum.request({

Choose a reason for hiding this comment

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

Suggested change
const response = await window.ethereum.request({
const response = await provider.request({

When connected to the [Starknet Snap](../../how-to/use-non-evm-networks/starknet/index.md), dapps
can use the Starknet Snap API to interact with users' Starknet accounts (for example, to send transactions).

Currently, the [`get-starknet`](https://github.com/starknet-io/get-starknet) library only supports the
Copy link

@stanleyyconsensys stanleyyconsensys Oct 1, 2024

Choose a reason for hiding this comment

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

do we really need to specific what SNAP api does get-starknet implemented

as the starknet community (builders) usually following the general standard on client side , which is get-starknet / starknet.js, they have no idea what Starknet Snap API is

i think we can change the angle, instead of telling ppl what SNAP API that we supported in get-starknet, we can tell ppl MetaMask Starknet Snap is almost fully support the functionality of get-starknet v3.3.0 (except on/off event listener)

cc @khanti42

Copy link
Collaborator

Choose a reason for hiding this comment

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

We want to be transparent about the two options that are available to dapp developers. So while we should recommend using get-starknet / starknet.js, we should still explain the alternative (wallet_invokeSnap & related methods).

Copy link

@stanleyyconsensys stanleyyconsensys Oct 2, 2024

Choose a reason for hiding this comment

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

@Montoya what i try to metion is using an API to represent a starknet network operation/function seem not that related

e.g get-starknet does not integrate starkNet_estimateFee, but it doesnt mean it cant do it, it is still possible to estimate the fee

so me and @khanti42 come up a conclusion that we use operation/method as the angle/entry, and each entity (get-starknet, snap), we attach the way to how to perform such action

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah great, that makes sense

Copy link

@stanleyyconsensys stanleyyconsensys left a comment

Choose a reason for hiding this comment

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

Add a new diagram to describe the architecture between Snap, Network, and get-starknet

Comment on lines +37 to +42
const showAccountInfo = async () => {
const account = await connectStarknetAccount();
if (account) {
document.getElementById("accountAddress").innerText = `Account Address: ${account}`;
}
};

Choose a reason for hiding this comment

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

Suggested change
const showAccountInfo = async () => {
const account = await connectStarknetAccount();
if (account) {
document.getElementById("accountAddress").innerText = `Account Address: ${account}`;
}
};
const connectStarknetAccount = async () => {
const starknet = await connect();
await starknet.enable(); // Prompts the user to connect their Starknet account using MetaMask
return starknet;
};
const showAccountInfo = async () => {
const starknet = await connectStarknetAccount();
if (account) {
document.getElementById("accountAddress").innerText = `Account Address: ${starknet.selectedAddress}`;
}
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants