Skip to content

Commit

Permalink
feat: sdk protocol update (#6755)
Browse files Browse the repository at this point in the history
* feat: connection workflow improvement and prevent double goBack issue

* feat: increase rpc timeout to 90s

* chore: typing error

* feat: update comm layer to 0.4.2

* feat: reset ready state on disconnect

* feat: update comm layer to 0.5.0

* chore: remove extra logs

* chore: comments

* build: update to comm layer 0.5.0
  • Loading branch information
abretonc7s authored Jul 12, 2023
1 parent 555d8cc commit afb3df9
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 75 deletions.
129 changes: 65 additions & 64 deletions app/core/SDKConnect/SDKConnect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ export class Connection extends EventEmitter2 {
this.setLoading(true);

this.remote = new RemoteCommunication({
platform: AppConstants.MM_SDK.PLATFORM,
platformType: AppConstants.MM_SDK.PLATFORM as 'metamask-mobile',
communicationServerUrl: AppConstants.MM_SDK.SERVER_URL,
communicationLayerPreference: CommunicationLayerPreference.SOCKET,
otherPublicKey,
Expand Down Expand Up @@ -258,6 +258,7 @@ export class Connection extends EventEmitter2 {
this.initialConnection = false;
this.otps = undefined;
}
this.isReady = false;
});

this.remote.on(
Expand Down Expand Up @@ -287,6 +288,22 @@ export class Connection extends EventEmitter2 {
this.approvalPromise = undefined;
}

// Make sure we always have most up to date originatorInfo even if already ready.
if (!updatedOriginatorInfo) {
console.warn(`Connection - clients_ready - missing originatorInfo`);
return;
}

this.originatorInfo = updatedOriginatorInfo;
updateOriginatorInfos({
channelId: this.channelId,
originatorInfo: updatedOriginatorInfo,
});

if (this.isReady) {
return;
}

Logger.log(
`Connection - clients_ready channel=${this.channelId} origin=${this.origin} initialConnection=${initialConnection} apiVersion=${apiVersion}`,
updatedOriginatorInfo,
Expand All @@ -296,11 +313,8 @@ export class Connection extends EventEmitter2 {
!this.initialConnection &&
this.origin === AppConstants.DEEPLINKS.ORIGIN_QR_CODE
) {
// clear previous pending approval
if (approvalController.get(this.channelId)) {
Logger.log(
`Connection - clients_ready - clearing previous pending approval`,
);
// cleaning previous pending approval
approvalController.reject(
this.channelId,
ethErrors.provider.userRejectedRequest(),
Expand Down Expand Up @@ -347,21 +361,6 @@ export class Connection extends EventEmitter2 {
this.sendAuthorized(true);
}

// Make sure we only initialize the bridge when originatorInfo is received.
if (!updatedOriginatorInfo) {
return;
}
this.originatorInfo = updatedOriginatorInfo;
updateOriginatorInfos({
channelId: this.channelId,
originatorInfo: updatedOriginatorInfo,
});

if (this.isReady) {
// Re-send otp message in case client didnd't receive disconnection.
return;
}

this.setupBridge(updatedOriginatorInfo);
this.isReady = true;
},
Expand All @@ -370,20 +369,17 @@ export class Connection extends EventEmitter2 {
this.remote.on(
EventType.MESSAGE,
async (message: CommunicationLayerMessage) => {
// Wait for bridge to be ready before handling messages.
while (!this.isReady) {
await wait(1000);
}

// TODO should probably handle this in a separate EventType.TERMINATE event.
// handle termination message
if (message.type === MessageType.TERMINATE) {
// Delete connection from storage
this.onTerminate({ channelId: this.channelId });
return;
}

// ignore anything other than RPC methods.
// ignore anything other than RPC methods
if (!message.method || !message.id) {
// ignore if it is protocol message
console.warn(
`Connection channel=${this.channelId} Invalid message format`,
message,
Expand Down Expand Up @@ -411,23 +407,22 @@ export class Connection extends EventEmitter2 {
this.requestsToRedirect[message?.id] = true;
}

// Wait for keychain to be unlocked before handling rpc calls.
const keyringController = (
Engine.context as { KeyringController: KeyringController }
).KeyringController;
await waitForKeychainUnlocked({ keyringController });

// Check if channel is permitted
this.setLoading(false);

// Wait for bridge to be ready before handling messages.
// It will wait until user accept/reject the connection request.
try {
await this.checkPermissions(message);
this.sendAuthorized();
this.setLoading(false);
if (needsRedirect) {
// Special case for eth_requestAccount, doens't need to queue because it comes from apporval request.
this.rpcQueueManager.add({
id: (message.id as string) ?? 'unknown',
method: message.method,
});
while (!this.isReady) {
await wait(1000);
}
this.sendAuthorized();
} catch (error) {
// Approval failed - redirect to app with error.
this.sendMessage({
Expand All @@ -442,6 +437,11 @@ export class Connection extends EventEmitter2 {
return;
}

this.rpcQueueManager.add({
id: (message.id as string) ?? 'unknown',
method: message.method,
});

// We have to implement this method here since the eth_sendTransaction in Engine is not working because we can't send correct origin
if (message.method === 'eth_sendTransaction') {
if (
Expand Down Expand Up @@ -509,7 +509,7 @@ export class Connection extends EventEmitter2 {
// Always disconnect - this should not happen, DAPP should always init the connection.
// A new channelId should be created after connection is removed.
// On first launch reconnect is set to false even if there was a previous existing connection in another instance.
// To avoid hanging on the socket forever, we automatically close it after 5seconds.
// To avoid hanging on the socket forever, we automatically close it.
this.removeConnection({ terminate: false });
});
}
Expand Down Expand Up @@ -547,6 +547,7 @@ export class Connection extends EventEmitter2 {
if (this.backgroundBridge) {
return;
}

this.backgroundBridge = new BackgroundBridge({
webview: null,
isMMSDK: true,
Expand Down Expand Up @@ -715,13 +716,12 @@ export class Connection extends EventEmitter2 {

async sendMessage(msg: any) {
const needsRedirect = this.requestsToRedirect[msg?.data?.id] !== undefined;
const rpcMethod = this.rpcQueueManager.getId(msg?.data?.id);
this.rpcQueueManager.remove(msg?.data?.id);
const queue = this.rpcQueueManager.get();

if (msg?.data?.id && queue[msg?.data?.id] !== undefined) {
this.rpcQueueManager.remove(msg?.data?.id);
}

Logger.log(
`Connection::sendMessage needsRedirect=${needsRedirect} rpcMethod=${rpcMethod}`,
msg,
);
this.remote.sendMessage(msg).catch((err) => {
console.warn(`Connection::sendMessage failed to send`, err);
});
Expand All @@ -734,26 +734,26 @@ export class Connection extends EventEmitter2 {

if (this.origin === AppConstants.DEEPLINKS.ORIGIN_QR_CODE) return;

waitForEmptyRPCQueue(this.rpcQueueManager)
.then(async () => {
// Queue might not be empty if it timedout ---Always force clear before to go back
this.rpcQueueManager.reset();
try {
await waitForEmptyRPCQueue(this.rpcQueueManager);
// Prevent double back issue android. (it seems that the app goes back randomly by itself)
if (wentBackMinimizer) {
// Skip, already went back.
return;
}

// Prevent double back issue android. (it seems that the app goes back randomly by itself)
if (wentBackMinimizer) {
// Skip, already went back.
return;
}
this.setLoading(false);
wentBackMinimizer = true;

this.setLoading(false);
Minimizer.goBack();
})
.catch((err) => {
console.warn(
`Connection::sendMessage error while waiting for empty rpc queue`,
err,
);
});
// Delay goback in order to display the feedback modal.
await wait(1000);
Minimizer.goBack();
} catch (err) {
console.warn(
`Connection::sendMessage wait for empty rpc queue error`,
err,
);
}
}
}

Expand Down Expand Up @@ -1238,10 +1238,11 @@ export class SDKConnect extends EventEmitter2 {
}
this.paused = false;
} else if (appState === 'background') {
// Reset wentBack state
wentBackMinimizer = true;
// Cancel rpc queue anytime app is backgrounded
this.rpcqueueManager.reset();
// TODO: remove below comments but currently keep for reference in case android double back issue still happens.
// wentBackMinimizer = true;
// // Cancel rpc queue anytime app is backgrounded
// this.rpcqueueManager.reset();

if (!this.paused) {
/**
* Pause connections after 20 seconds of the app being in background to respect device resources.
Expand Down
2 changes: 1 addition & 1 deletion app/core/SDKConnect/utils/wait.util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const waitForKeychainUnlocked = async ({
}
};

const MAX_QUEUE_LOOP = 50; // 50 seconds
const MAX_QUEUE_LOOP = 90; // seconds
export const waitForEmptyRPCQueue = async (manager: RPCQueueManager) => {
let i = 0;
let queue = Object.keys(manager.get());
Expand Down
12 changes: 6 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@
"@metamask/permission-controller": "~3.0.0",
"@metamask/phishing-controller": "^3.0.0",
"@metamask/preferences-controller": "^2.1.0",
"@metamask/sdk-communication-layer": "0.3.0",
"@metamask/sdk-communication-layer": "^0.5.0",
"@metamask/signature-controller": "^2.0.0",
"@metamask/swaps-controller": "^6.8.0",
"@metamask/transaction-controller": "4.0.0",
Expand Down Expand Up @@ -333,11 +333,11 @@
"zxcvbn": "4.4.2"
},
"devDependencies": {
"@actions/core": "^1.10.0",
"@actions/github": "^5.1.1",
"@babel/core": "^7.20.0",
"@babel/preset-env": "^7.20.0",
"@babel/runtime": "^7.20.0",
"@actions/core": "^1.10.0",
"@actions/github": "^5.1.1",
"@cucumber/message-streams": "^4.0.1",
"@cucumber/messages": "^22.0.0",
"@ethersproject/contracts": "^5.7.0",
Expand All @@ -355,14 +355,14 @@
"@storybook/addon-ondevice-actions": "^5.3.23",
"@storybook/addon-ondevice-knobs": "^5.3.25",
"@storybook/react-native": "^5.3.25",
"@testing-library/react-hooks": "^8.0.1",
"@testing-library/react": "14.0.0",
"@testing-library/react-hooks": "^8.0.1",
"@testing-library/react-native": "12.1.2",
"@types/enzyme": "^3.10.9",
"@types/is-url": "^1.2.30",
"@types/jest": "^27.2.0",
"@types/qs": "^6.9.7",
"@types/node": "^17.0.21",
"@types/qs": "^6.9.7",
"@types/react": "^17.0.11",
"@types/react-native": "^0.64.10",
"@types/react-native-background-timer": "^2.0.0",
Expand Down Expand Up @@ -432,8 +432,8 @@
"regenerator-runtime": "0.13.9",
"rn-nodeify": "10.3.0",
"stack-beautifier": "1.0.2",
"typescript": "^4.4.2",
"ts-node": "^10.5.0",
"typescript": "^4.4.2",
"wdio-cucumberjs-json-reporter": "^4.4.3",
"xml2js": "^0.5.0"
},
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5423,10 +5423,10 @@
resolved "https://registry.yarnpkg.com/@metamask/safe-event-emitter/-/safe-event-emitter-2.0.0.tgz#af577b477c683fad17c619a78208cede06f9605c"
integrity sha512-/kSXhY692qiV1MXu6EeOZvg5nECLclxNXcKCxJ3cXQgYuRymRHpdx/t7JXfsK+JLjwA1e1c1/SBrlQYpusC29Q==

"@metamask/sdk-communication-layer@0.3.0":
version "0.3.0"
resolved "https://registry.yarnpkg.com/@metamask/sdk-communication-layer/-/sdk-communication-layer-0.3.0.tgz#ee7eac4f108bcf5d2ea0edfb9fedd97d1bf10360"
integrity sha512-Hubhl7nbXzsxUKCVZb0ZCoSqM9srAIUF3mZsoFvaGVeBRgRICmyutrjF/hyczgoQ4QDxgT8C13YgjJRXSdrdGQ==
"@metamask/sdk-communication-layer@^0.5.0":
version "0.5.0"
resolved "https://registry.yarnpkg.com/@metamask/sdk-communication-layer/-/sdk-communication-layer-0.5.0.tgz#a2333ab1f0d96c52aed9f749e7539e9e927f53db"
integrity sha512-lcA9KpOM332oPhFZVsdeJYF1OJAQP7LmHCSrCOyBOLrvgaRhNxCSsgMid9shGuN1PVozDPEQXljRvJQOyfRpsg==
dependencies:
cross-fetch "^3.1.5"
date-fns "^2.29.3"
Expand Down

0 comments on commit afb3df9

Please sign in to comment.