stable encoding of map

[tharvik]

Currently, encoder.handleMap generates an unstable output, as Value.MapKeys is used to retrieve the keys of a given map. To would allow to use map in many serialized state, such as conscensus in byzcoin contract.

[ineiti]

I'm not sure this is something that can and should be fixed in dedis/protobuf. Imagine you fix it, and somebody sends a data from javascript, but in the wrong order. Now if you suppose that the order should be correct on the receiving side, you're screwed again.

Proposed fix: add a CheckForMap method that recurses in the structure and checks if a map is present. Then use this method in ByzCoin when registering a contract, and throw an error if somebody uses it.

[tharvik]

Imagine you fix it, and somebody sends a data from javascript, but in the wrong order. Now if you suppose that the order should be correct on the receiving side, you're screwed again.

I don't really get what you mean

• javascript is not used for concensus so it cannot create a wrong serialisation for StateChange.Value
• no need to suppose that the order would be correct on the receiver side as map is an unsorted collection

What I'm trying to fix is that the following doesn't yield the same encoding, which is not really important when used for data transmission but is when using it for byte equality

Encode(map[string]bool{"a": true, "b": true})
Encode(map[string]bool{"b": true, "a": true})

[ineiti]

Yes, been bitten by that one, too. But from what I understand, you want to make protobuf.Encode always return the same bytestring, even when maps are involved. Which would lead to people using maps in contracts. Which would lead to javascript code sending ClientTransactions with these maps. Which would lead to confusion, unless you tell javascript to do the same encoding.

So it's easier not to use maps at all whenever the structure will be used in anything that involves hashing or such.

Makes more sense now?

[tharvik]

Yes, been bitten by that one, too. But from what I understand, you want to make protobuf.Encode always return the same bytestring, even when maps are involved. Which would lead to people using maps in contracts. Which would lead to javascript code sending ClientTransactions with these maps.

Correct; it is nicer to be able to serialise most basic data structures.

Which would lead to confusion, unless you tell javascript to do the same encoding.

Why would it lead to confusion? Or rather, what kind of confusion? I don't follow.

The serialisation is done by a single client, not redone on every node, so it doesn't need to be stable, only to be done once.

So it's easier not to use maps at all whenever the structure will be used in anything that involves hashing or such.

Yep, I intended this issue to be more of a "let's do it in a while" one, as it is not needed currently, but it would be a nice improvement IMO.

[ineiti]

It's a little bit artificial, but I can see something like this happen:

1. javascript sends a ClientTransaction with a protobuf-encoded map to be included in an instance. Only, javascript doesn't have the same order as protobuf.Encode
2. the contract decodes the structure containing the map, verifies it's correct, and then re-encodes it to store it in the instance. Only now, protobuf.Encode changed the order of the elements compared to how javascript sent them
3. the hash of the structure in javascript and the hash of the structure in the instance might now be different

Catching that error seems as difficult as catching the 'standard' map in the contract error...

And now you know why the Arguments are a key/value slice, and not a map ;)

So to me, an error being thrown when you register a contract that uses maps, is the best way...

[tharvik]

So to me, an error being thrown when you register a contract that uses maps, is the best way...

Closed in favor of https://github.com/dedis/cothority/issues/2256