feat: add padding#1959
Conversation
Signed-off-by: F Bojarski <ceciestunepoubelle@protonmail.ch>
Signed-off-by: F Bojarski <ceciestunepoubelle@protonmail.ch>
Signed-off-by: F Bojarski <ceciestunepoubelle@protonmail.ch>
Signed-off-by: F Bojarski <ceciestunepoubelle@protonmail.ch>
DavePearce
left a comment
There was a problem hiding this comment.
Overall, it seems reasonable from my perspective --- though it is getting complicated. The only concern I have is that I don't see any solution to the empty module case.
| // | ||
| // TODO: an empty copy-row module has no row to replicate, so it falls | ||
| // back to the constant padding value, which is generally not a valid | ||
| // witness. Such modules need a synthesized/precomputed padding row. |
There was a problem hiding this comment.
Such modules need a synthesized/precomputed padding row.
So, the question is where this comes from.
| p.height = nsize | ||
| } | ||
|
|
||
| // UseLastRowAsPadding sets each (already filled) column's padding value to its |
There was a problem hiding this comment.
Is there a specific reason to choose the final row? It doesn't matter, I'm just wondering.
| ) | ||
| // | ||
| if callee.IsAtomic() { | ||
| if callee.IsAtomic() && !callee.ContainsMemoryAccess() { |
There was a problem hiding this comment.
Hmmmmm, interesting one. We didn't have memory in zkASM, so I didn't come across this case before.
| // assignActivityColumn fills the single $ret activity column of an atomic | ||
| // memory-touching function with 1 on every (active) row and returns its name. | ||
| // Padding rows are left at the column's zero padding value, so $ret==0 there. | ||
| func (p *FullObserver[W, I, M]) assignActivityColumn(m machine.Module, |
There was a problem hiding this comment.
FYI --- I am rebuilding the FullObserver so this will change somewhat. Specifically, it will be moved into the constraints package as a standalone "post-processing" function.
No description provided.