search

cambrian / accumulator

Cryptographic accumulators in Rust.
https://cambrian.github.io/accumulator
MIT License
133 stars 34 forks source link

Small changes needed for sim repo #11

Closed whaatt closed 5 years ago

whaatt commented 5 years ago

Also some minor reordering of derives

eddiew commented 5 years ago

Actually, requiring PartialEq for Group is fine, just leave a comment that it's to help auto-derive the PartialEq for Accumulator

whaatt commented 5 years ago

changed the derives

re: group element in Accumulator -- accumulators are used to store witnesses in the simulation, and updating these witnesses requires exponentiation of the element that isn't supported by any of the existing functions

we could potentially move that functionality into this library if we don't want to expose the black box

whaatt commented 5 years ago

could be something like acc.exp_(val) with the underscore denoting that this isn't a typical accumulator operation

eddiew commented 5 years ago

Updating the witness is basically an add. Maybe add a fn add_without_proof?

On Wed, Feb 6, 2019 at 3:31 PM Sanjay Kannan notifications@github.com wrote:

changed the derives

re: group element in Accumulator -- accumulators are used to store witnesses in the simulation, and updating these witnesses requires exponentiation of the element that isn't supported by any of the existing functions

we could potentially move that functionality into this library if we don't want to expose the black box

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/1protocol/accumulator/pull/11#issuecomment-461176007, or mute the thread https://github.com/notifications/unsubscribe-auth/ABsmxWJxABwdk5vSSF5pQT069Mhs0zBXks5vKzuHgaJpZM4amCA9 .

whaatt commented 5 years ago

it's not really though, because (for instance) to get a witness for a subset of elements in an aggregate witness, you're usually doing agg_witness ^ (agg_product / subset_product)

calling this an add is confusing

eddiew commented 5 years ago

You would want a corresponding delete_without_proof too

On Wed, Feb 6, 2019 at 3:38 PM Eddie Wang eddie@vest.com wrote:

Updating the witness is basically an add. Maybe add a fn add_without_proof?

On Wed, Feb 6, 2019 at 3:31 PM Sanjay Kannan notifications@github.com wrote:

changed the derives

re: group element in Accumulator -- accumulators are used to store witnesses in the simulation, and updating these witnesses requires exponentiation of the element that isn't supported by any of the existing functions

we could potentially move that functionality into this library if we don't want to expose the black box

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/1protocol/accumulator/pull/11#issuecomment-461176007, or mute the thread https://github.com/notifications/unsubscribe-auth/ABsmxWJxABwdk5vSSF5pQT069Mhs0zBXks5vKzuHgaJpZM4amCA9 .

whaatt commented 5 years ago

Note that the division there cannot be implemented as a delete (which would require precisely the witness you're trying to compute). You have to pre-compute (agg_product / subset_product), erroring as needed if the division produces a remainder