Implement caching for parameters and identity storage contracts #2834

milosstanisavljevic · 2023-12-11T14:10:09Z

Description

Added listener for Parameters storage contract. Implemented caching for functions inside parameters storage.

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Tested in the local env, creating assets locally.

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

src/modules/blockchain/implementation/web3-service.js

0xbraindevd

Looks good, but there are other ways how to reduce number of calls. Just to name a few:

In the ValidationService we're separately checking assertionSize, assertionTriplesNumber and assertionChunksNumber. We're using 3 calls here. Instead, we can just use 1 call getAssertion, which would return all the data
callScoreFunction can be replaced with local implementation of the score function (same as we did for hashing) [Now as I see we don't use this function]
One more idea, maybe we can validate bid on publisher node before sending requests instead of validating on every node?
Potentially we could also cache getLog2PLDSFParams, but we need to add events for that contract

There could be more optimizations, just ran through the code quickly

P.S. Looks like we're missing validateAssertion for update receiver. Not related to the topic, but adding it here just not to forget later

0xbraindevd · 2023-12-11T23:33:23Z

src/modules/blockchain/implementation/web3-service.js

@@ -406,10 +416,16 @@ class Web3Service {

    async callContractFunction(contractInstance, functionName, args) {


Since we're using this function for identityId, old logic for getIdentityId function should be removed and we can add it to cached parameters since it's immutable

0xbraindevd · 2023-12-12T08:36:03Z

One more idea we should consider, what if we use MySQL as a key-value store instead of caching in memory?

zeroxbt · 2023-12-12T23:58:09Z

src/constants/constants.js

@@ -586,3 +588,16 @@ export const BLOCK_TIME_MILLIS = {
 };

 export const TRANSACTION_CONFIRMATIONS = 1;
+
+export const CACHED_FUNCTIONS = {


Could you also please move the cached values from the scoring function ?

zeroxbt · 2023-12-13T00:06:34Z

src/modules/blockchain/implementation/web3-service.js

            try {
                // eslint-disable-next-line no-await-in-loop
                result = await contractInstance[functionName](...args);
+                if (CACHED_FUNCTIONS.ParametersStorage.includes(functionName)) {
+                    this.cacheParameter(functionName, result);
+                }
            } catch (error) {


Is there a reason why we don't call "handleError" in the catch block anymore ? This was previously used to switch RPCs in case the error was due to RPC being down.
If this is intended, then the while loop is not necessary and can be removed.

We switched to a new provider implementation in the ethers.js that automatically hits different RPC after defined timeout, while loop probably can be removed as we're not retrying the call anyway (because of throwing new error)

0xbraindevd · 2023-12-13T11:54:15Z

src/constants/constants.js

@@ -589,15 +589,19 @@ export const BLOCK_TIME_MILLIS = {

 export const TRANSACTION_CONFIRMATIONS = 1;

+export const CACHE_DATA_TYPES = {


Not sure I get why would we need this

Still don't think it's needed, for the specific events we have predefined types in the ABI

e.g. for ParametersStorage it can't be anything else except Number, so I would just cast values based on the types in the ABI

0xbraindevd · 2023-12-13T11:55:04Z

src/modules/blockchain/implementation/web3-service.js

@@ -272,11 +273,28 @@ class Web3Service {
    }

    cacheParameter(parameterName, parameterValue) {
-        resultCache[parameterName] = parameterValue;
+        console.log('PARAMETER IME', parameterName);
+        const found = Object.values(CACHED_FUNCTIONS)


Why not just add contractName as argument for the function?

0xbraindevd · 2023-12-13T11:55:10Z

src/modules/blockchain/implementation/web3-service.js

    }

    getCachedValue(parameterName) {
-        return resultCache[parameterName];
+        if (CACHED_FUNCTIONS.ParametersStorage.some((item) => item.name === parameterName)) {


Why not just add contractName as argument for the function?

0xbraindevd · 2023-12-14T13:21:46Z

src/constants/constants.js

+};
+
+export const CACHED_FUNCTIONS = {
+    ParametersStorage: [


Let's make it an object, with parameterName as key and type as value, then we don't need to iterate through all parameters every time.

If we remove types as I suggested in the comment above, we can just make it a Set with constant O(1) lookup

0xbraindevd · 2023-12-14T13:22:22Z

src/modules/blockchain/implementation/web3-service.js

+    cacheParameter(contractName, parameterName, parameterValue) {
+        const found =
+            CACHED_FUNCTIONS[contractName] &&
+            CACHED_FUNCTIONS[contractName].find((item) => item.name === parameterName);


This is not needed if CACHED_FUNCTIONS is an object/Set instead of array

0xbraindevd · 2023-12-14T13:25:18Z

src/constants/constants.js

@@ -589,15 +589,19 @@ export const BLOCK_TIME_MILLIS = {

 export const TRANSACTION_CONFIRMATIONS = 1;

+export const CACHE_DATA_TYPES = {


Still don't think it's needed, for the specific events we have predefined types in the ABI

e.g. for ParametersStorage it can't be anything else except Number, so I would just cast values based on the types in the ABI

0xbraindevd · 2023-12-14T13:31:04Z

src/modules/blockchain/implementation/web3-service.js

+            CACHED_FUNCTIONS[contractName].find((item) => item.name === parameterName);
+        if (found) {
+            const { type } = found;
+            switch (type) {


Following the suggestion to remove types from constants, I would make it a separate function somewhere in the services or even utils for casting based on types of the event arguments in the ABI and cast it directly in the event handler instead of doing it here

0xbraindevd · 2023-12-14T13:31:54Z

src/modules/blockchain/implementation/web3-service.js

+    getCachedValue(contractName, parameterName) {
+        if (
+            CACHED_FUNCTIONS[contractName] &&
+            CACHED_FUNCTIONS[contractName].some((item) => item.name === parameterName)


This is not needed if CACHED_FUNCTIONS is an object/Set instead of array

0xbraindevd · 2023-12-14T13:33:17Z

src/modules/blockchain/implementation/web3-service.js

-        while (result === undefined) {
+        result = this.getCachedValue(contractName, functionName);
+
+        while (!result) {


while loop here is redundant, we won't ever reiterate here

0xbraindevd · 2023-12-14T13:33:56Z

src/modules/blockchain/implementation/web3-service.js

@@ -404,12 +432,15 @@ class Web3Service {
        }
    }

-    async callContractFunction(contractInstance, functionName, args) {
+    async callContractFunction(contractInstance, functionName, args, contractName = null) {


Can't we add name field to the contractInstance during initialization?

src/modules/blockchain/implementation/web3-service.js

Co-authored-by: Nikola Todorovic <nikolaztodorovic26@gmail.com>

Milos Stanisavljevic added 2 commits December 11, 2023 14:12

Implementation of caching and parameter storage handler

a8c7806

caching code improvement and removing unnecessary logs

d0870db

milosstanisavljevic self-assigned this Dec 11, 2023

milosstanisavljevic requested review from branarakic, djordjekovac, NZT48, zeroxbt and 0xbraindevd as code owners December 11, 2023 14:10

djordjekovac reviewed Dec 11, 2023

View reviewed changes

src/modules/blockchain/implementation/web3-service.js Outdated Show resolved Hide resolved

Milos Stanisavljevic added 2 commits December 11, 2023 15:39

code improvement

ade1090

undefined bugfix

3176845

0xbraindevd reviewed Dec 12, 2023

View reviewed changes

zeroxbt reviewed Dec 12, 2023

View reviewed changes

zeroxbt reviewed Dec 13, 2023

View reviewed changes

Milos Stanisavljevic added 2 commits December 13, 2023 12:27

updated cache to be scalable

829599f

updated cache to be scalable

4ac849d

0xbraindevd requested changes Dec 13, 2023

View reviewed changes

added contractName as parameter in caching

d44ed6d

0xbraindevd requested changes Dec 14, 2023

View reviewed changes

djordjekovac added 3 commits December 25, 2023 12:10

Updated Caching of contract calls

76ca352

Updated caching

04fbd45

Merge branch 'v6/develop' into v6/improvement/reduce-rpc-calls

962de73

NZT48 approved these changes Dec 25, 2023

View reviewed changes

src/modules/blockchain/implementation/web3-service.js Outdated Show resolved Hide resolved

Update src/modules/blockchain/implementation/web3-service.js

4810571

Co-authored-by: Nikola Todorovic <nikolaztodorovic26@gmail.com>

NZT48 changed the title ~~V6/improvement/reduce rpc calls~~ Implement caching for parameters and identity storage contracts Dec 25, 2023

djordjekovac merged commit 864fc31 into v6/develop Dec 25, 2023
3 checks passed

djordjekovac deleted the v6/improvement/reduce-rpc-calls branch December 25, 2023 11:54

This was referenced Dec 25, 2023

OriginTrail Devnet Prerelease v6.1.1 #2851

Merged

OriginTrail Devnet Release v6.1.1 #2852

Merged

This was referenced Dec 27, 2023

OriginTrail Testnet Prerelease v6.1.2 #2858

Merged

OriginTrail Testnet Release v6.1.2 #2859

Merged

OriginTrail Mainnet Prerelease v6.1.2 #2870

Merged

OriginTrail Mainnet Release v6.1.2 #2871

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement caching for parameters and identity storage contracts #2834

Implement caching for parameters and identity storage contracts #2834

milosstanisavljevic commented Dec 11, 2023 •

edited

Loading

0xbraindevd left a comment •

edited

Loading

0xbraindevd Dec 11, 2023

0xbraindevd commented Dec 12, 2023

zeroxbt Dec 12, 2023

zeroxbt Dec 13, 2023

0xbraindevd Dec 13, 2023

0xbraindevd Dec 13, 2023

0xbraindevd Dec 14, 2023

0xbraindevd Dec 13, 2023

0xbraindevd Dec 13, 2023

0xbraindevd Dec 14, 2023

0xbraindevd Dec 14, 2023

0xbraindevd Dec 14, 2023

0xbraindevd Dec 14, 2023

0xbraindevd Dec 14, 2023

0xbraindevd Dec 14, 2023

0xbraindevd Dec 14, 2023

		@@ -406,10 +416,16 @@ class Web3Service {

		async callContractFunction(contractInstance, functionName, args) {

		@@ -589,15 +589,19 @@ export const BLOCK_TIME_MILLIS = {

		export const TRANSACTION_CONFIRMATIONS = 1;

		export const CACHE_DATA_TYPES = {

Implement caching for parameters and identity storage contracts #2834

Implement caching for parameters and identity storage contracts #2834

Conversation

milosstanisavljevic commented Dec 11, 2023 • edited Loading

Description

Type of change

How Has This Been Tested?

Checklist:

0xbraindevd left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xbraindevd commented Dec 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milosstanisavljevic commented Dec 11, 2023 •

edited

Loading

0xbraindevd left a comment •

edited

Loading