Code review follow up

multiversx · Sep 10, 2024 · 275bb49 · 275bb49
1 parent 14b2f5f
commit 275bb49
Show file tree

Hide file tree

Showing 8 changed files with 60 additions and 38 deletions.
diff --git a/src/abi/typeFormulaParser.ts b/src/abi/typeFormulaParser.ts
@@ -81,9 +81,9 @@ export class TypeFormulaParser {
     private acquireTypeWithParameters(stack: any[]): TypeFormula {
         const typeParameters = this.acquireTypeParameters(stack);
         const typeName = stack.pop();
-        const metadata = typeParameters[0].name;
 
         if (typeName === "ManagedDecimal" || typeName === "ManagedDecimalSigned") {
+            const metadata = typeParameters[0].name;
             const typeFormula = new TypeFormula(typeName, [], metadata);
             return typeFormula;
         }

diff --git a/src/smartcontracts/codec/managedDecimal.ts b/src/smartcontracts/codec/managedDecimal.ts
@@ -34,14 +34,14 @@ export class ManagedDecimalCodec {
         }
 
         const value = bufferToBigInt(buffer);
-        const scale = parseInt(type.getMetadata());
+        const metadata = type.getMetadata();
+        const scale = typeof metadata === "number" ? metadata : 0;
         return new ManagedDecimalValue(value, scale);
     }
 
     encodeNested(value: ManagedDecimalValue): Buffer {
         let buffers: Buffer[] = [];
-        const managedDecimalType = value.getType() as ManagedDecimalType;
-        if (managedDecimalType.isVariable()) {
+        if (value.isVariable()) {
             buffers.push(Buffer.from(this.binaryCodec.encodeNested(new BigUIntValue(value.valueOf()))));
             buffers.push(Buffer.from(this.binaryCodec.encodeNested(new U32Value(value.getScale()))));
         } else {

diff --git a/src/smartcontracts/typesystem/managedDecimal.spec.ts b/src/smartcontracts/typesystem/managedDecimal.spec.ts
@@ -1,11 +1,10 @@
 import { assert } from "chai";
 import { ManagedDecimalType, ManagedDecimalValue } from "./managedDecimal";
-import BigNumber from "bignumber.js";
 
 describe("test managed decimal", () => {
     it("should get correct metadata set", () => {
-        let type = new ManagedDecimalType("8");
-        const expectedMetadata = "8";
+        const type = new ManagedDecimalType(8);
+        const expectedMetadata = 8;
 
         assert.equal(type.getMetadata(), expectedMetadata);
         assert.isFalse(type.isVariable());
@@ -20,32 +19,36 @@ describe("test managed decimal", () => {
     });
 
     it("should return the expected values for scale and metadata", () => {
-        let firstValue = new ManagedDecimalValue(new BigNumber(1), 2, false);
-        let secondValue = new ManagedDecimalValue(new BigNumber(2), 2, false);
-        const expectedMetadata = "2";
-        const type = firstValue.getType() as ManagedDecimalType;
+        const firstValue = new ManagedDecimalValue("1", 2, false);
+        const secondValue = new ManagedDecimalValue("2", 2, false);
+        const expectedMetadata = 2;
+        const type = firstValue.getType();
 
         assert.equal(type.getMetadata(), expectedMetadata);
-        assert.isFalse(type.isVariable());
+        assert.isFalse(firstValue.isVariable());
         assert.equal(firstValue.getScale(), 2);
         assert.equal(firstValue.toString(), "1.00");
         assert.isFalse(firstValue.equals(secondValue));
     });
 
     it("should compare correctly two managed decimals even with different scale", () => {
-        let firstValue = new ManagedDecimalValue(new BigNumber(1.234), 3, false);
-        let secondValue = new ManagedDecimalValue(new BigNumber(12.34), 2, false);
+        const firstValue = new ManagedDecimalValue("1.234", 3, false);
+        const secondValue = new ManagedDecimalValue("12.34", 2, false);
 
         assert.isFalse(firstValue.equals(secondValue));
     });
 
+    it.only("should compare correctly two managed decimals even with different scale", () => {
+        const firstValue = new ManagedDecimalValue("1.234", 3, false);
+        const secondValue = new ManagedDecimalValue("1.234", 3, false);
+
+        assert.isTrue(firstValue.equals(secondValue));
+    });
+
     it("should set the correct scale when variable decimals", () => {
-        let value = new ManagedDecimalValue(new BigNumber(1.3), 2, true);
-        const expectedMetadata = "usize";
-        const type = value.getType() as ManagedDecimalType;
+        const value = new ManagedDecimalValue("1.3", 2, true);
 
-        assert.equal(type.getMetadata(), expectedMetadata);
-        assert.isTrue(type.isVariable());
+        assert.isTrue(value.isVariable());
         assert.equal(value.toString(), "1.30");
         assert.equal(value.getScale(), 2);
     });

diff --git a/src/smartcontracts/typesystem/managedDecimal.ts b/src/smartcontracts/typesystem/managedDecimal.ts
@@ -4,15 +4,15 @@ import { Type, TypedValue } from "./types";
 export class ManagedDecimalType extends Type {
     static ClassName = "ManagedDecimalType";
 
-    constructor(metadata: any) {
+    constructor(metadata: number | "usize") {
         super("ManagedDecimal", undefined, undefined, metadata);
     }
 
     getClassName(): string {
         return ManagedDecimalType.ClassName;
     }
 
-    getMetadata(): string {
+    getMetadata(): number | "usize" {
         return this.metadata;
     }
 
@@ -25,11 +25,13 @@ export class ManagedDecimalValue extends TypedValue {
     static ClassName = "ManagedDecimalValue";
     private readonly value: BigNumber;
     private readonly scale: number;
+    private readonly variable: boolean;
 
-    constructor(value: BigNumber.Value, scale: number, isVar: boolean = false) {
-        super(new ManagedDecimalType(isVar ? "usize" : scale));
+    constructor(value: BigNumber.Value, scale: number, isVariable: boolean = false) {
+        super(new ManagedDecimalType(isVariable ? "usize" : scale));
         this.value = new BigNumber(value);
         this.scale = scale;
+        this.variable = isVariable;
     }
 
     getClassName(): string {
@@ -52,7 +54,7 @@ export class ManagedDecimalValue extends TypedValue {
             return false;
         }
 
-        return this.value == other.value;
+        return new BigNumber(this.value).eq(other.value);
     }
 
     valueOf(): BigNumber {
@@ -62,20 +64,24 @@ export class ManagedDecimalValue extends TypedValue {
     toString(): string {
         return this.value.toFixed(this.scale);
     }
+
+    isVariable(): boolean {
+        return this.variable;
+    }
 }
 
 export class ManagedDecimalSignedType extends Type {
     static ClassName = "ManagedDecimalSignedType";
 
-    constructor(metadata: any) {
+    constructor(metadata: number | "usize") {
         super("ManagedDecimalSigned", undefined, undefined, metadata);
     }
 
     getClassName(): string {
         return ManagedDecimalType.ClassName;
     }
 
-    getMetadata(): string {
+    getMetadata(): number | "usize" {
         return this.metadata;
     }
 
@@ -88,11 +94,13 @@ export class ManagedDecimalSignedValue extends TypedValue {
     static ClassName = "ManagedDecimalSignedValue";
     private readonly value: BigNumber;
     private readonly scale: number;
+    private readonly variable: boolean;
 
-    constructor(value: BigNumber.Value, scale: number, isVariableDecimals: boolean = false) {
-        super(new ManagedDecimalSignedType(isVariableDecimals ? "usize" : scale));
+    constructor(value: BigNumber.Value, scale: number, isVariable: boolean = false) {
+        super(new ManagedDecimalSignedType(isVariable ? "usize" : scale));
         this.value = new BigNumber(value);
         this.scale = scale;
+        this.variable = isVariable;
     }
 
     getClassName(): string {
@@ -111,7 +119,7 @@ export class ManagedDecimalSignedValue extends TypedValue {
             return false;
         }
 
-        return this.value == other.value;
+        return new BigNumber(this.value).eq(other.value);
     }
 
     valueOf(): BigNumber {
@@ -121,4 +129,8 @@ export class ManagedDecimalSignedValue extends TypedValue {
     toString(): string {
         return this.value.toFixed(this.scale);
     }
+
+    isVariable(): boolean {
+        return this.variable;
+    }
 }
diff --git a/src/smartcontracts/typesystem/typeExpressionParser.ts b/src/smartcontracts/typesystem/typeExpressionParser.ts
@@ -1,7 +1,7 @@
 import { TypeFormula } from "../../abi/typeFormula";
 import { TypeFormulaParser } from "../../abi/typeFormulaParser";
 import { ErrTypingSystem } from "../../errors";
-import { Type, TypeCardinality } from "./types";
+import { Type } from "./types";
 
 export class TypeExpressionParser {
     private readonly backingTypeFormulaParser: TypeFormulaParser;
@@ -26,6 +26,6 @@ export class TypeExpressionParser {
 
     private typeFormulaToType(typeFormula: TypeFormula): Type {
         const typeParameters = typeFormula.typeParameters.map((typeFormula) => this.typeFormulaToType(typeFormula));
-        return new Type(typeFormula.name, typeParameters, TypeCardinality.fixed(1), typeFormula.metadata);
+        return new Type(typeFormula.name, typeParameters, undefined, typeFormula.metadata);
     }
 }
diff --git a/src/smartcontracts/typesystem/typeMapper.ts b/src/smartcontracts/typesystem/typeMapper.ts
@@ -174,11 +174,13 @@ export class TypeMapper {
 
     private learnType(type: Type): void {
         if (type.getName() === "ManagedDecimal" || type.getName() === "ManagedDecimalSigned") {
-            this.learnedTypesMap.delete(`${type.getName()}_${type.getMetadata()}`);
-            this.learnedTypesMap.set(`${type.getName()}_${type.getMetadata()}`, type);
+            const learnedTypeKey = `${type.getName()}_${type.getMetadata()}`;
+            this.learnedTypesMap.delete(learnedTypeKey);
+            this.learnedTypesMap.set(learnedTypeKey, type);
         } else {
-            this.learnedTypesMap.delete(type.getName());
-            this.learnedTypesMap.set(type.getName(), type);
+            const learnedTypeKey = type.getName();
+            this.learnedTypesMap.delete(learnedTypeKey);
+            this.learnedTypesMap.set(learnedTypeKey, type);
         }
     }
 

diff --git a/src/smartcontracts/typesystem/types.spec.ts b/src/smartcontracts/typesystem/types.spec.ts
@@ -64,11 +64,16 @@ describe("test types", () => {
             parser.parse("Option<u32>").getFullyQualifiedName(),
             "multiversx:types:Option<multiversx:types:u32>",
         );
-        assert.equal(new ManagedDecimalType("8").getFullyQualifiedName(), "multiversx:types:ManagedDecimal*8*");
+        assert.equal(new ManagedDecimalType(8).getFullyQualifiedName(), "multiversx:types:ManagedDecimal*8*");
+        assert.equal(new ManagedDecimalType("usize").getFullyQualifiedName(), "multiversx:types:ManagedDecimal*usize*");
         assert.equal(
-            new ManagedDecimalSignedType("8").getFullyQualifiedName(),
+            new ManagedDecimalSignedType(8).getFullyQualifiedName(),
             "multiversx:types:ManagedDecimalSigned*8*",
         );
+        assert.equal(
+            new ManagedDecimalSignedType("usize").getFullyQualifiedName(),
+            "multiversx:types:ManagedDecimalSigned*usize*",
+        );
     });
 
     it("types and values should have correct JavaScript class hierarchy", () => {

diff --git a/src/smartcontracts/typesystem/types.ts b/src/smartcontracts/typesystem/types.ts
@@ -11,7 +11,7 @@ export class Type {
     private readonly name: string;
     private readonly typeParameters: Type[];
     private readonly cardinality: TypeCardinality;
-    protected readonly metadata: string;
+    protected readonly metadata: any;
 
     public constructor(
         name: string,