High-level declarative workload classes implementation issues

[aklenik]

1. The DeclarativeWorkloadModuleBase should just simply extend the WorkloadModuleBase class, which performs most of the initialization. Then the initializeWorkloadModule function can be overridden to perform the extra init step (after calling the super init).
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js

   Line 24 in 64f0a67

   class DeclarativeWorkloadModuleBase extends WorkloadModuleInterface {

2. The following lines look "weird", not really the JS way. Simply use this.something = {}; instead
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js

   Line 88 in 64f0a67

   this.parameters = new Object();

   
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js

   Line 96 in 64f0a67

   this.contracts = new Object();

3. The intention is cleaner and easier to read if you use for(const key of Object.keys(roundArguments)) { ... } to iterate through the keys of an object.
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js

   Line 89 in 64f0a67

   for(const property in roundArguments) {

4. You don't need to save this on the instance level, won't use it outside of this function:
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js

   Line 95 in 64f0a67

   this.behavior = roundArguments.behavior;

5. This doesn't enumerate the objects, only the key names (string), whatever those are for array entries. You need for(const contractObject of roundArguments.contracts) {...} (instead of in)
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js

   Line 97 in 64f0a67

   for(const contract in this.behavior.contracts) {

6. You should pass the entire contract object for the constructor (plus some extra arguments) since you'll need the function selector part later:

   this.contracts[contractObject.name] = new Contract(contractObject, variables, parameters, valueProviderFactory);

   Instead of:
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js

   Line 98 in 64f0a67

   this.contracts[contract.name] = new Contract(contract.functions);

7. You don't need these, the Contract just passes them down the hierarchy:
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/contract.js

   Lines 29 to 30 in 64f0a67

   	this.variables = variables;
   	this.parameters = parameters;

8. Instead of this:
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/contract.js

   Line 32 in 64f0a67

   for(const element in this.functions) {

   
   You need to construct an object for each function and store them:

   for (const functionObject of functions) {
       this.functions[functionObject.name] = new ContractFunction(functionObject, variables, parameters, valueProviderFactory);
   }

[aklenik]

The above issues are fixed 👍

Additional remarks:

9. Since the ValueProviderFactory already holds the variables and parameters, there's no need to propagate them along the hierarchy, my bad. So the following lines can drop the variables/parameters arguments:
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js

   Line 60 in 8128511

   this.contracts[contractObject.name] = new Contract(contractObject, this.variables, this.parameters, valueProviderFactory);

   
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/contract.js

   Line 28 in 8128511

   constructor(contract, variables, parameters, valueProviderFactory) {

   
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/contract.js

   Line 31 in 8128511

   this.functions[functionObject.name] = new ContractFunction(functionObject, variables, parameters, valueProviderFactory);

   
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/contract-function.js

   Line 28 in 8128511

   constructor(functionReference, variables, parameters, valueProviderFactory) {

   
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/contract-function.js

   Line 31 in 8128511

   this.parameters[parameterObject.name] = new ContractFunctionParameter(parameterObject, variables, parameters, valueProviderFactory);

   
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/contract-function-parameter.js

   Line 27 in 8128511

   constructor(parameterReference, variables, parameters, valueProviderFactory) {

10. What is parameterList? I guess it's a leftover variable. Also there is a trailing space linting error before this part:
    

    caliper/packages/caliper-core/lib/worker/workload/declarative/contract-function-parameter.js

    Lines 29 to 35 in 8128511

    	if(this.parameterList === undefined) {
    	throw new Error('Incorrect Value for parmeterList: undefined');
    	}
    	if(this.parameterList.length < 1) {
    	throw new Error('Insufficient length of parameterList');
    	}

11. This should be saved as this.valueProvider, its intention is more clear that way:
    

    caliper/packages/caliper-core/lib/worker/workload/declarative/contract-function-parameter.js

    Line 37 in 8128511

    this.parameter = valueProviderFactory.createValueProvider(parameterReference.type, parameterReference.options);

12. The overridden super function must be called before this line, so the parameters are saved in the base class:
    

    caliper/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js

    Line 44 in 8128511

    this.variables = {

Next tasks

T1. The generateValue call should return the value generated by the underlying value provider:

caliper/packages/caliper-core/lib/worker/workload/declarative/contract-function-parameter.js

Lines 44 to 47 in 8128511

	generateValue() {
	return this.parameter;
	}
	}

T2. The generateCallArguments call should return a value for every parameter of a function (by calling their generateValue function). The new values should be returned in a key-value object style: { param1Name: value1; param2Name: value2; }

caliper/packages/caliper-core/lib/worker/workload/declarative/contract-function.js

Lines 38 to 40 in 8128511

	generateCallArguments() {
	return this.parameters;
	}

T3. The DeclarativeWorkloadModuleBase class should have a "selector" variable, that can select a uniformly random contract name from the available contracts (reusing hardwired value provider objects that achieve exactly what we want).

T4. The Contract class should have a "selector" variable, that can select a uniformly random function name from the available functions (reusing hardwired value provider objects that achieve exactly what we want).

[aklenik]

Next remarks:

1. You are missing the new operator from here:
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js

   Line 59 in d6d81d4

   const valueProviderFactory = ValueProviderFactory(this.variables, this.parameters);

2. Should also check that the list has at least one element:
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/value-providers/uniform-random-list-item-value-provider.js

   Line 25 in d6d81d4

   if(this.referenceList === undefined) {

3. Will this give a correct range?
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/value-providers/uniform-random-list-item-value-provider.js

   Line 34 in d6d81d4

   return this.referenceList[Math.floor(Math.random() * (this.referenceList.length - 1))];

   
   For a 1 element list: floor([0,1) * 0) = floor(0) = 0 (correct)
   For a 2 elements list: floor([0,1) * 1) = floor([0,1)) = 0 (always the first is selected)
   For a 3 elements list: floor([0,1) * 2) = floor([0,2)) = [0,1] (the last is never selected)
   For a 4 elements list: floor([0,1) * 3) = floor([0,3)) = [0,2] (the last is never selected)
   ... and so on. So the last element is never selected. You need to multiply with only this.referenceList.length
4. The selectors in the workload module and in the Contract class should be called contractSelector and functionSelector, respectively.
5. Don't overwrite your ContractFunctionParameter instances here, you will also need them on the next generate call:
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/contract-function.js

   Line 39 in d6d81d4

   this.parameters[parameterKey] = this.parameters[parameterKey].generateValue();

   
   Instead, create a new empty object (like callAruments) and add the newly generated values to that.

[aklenik]

Issues

1. generatedValues is a local variable, so don't need the this
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/contract-function.js

   Line 40 in e5e18fa

   this.generatedValues[parameterKey] = this.parameters[parameterKey].generateValue();

2. The first call needs to be a super call to the same function of the base class, so it initializes the instance variables:
   

   caliper/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js

   Lines 44 to 45 in e5e18fa

   	async initializeWorkloadModule(workerIndex, totalWorkers, roundIndex, roundArguments, sutAdapter, sutContext) {
   	this.variables = {

   
   So add this as first line in the function:

   await super.initializeWorkloadModule(workerIndex, totalWorkers, roundIndex, roundArguments, sutAdapter, sutContext);

Tasks

T1. Randomly select a contract function, and return its generated arguments:

caliper/packages/caliper-core/lib/worker/workload/declarative/contract.js

Lines 38 to 44 in e5e18fa

	generateCallArguments() {
	return this.functions;
	/**
	* return this.functions[this.selector.generateValue()].generateCallArguments();
	*/
	}
	}

T2. Randomly select a contract and returns its (randomly selected function's) generated arguments:

caliper/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js

Line 71 in e5e18fa

async submitTransaction() {}

T3. submitTransaction should call a new submitWithArguments(generatedArguments) function, that can be implemented by derived classes (it should throw an error in the DeclarativeWorkloadModuleBase class). So as for now, we leave the actual "custom args to connector args" transformation to be implemented in derived classes, and we won't automate it for now.

[aklenik]

Awesome 👍 The only thing left is to expose the workload module class from the core package, so users can use it for their workload modules as a base class.

Just add a similar export to the end of this file:

caliper/packages/caliper-core/index.js

Line 32 in 361c03b

module.exports.WorkloadModuleBase = require('./lib/worker/workload/workloadModuleBase');