-
Notifications
You must be signed in to change notification settings - Fork 41
[6.0] Fix semantics of AvlTree.insert & new AvlTree.insertOrUpdate method #1038
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
c1e5ba3
0c74184
754d624
a506be8
b80b3af
ea31523
2c501a7
18a6a77
45a4686
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ package sigmastate.eval | |
| import debox.cfor | ||
| import org.ergoplatform.ErgoBox | ||
| import org.ergoplatform.ErgoBox.TokenId | ||
| import scorex.crypto.authds.avltree.batch.{Insert, Lookup, Remove, Update} | ||
| import scorex.crypto.authds.avltree.batch.{Insert, InsertOrUpdate, Lookup, Remove, Update} | ||
| import scorex.crypto.authds.{ADKey, ADValue} | ||
| import scorex.util.encode.Base16 | ||
| import sigma.ast.SType.AnyOps | ||
|
|
@@ -91,7 +91,7 @@ object Extensions { | |
| val bv = CAvlTreeVerifier(tree, proof) | ||
| entries.forall { case (key, value) => | ||
| val insertRes = bv.performOneOperation(Insert(ADKey @@ key.toArray, ADValue @@ value.toArray)) | ||
| if (insertRes.isFailure) { | ||
| if (insertRes.isFailure && !VersionContext.current.isV6SoftForkActivated) { | ||
| syntax.error(s"Incorrect insert for $tree (key: $key, value: $value, digest: ${tree.digest}): ${insertRes.failed.get}}") | ||
| } | ||
| insertRes.isSuccess | ||
|
|
@@ -120,6 +120,24 @@ object Extensions { | |
| } | ||
| } | ||
|
|
||
| def insertOrUpdate( | ||
| entries: Coll[(Coll[Byte], Coll[Byte])], | ||
| proof: Coll[Byte]): Option[AvlTree] = { | ||
| if (!tree.isInsertAllowed || !tree.isUpdateAllowed) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both flags are required, however for each given tree it will either be insert or update (but not both).
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. " for each given tree it will either be insert or update (but not both)" - no, it could be insert for one key and update for another |
||
| None | ||
| } else { | ||
| val bv = CAvlTreeVerifier(tree, proof) | ||
| entries.forall { case (key, value) => | ||
| val insertRes = bv.performOneOperation(InsertOrUpdate(ADKey @@ key.toArray, ADValue @@ value.toArray)) | ||
| insertRes.isSuccess | ||
| } | ||
| bv.digest match { | ||
| case Some(d) => Some(tree.updateDigest(Colls.fromArray(d))) | ||
| case _ => None | ||
| } | ||
| } | ||
| } | ||
|
|
||
| def remove(operations: Coll[Coll[Byte]], proof: Coll[Byte]): Option[AvlTree] = { | ||
| if (!tree.isRemoveAllowed) { | ||
| None | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -146,7 +146,7 @@ class CErgoTreeEvaluator( | |||||||||||
| val insertRes = bv.performInsert(key.toArray, value.toArray) | ||||||||||||
| // TODO v6.0: throwing exception is not consistent with update semantics | ||||||||||||
| // however it preserves v4.0 semantics (see https://github.com/ScorexFoundation/sigmastate-interpreter/issues/908) | ||||||||||||
|
||||||||||||
| if (insertRes.isFailure) { | ||||||||||||
| if (insertRes.isFailure && !VersionContext.current.isV6SoftForkActivated) { | ||||||||||||
| syntax.error(s"Incorrect insert for $tree (key: $key, value: $value, digest: ${tree.digest}): ${insertRes.failed.get}}") | ||||||||||||
| } | ||||||||||||
| res = insertRes.isSuccess | ||||||||||||
|
|
@@ -173,7 +173,7 @@ class CErgoTreeEvaluator( | |||||||||||
| // when the tree is empty we still need to add the insert cost | ||||||||||||
| val nItems = Math.max(bv.treeHeight, 1) | ||||||||||||
|
|
||||||||||||
| // here we use forall as looping with fast break on first failed tree oparation | ||||||||||||
| // here we use forall as looping with fast break on first failed tree operation | ||||||||||||
| operations.forall { case (key, value) => | ||||||||||||
| var res = true | ||||||||||||
| // the cost of tree update is O(bv.treeHeight) | ||||||||||||
|
|
@@ -192,6 +192,37 @@ class CErgoTreeEvaluator( | |||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| override def insertOrUpdate_eval( | ||||||||||||
| mc: MethodCall, tree: AvlTree, | ||||||||||||
| operations: KeyValueColl, proof: Coll[Byte]): Option[AvlTree] = { | ||||||||||||
| addCost(isUpdateAllowed_Info) | ||||||||||||
| addCost(isInsertAllowed_Info) | ||||||||||||
| if (!(tree.isUpdateAllowed && tree.isInsertAllowed)) { | ||||||||||||
| None | ||||||||||||
| } else { | ||||||||||||
| val bv = createVerifier(tree, proof) | ||||||||||||
| // when the tree is empty we still need to add the insert cost | ||||||||||||
| val nItems = Math.max(bv.treeHeight, 1) | ||||||||||||
|
|
||||||||||||
| // here we use forall as looping with fast break on first failed tree operation | ||||||||||||
| operations.forall { case (key, value) => | ||||||||||||
| var res = true | ||||||||||||
| // the cost of tree update is O(bv.treeHeight) | ||||||||||||
| addSeqCost(UpdateAvlTree_Info, nItems) { () => | ||||||||||||
|
||||||||||||
| addSeqCost(UpdateAvlTree_Info, nItems) { () => | |
| // Here (and in the previous methods) the cost is not properly approximated. | |
| // When the tree is small (or empty), but there are many `operations`, the treeHeight will grow on every iteration. | |
| // So should the cost on every iteration. | |
| addSeqCost(UpdateAvlTree_Info, nItems) { () => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense to describe each case of returned result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done