Introduce OrderedMap into CBOR and use it in Cardano

[gabrielKerekes]

Ordering output multi asset policies and the tokens in them causes interoperability problems with cardano-cli because cardano-cli adds policies and tokens into the transaction as they come. Trezor sorts the dicts going into CBOR by default, which is in line with Canonical CBOR definition from the CBOR RFC. We're working on a CIP which would define canonical CBOR spec for Cardano which would be in line with what cardano-cli does and would standardise transaction serialisation format. Until this CIP is accepted, we should be including the assets in the order in which they came. That's what this PR does.

As Python's dicts don't preserve the order of items and OrderedDict from collections or ucollections is not available, we needed to introduce a new container, OrderedMap, which is a simple implementation of a KVP list which preserves the order of its items while being serialised into CBOR as a Map. The OrderedMap implements only the necessary functionality - it's in no way a complete container, but IMHO that is OK for our purposes. We weren't able to make OrderedMap correctly type annotated and generic i.e. class OrderedMap(Generic[K, V]). We tried doing something similar as is done in mathcheck.py but it didn't work since Generic[K, V] is not valid Python syntax. OrderedMap is now used in Cardano transaction signing for serialising policies, assets and also withdrawals, since these items might previously get reordered when being serialised. Since we don't want duplicate withdrawals to be included in CBOR we also validate that there are no duplicate paths for withdrawals - previously they were simply overwritten in the dict and it seems the validation should have been there also then.

It would be awesome if we could get this into the July release 🙏

[matejcik]

We tried doing something similar as is done in mathcheck.py but it didn't work since Generic[K, V] is not valid Python syntax

Generic[K, V] is perfectly valid:

K = V = 0
Generic = { (0, 0): object }

class X(Generic[K, V]):
	pass

[gabrielKerekes]

Generic[K, V] is perfectly valid:

Nice, thanks. Updated in 11dd985 and a68ee6a.

[matejcik]

please squash&rebase.
we removed the need for input_flows in paginated tests so you can get rid of that too.

as an aside, if you could check that new behavior in pagination (each paging event sends a ButtonRequest) works fine with Cardano wallets, that would be nice

[gabrielKerekes]

Squashed and rebased.

I tried generating the test fixtures twice - with an emulator running without animations and without any emulator running - however it changed all of the fixtures both times. Is this currently expected or should I play around with it?

I tried signing a transaction and deriving an address through Adalite and it worked fine with the physical device, but it was failing on the emulator - as soon as I swiped, the action failed. I used https://github.com/trezor/trezord-go and ./trezord-go -e 21324 to communicate with the emulator. Daedalus is currently syncing so I'll also try that when it's done.

[matejcik]

I tried generating the test fixtures twice - with an emulator running without animations and without any emulator running - however it changed all of the fixtures both times. Is this currently expected or should I play around with it?

something must be going wrong, because without emulator you can't generate the UI fixtures. (the images aren't exported from hardware)

[matejcik]

fixture added in 2491f87

[matejcik]

CI running in https://gitlab.com/satoshilabs/trezor/trezor-firmware/-/pipelines/326594654

[matejcik]

I tried signing a transaction and deriving an address through Adalite and it worked fine with the physical device, but it was failing on the emulator - as soon as I swiped, the action failed. I used trezor/trezord-go and ./trezord-go -e 21324 to communicate with the emulator.

Can't reproduce the problem, so I'm going to assume that this is some sort of glitch. If you managed to isolate it, that would be great though.

[matejcik]

CI green, merging.
thanks!

[gabrielKerekes]

something must be going wrong, because without emulator you can't generate the UI fixtures. (the images aren't exported from hardware)

When I said "without any emulator running", I meant that I had no emulator started up when calling make test_emu_ui_record - I suppose the command starts its own emulator if there isn't one already running?

I'll give it another try tomorrow. I'm recently switched from Linux to a M1 macbook with nix-shell so that might be a part of the problem.

Thanks for adding the fixture and for merging 👍

[matejcik]

I suppose the command starts its own emulator if there isn't one already running?

Exactly. I'm actually not sure what happens if there is an emulator running; I'd expect some sort of conflict and weird state.

Also to save time you can run only the tests that you need via pytest, using argument --ui=test for generating diff or --ui=record for recording the fixture.

For example this is what I do: pytest tests/device_tests -m cardano --ui=test

[gabrielKerekes]

Hi @matejcik, sorry for getting back to you a bit late. Thanks for the filtering tip, I'm not sure why I haven't used it with the UI tests.

I tried generating the fixtures again on the mac a couple of times, but all the fixtures always changed. I tried running with nix-shell and without it (although emulator was still built through nix-shell). It works fine on the Linux machine also with nix-shell. I haven't looked deeper into it as I don't understand enough details of what's going on. I'll play around with it again the next time I'll be generating the fixtures.