-
Notifications
You must be signed in to change notification settings - Fork 41
Prohibition for using 6.0 types in register and context extension vars #1047
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
Conversation
| case c: Constant[_] => v6TypeCheck(c.tpe) | ||
| case c: EvaluatedCollection[_, _] => v6TypeCheck(c.elementType) | ||
| case GroupGenerator => | ||
| } |
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.
What is the motivation to add and check this rule?
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.
The problem is that deserialization of values in registers and context extension is not versioned, so newly supported serializable types (Header, Option[], UnsignedBigInt) can't be put there (but serialized as Coll[Byte] value can be put there to be used with Global.deserialize )
| } | ||
| } | ||
| v match { | ||
| case c: Constant[_] => v6TypeCheck(c.tpe) |
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.
It seems it is possible to serialize/parse Tuple with v6 type
property("impossible to use v6 types in box registers") {
val trueProp = ErgoTreePredef.TrueProp(ErgoTree.defaultHeaderWithVersion(3))
val b = new ErgoBoxCandidate(1L, trueProp, 1,
additionalRegisters = Map(R4 -> Tuple(UnsignedBigIntConstant(new BigInteger("1")), IntConstant(1))))
VersionContext.withVersions(3, 3) {
val bs = ErgoBoxCandidate.serializer.toBytes(b)
a[sigma.validation.ValidationException] should be thrownBy ErgoBoxCandidate.serializer.fromBytes(bs)
}
}doesn't throw exception
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.
@sethdusek fixed, the reason was c.elementType being SAny for Tuple. Made test for UBI as second tuple element, and collection as well
| } | ||
| } | ||
| v match { | ||
| case c: Constant[_] => v6TypeCheck(c.tpe) |
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.
This seems to be insufficient in its current form, for example a Constant with type STuple(.., UnsignedBigInt | Option | Header) doesn't get caught by this. Example of type (Byte, Short, Byte, UnsignedBigInt) (Constant was built using sigma-rust so passed as base16 bytes here).
val reader = new SigmaByteReader(new VLQByteBufferReader(ByteBuffer.wrap(decodeBytes("5402030209050a050105").toArray)), new ConstantStore(), false)
val v = VersionContext.withVersions(3, 3) {ConstantSerializer(DeserializationSigmaBuilder).parse(reader).asInstanceOf[Constant[STuple]] }
val b2 = new ErgoBoxCandidate(1L, trueProp, 1,
additionalRegisters = Map(R4 -> v))
VersionContext.withVersions(3, 3) {
val bs2 = ErgoBoxCandidate.serializer.toBytes(b2)
a[sigma.validation.ValidationException] should be thrownBy ErgoBoxCandidate.serializer.fromBytes(bs2)
}I think the checks should be recursive
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.
Indeed, recursive checks done
| } | ||
| v match { | ||
| case c: Constant[_] => v6TypeCheck(c.tpe) | ||
| case t: Tuple => if(t.items.length != 2) { |
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.
It's possible to parse tuples with length > 2 although they don't seem to be very useful since they can't be evaluated, but they could be stuffed in register/contextextension
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.
addressed, no length checked now
| } | ||
| } | ||
|
|
||
| def step(s: SType): Unit = { |
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 seem to be sufficient for Coll type that's nested in another type like Tuple or Coll
val b7 = new ErgoBoxCandidate(1L, trueProp, 1,
additionalRegisters = Map(R4 -> ConcreteCollection(Seq(ConcreteCollection(Seq(UnsignedBigIntConstant(new BigInteger("1"))), SUnsignedBigInt)), (SCollection(SUnsignedBigInt)))))
VersionContext.withVersions(3, 3) {
val bs = ErgoBoxCandidate.serializer.toBytes(b7)
a[sigma.validation.ValidationException] should be thrownBy ErgoBoxCandidate.serializer.fromBytes(bs)
}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.
oh right, fixed in 845c3e0
No description provided.