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
BREAKING: Remove wallet
global in favor of snaps
and ethereum
#939
Conversation
552a4b5
to
09d913d
Compare
3d9c99d
to
a2d249c
Compare
wallet
global in favor of snaps
and ethereum
wallet
global in favor of snaps
and ethereum
wallet
global in favor of snap
and ethereum
a2d249c
to
18c8a47
Compare
packages/snaps-controllers/src/services/iframe/IframeExecutionService.test.ts
Outdated
Show resolved
Hide resolved
packages/snaps-controllers/src/services/node/NodeProcessExecutionService.test.ts
Outdated
Show resolved
Hide resolved
packages/snaps-controllers/src/services/node/NodeThreadExecutionService.test.ts
Outdated
Show resolved
Hide resolved
packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts
Outdated
Show resolved
Hide resolved
packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts
Outdated
Show resolved
Hide resolved
packages/snaps-execution-environments/src/common/endowments/index.ts
Outdated
Show resolved
Hide resolved
packages/snaps-execution-environments/src/common/endowments/index.ts
Outdated
Show resolved
Hide resolved
packages/snaps-types/src/types.d.ts
Outdated
@@ -22,7 +22,9 @@ export type OnCronjobHandler = (args: { | |||
request: JsonRpcRequest<unknown[] | { [key: string]: unknown }>; | |||
}) => Promise<unknown>; | |||
|
|||
export type SnapProvider = MetaMaskInpageProvider; | |||
export type SnapAPI = { request: StreamProvider['request'] }; |
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.
Maybe "SnapsProvider"?
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.
afaik we specifically wanted to move away from calling it a provider
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.
"SnapsAPI"?
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.
Maybe? 🤔
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.
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.
SnapsGlobalObject
, perhaps? I'm inspired by the TC39 spec, where e.g. the JSON
global is just called "The JSON object". In natural language we should refer to the SnapsGlobalObject
as "the snaps
object" or "the global snaps
object".
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.
Wait, this seems to imply that the object should be called snaps
. Is that intended?
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.
SnAPI
f74b474
to
93771da
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. Just confirm with @rekmarks if it should be named SnapsObject
or SnapObject
before merging.
wallet
global in favor of snap
and ethereum
wallet
global in favor of snaps
and ethereum
hi, can i please ask why adding the it's hard to imagine a snap so simple that it can get away without invoking |
This was an oversight in the current version of Flask. We plan to make it a regular, not scary, permission before release to MetaMask extension. |
@firnprotocol This should be fixed in the latest version of Flask released to both the Chrome and FF stores! |
@FrederikBolding cool, thank you; i can confirm that it's been fixed. unfortunately I can't proceed with development whatsoever until this discussion is resolved. this also means I will miss the deadline for auditing and getting "allowlisted" for snaps v1, which is a disappointment. |
Removes the
wallet
object. Replacing it withsnaps
andethereum
.snaps
is always exposed to a snap whileethereum
requires a new permissionendowment:ethereum-provider
.Fixes #908
Fixes #891