tact-lang / tact

Tact compiler main repository
https://tact-lang.org
MIT License
280 stars 56 forks source link

Map comparison can produce false-negative results #196

Open Gusarich opened 3 months ago

Gusarich commented 3 months ago

I've noticed that Tact allows to compare Maps with == and != operators while working on #195: https://github.com/tact-lang/tact/blob/7ef2ae17baef890d6489688d523bcdf77ce7b561/src/generator/writers/writeExpression.ts#L288-L293 If we look at the implementation of __tact_cell_eq_nullable (same for neq operation) we can see how exactly it compares maps in this case: https://github.com/tact-lang/tact/blob/7ef2ae17baef890d6489688d523bcdf77ce7b561/src/generator/writers/writeStdlib.ts#L941-L952

As you can see, it simply compares their hashes. This is fine for Cell comparisons, but not for Map because the same dictionaries in TVM can have many representations. In most cases it'll still work, but that's just because most serializers are written identically to the one from TON monorepo. It's still possible to break the comparison to produce false-negative results by serializing a map manually or by changing the logic of serializer in some library.

We should either remove the ability to compare maps or highlight that behaviour for non-empty maps.

anton-trunov commented 3 months ago

We should either remove the ability to compare maps or highlight that behaviour for non-empty maps.

Removing a feature is not backwards compatible, we should fix this by making sure that for each key-value entry in both maps those are equal and document that comparing maps for equality/non-equality can be computationally expensive as it requires map traversals.

anton-trunov commented 2 months ago

@Gusarich Can you actually provide a counter-example? We will put it into the test suite when there is a PR resolving it

Gusarich commented 2 months ago

@Gusarich Can you actually provide a counter-example? We will put it into the test suite when there is a PR resolving it

Smallest example. We have two maps:

const d1 = beginCell()
    .storeUint(2, 2) // long label
    .storeUint(8, 4) // key length
    .storeUint(1, 8) // key
    .storeBit(true) // value
    .endCell();

const d2 = beginCell()
    .storeUint(0, 1) // short label
    .storeUint(0b111111110, 9) // key length
    .storeUint(1, 8) // key
    .storeBit(true) // value
    .endCell();

both of them represent the following dictionary: { 1 => true }, however, they're serialized differently. d1 uses long label for the key and d2 uses short label.

Now let's suppose we have a contract that checks their equity:

message CheckMapsEquality {
    d1: map<Int as uint8, Bool>;
    d2: map<Int as uint8, Bool>;
}

contract Issue196 {
    init() {}
    receive(msg: CheckMapsEquality) {
        require(msg.d1 == msg.d2, "maps are not equal");
    }
}

And then we call it with these two map cells:

await treasure.send({
    to: contract.address,
    value: toNano("0.1"),
    body: beginCell()
        .storeUint(2573430445, 32)
        .storeMaybeRef(d1)
        .storeMaybeRef(d2)
        .endCell(),
    init: contract.init,
});

The call results maps are not equal error.

The reason why I called the contract manually by sending the message from treasury instead of using contract.send(...) is that because in the second case it would requite me to provide Dictionary objects from ton-core instead of cells and then ton-core would just serialize them on its own.