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

chore(trezor): Upgrade Trezor Connect to v9 #74

Merged
merged 4 commits into from
Sep 7, 2022

Conversation

sime
Copy link
Contributor

@sime sime commented Jul 28, 2022

Quick POC of integrating Trezor Connect v9 beta.

v9 Docs: https://github.com/trezor/trezor-suite/tree/develop/packages/connect

Copy link

@Rob-Ferguson Rob-Ferguson left a comment

Choose a reason for hiding this comment

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

image

It looks like we should be using the @trezor/connect-web package instead.

@sime
Copy link
Contributor Author

sime commented Aug 4, 2022

Hi @Rob-Ferguson

I just pushed a new package.json with @trezor/connect-web.

Can I run this UI that you screenshotted locally?

Copy link

@Rob-Ferguson Rob-Ferguson left a comment

Choose a reason for hiding this comment

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

I pulled in the latest change and tried again, but I'm still seeing the same This package is not suitable to work with browser. Use @trezor/connect-web package instead error.

@Rob-Ferguson
Copy link

Can I run this UI that you screenshotted locally?

The screenshot is from running Caravan, our open-source multisig coordinator, locally on my machine. You should also be able to run it locally and (hopefully) recreate the issue yourself by following these steps:

  1. I'm assuming you already have unchained-wallets cloned locally, so you should just need to run npm run compile from that directory to compile the package.
  2. Clone the caravan repo locally (preferably under the same parent directory as unchained-wallets)
  3. Update the package.json in the caravan directory to point to the this compiled version of unchained-wallets (i.e. update the dependencies section to include "unchained-wallets": "file:../unchained-wallets"
  4. Run npm i and npm start from within the caravan directory.
  5. It should spin up a local Caravan instance at https://localhost:3000/caravan in the browser
  6. The Caravan test suite specifically (from screenshot) is at https://localhost:3000/caravan#/test (hosted version available here), which can also be accessed by clicking the Test Suite option from the dropdown menu at the top right.
  7. From there, load a Trezor device with the seed phrase provided on the test suite page, and you can run through each test manually to validate that the device firmware is producing the exact output our platform expects for various signature/address combinations. This is what the Unchained QA team runs to initially validate each new Trezor firmware release.

The issue I run into is after Caravan spins up, I try to detect a hardware device or start the test suite, and it throws the error immediately, rather than letting me proceed.

cc @waldenraines in case the above doesn't make sense.

@@ -23,7 +23,7 @@ import {
} from "./trezor";
import {ECPair, payments} from "bitcoinjs-lib";

const TrezorConnect = require("trezor-connect").default;
const TrezorConnect = require("@trezor/connect").default;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these be importing connect web?

@sime
Copy link
Contributor Author

sime commented Aug 9, 2022

Thanks for the tip @waldenraines. I pushed a change.

Would you mind trying the tests again @Rob-Ferguson, I have looked into running that UI you detailed.

Copy link

@Rob-Ferguson Rob-Ferguson left a comment

Choose a reason for hiding this comment

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

image

I pulled down the latest commit, rebuilt everything, and started Caravan locally again. This time, I was able to run through the entire test suite with a Trezor Model 1 without issue. 🎉

I noticed when the Trezor connect window would pop up that it did appear to be referencing the new version 9 - i.e., connect.trezor.io/9 instead of connect.trezor.io/8.

@sime
Copy link
Contributor Author

sime commented Aug 10, 2022

Hi @Rob-Ferguson are you doing anything special for open the window?

I see you have an environment variable ENV_TREZOR_CONNECT_URL, are you specifying anything there (just a guess)?

Misread your message, sorry!

@Hannsek
Copy link

Hannsek commented Aug 26, 2022

Hi @Rob-Ferguson,
I just want to let you know that we have released Connect v9. It is now GA. https://www.npmjs.com/package/@trezor/connect/v/9.0.0

@steveblackmon
Copy link
Contributor

travis integration seems to be broken at the moment, but i built this PR locally and confirmed it passed tests

@steveblackmon steveblackmon merged commit c759510 into unchained-capital:master Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants