-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Initial Multichain API docs #1621
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
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
For MetaMask, I do not believe we need to document the scenario where |
# Conflicts: # docusaurus.config.js # wallet-sidebar.js
…nto 1566-multichain # Conflicts: # docusaurus.config.js # wallet-sidebar.js
# Conflicts: # docs/whats-new.md
Sorry to change this, but the multichain API will essentially be rolling directly to production (vs Flask). We will still need to describe it as "experimental" in the API reference though. |
# Conflicts: # docs/whats-new.md
--- | ||
description: Learn about the Multichain API. | ||
sidebar_custom_props: | ||
flask_only: true |
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.
it's flask and beta soon i believe. @vandan
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.
To clarify, should we remove the flask label and replace it with a note that the feature is experimental / a beta release?
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.
Yes, let's remove the Flask label. We can just make a note that the APIs are considered experimental and may be subject to changes.
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.
Since we're on our way to hitting prod soon. Let's do no flask label, no beta label, just an experimental label. I think that's what Vandan is saying
scope: msg.data.params.scope, | ||
method: msg.data.params.notification.method, | ||
subscription: msg.data.params.notification.params.subscription, | ||
number: msg.data.params.notification.params.result.number |
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.
this is specifically for making eth_subscription events over wallet_notify easier to read. It will break if there are other wallet_notify events that are not eth_subscription. Perhaps we remove this block entirely, or make the guards more precise?
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.
If we remove, would we be removing the entire message handling block?
// Set up message listener for events.
extensionPort.onMessage.addListener((msg) => {
// Format wallet_notify events to be able to read them later.
if (msg.data.method === "wallet_notify") {
console.log("wallet_notify:", {
scope: msg.data.params.scope,
method: msg.data.params.notification.method,
subscription: msg.data.params.notification.params.subscription,
number: msg.data.params.notification.params.result.number
})
return;
}
console.log(msg.data)
})
Alternatively, we can specify that this example specifically makes eth_subscription
events easier to read.
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.
// Set up message listener for events.
extensionPort.onMessage.addListener((msg) => {
if (msg.data.method === "wallet_notify") {
console.log("wallet_notify:", msg.data.params)
return;
}
console.log(msg.data)
})
something like this?
my concern is that the following block will cause an exception if a wallet_notify event comes through that is not eth_subscription. Also given that this particular example is in the generic Multichain API examples, it may be nice for the listener example to also be ecosystem agnostic
if (msg.data.method === "wallet_notify") {
console.log("wallet_notify:", {
scope: msg.data.params.scope,
method: msg.data.params.notification.method,
subscription: msg.data.params.notification.params.subscription,
number: msg.data.params.notification.params.result.number
})
return;
}
}, | ||
"sessionProperties": { | ||
"expiry": "2022-12-24T17:07:31+00:00", | ||
"caip154-mandatory": "true" |
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.
@adonesky1 we technically don't enforce these yet
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.
Yeah let's remove this
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.
Should I remove the whole sessionProperties
or just "caip154-mandatory": "true"
?
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.
yeahh... hmm... let's remove sessionProperties
entirely until we actually support it to avoid confusion. Thank you for clarifying!
Description
Initial Multichain API docs.
Issue(s) fixed
Fixes #1566
Preview
(Links updated Mar 5)
Checklist
Complete this checklist before merging your PR: