Repository: ergoplatform/sigma-rust (language: rust)

Issue:
`ErgoTree::set_constant` silently fails if type differs
### Motivation
When setting(swapping) the constant with a different type the resulted ErgoTree becomes invalid. 

### Solution
#### Option 1.
Make `ErgoTree::set_constant` return `Result` with error (wrong type, index out of bounds).

https://github.com/ergoplatform/sigma-rust/blob/3b1b3a5cee6fdafa5366d76f0e94902eea6fa223/ergotree-ir/src/ergo_tree.rs#L33-L35

Cons:
- while rustc will demand the result not to be ignored, Wasm API allows the result to be ignored, thus silently failing to set the constant.
#### Option 2.
Move to immutable model. Alter `ErgoTree::set_constant` to become:
```rust
pub fn with_constant(self, index: usize, constant: Constant) -> Result<ErgoTree, ErgoTreeConstantError>;
```
consuming the `self` and returning updated ErgoTree with a constant set will force Wasm API consumers to handle the error case.


Discussion / hints:

Fix patch:
diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml
index ce9c5330b..5e3600464 100644
--- a/.github/workflows/android.yml
+++ b/.github/workflows/android.yml
@@ -30,5 +30,6 @@ jobs:
         with:
           api-level: 29
           ndk: 21.3.6528147 
+          emulator-build: 6110076
           script: cd ./bindings/ergo-lib-jni && ./gradlew connectedCheck
 
diff --git a/bindings/ergo-lib-wasm/src/ergo_tree.rs b/bindings/ergo-lib-wasm/src/ergo_tree.rs
index 0696a69dc..1e7fda8bd 100644
--- a/bindings/ergo-lib-wasm/src/ergo_tree.rs
+++ b/bindings/ergo-lib-wasm/src/ergo_tree.rs
@@ -59,17 +59,13 @@ impl ErgoTree {
             .map_err(|e| JsValue::from_str(&format!("{:?}", e)))
     }
 
