trezor / trezor-suite

Trezor Suite Monorepo
https://trezor.io/trezor-suite
Other
723 stars 251 forks source link

uiResponse doesn't target device #4864

Open floating opened 5 years ago

floating commented 5 years ago

For methods like getPublicKey you can target a devices with a "device" parameter. This doesn't seem to be working for the uiResponse method.

szymonlesisz commented 5 years ago

uiResponse shouldn't work at all with popup mode, because hosting page cannot be trusted. uiResponse can be used only in "trusted" mode (localhost, trezor.io origins OR electron self hosted connect)

floating commented 5 years ago

Yes, I am using it in trusted mode

szymonlesisz commented 5 years ago

could you provide some code example? because i don't understand the problem.

uiRequest is emitted from connect to hosting page and pending promise inside connect knows which device needs to be used by uiResponse, thats why you don't have to provide this info

floating commented 5 years ago

I see, I'm running into an error where uiResponse seems to be targeting the wrong device.

import TrezorConnect from 'trezor-connect'

const devices = []

const getPublicKey = (device, path = `m/44'/60'/0'/0`) => {
  TrezorConnect.getPublicKey({ device, path }).then(result => {
    console.log(device.label, result)
  }).catch(console.error)
}

TrezorConnect.on('DEVICE_EVENT', e => {
  console.log('DEVICE_EVENT', e.type, e.payload.label)
  if (e.type === 'device-connect') {
    devices.push(e.payload)
    getPublicKey(e.payload)
  }
})
TrezorConnect.on('UI_EVENT', e => {
  console.log('UI_EVENT', e.type, e)
})

const manifest = { email: 'test', appUrl: 'test' }
const connectSrc = 'https://sisyfos.trezor.io/connect-electron/'
const config = { connectSrc, manifest, popup: false, webusb: false, debug: false, lazyLoad: false }

TrezorConnect.init(config).then(() => {}).catch(err => console.warn('TrezorConnect Init Error', err))

window.inputPin = pin => TrezorConnect.uiResponse({ type: 'ui-receive_pin', payload: pin })

window.getStatus = () => devices.forEach(device => getPublicKey(device))

Steps: 1 - Plug in Trezor One 2 - Plug in Trezor T and input Trezor T pin 3 - Use inputPin to input pin for Trezor One

If I repeat these steps and skip step 2 everything works fine

szymonlesisz commented 5 years ago

great, i will have a look at it, but i will be able to add potential fix in the next major release. Im in the process of rewriting connect to multiple packages (node, react-native, electron, web) and thats the main task for now.

As a workaround i suggest you to call getPublicKey in async/await loop:, get PK of first device, then get PK of second device, like:

devices.forEach(async device => await getPublicKey(device))

floating commented 5 years ago

Great, glad to hear you're working on an upgrade path for node users.

As far as the workaround, the getStatus call is actually unrelated to the problem and not used in the steps to repeat. The issue is calling uiResponse after a different device has been connected.

szymonlesisz commented 5 years ago

ah, i get it, its because you are also calling "getPublicKey" right after device is connected. so the workaround should be like:

let yourDeferredPromise;

const getPublicKey = (device, path = `m/44'/60'/0'/0`) => {
  yourDeferredPromise = createDeferredPromise();
  TrezorConnect.getPublicKey({ device, path }).then(result => {
    console.log(device.label, result)
    yourDeferredPromise.resolve();
  }).catch(console.error)
}

TrezorConnect.on('DEVICE_EVENT', async e => {
  if (e.type === 'device-connect') {
    if (yourDeferredPromise) await yourDeferredPromise;
    getPublicKey(e.payload)
  }
})
floating commented 5 years ago

https://sisyfos.trezor.io/connect-electron is no longer available and broke the current build. What is the official connect source with electron event functionality?