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.

Sign up for GitHub

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

DAPP action replay improvements #16250

Merged
merged 12 commits into from
Nov 29, 2022
Merged

DAPP action replay improvements #16250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8a3bb80
DAPP action replay
jpuri Nov 14, 2022
7618a4f
Merge branch 'develop' into dapp_action_replay_improvements
jpuri Nov 14, 2022
b72b31c
Merge branch 'develop' into dapp_action_replay_improvements
jpuri Nov 21, 2022
e964439
Merge branch 'develop' into dapp_action_replay_improvements
jpuri Nov 21, 2022
9c9f8a4
fix
jpuri Nov 21, 2022
e5cbbbe
Merge branch 'develop' into dapp_action_replay_improvements
jpuri Nov 21, 2022
ab819c7
Merge branch 'develop' of github.com:MetaMask/metamask-extension into…
jpuri Nov 22, 2022
f128458
merge
jpuri Nov 24, 2022
5b664a9
Merge branch 'develop' into dapp_action_replay_improvements
jpuri Nov 24, 2022
44acf02
fix
jpuri Nov 24, 2022
e4bcda4
merge
jpuri Nov 28, 2022
50e2322
fix
jpuri Nov 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions app/scripts/contentscript.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,14 @@ const setupPageStreams = () => {
pageChannel = pageMux.createStream(PROVIDER);
};

// The field below is used to ensure that replay is done only once for each restart.
let METAMASK_EXTENSION_CONNECT_SENT = false;

const setupExtensionStreams = () => {
METAMASK_EXTENSION_CONNECT_SENT = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe METAMASK_EXTENSION_CONNECT_SENT -> METAMASK_EXTENSION_CAN_SEND_RETRY? thoughts?

cc: @naugtur

Copy link
Member

@Gudahtt Gudahtt Nov 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Agreed that calling this "connect sent" isn't ideal, because it doesn't really represent whether the extension has connected. After we send the retry command, it remains true that the extension has connected, but this is false.

Maybe instead of "can send retry", it should be "should send retry"? Or "send retry when ready"

OR maybe it would be simpler to flip this around. Use this boolean to track when the retry has already been sent, call it "METAMASK_EXTENSION_RETRY_SENT", and set it to false here. Then we can set to true in extensionStreamMessageListener

Copy link
Contributor

Choose a reason for hiding this comment

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

How about we call it what it is - a semaphore.

METAMASK_EXTENSION_CONNECT_SEMAPHORE

Copy link
Contributor

Choose a reason for hiding this comment

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

Code looks good to me but I agree with @Gudahtt 's comment about variable naming

extensionPort = browser.runtime.connect({ name: CONTENT_SCRIPT });
extensionStream = new PortStream(extensionPort);
extensionStream.on('data', extensionStreamMessageListener);

// create and connect channel muxers
// so we can handle the channels individually
Expand Down Expand Up @@ -522,6 +527,38 @@ function logStreamDisconnectWarning(remoteLabel, error) {
);
}

/**
* The function notifies inpage when the extension stream connection is ready. When the
* 'metamask_chainChanged' method is received from the extension, it implies that the
* background state is completely initialized and it is ready to process method calls.
* This is used as a notification to replay any pending messages in MV3.
*
* @param {object} msg - instance of message received
*/
function extensionStreamMessageListener(msg) {
if (
METAMASK_EXTENSION_CONNECT_SENT &&
digiwand marked this conversation as resolved.
Show resolved Hide resolved
isManifestV3 &&
msg.data.method === 'metamask_chainChanged'
digiwand marked this conversation as resolved.
Show resolved Hide resolved
) {
METAMASK_EXTENSION_CONNECT_SENT = false;
window.postMessage(
{
target: INPAGE, // the post-message-stream "target"
data: {
// this object gets passed to obj-multiplex
name: PROVIDER, // the obj-multiplex channel name
data: {
jsonrpc: '2.0',
method: 'METAMASK_EXTENSION_CONNECT_CAN_RETRY',
},
},
},
window.location.origin,
);
}
}

/**
* This function must ONLY be called in pump destruction/close callbacks.
* Notifies the inpage context that streams have failed, via window.postMessage.
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
"@metamask/permission-controller": "^1.0.0",
"@metamask/phishing-controller": "^1.0.0",
"@metamask/post-message-stream": "^6.0.0",
"@metamask/providers": "^10.0.0",
"@metamask/providers": "^10.2.1",
"@metamask/rate-limit-controller": "^1.0.0",
"@metamask/rpc-methods": "^0.24.1",
"@metamask/slip44": "^2.1.0",
Expand Down
18 changes: 9 additions & 9 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3183,10 +3183,10 @@
"@metamask/base-controller" "~1.0.0"
"@metamask/controller-utils" "~1.0.0"

"@metamask/providers@^10.0.0", "@metamask/providers@^10.2.0":
version "10.2.0"
resolved "https://registry.yarnpkg.com/@metamask/providers/-/providers-10.2.0.tgz#8131de667db0c55a61a150438c2a7f17b2d53615"
integrity sha512-qO3cOZZr/YJ8LLOqhR+51GGBiRknalfV/na7hwXyqZ1R/uxeLeNdqCyg+g8l3Z8JcLoEiaKGNJOEV3FFyLw8mQ==
"@metamask/providers@^10.2.0", "@metamask/providers@^10.2.1":
version "10.2.1"
resolved "https://registry.yarnpkg.com/@metamask/providers/-/providers-10.2.1.tgz#61304940adeccc7421dcda30ffd1d834273cc77b"
integrity sha512-p2TXw2a1Nb8czntDGfeIYQnk4LLVbd5vlcb3GY//lylYlKdSqp+uUTegCvxiFblRDOT68jsY8Ib1VEEzVUOolA==
dependencies:
"@metamask/object-multiplex" "^1.1.0"
"@metamask/safe-event-emitter" "^2.0.0"
Expand All @@ -3197,7 +3197,7 @@
fast-deep-equal "^2.0.1"
is-stream "^2.0.0"
json-rpc-engine "^6.1.0"
json-rpc-middleware-stream "^4.2.0"
json-rpc-middleware-stream "^4.2.1"
pump "^3.0.0"
webextension-polyfill-ts "^0.25.0"

Expand Down Expand Up @@ -16031,10 +16031,10 @@ json-rpc-middleware-stream@^2.1.1:
readable-stream "^2.3.3"
safe-event-emitter "^1.0.1"

json-rpc-middleware-stream@^4.2.0:
version "4.2.0"
resolved "https://registry.yarnpkg.com/json-rpc-middleware-stream/-/json-rpc-middleware-stream-4.2.0.tgz#a235814e031a2f85cc14b213aa7d42b75527104b"
integrity sha512-vaPaFVnhozfAVx6ImO3YuSrzl6A1OCktDi9Prw6NASn2VRYfe3pzv+2QGzFHRjViztr61oHi6KC3k8cujrfK7A==
json-rpc-middleware-stream@^4.2.0, json-rpc-middleware-stream@^4.2.1:
version "4.2.1"
resolved "https://registry.yarnpkg.com/json-rpc-middleware-stream/-/json-rpc-middleware-stream-4.2.1.tgz#e5cb8795ebfd7503c6ceaa43daaf065687cc2f22"
integrity sha512-6QKayke/8lg0nTjOpRCq4JCgRx7bVybldmloIfY21HSDV0GUevcV9i8DJNvuKTJx4KR9EDIf6HTStAnEovGUvA==
dependencies:
"@metamask/safe-event-emitter" "^2.0.0"
readable-stream "^2.3.3"
Expand Down