Skip to content

Remove unused RPC handler hook#20580

Closed
rekmarks wants to merge 3 commits intodevelopfrom
remove-unused-rpc-handler-hook
Closed

Remove unused RPC handler hook#20580
rekmarks wants to merge 3 commits intodevelopfrom
remove-unused-rpc-handler-hook

Conversation

@rekmarks
Copy link
Copy Markdown
Member

Explanation

Removes an unused RPC handler hook that became unused in #20376. Also adds a check to fail immediately at runtime if extraneous hooks are provided, to prevent this from happening again.

Pre-merge reviewer checklist

  • Manual testing (e.g. pull and build branch, run in browser, test code being changed)
  • PR is linked to the appropriate GitHub issue
  • IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

@rekmarks rekmarks requested a review from a team as a code owner August 24, 2023 06:12
@rekmarks rekmarks added the team-extension-platform Extension Platform team label Aug 24, 2023
legobeat

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

@legobeat legobeat left a comment

Choose a reason for hiding this comment

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

Failing unit tests


  403 passing (5s)
  3 failing

  1) MetaMaskController
       MetaMaskController Behaviour
         #setupUntrustedCommunication
           adds a tabId and origin to requests:
     Error: Received unexpected hooks:

setApprovalFlowLoadingText
showApprovalSuccess
showApprovalError
getNetworkConfigurations

      at createMethodMiddleware (app/scripts/lib/rpc-method-middleware/createMethodMiddleware.js:10:1)
      at MetamaskController.currUseCurrencyRateCheck [as setupProviderEngine] (app/scripts/metamask-controller.js:747:60)
      at MetamaskController.setupProviderConnection (app/scripts/metamask-controller.js:713:39)
      at MetamaskController.setupUntrustedCommunication (app/scripts/metamask-controller.js:655:21)
      at Context.<anonymous> (app/scripts/metamask-controller.test.js:1067:11)
      at processImmediate (node:internal/timers:476:21)

  2) MetaMaskController
       MetaMaskController Behaviour
         #setupUntrustedCommunication
           should add only origin to request if tabId not provided:
     Error: Received unexpected hooks:

setApprovalFlowLoadingText
showApprovalSuccess
showApprovalError
getNetworkConfigurations

      at createMethodMiddleware (app/scripts/lib/rpc-method-middleware/createMethodMiddleware.js:10:1)
      at MetamaskController.currUseCurrencyRateCheck [as setupProviderEngine] (app/scripts/metamask-controller.js:747:60)
      at MetamaskController.setupProviderConnection (app/scripts/metamask-controller.js:713:39)
      at MetamaskController.setupUntrustedCommunication (app/scripts/metamask-controller.js:655:21)
      at Context.<anonymous> (app/scripts/metamask-controller.test.js:1109:11)
      at processImmediate (node:internal/timers:476:21)

  3) MetaMaskController
       MetaMaskController Behaviour
         #setupTrustedCommunication
           sets up controller JSON-RPC api for trusted communication:
     Error: Received unexpected hooks:

setApprovalFlowLoadingText
showApprovalSuccess
showApprovalError
getNetworkConfigurations

      at createMethodMiddleware (app/scripts/lib/rpc-method-middleware/createMethodMiddleware.js:10:1)
      at MetamaskController.currUseCurrencyRateCheck [as setupProviderEngine] (app/scripts/metamask-controller.js:747:60)
      at MetamaskController.setupProviderConnection (app/scripts/metamask-controller.js:713:39)
      at MetamaskController.chunk [as setupTrustedCommunication] (app/scripts/metamask-controller.js:670:5)
      at Context.<anonymous> (app/scripts/metamask-controller.test.js:1149:24)
      at processImmediate (node:internal/timers:476:21)



Error: Exited with code '1'
    at runInShell (/home/circleci/project/development/lib/run-command.js:134:29)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async runMocha (/home/circleci/project/test/run-unit-tests.js:81:3)
    at async start (/home/circleci/project/test/run-unit-tests.js:183:7)

Exited with code exit status 1

@github-actions
Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@github-actions
Copy link
Copy Markdown
Contributor

This PR has been automatically marked as stale because it has not had recent activity in the last 60 days. It will be closed in 14 days. Thank you for your contributions.

@github-actions github-actions bot added the stale issues and PRs marked as stale label Oct 28, 2023
@legobeat
Copy link
Copy Markdown
Contributor

@rekmarks ?

@github-actions github-actions bot removed the stale issues and PRs marked as stale label Oct 30, 2023
@gauthierpetetin
Copy link
Copy Markdown
Contributor

Hi @rekmarks , is this ticket ready for review?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 3, 2024

This PR has been automatically marked as stale because it has not had recent activity in the last 60 days. It will be closed in 14 days. Thank you for your contributions.

@github-actions github-actions bot added the stale issues and PRs marked as stale label Feb 3, 2024
@github-actions
Copy link
Copy Markdown
Contributor

This PR was closed because there has been no follow up activity in the last 14 days. Thank you for your contributions.

@github-actions github-actions bot closed this Feb 17, 2024
@rekmarks
Copy link
Copy Markdown
Member Author

rekmarks commented May 2, 2024

Superseded by #24357

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale issues and PRs marked as stale team-extension-platform Extension Platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants