Convex-Dev / convex

Convex Main Repository - Decentralised platform for the Internet of Value
https://convex.world
Other
93 stars 29 forks source link

Etch: novelty handler always called #299

Open helins opened 3 years ago

helins commented 3 years ago

I noticed the novelty handler is called no matter what, even when I know for a fact a ref is not novel.

Unless I am missing something, it seems to be coded like that: https://github.com/Convex-Dev/convex/blob/34279b17ccd4659db8585f04a090aea8603818a7/convex-core/src/main/java/etch/EtchStore.java#L159-L160

Whitepaper mentions novelty detection is important for incremental updates, hasn't this cause problems?

mikera commented 3 years ago

At this point, we have already checked that the Ref is not already at the required status (i.e. is novel) so should be OK (lines 109 and 124).

If you can find a test case where this fails though useful to know!

helins commented 3 years ago

I might have misunderstood the exact purpose of the novelty handler. I was under the impression that any hash already present in the DB would not trigger it. But of you work with a ref that is not reported as stored yet AND its cell is reported as embedded, it goes through the checks you mention.

For instance, I was using a vector [0 1 2 3 4 5 6 7 8 9], which returns true on .isEmbedded(). Does it make sense? I guess I am sometimes surprised by what is being considered to be embedded.

mikera commented 3 years ago

Yes that vector is embedded. In general most things with an encoding of 140 bytes or less are embedded.

The main purpose of the novelty handler is to identify which parts of the merkle tree data structure need to be transmitted to remote peers in order for them to have a full information set in their store. Hence embedded values don't really need to be considered as novelty (they will be included in the encoding of other cells)