fix issue #224 : add field_tag into address representation

[DreamWuGit]

Try to resolve issue #224

[DreamWuGit]

@z2trillion @ed255 could you have a look ?

[ed255]

LGTM!

I think this is a nice solution for the issue :). Now the order of fields of a TxLog is set so that it can follow the lexicographic ordering without scrambling rows, because following this approach field_tag will have higher precedence than index (which was not the case if address = log_id || index). And since the address is also represented as 16 bit limbs columns in the state circuit, we can still do checks on log_id, field_tag and index individually without any penalty.

[z2trillion]

Isn't the ordering unchanged with this packing? Previously it was:

(tx_id, log_id, field_tag, index), 

and with this change it will be

(tx_id, log_id, field_tag, index, field_tag), 

which is equivalent?

(This is all in a world where privacy-ethereum/zkevm-circuits#566 gets merged in and restores the sort order of the keys to be key1, key2, key3, key4).

[ed255]

Isn't the ordering unchanged with this packing? Previously it was:

(tx_id, log_id, field_tag, index), 

and with this change it will be

(tx_id, log_id, field_tag, index, field_tag), 

which is equivalent?

This is correct, from the spec point of view. But in between we currently have this in the circuits:

(tx_id, log_id, index, field_tag)

See https://github.com/privacy-scaling-explorations/zkevm-circuits/blob/2aa9875d9bcf5dd8937af7d88f121dad1a8877ed/zkevm-circuits/src/evm_circuit/witness.rs#L766

AFAIK the aim here was to remove the index from the key4 (storageKey) which is RLC encoded (and also expressed as 8 bit limbs) and put it somewhere else that allows simpler raw access: the key2 (address) which is also expressed as 16 bit limbs (which is convenient because log_id, field_tag and index fit in 16 bits)

(This is all in a world where privacy-scaling-explorations/zkevm-circuits#566 gets merged in and restores the sort order of the keys to be key1, key2, key3, key4).

NOTE: I haven't taken a look yet at that PR.

[DreamWuGit]

I merged this PR change into #220 , closing this, any concern can discuss there if you have.