feat(avm): bounded mle implementation #8668
Conversation
jeanmon
requested review from
IlyasRidhuan,
fcarreiro and
Maddiaa0
as code owners
| @@ -295,6 +295,7 @@ global AVM_VERIFICATION_KEY_LENGTH_IN_FIELDS: u32 = 2 + 16 * 4; | |||
| // To determine latest value, hover `COMPUTED_AVM_PROOF_LENGTH_IN_FIELDS` | |||
| // in barretenberg/cpp/src/barretenberg/vm/avm/generated/flavor.hpp | |||
| global AVM_PROOF_LENGTH_IN_FIELDS: u32 = 3817; | |||
| global AVM_PUBLIC_COLUMN_MAX_SIZE_LOG = 8; | |||
There was a problem hiding this comment.
Ahhhh this LOG is from Logarithm, right? I was confused for a while thinking about (unencrypted) logs. Maybe use LOG2 or LOGARITHM?
| const size_t n = evaluation_points.size(); | ||
| ASSERT(dim <= n); | ||
|
|
||
| std::span<const Fr, std::dynamic_extent> truncated_points = evaluation_points.first(dim); |
There was a problem hiding this comment.
Do you need to be explicit about dynamic extent? From https://en.cppreference.com/w/cpp/container/span it looks like it's the default, that'd mean you could do std::span<const Fr> truncated_points = evaluation_points.first(dim);
Also I wonder, do you need this function?
Cant you use the existing partial_evaluate_mle or evaluate_mle and create a span in the caller doing exactly std::span<const Fr> truncated_points = evaluation_points.first(dim); and passing that?
There was a problem hiding this comment.
Sort of along the lines of what @fcarreiro is saying, I wonder if this should just become the default behavior of evaluate_mle. Instead of explicitly providing a dim the function could just compute something like dim = msb(poly.start_offset() + size()) + 1 then perform the logic you've laid out here. This would in theory lead to even better efficiency unless your polys always have dim = AVM_PUBLIC_COLUMN_MAX_SIZE_LOG. In this case your tests could compare two equivalent polys, one of which takes advantage of Adams new structured poly and one of which doesn't.
| const size_t n = evaluation_points.size(); | ||
| ASSERT(dim <= n); | ||
|
|
||
| std::span<const Fr, std::dynamic_extent> truncated_points = evaluation_points.first(dim); |
There was a problem hiding this comment.
Sort of along the lines of what @fcarreiro is saying, I wonder if this should just become the default behavior of evaluate_mle. Instead of explicitly providing a dim the function could just compute something like dim = msb(poly.start_offset() + size()) + 1 then perform the logic you've laid out here. This would in theory lead to even better efficiency unless your polys always have dim = AVM_PUBLIC_COLUMN_MAX_SIZE_LOG. In this case your tests could compare two equivalent polys, one of which takes advantage of Adams new structured poly and one of which doesn't.
Resolves #8651