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

feat: externally_connectable CAIP delivery and enveloping #25075

Merged
merged 57 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
e0c7edb
WIP
jiexi Jun 3, 2024
370dc06
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 3, 2024
c1ae3db
WIP
jiexi Jun 5, 2024
61e9697
WIP PortStream bypass sanity check (working)
jiexi Jun 5, 2024
1f8cfd2
WIP wrapped stream (working)
jiexi Jun 5, 2024
5db9a97
cleanup inpage
jiexi Jun 5, 2024
6ef86a4
DRY caip stream
jiexi Jun 5, 2024
97469f9
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 5, 2024
1f58e7e
Rename. WIP spec
jiexi Jun 5, 2024
a9b741f
add SplitStream specs
jiexi Jun 5, 2024
7df64f3
add CaipToMultiplexStream, MultiplexToCaipStream specs
jiexi Jun 5, 2024
7db9d75
lint
jiexi Jun 5, 2024
65c9d68
WIP createCaipStream spec
jiexi Jun 5, 2024
c8b66b5
add createCaipStream specs
jiexi Jun 6, 2024
bbb33ae
lint
jiexi Jun 6, 2024
edab0fe
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 6, 2024
040d300
add BARAD_DUR flag
jiexi Jun 6, 2024
2acb719
dry background trackDappView
jiexi Jun 6, 2024
9792594
move externally_connectable manifest wildcard behind BARAD_DUR
jiexi Jun 6, 2024
7372604
jsdoc
jiexi Jun 6, 2024
b82888b
restore inpage
jiexi Jun 11, 2024
64ca655
Move caip stream closer to provider. Replace caip<->multiplex transfo…
jiexi Jun 11, 2024
714d044
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 11, 2024
29ea68c
Rename connectExternalDapp to connectExternalCaip
jiexi Jun 11, 2024
64ba990
actually restore inpage
jiexi Jun 11, 2024
46bfee7
Fix createCaipStream specs
jiexi Jun 11, 2024
e24ef63
use createDeferredPromise instead
jiexi Jun 11, 2024
379ce8a
rename onData to readFromStream
jiexi Jun 12, 2024
98ff148
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 12, 2024
cd3dd02
fix method names. add setupUntrustedCommunicationCaip
jiexi Jun 12, 2024
7f1eb8a
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 12, 2024
cbfd014
lint
jiexi Jun 12, 2024
5d65634
Merge remote-tracking branch 'origin/jl/mmp-2528/externally_connectab…
jiexi Jun 12, 2024
3652ab3
lint
jiexi Jun 12, 2024
a239e64
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 13, 2024
4f4f999
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 14, 2024
5b667f2
Rename connectExternalLegacy to connectExternalExtension
jiexi Jun 14, 2024
0c3f000
rename _subjectType to inputSubjectType
jiexi Jun 14, 2024
ab7767b
use messenger where possible
jiexi Jun 18, 2024
7fc0b23
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 18, 2024
ccd10ab
Rename setupUntrustedCommunicationLegacy to setupUntrustedCommunicati…
jiexi Jun 18, 2024
2eeac73
lint
jiexi Jun 18, 2024
600daed
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 21, 2024
ee4dec5
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 21, 2024
dc6c189
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 21, 2024
d27526a
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 24, 2024
571cf9e
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 25, 2024
1ced8f3
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 25, 2024
487b9ca
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 26, 2024
323ab1e
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 26, 2024
7d3d5f6
Bifurcate streams
jiexi Jun 26, 2024
2258ee7
Merge branch 'develop' into jl/mmp-2528/externally_connectable-caip-e…
jiexi Jun 26, 2024
3c8b832
fix specs
jiexi Jun 26, 2024
439f27e
Update app/scripts/metamask-controller.js
jiexi Jun 26, 2024
9f35203
remove requestAccountTabIds for CAIP
jiexi Jun 26, 2024
fa34c32
Merge remote-tracking branch 'origin/jl/mmp-2528/externally_connectab…
jiexi Jun 26, 2024
c8ce2f8
lint
jiexi Jun 26, 2024
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
6 changes: 6 additions & 0 deletions app/manifest/v2/_barad_dur.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"externally_connectable": {
"matches": ["http://*/*", "https://*/*"],
"ids": ["*"]
}
}
6 changes: 6 additions & 0 deletions app/manifest/v3/_barad_dur.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"externally_connectable": {
"matches": ["http://*/*", "https://*/*"],
"ids": ["*"]
}
}
170 changes: 108 additions & 62 deletions app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import debounce from 'debounce-stream';
import log from 'loglevel';
import browser from 'webextension-polyfill';
import { storeAsStream } from '@metamask/obs-store';
import { hasProperty, isObject } from '@metamask/utils';
import { isObject } from '@metamask/utils';
///: BEGIN:ONLY_INCLUDE_IF(snaps)
import { ApprovalType } from '@metamask/controller-utils';
///: END:ONLY_INCLUDE_IF
Expand Down Expand Up @@ -188,7 +188,8 @@ const sendReadyMessageToTabs = async () => {

// These are set after initialization
let connectRemote;
let connectExternal;
let connectExternalExtension;
let connectExternalCaip;

browser.runtime.onConnect.addListener(async (...args) => {
// Queue up connection attempts here, waiting until after initialization
Expand All @@ -201,7 +202,13 @@ browser.runtime.onConnectExternal.addListener(async (...args) => {
// Queue up connection attempts here, waiting until after initialization
await isInitialized;
// This is set in `setupController`, which is called as part of initialization
connectExternal(...args);
const port = args[0];

if (port.sender.tab?.id && process.env.BARAD_DUR) {
adonesky1 marked this conversation as resolved.
Show resolved Hide resolved
connectExternalCaip(...args);
} else {
connectExternalExtension(...args);
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we need the legacy API in all cases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be clear, there are 3 APIs now. Legacy, Current/EIP1193, CAIP

When you say Legacy, I assume you are referring to the Current/EIP1193.

I don't believe we are serving EIP1193 over externally_connectable connections from webpages. @adonesky1

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah. @shanejonas just to clarify, this browser.runtime.onConnectExternal.addListener(... block only for externally_connectable connections and we don't need to serve the EIP1193 provider to any consumers over that transport

}
});

function saveTimestamp() {
Expand Down Expand Up @@ -468,44 +475,73 @@ export async function loadStateFromPersistence() {
* which should only be tracked only after a user opts into metrics and connected to the dapp
*
* @param {string} origin - URL of visited dapp
* @param {object} connectSitePermissions - Permission state to get connected accounts
* @param {object} preferencesController - Preference Controller to get total created accounts
*/
function emitDappViewedMetricEvent(
origin,
connectSitePermissions,
preferencesController,
) {
function emitDappViewedMetricEvent(origin) {
const { metaMetricsId } = controller.metaMetricsController.state;
if (!shouldEmitDappViewedEvent(metaMetricsId)) {
return;
}

// A dapp may have other permissions than eth_accounts.
// Since we are only interested in dapps that use Ethereum accounts, we bail out otherwise.
if (!hasProperty(connectSitePermissions.permissions, 'eth_accounts')) {
const permissions = controller.controllerMessenger.call(
'PermissionController:getPermissions',
origin,
);
const numberOfConnectedAccounts =
permissions?.eth_accounts?.caveats[0]?.value.length;
if (!numberOfConnectedAccounts) {
return;
}

const numberOfTotalAccounts = Object.keys(
preferencesController.store.getState().identities,
).length;
const connectAccountsCollection =
connectSitePermissions.permissions.eth_accounts.caveats;
if (connectAccountsCollection) {
const numberOfConnectedAccounts = connectAccountsCollection[0].value.length;
controller.metaMetricsController.trackEvent({
event: MetaMetricsEventName.DappViewed,
category: MetaMetricsEventCategory.InpageProvider,
referrer: {
url: origin,
},
properties: {
is_first_visit: false,
number_of_accounts: numberOfTotalAccounts,
number_of_accounts_connected: numberOfConnectedAccounts,
},
});
const preferencesState = controller.controllerMessenger.call(
'PreferencesController:getState',
);
const numberOfTotalAccounts = Object.keys(preferencesState.identities).length;

controller.metaMetricsController.trackEvent({
event: MetaMetricsEventName.DappViewed,
category: MetaMetricsEventCategory.InpageProvider,
referrer: {
url: origin,
},
properties: {
is_first_visit: false,
number_of_accounts: numberOfTotalAccounts,
number_of_accounts_connected: numberOfConnectedAccounts,
},
});
}

/**
* Track dapp connection when loaded and permissioned
*
* @param {Port} remotePort - The port provided by a new context.
*/
function trackDappView(remotePort) {
if (!remotePort.sender || !remotePort.sender.tab || !remotePort.sender.url) {
return;
}
const tabId = remotePort.sender.tab.id;
const url = new URL(remotePort.sender.url);
const { origin } = url;

// store the orgin to corresponding tab so it can provide infor for onActivated listener
if (!Object.keys(tabOriginMapping).includes(tabId)) {
tabOriginMapping[tabId] = origin;
}

const isConnectedToDapp = controller.controllerMessenger.call(
'PermissionController:hasPermissions',
origin,
);

// when open a new tab, this event will trigger twice, only 2nd time is with dapp loaded
const isTabLoaded = remotePort.sender.tab.title !== 'New Tab';

// *** Emit DappViewed metric event when ***
// - refresh the dapp
// - open dapp in a new tab
if (isConnectedToDapp && isTabLoaded) {
emitDappViewedMetricEvent(origin);
}
}

Expand Down Expand Up @@ -709,27 +745,7 @@ export function setupController(
const url = new URL(remotePort.sender.url);
const { origin } = url;

// store the orgin to corresponding tab so it can provide infor for onActivated listener
if (!Object.keys(tabOriginMapping).includes(tabId)) {
tabOriginMapping[tabId] = origin;
}
const connectSitePermissions =
controller.permissionController.state.subjects[origin];
// when the dapp is not connected, connectSitePermissions is undefined
const isConnectedToDapp = connectSitePermissions !== undefined;
// when open a new tab, this event will trigger twice, only 2nd time is with dapp loaded
const isTabLoaded = remotePort.sender.tab.title !== 'New Tab';

// *** Emit DappViewed metric event when ***
// - refresh the dapp
// - open dapp in a new tab
if (isConnectedToDapp && isTabLoaded) {
emitDappViewedMetricEvent(
origin,
connectSitePermissions,
controller.preferencesController,
);
}
trackDappView(remotePort);

remotePort.onMessage.addListener((msg) => {
if (
Expand All @@ -740,22 +756,56 @@ export function setupController(
}
});
}
connectExternal(remotePort);
connectExternalExtension(remotePort);
}
};

// communication with page or other extension
connectExternal = (remotePort) => {
connectExternalExtension = (remotePort) => {
const portStream =
overrides?.getPortStream?.(remotePort) || new PortStream(remotePort);
controller.setupUntrustedCommunicationEip1193({
connectionStream: portStream,
sender: remotePort.sender,
});
};

connectExternalCaip = async (remotePort) => {
if (metamaskBlockedPorts.includes(remotePort.name)) {
return;
}

// this is triggered when a new tab is opened, or origin(url) is changed
if (remotePort.sender && remotePort.sender.tab && remotePort.sender.url) {
const tabId = remotePort.sender.tab.id;
const url = new URL(remotePort.sender.url);
const { origin } = url;

trackDappView(remotePort);

// TODO: remove this when we separate the legacy and multichain rpc pipelines
Copy link
Member

Choose a reason for hiding this comment

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

Curious to know what this refers to, i.e. what does it mean to "separate the legacy and multichain RPC pipelines", and how is this requestAccountTabIds state related?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Gudahtt we are intending to create a separate JSON RPC pipeline for the new API (ticket here) for long term maintainability

Copy link
Contributor

Choose a reason for hiding this comment

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

@Gudahtt just want to confirm you've seen and understood this roadmap?

remotePort.onMessage.addListener((msg) => {
if (
msg.type === 'caip-x' &&
jiexi marked this conversation as resolved.
Show resolved Hide resolved
msg.data &&
msg.data.method === MESSAGE_TYPE.ETH_REQUEST_ACCOUNTS
) {
requestAccountTabIds[origin] = tabId;
}
});
}

const portStream =
overrides?.getPortStream?.(remotePort) || new PortStream(remotePort);
controller.setupUntrustedCommunication({

controller.setupUntrustedCommunicationCaip({
connectionStream: portStream,
sender: remotePort.sender,
});
};

if (overrides?.registerConnectListeners) {
overrides.registerConnectListeners(connectRemote, connectExternal);
overrides.registerConnectListeners(connectRemote, connectExternalExtension);
}

//
Expand Down Expand Up @@ -1050,11 +1100,7 @@ function onNavigateToTab() {
// when the dapp is not connected, connectSitePermissions is undefined
const isConnectedToDapp = connectSitePermissions !== undefined;
if (isConnectedToDapp) {
emitDappViewedMetricEvent(
currentOrigin,
connectSitePermissions,
controller.preferencesController,
);
emitDappViewedMetricEvent(currentOrigin);
}
}
}
Expand Down
44 changes: 37 additions & 7 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ import {
getSmartTransactionsOptInStatus,
getCurrentChainSupportsSmartTransactions,
} from '../../shared/modules/selectors';
import { createCaipStream } from '../../shared/modules/caip-stream';
import { BaseUrl } from '../../shared/constants/urls';
import { BalancesController as MultichainBalancesController } from './lib/accounts/BalancesController';
import {
Expand Down Expand Up @@ -4863,17 +4864,21 @@ export default class MetamaskController extends EventEmitter {
* @param {MessageSender | SnapSender} options.sender - The sender of the messages on this stream.
* @param {string} [options.subjectType] - The type of the sender, i.e. subject.
*/
setupUntrustedCommunication({ connectionStream, sender, subjectType }) {
setupUntrustedCommunicationEip1193({
connectionStream,
sender,
subjectType,
}) {
const { completedOnboarding } = this.onboardingController.store.getState();
const { usePhishDetect } = this.preferencesController.store.getState();

let _subjectType;
let inputSubjectType;
if (subjectType) {
_subjectType = subjectType;
inputSubjectType = subjectType;
} else if (sender.id && sender.id !== this.extension.runtime.id) {
_subjectType = SubjectType.Extension;
inputSubjectType = SubjectType.Extension;
} else {
_subjectType = SubjectType.Website;
inputSubjectType = SubjectType.Website;
}

if (usePhishDetect && completedOnboarding && sender.url) {
Expand Down Expand Up @@ -4901,7 +4906,7 @@ export default class MetamaskController extends EventEmitter {
this.setupProviderConnection(
mux.createStream('metamask-provider'),
sender,
_subjectType,
inputSubjectType,
);

// TODO:LegacyProvider: Delete
Expand All @@ -4911,6 +4916,31 @@ export default class MetamaskController extends EventEmitter {
}
}

/**
* Used to create a CAIP stream for connecting to an untrusted context.
*
* @param options - Options bag.
* @param {ReadableStream} options.connectionStream - The Duplex stream to connect to.
* @param {MessageSender | SnapSender} options.sender - The sender of the messages on this stream.
* @param {string} [options.subjectType] - The type of the sender, i.e. subject.
*/

setupUntrustedCommunicationCaip({ connectionStream, sender, subjectType }) {
let inputSubjectType;
if (subjectType) {
inputSubjectType = subjectType;
} else if (sender.id && sender.id !== this.extension.runtime.id) {
inputSubjectType = SubjectType.Extension;
} else {
inputSubjectType = SubjectType.Website;
}

const caipStream = createCaipStream(connectionStream);

// messages between subject and background
this.setupProviderConnection(caipStream, sender, inputSubjectType);
}

/**
* Used to create a multiplexed stream for connecting to a trusted context,
* like our own user interfaces, which have the provider APIs, but also
Expand Down Expand Up @@ -5112,7 +5142,7 @@ export default class MetamaskController extends EventEmitter {
* @param connectionStream
*/
setupSnapProvider(snapId, connectionStream) {
this.setupUntrustedCommunication({
this.setupUntrustedCommunicationEip1193({
connectionStream,
sender: { snapId },
subjectType: SubjectType.Snap,
Expand Down
Loading