feat(avm)!: Note hash exists opcode#15699
Conversation
sirasistant
requested review from
IlyasRidhuan,
LeilaWang,
Maddiaa0,
dbanks12,
fcarreiro and
jeanmon
as code owners
sirasistant
removed request for
IlyasRidhuan,
LeilaWang,
Maddiaa0,
dbanks12,
fcarreiro and
jeanmon
|
|
||
| pol PREV_LEAF_VALUE_UNIQUE_NOTE_HASH_DIFF = prev_leaf_value - unique_note_hash; | ||
| pol commit prev_leaf_value_unique_note_hash_diff_inv; | ||
| sel * (PREV_LEAF_VALUE_UNIQUE_NOTE_HASH_DIFF * (exists * (1 - prev_leaf_value_unique_note_hash_diff_inv) + prev_leaf_value_unique_note_hash_diff_inv) - 1 + exists) = 0; |
There was a problem hiding this comment.
I'd comment "exists = 1 if prev_leaf_value == unique_note_hash"
| auto& memory = context.get_memory(); | ||
| get_gas_tracker().consume_gas(); | ||
|
|
||
| auto slot = memory.get(slot_addr); | ||
| set_and_validate_inputs(opcode, { slot }); | ||
|
|
||
| get_gas_tracker().consume_gas(); | ||
|
|
There was a problem hiding this comment.
I see consume_gas before set_and_validate_inputs in some* cases....
There was a problem hiding this comment.
That's wrong (before) most likely. Gas comes after registers.
There was a problem hiding this comment.
Yes I saw that and thought it could create trouble
| pol commit note_hash_leaf_in_range; | ||
|
|
||
| pol LEAF_INDEX_GTE_NOTE_HASH_LEAF_COUNT = register[1] - constants.NOTE_HASH_TREE_LEAF_COUNT; | ||
| pol LEAF_INDEX_LT_NOTE_HASH_LEAF_COUNT = constants.NOTE_HASH_TREE_LEAF_COUNT - register[1] - 1; | ||
| pol commit note_hash_leaf_index_leaf_count_cmp_diff; | ||
| sel_execute_notehash_exists * ((LEAF_INDEX_LT_NOTE_HASH_LEAF_COUNT - LEAF_INDEX_GTE_NOTE_HASH_LEAF_COUNT) * note_hash_leaf_in_range + LEAF_INDEX_GTE_NOTE_HASH_LEAF_COUNT - note_hash_leaf_index_leaf_count_cmp_diff) = 0; |
There was a problem hiding this comment.
It's not clear to me exactly what this is doing. I imagine it's constraining note_hash_leaf_in_range, but i think it deserves a comment. And maybe @jeanmon can just take a brief look at it.
There was a problem hiding this comment.
This is using a standard comparison between range constrained numbers https://hackmd.io/moq6viBpRJeLpWrHAogCZw?view#Comparison-between-range-constrained-numbers
There was a problem hiding this comment.
Added a comment
There was a problem hiding this comment.
It is a standard constraint @dbanks12.
@sirasistant I do not see any boolean condition on note_hash_leaf_in_range though
As another comment, you might want to use the new gt gadget for doing this. Maybe we can discuss with the team if we want to migrate all these instances to the new gt gadget.
There was a problem hiding this comment.
As another comment, you might want to use the new gt gadget for doing this. Maybe we can discuss with the team if we want to migrate all these instances to the new gt gadget.
It's not merged right?
I do not see any boolean condition on note_hash_leaf_in_range though
will add!
4 participants
Implement note hash exists, modify note hash tree check to expose a exists boolean that is one when the note hash doesn't match the leaf value