-    /// Sets new constant value for a given index in constants list (as stored in serialized ErgoTree),
-    /// and returns previous constant or None if index is out of bounds
-    /// or error if constants parsing were failed
-    pub fn set_constant(
-        &mut self,
-        index: usize,
-        constant: &Constant,
-    ) -> Result<Option<Constant>, JsValue> {
+    /// Consumes the calling ErgoTree and returns new ErgoTree with a new constant value
+    /// for a given index in constants list (as stored in serialized ErgoTree), or an error.
+    /// After the call the calling ErgoTree will be null.
+    pub fn with_constant(self, index: usize, constant: &Constant) -> Result<ErgoTree, JsValue> {
         self.0
-            .set_constant(index, constant.clone().into())
-            .map(|opt| opt.map(|c| c.into()))
+            .with_constant(index, constant.clone().into())
+            .map(ErgoTree)
             .map_err(|e| JsValue::from_str(&format!("{:?}", e)))
     }
 
diff --git a/ergo-lib/CHANGELOG.md b/ergo-lib/CHANGELOG.md
index 7653d11fa..0cc4d6c66 100644
--- a/ergo-lib/CHANGELOG.md
+++ b/ergo-lib/CHANGELOG.md
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Changed
 - `ergotree-ir::mir::constant::constant_placeholder` module is public;
+- `ErgoTree::set_constant` is removed in favor of `ErgoTree::with_constant` with an added check for matching constant type[#323](https://github.com/ergoplatform/sigma-rust/pull/323);
 
 ## [0.13.3] - 2021-06-11
 
diff --git a/ergotree-ir/src/ergo_tree.rs b/ergotree-ir/src/ergo_tree.rs
index 7af51ec68..725cd6480 100644
--- a/ergotree-ir/src/ergo_tree.rs
+++ b/ergotree-ir/src/ergo_tree.rs
@@ -30,14 +30,29 @@ struct ParsedTree {
 }
 
 impl ParsedTree {
-    /// Sets new constant value for a given index in constants list (as stored in serialized ErgoTree),
-    /// and returns either previous constant or None if index is out of bounds
-    fn set_constant(&mut self, index: usize, constant: Constant) -> Option<Constant> {
-        if index >= self.constants.len() {
-            None
+    /// Returns new ParsedTree with a new constant value for a given index in constants list
+    /// (as stored in serialized ErgoTree), or an error
+    fn with_constant(self, index: usize, constant: Constant) -> Result<Self, SetConstantError> {
+        let mut new_constants = self.constants.clone();
+        if let Some(old_constant) = self.constants.get(index) {
+            if constant.tpe == old_constant.tpe {
+                let _ = std::mem::replace(&mut new_constants[index], constant);
+                Ok(Self {
+                    constants: new_constants,
+                    root: self.root,
+                })
+            } else {
+                Err(SetConstantError::TypeMismatch(format!(
+                    "with_constant: expected constant type to be {:?}, got {:?}",
+                    old_constant.tpe, constant.tpe
+                )))
+            }
         } else {
-            let replaced = std::mem::replace(&mut self.constants[index], constant);
-            Some(replaced)
+            Err(SetConstantError::OutOfBounds(format!(
+                "with_constant: index({0}) out of bounds (lengh = {1})",
+                index,
+                self.constants.len()
+            )))
         }
     }
 
@@ -71,6 +86,15 @@ impl ParsedTree {
     }
 }
 
+/// Errors on fail to set a new constant value
+#[derive(Debug)]
+pub enum SetConstantError {
+    /// Index is out of bounds
+    OutOfBounds(String),
+    /// Existing constant type differs from the provided new constant type
+    TypeMismatch(String),
+}
+
 /// Currently we define meaning for only first byte, which may be extended in future versions.
 ///  7  6  5  4  3  2  1  0
 ///  -------------------------
@@ -383,18 +407,18 @@ impl ErgoTree {
             .map_err(|e| e.clone())
     }
 
-    /// Sets new constant value for a given index in constants list (as stored in serialized ErgoTree),
-    /// and returns previous constant or None if index is out of bounds
-    /// or error if constants parsing were failed
-    pub fn set_constant(
-        &mut self,
+    /// Returns new ErgoTree with a new constant value for a given index in constants list (as stored in serialized ErgoTree),
+    /// or an error
+    pub fn with_constant(
+        self,
         index: usize,
         constant: Constant,
-    ) -> Result<Option<Constant>, ErgoTreeConstantsParsingError> {
-        self.tree
-            .as_mut()
-            .map(|tree| tree.set_constant(index, constant))
-            .map_err(|e| e.clone())
+    ) -> Result<Self, ErgoTreeConstantError> {
+        let parsed_tree = self.tree?;
+        Ok(Self {
+            header: self.header,
+            tree: Ok(parsed_tree.with_constant(index, constant)?),
+        })
     }
 
     /// Serialized proposition expression of SigmaProp type with
@@ -404,6 +428,15 @@ impl ErgoTree {
     }
 }
 
+/// Constants related errors
+#[derive(Debug, From)]
+pub enum ErgoTreeConstantError {
+    /// Fail to parse a constant when deserializing an ErgoTree
+    ParsingError(ErgoTreeConstantsParsingError),
+    /// Fail to set a new constant value
+    SetConstantError(SetConstantError),
+}
+
 impl From<Expr> for ErgoTree {
     fn from(expr: Expr) -> Self {
         match &expr {
@@ -715,27 +748,27 @@ mod tests {
             tpe: SType::SBoolean,
             v: Value::Boolean(false),
         });
-        let mut ergo_tree = ErgoTree::new(ErgoTreeHeader::v0(true), &expr);
-        assert_eq!(
-            ergo_tree.set_constant(0, true.into()).unwrap().unwrap(),
-            false.into()
-        );
-        assert_eq!(ergo_tree.get_constant(0).unwrap().unwrap(), true.into());
+        let ergo_tree = ErgoTree::new(ErgoTreeHeader::v0(true), &expr);
+        let new_ergo_tree = ergo_tree.with_constant(0, true.into()).unwrap();
+        assert_eq!(new_ergo_tree.get_constant(0).unwrap().unwrap(), true.into());
     }
 
     #[test]
     fn dex_t2tpool_parse() {
         let base16_str = "19a3030f0400040204020404040404060406058080a0f6f4acdbe01b058080a0f6f4acdbe01b050004d00f0400040005000500d81ad601b2a5730000d602e4c6a70405d603db63087201d604db6308a7d605b27203730100d606b27204730200d607b27203730300d608b27204730400d609b27203730500d60ab27204730600d60b9973078c720602d60c999973088c720502720bd60d8c720802d60e998c720702720dd60f91720e7309d6108c720a02d6117e721006d6127e720e06d613998c7209027210d6147e720d06d615730ad6167e721306d6177e720c06d6187e720b06d6199c72127218d61a9c72167218d1edededededed93c27201c2a793e4c672010405720292c17201c1a793b27203730b00b27204730c00938c7205018c720601ed938c7207018c720801938c7209018c720a019593720c730d95720f929c9c721172127e7202069c7ef07213069a9c72147e7215067e9c720e720206929c9c721472167e7202069c7ef0720e069a9c72117e7215067e9c721372020695ed720f917213730e907217a19d721972149d721a7211ed9272199c7217721492721a9c72177211";
         let tree_bytes = base16::decode(base16_str.as_bytes()).unwrap();
-        let mut tree = ErgoTree::sigma_parse_bytes(&tree_bytes).unwrap();
+        let tree = ErgoTree::sigma_parse_bytes(&tree_bytes).unwrap();
         dbg!(&tree);
         assert!(tree.header.has_size());
         assert!(tree.header.is_constant_segregation());
         assert_eq!(tree.header.version(), ErgoTreeVersion::V1);
-        tree.set_constant(7, 1i64.into()).unwrap();
-        assert_eq!(tree.get_constant(7).unwrap().unwrap(), 1i64.into());
-        tree.set_constant(8, 2i64.into()).unwrap();
-        assert_eq!(tree.get_constant(8).unwrap().unwrap(), 2i64.into());
-        assert!(tree.sigma_serialize_bytes().len() > 1);
+        let new_tree = tree
+            .with_constant(7, 1i64.into())
+            .unwrap()
+            .with_constant(8, 2i64.into())
+            .unwrap();
+        assert_eq!(new_tree.get_constant(7).unwrap().unwrap(), 1i64.into());
+        assert_eq!(new_tree.get_constant(8).unwrap().unwrap(), 2i64.into());
+        assert!(new_tree.sigma_serialize_bytes().len() > 1);
     }
 }


Test patch:
diff --git a/bindings/ergo-lib-wasm/tests/test_ergo_tree.js b/bindings/ergo-lib-wasm/tests/test_ergo_tree.js
index 5b9aee3f0..ceb401d75 100644
--- a/bindings/ergo-lib-wasm/tests/test_ergo_tree.js
+++ b/bindings/ergo-lib-wasm/tests/test_ergo_tree.js
@@ -2,7 +2,8 @@ import { expect, assert } from 'chai';
 
 import {
   ErgoTree,
-  Constant
+  Constant,
+  I64
 } from '../pkg/ergo_lib_wasm';
 
 it('constants_len', async () => {
@@ -21,7 +22,7 @@ it('get_constant', async () => {
   assert(tree.get_constant(1) != null);
 });
 
-it('get_constant out of bounds', async () => {
+it('get_constant, out of bounds', async () => {
   let tree_bytes_base16_str = "100204a00b08cd021dde34603426402615658f1d970cfa7c7bd92ac81a8b16eeebff264d59ce4604ea02d192a39a8cc7a70173007301";
   let tree = ErgoTree.from_base16_bytes(tree_bytes_base16_str);
   assert(tree != null);
@@ -29,19 +30,28 @@ it('get_constant out of bounds', async () => {
   assert(tree.get_constant(3) == null);
 });
 
-it('set_constant', async () => {
+it('with_constant', async () => {
   let tree_bytes_base16_str = "100204a00b08cd021dde34603426402615658f1d970cfa7c7bd92ac81a8b16eeebff264d59ce4604ea02d192a39a8cc7a70173007301";
   let tree = ErgoTree.from_base16_bytes(tree_bytes_base16_str);
   assert(tree.constants_len() == 2);
   let constant = Constant.from_i32(99);
-  assert(tree.set_constant(0, constant) != null);
-  assert(tree.get_constant(0).to_i32() == 99);
+  let new_tree = tree.with_constant(0, constant);
+  assert(new_tree != null);
+  assert(new_tree.get_constant(0).to_i32() == 99);
 });
 
-it('set_constant out of bounds', async () => {
+it('with_constant, out of bounds', async () => {
   let tree_bytes_base16_str = "100204a00b08cd021dde34603426402615658f1d970cfa7c7bd92ac81a8b16eeebff264d59ce4604ea02d192a39a8cc7a70173007301";
   let tree = ErgoTree.from_base16_bytes(tree_bytes_base16_str);
   assert(tree.constants_len() == 2);
   let constant = Constant.from_i32(99);
-  assert(tree.set_constant(3, constant) == null);
+  expect(function() {tree.with_constant(3, constant);}).throw();
+});
+
+it('with_constant, type mismatch', async () => {
+  let tree_bytes_base16_str = "100204a00b08cd021dde34603426402615658f1d970cfa7c7bd92ac81a8b16eeebff264d59ce4604ea02d192a39a8cc7a70173007301";
+  let tree = ErgoTree.from_base16_bytes(tree_bytes_base16_str);
+  assert(tree.constants_len() == 2);
+  let constant = Constant.from_i64(I64.from_str("324234"));
+  expect(function() {tree.with_constant(0, constant);}).throw();
 });