Consensys / corset

4 stars 10 forks source link

Interleaved columns have values which are not in the source columns #100

Closed letypequividelespoubelles closed 1 month ago

letypequividelespoubelles commented 1 month ago

To reproduce : corset java constraints

Test successionOverlappingMstore() in MemoryTests.java, corset inspect module MMIO, columns VAL_ABC and VAL_ABC_NEW have values which are not in the source columns (VAL_A, VAL_B, VAL_C, VAL_A_NEW, VAL_B_NEW, VAL_C_NEW)

DavePearce commented 1 month ago

Connected with #94 ?

DavePearce commented 1 month ago

@letypequividelespoubelles Its not really enough information for me to reproduce this error. Can you include the trace that that you are running corset check with? And, ideally, a bit more info about which row/column in that file is causing the problem.

letypequividelespoubelles commented 1 month ago

Maybe just a display issue of corset inspect, as (presumably) in #112 ?

DavePearce commented 1 month ago

Ok, recreation steps:

  1. Checked out head of feat/issue-142/mmio (commit cc64d1d)
  2. Rebuild zkevm-constraints/zkevm.bin
  3. Within the inspector select mmio module, and goto column 97 (?)

Screenshot attached.

issue_100

The problematic number is the one starting 2668881309183831085987251822994260480

DavePearce commented 1 month ago

NOTES

DavePearce commented 1 month ago

Ok, so this is starting to look like a presentation error. The offending number does exist in the original trace file. Specifically, we have this equivalence:

2668881309183831085987251822994260480 = 0x202020202020202020202020202020

So, the remaining question is why this is not being displayed in the correct form.

DavePearce commented 1 month ago

Ok, for reasons unknown, the column base is set to Dec for these columns rather than Hex (as for other columns). I guess because they are computed columns?

DavePearce commented 1 month ago

It is possible to override the default base for an interleaved column like so:

(module test)
(defcolumns (A :byte) (B :byte))
(definterleaved (C :display :hex) (A B))

However, I think the default should be Hex anyway.

UPDATE: Yes Hex is the default for a column unless specified otherwise.