HeliosLang / compiler

Helios is a DSL for writing Cardano smart contracts. This library lets you compile Helios scripts and build Cardano transactions.
https://www.hyperion-bt.org/helios-book
BSD 3-Clause "New" or "Revised" License
142 stars 31 forks source link

Assets should be sorted by length first, and then lexically. #113

Closed papag00se closed 11 months ago

papag00se commented 1 year ago

Hardware wallets like Ledger and Trezor reconstruct the transaction when signing. When they do this, the assets end up sorted differently than the Helios sort. When submitting a transaction that has multiple assets listed within a policy, hardware wallets fail because they produce a different txhash.

According to the CBOR spec, length should be considered first, then lexical order.

*  If two keys have different lengths, the shorter one 
sorts earlier;

*  If two keys have the same length, the one with the 
lower value in (byte-wise) lexical order sorts earlier.

When we replace the code in ByteArrayData.comp with the following code, the hardware wallet transactions are successful:

static comp(a, b) {
    if (a.length != b.length) {
        return a.length < b.length ? -1 : 1;
    } else {
        for (let i = 0; i < a.length; i++) {
            if (a[i] != b[i]) {
                return a[i] < b[i] ? -1 : 1;
            }
        }
        return 0;
    }
}
papag00se commented 1 year ago

I believe this is related to https://github.com/Hyperion-BT/helios/issues/92

christianschmitz commented 1 year ago

This is indeed related to #92, possibly not caught then because @lley154 was using token names of the same length.

I'll add a new static method to ByteArrayData called compLengthFirst.

Note: the current comp is also used for the UPLC builtins lessThanByteString and lessThanEqualsByteString, which is implemented according to the spec, which specifically gives an example of a bytestring longer than another sorting first (#23456789 < #24).

christianschmitz commented 12 months ago

v0.16.4 hopefully does this correctly

papag00se commented 11 months ago

Fixed!