address PR reviews

Author: kantai

clarity/src/vm/analysis/read_only_checker/mod.rs

@@ -292,6 +292,7 @@ impl<'a, 'b> ReadOnlyChecker<'a, 'b> {
             }
             FromConsensusBuff => {
                 // Check only the second+ arguments: the first argument is a type parameter
+                check_argument_count(2, args)?;
                 self.check_each_expression_is_read_only(&args[1..])
             }
             AtBlock => {

clarity/src/vm/analysis/type_checker/tests/mod.rs

@@ -75,6 +75,54 @@ fn ascii_type(size: u32) -> TypeSignature {
     TypeSignature::SequenceType(StringType(ASCII(size.try_into().unwrap()))).into()
 }
 
+#[test]
+fn test_from_consensus_buff() {
+    let good = [
+        ("(from-consensus-buff int 0x00)", "(optional int)"),
+        (
+            "(from-consensus-buff { a: uint, b: principal } 0x00)",
+            "(optional (tuple (a uint) (b principal)))",
+        ),
+    ];
+
+    let bad = [
+        (
+            "(from-consensus-buff)",
+            CheckErrors::IncorrectArgumentCount(2, 0),
+        ),
+        (
+            "(from-consensus-buff 0x00 0x00 0x00)",
+            CheckErrors::IncorrectArgumentCount(2, 3),
+        ),
+        (
+            "(from-consensus-buff 0x00 0x00)",
+            CheckErrors::InvalidTypeDescription,
+        ),
+        (
+            "(from-consensus-buff int u6)",
+            CheckErrors::TypeError(TypeSignature::max_buffer(), TypeSignature::UIntType),
+        ),
+        (
+            "(from-consensus-buff (buff 1048576) 0x00)",
+            CheckErrors::ValueTooLarge,
+        ),
+    ];
+
+    for (good_test, expected) in good.iter() {
+        let type_result = type_check_helper(good_test).unwrap();
+        assert_eq!(expected, &type_result.to_string());
+
+        assert!(
+            type_result.admits(&execute_v2(good_test).unwrap().unwrap()),
+            "The analyzed type must admit the evaluated type"
+        );
+    }
+
+    for (bad_test, expected) in bad.iter() {
+        assert_eq!(expected, &type_check_helper(&bad_test).unwrap_err().err);
+    }
+}
+
 #[test]
 fn test_to_consensus_buff() {
     let good = [
@@ -106,6 +154,11 @@ fn test_to_consensus_buff() {
         ("(to-consensus-buff 0x00)", "(optional (buff 6))"),
         ("(to-consensus-buff \"a\")", "(optional (buff 6))"),
         ("(to-consensus-buff u\"ab\")", "(optional (buff 13))"),
+        ("(to-consensus-buff 'STB44HYPYAT2BB2QE513NSP81HTMYWBJP02HPGK6)", "(optional (buff 151))"),
+        ("(to-consensus-buff 'STB44HYPYAT2BB2QE513NSP81HTMYWBJP02HPGK6.abcdeabcdeabcdeabcdeabcdeabcdeabcdeabcde)", "(optional (buff 151))"),
+        ("(to-consensus-buff true)", "(optional (buff 1))"),
+        ("(to-consensus-buff -1)", "(optional (buff 17))"),
+        ("(to-consensus-buff u1)", "(optional (buff 17))"),
         ("(to-consensus-buff (list 1 2 3 4))", "(optional (buff 73))"),
         (
             "(to-consensus-buff { apple: u1, orange: 2, blue: true })",

clarity/src/vm/docs/mod.rs

@@ -2000,6 +2000,8 @@ consensus buffer length based on the inferred type of the supplied value.
 (to-consensus-buff true) ;; Returns (some 0x03)
 (to-consensus-buff false) ;; Returns (some 0x04)
 (to-consensus-buff none) ;; Returns (some 0x09)
+(to-consensus-buff 'SZ2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKQ9H6DPR) ;; Returns (some 0x051fa46ff88886c2ef9762d970b4d2c63678835bd39d)
+(to-consensus-buff { abc: 3, def: 4 }) ;; Returns (some 0x0c00000002036162630000000000000000000000000000000003036465660000000000000000000000000000000004)
 "#,
 };
 
@@ -2020,6 +2022,8 @@ to deserialize the type, the method returns `none`.
 (from-consensus-buff bool 0x0000000000000000000000000000000001) ;; Returns none
 (from-consensus-buff bool 0x03) ;; Returns (some true)
 (from-consensus-buff bool 0x04) ;; Returns (some false)
+(from-consensus-buff principal 0x051fa46ff88886c2ef9762d970b4d2c63678835bd39d) ;; Returns (some SZ2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKQ9H6DPR)
+(from-consensus-buff { abc: int, def: int } 0x0c00000002036162630000000000000000000000000000000003036465660000000000000000000000000000000004) ;; Returns (some (tuple (abc 3) (def 4)))
 "#,
 };
 

clarity/src/vm/tests/simple_apply_eval.rs

@@ -322,11 +322,6 @@ fn test_from_consensus_buff_type_checks() {
         ),
     ];
 
-    for (input, expected) in vectors.iter() {
-        let result = vm_execute_v2(input).expect_err("Should raise an error");
-        eprintln!("{}", result.to_string());
-    }
-
     for (input, expected) in vectors.iter() {
         let result = vm_execute_v2(input).expect_err("Should raise an error");
         assert_eq!(&result.to_string(), expected);

clarity/src/vm/types/serialization.rs

@@ -294,7 +294,7 @@ macro_rules! check_match {
 impl TypeSignature {
     /// Return the maximum length of the consensus serialization of a
     /// Clarity value of this type. The returned length *may* not fit
-    /// in in a Clarity buffer! For example, the maximum serialized
+    /// in a Clarity buffer! For example, the maximum serialized
     /// size of a `(buff 1024*1024)` is `1+1024*1024` because of the
     /// type prefix byte. However, that is 1 byte larger than the maximum
     /// buffer size in Clarity.
@@ -422,8 +422,7 @@ impl Value {
         r: &mut R,
         expected_type: Option<&TypeSignature>,
     ) -> Result<Value, SerializationError> {
-        let mut bound_reader = BoundReader::from_reader(r, BOUND_VALUE_SERIALIZATION_BYTES as u64);
-        Value::inner_deserialize_read(&mut bound_reader, expected_type, 0)
+        Self::deserialize_read_count(r, expected_type).map(|(value, _)| value)
     }
 
     /// Deserialize just like `deserialize_read` but also
@@ -433,8 +432,30 @@ impl Value {
         expected_type: Option<&TypeSignature>,
     ) -> Result<(Value, u64), SerializationError> {
         let mut bound_reader = BoundReader::from_reader(r, BOUND_VALUE_SERIALIZATION_BYTES as u64);
-        Value::inner_deserialize_read(&mut bound_reader, expected_type, 0)
-            .map(|value| (value, bound_reader.num_read()))
+        let value = Value::inner_deserialize_read(&mut bound_reader, expected_type, 0)?;
+        let bytes_read = bound_reader.num_read();
+        if let Some(expected_type) = expected_type {
+            let expect_size = match expected_type.max_serialized_size() {
+                Ok(x) => x,
+                Err(e) => {
+                    warn!(
+                        "Failed to determine max serialized size when checking expected_type argument";
+                        "err" => ?e
+                    );
+                    return Ok((value, bytes_read));
+                }
+            };
+
+            assert!(
+                expect_size as u64 >= bytes_read,
+                "Deserialized more bytes than expected size during deserialization. Expected size = {}, bytes read = {}, type = {}",
+                expect_size,
+                bytes_read,
+                expected_type,
+            );
+        }
+
+        Ok((value, bytes_read))
     }
 
     fn inner_deserialize_read<R: Read>(