Skip to content

feat: add padding#1959

Draft
letypequividelespoubelles wants to merge 7 commits into
mainfrom
1761-feat-padding-for-one-line-functions-in-zkc-constraints
Draft

feat: add padding#1959
letypequividelespoubelles wants to merge 7 commits into
mainfrom
1761-feat-padding-for-one-line-functions-in-zkc-constraints

Conversation

@letypequividelespoubelles

Copy link
Copy Markdown
Contributor

No description provided.

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>
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 DavePearce left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread pkg/ir/trace_builder.go
//
// 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such modules need a synthesized/precomputed padding row.

So, the question is where this comes from.

Comment thread pkg/trace/array_trace.go
p.height = nsize
}

// UseLastRowAsPadding sets each (already filled) column's padding value to its

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: padding for one-line functions in ZkC constraints

2 participants