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). https://github.com/bistaastha/caliper/blob/64f0a67a9264197c8288845d2d2ef17bf433425d/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js#L24
2. The following lines look "weird", not really the JS way. Simply use this.something = {}; instead https://github.com/bistaastha/caliper/blob/64f0a67a9264197c8288845d2d2ef17bf433425d/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js#L88 https://github.com/bistaastha/caliper/blob/64f0a67a9264197c8288845d2d2ef17bf433425d/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js#L96
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. https://github.com/bistaastha/caliper/blob/64f0a67a9264197c8288845d2d2ef17bf433425d/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js#L89
4. You don't need to save this on the instance level, won't use it outside of this function: https://github.com/bistaastha/caliper/blob/64f0a67a9264197c8288845d2d2ef17bf433425d/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js#L95
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) https://github.com/bistaastha/caliper/blob/64f0a67a9264197c8288845d2d2ef17bf433425d/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js#L97
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: https://github.com/bistaastha/caliper/blob/64f0a67a9264197c8288845d2d2ef17bf433425d/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js#L98

7. You don't need these, the Contract just passes them down the hierarchy: https://github.com/bistaastha/caliper/blob/64f0a67a9264197c8288845d2d2ef17bf433425d/packages/caliper-core/lib/worker/workload/declarative/contract.js#L29-L30
8. Instead of this: https://github.com/bistaastha/caliper/blob/64f0a67a9264197c8288845d2d2ef17bf433425d/packages/caliper-core/lib/worker/workload/declarative/contract.js#L32 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: https://github.com/bistaastha/caliper/blob/8128511f5ddbc144e574646e5c500f604984ec6a/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js#L60 https://github.com/bistaastha/caliper/blob/8128511f5ddbc144e574646e5c500f604984ec6a/packages/caliper-core/lib/worker/workload/declarative/contract.js#L28 https://github.com/bistaastha/caliper/blob/8128511f5ddbc144e574646e5c500f604984ec6a/packages/caliper-core/lib/worker/workload/declarative/contract.js#L31 https://github.com/bistaastha/caliper/blob/8128511f5ddbc144e574646e5c500f604984ec6a/packages/caliper-core/lib/worker/workload/declarative/contract-function.js#L28 https://github.com/bistaastha/caliper/blob/8128511f5ddbc144e574646e5c500f604984ec6a/packages/caliper-core/lib/worker/workload/declarative/contract-function.js#L31 https://github.com/bistaastha/caliper/blob/8128511f5ddbc144e574646e5c500f604984ec6a/packages/caliper-core/lib/worker/workload/declarative/contract-function-parameter.js#L27
10. What is parameterList? I guess it's a leftover variable. Also there is a trailing space linting error before this part: https://github.com/bistaastha/caliper/blob/8128511f5ddbc144e574646e5c500f604984ec6a/packages/caliper-core/lib/worker/workload/declarative/contract-function-parameter.js#L29-L35
11. This should be saved as this.valueProvider, its intention is more clear that way: https://github.com/bistaastha/caliper/blob/8128511f5ddbc144e574646e5c500f604984ec6a/packages/caliper-core/lib/worker/workload/declarative/contract-function-parameter.js#L37
12. The overridden super function must be called before this line, so the parameters are saved in the base class: https://github.com/bistaastha/caliper/blob/8128511f5ddbc144e574646e5c500f604984ec6a/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js#L44

    Next tasks

T1. The generateValue call should return the value generated by the underlying value provider: https://github.com/bistaastha/caliper/blob/8128511f5ddbc144e574646e5c500f604984ec6a/packages/caliper-core/lib/worker/workload/declarative/contract-function-parameter.js#L44-L47

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; } https://github.com/bistaastha/caliper/blob/8128511f5ddbc144e574646e5c500f604984ec6a/packages/caliper-core/lib/worker/workload/declarative/contract-function.js#L38-L40

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: https://github.com/bistaastha/caliper/blob/d6d81d40d40a3de2c47d45e0f710168a9001dc74/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js#L59
2. Should also check that the list has at least one element: https://github.com/bistaastha/caliper/blob/d6d81d40d40a3de2c47d45e0f710168a9001dc74/packages/caliper-core/lib/worker/workload/declarative/value-providers/uniform-random-list-item-value-provider.js#L25
3. Will this give a correct range? https://github.com/bistaastha/caliper/blob/d6d81d40d40a3de2c47d45e0f710168a9001dc74/packages/caliper-core/lib/worker/workload/declarative/value-providers/uniform-random-list-item-value-provider.js#L34 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: https://github.com/bistaastha/caliper/blob/d6d81d40d40a3de2c47d45e0f710168a9001dc74/packages/caliper-core/lib/worker/workload/declarative/contract-function.js#L39 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 https://github.com/bistaastha/caliper/blob/e5e18fabf34d6525c89353dc3ea894ad216c0988/packages/caliper-core/lib/worker/workload/declarative/contract-function.js#L40
2. The first call needs to be a super call to the same function of the base class, so it initializes the instance variables: https://github.com/bistaastha/caliper/blob/e5e18fabf34d6525c89353dc3ea894ad216c0988/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js#L44-L45 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: https://github.com/bistaastha/caliper/blob/e5e18fabf34d6525c89353dc3ea894ad216c0988/packages/caliper-core/lib/worker/workload/declarative/contract.js#L38-L44 T2. Randomly select a contract and returns its (randomly selected function's) generated arguments: https://github.com/bistaastha/caliper/blob/e5e18fabf34d6525c89353dc3ea894ad216c0988/packages/caliper-core/lib/worker/workload/declarative/declarative-workload-module-base.js#L71 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: https://github.com/bistaastha/caliper/blob/361c03b8587245e2d12447c2d774c5bf3e289091/packages/caliper-core/index.js#L32