-
Notifications
You must be signed in to change notification settings - Fork 41
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
VersionContext added #705
VersionContext added #705
Conversation
# Conflicts: # sigmastate/src/main/scala/sigmastate/interpreter/InterpreterContext.scala # sigmastate/src/test/scala/org/ergoplatform/ErgoLikeTransactionSpec.scala
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.
Found some problems with updated ErgoLikeContext:
- remove default value for "val activatedScriptVersion: Byte'
- add the new field to JSON codecs
@@ -400,7 +400,8 @@ trait JsonCodecs { | |||
"extension" -> ctx.extension.asJson, | |||
"validationSettings" -> ctx.validationSettings.asJson, | |||
"costLimit" -> ctx.costLimit.asJson, | |||
"initCost" -> ctx.initCost.asJson | |||
"initCost" -> ctx.initCost.asJson, | |||
"activatedVersion" -> ctx.activatedScriptVersion.asJson |
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.
"scriptVersion" is more clean. Also, in particular, this JSON can be provided via /executeWithContext API, where activation is irrelevant
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.
fixed
@@ -417,7 +418,11 @@ trait JsonCodecs { | |||
validationSettings <- cursor.downField("validationSettings").as[SigmaValidationSettings] | |||
costLimit <- cursor.downField("costLimit").as[Long] | |||
initCost <- cursor.downField("initCost").as[Long] | |||
} yield new ErgoLikeContext(lastBlockUtxoRoot, Colls.fromArray(headers.toArray), preHeader, | |||
dataBoxes, boxesToSpend, spendingTransaction, selfIndex, extension, validationSettings, costLimit, initCost) | |||
version <- cursor.downField("activatedVersion").as[Byte] |
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.
"scriptVersion"
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.
fixed
* [[sigmastate.Values.ConstantPlaceholder]] | ||
* @param resolvePlaceholdersToConstants if true then resolved constants will be | ||
* substituted in the tree instead of the placeholder. | ||
* @param versionContext optional version context to support soft and |
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.
Doesn't look like optional parameter (Option[...])
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.
parameter removed
@@ -1058,6 +1058,13 @@ object Values { | |||
(header | version).toByte | |||
} | |||
|
|||
@inline def headerWithVersion(version: Byte): Byte = { |
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.
ScalaDoc missed
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.
added
@inline def headerWithVersion(version: Byte): Byte = { | ||
var h = updateVersionBits(DefaultHeader, version) | ||
if (version > 0) | ||
h = (h | ErgoTree.SizeFlag).toByte |
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.
logic unclear, please add comments
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.
clarified
object VersionContext { | ||
|
||
/** Descriptor of the max supported versions. */ | ||
val MaxSupportedVersion = VersionContext( |
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.
Used in testing code only, let's move to the testing code then ? Or eliminate, as it is used once.
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.
VersionContext removed entirely
@@ -79,6 +93,7 @@ abstract class SigmaSerializer[TFamily, T <: TFamily] extends Serializer[TFamily | |||
val sigmaByteReader = new SigmaByteReader(r, | |||
new ConstantStore(), | |||
resolvePlaceholdersToConstants = false, | |||
versionContext = VersionContext(0), |
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.
Magic number
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.
removed
@@ -17,29 +18,42 @@ object SigmaSerializer { | |||
val MaxPropositionSize: Int = SigmaConstants.MaxPropositionBytes.value | |||
val MaxTreeDepth: Int = SigmaConstants.MaxTreeDepth.value | |||
|
|||
/** Helper function to be use in serializers. | |||
//TODO v5.0: remove default value of versionContext parameter | |||
// All usages of this method should pass a version context explicitly. |
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.
Why not to do it now?
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.
removed
No description provided.