search

trezor / python-trezor

:snake: Don't use this repo, use the new monorepo instead:
https://github.com/trezor/trezor-firmware
GNU Lesser General Public License v3.0
201 stars 194 forks source link

Change network to protocol magic in cardano #342

Closed refi93 closed 5 years ago

refi93 commented 5 years ago

Related to https://github.com/trezor/trezor-core/pull/417

prusnak commented 5 years ago

Depends on https://github.com/trezor/trezor-common/pull/244

matejcik commented 5 years ago

please wait until the trezor-common PR is accepted and then update the reference to trezor-common submodule

refi93 commented 5 years ago

@jpochyla I tried adding to test_cardano_sign_tx function (https://github.com/trezor/python-trezor/pull/342/files#diff-ecde254718349399eb838e55d428d744L89) the following swipe and button press event emulations, following your suggestion in https://github.com/trezor/trezor-core/pull/417#issuecomment-454422223:

    ...
    with client:
        client.set_expected_responses(expected_responses)
        client.debug.swipe_down()
        client.debug.press_yes()
        client.debug.swipe_down()
        client.debug.press_yes()

and Trezor during this test swipes only through the first screen, confirms it and hangs on the second screen, waiting for the second swipe, although I have there the second swipe_down instruction. Seems like the second swipe down and press_yes events happen prematurely or do not happen at all. I tried even adding time.sleep(1) in between them, as in other examples in the code, without success, which I kind of expected and understand, since the sleep happens before sending the transaction to Trezor. I also tried putting them in different places without success.

So how to properly make the second swipe and "hold to confirm"?

matejcik commented 5 years ago

@jpochyla I tried adding to test_cardano_sign_tx function (trezor/python-trezor/pull/342/files#diff-ecde254718349399eb838e55d428d744L89) the following swipe and button press event emulations, following your suggestion in trezor/trezor-core#417 (comment):

    ...
    with client:
        client.set_expected_responses(expected_responses)
        client.debug.swipe_down()
        client.debug.press_yes()
        client.debug.swipe_down()
        client.debug.press_yes()

that's not at all what the referenced code does though?

you need to put the input interactions into a separate "input flow" function, set it up as a receiver with client.set_input_flow, and use yield statements to wait for ButtonRequests instead of spamming all input instructions in one go

refi93 commented 5 years ago

@matejcik I see, thanks. So I tried it this way:

    ...
    def input_flow():
        client.debug.swipe_down()
        client.debug.press_yes()

        yield

        client.debug.swipe_down()
        client.debug.press_yes()

    with client:
        client.set_expected_responses(expected_responses)
        client.set_input_flow(input_flow)
        response = cardano.sign_tx(
            client, inputs, outputs, transactions, protocol_magic
        )
        assert response.tx_hash.hex() == tx_hash
        assert response.tx_body.hex() == tx_body

and Trezor still hangs on the second screen. It seems as if the control is never returned to input_flow() after the yield statement. I tried also adding sleep statements between the input instructions and moving the yield statement within the input_flow function, both without success. What am I missing? To be honest, I don't have much experience with Python, particularly with generator functions, therefore I can be doing a dumb mistake somewhere, so don't hesitate to point it out. Thanks!

matejcik commented 5 years ago

Most likely you need to yield at start of the input_flow() function; otherwise, the first two input calls run immediately. In general, each yield translates to "wait until ButtonRequest", so there should be as many yields as there are expected ButtonRequests, which in your case is 4?

refi93 commented 5 years ago

yes, I tried putting a yield at the beginning of the input_flow() function, but it also seems to hang and it doesn't get past that yield (tried throwing an exception right after the yield, which was not thrown). It seems as if no ButtonRequests were coming from Trezor

matejcik commented 5 years ago

that could be the case. run the test with pytest -v -s to see messages as they come and go. i might try and give your PR a go later. this needs https://github.com/trezor/trezor-core/pull/417 to work, correct?

refi93 commented 5 years ago

yes, and https://github.com/trezor/trezor-common/pull/244 , since there were changes to the message definitions

prusnak commented 5 years ago

I just merged https://github.com/trezor/trezor-common/pull/244 into master

matejcik commented 5 years ago

well, your code doesn't seem to be sending the ButtonRequests, so that's a problem right there.

refi93 commented 5 years ago

@matejcik thanks! I just added them to https://github.com/trezor/trezor-core/pull/417 and it works

refi93 commented 5 years ago

moreover, I added the test case from this unmerged PR: https://github.com/trezor/python-trezor/pull/329 so it can be closed after approving and merging this one

matejcik commented 5 years ago

will merge this when https://github.com/trezor/trezor-core/pull/417 is ready

refi93 commented 5 years ago

I recently added tests for cardano address validation - a recent issue we came across