Support non-existing proofs in RwTable#907
Conversation
github-actions
Bot
added
crate-bus-mapping
| /// Config for StateCircuit | ||
| #[derive(Clone)] | ||
| pub struct StateCircuitConfig<F> { | ||
| selector: Column<Fixed>, // Figure out why you get errors when this is Selector. |
There was a problem hiding this comment.
Not sure what's the error, but perhaps because gates require to have a fixed column in the expression (lookups don't)?
There was a problem hiding this comment.
I originally left this comment. Gates require a fixed column, but Selector is a fixed column. I think the check in halo2 may not have taken this into account, but I have not investigated deeper.
silathdiir
force-pushed
the
feat/account-non-existing-proof
branch
from
3ce32a7 to
9a3ef55
Compare
|
@z2trillion Can you take a look into this PR when you have time? |
| /// Config for StateCircuit | ||
| #[derive(Clone)] | ||
| pub struct StateCircuitConfig<F> { | ||
| selector: Column<Fixed>, // Figure out why you get errors when this is Selector. |
There was a problem hiding this comment.
I originally left this comment. Gates require a fixed column, but Selector is a fixed column. I think the check in halo2 may not have taken this into account, but I have not investigated deeper.
| // For Rw::AccountStorage, identify if committed value or new value is zero. | ||
| // Will do lookup for ProofType::StorageDoesNotExist if both are zero, | ||
| // otherwise do lookup for ProofType::StorageChanged (either is non-zero). | ||
| is_initial_value_zero: Column<Advice>, |
There was a problem hiding this comment.
Can you combine these two with a BatchedIsZeroGadget?
There was a problem hiding this comment.
Fixed to merge both is_initial_value_zero and is_new_value_zero to one column is_non_exist as:
(F::one() - committed_value * committed_value.invert()) *
(F::one() - new_value * new_value.invert())
And it made degree reduce from 13 to 12.
There was a problem hiding this comment.
It would be better to use the BatchedIsZeroGadget in zkevm-circuits/src/evm_circuit/util/math_gadget.rs.
There was a problem hiding this comment.
I tried to replace this field with BatchedIsZeroGadget, but it seems that the first parameter of function BatchedIsZeroGadget::construct is evm_circuit::constraint_builder::ConstraintBuilder (not state_circuit::constraint_builder::ConstraintBuilder).
And it seems that the state_circuit::constraint_builder::ConstraintBuilder needs this StateCircuitConfig (this field belongs to) to construct here.
@z2trillion Any suggestion to refactor BatchedIsZeroGadget or state_circuit::constraint_builder::ConstraintBuilder? Thanks.
There was a problem hiding this comment.
Sorry about this. I didn't realize that BatchedIsZeroGadget requires the evm ConstraintBuilder to construct. Can you just add a todo here: "TODO: use BatchedIsZeroGadget here once it no longer requires the evm circuit constraint builder."
There was a problem hiding this comment.
Yes. I see. Thanks 🙏.
I will add TODO: use BatchedIsZeroGadget here once it no longer requires the evm circuit constraint builder. in the BALANCE circuit PR #683 (which will use this non-existing proof code).
…umn `is_non_exist`.
…` to one column.
4 participants
Close #895
Supposed to move privacy-ethereum/zkevm-specs#291 from spec to circuit.