Skip to content

refs #1 - fixes the concurrency problem#14

Open
TiagoV-PDMFC wants to merge 1 commit into
OpenDSU:masterfrom
TiagoV-PDMFC:refs1_concurrency
Open

refs #1 - fixes the concurrency problem#14
TiagoV-PDMFC wants to merge 1 commit into
OpenDSU:masterfrom
TiagoV-PDMFC:refs1_concurrency

Conversation

@TiagoV-PDMFC

Copy link
Copy Markdown

as mentioned, this aims to solve this issue:


 sc.setMainDSU(walletDSU);
 sc.refreshSecurityContext()
sc.on("initialised", () => {
    const keySSI = KeySSIApi.createTemplateSeedSSI(...args)
    resolver.createDSU(keyssi, (err, dsu) => {
       callback(err, dsu)
    });
});```

If you run this code in parallel using the same walletDSU as a SC, this will break under the createDSU call, since it will try to concurrently store the seedSSI in the current enclave and trigger a out of SYNC error.

After inspecting your code we notices the Seed, Const and Array DSU creation methods  **NOW force** the existence on an enclave and store their KeySSI on it. I'm not sure that's the indented behavior, after all the method is not called `createDSUAndEnclaveIfItDoesNotExistAndStoreKeySSIInEnclave` which would currently be the accurate description.

This pull request reverts the default `createDSU` implementation to not force an enclave by default (I left the PathSSI DSU creation method unchanged because its implementation did not have an `if (enclave) ...` and because I don't know the purpose of a PathKeySSI.

I ran your tests and I'm sorry to say 2 tests failed (on top the the ones already originally failing). I did not try and resolve them since I do not understand why this `enclave by default` was added (effectively killing concurrency and duplicating the amount of writes in the blockchain per simple DSU).

 I don't understand what feature you're implementing that needs this and cannot call the `wc_create<XXX>SSSI` directly passing it the enclave when required, but in order to be able to have multiple instances of the same wallet running operations concurrently (**which we very much need for performance and to avoid the `Singleton Security Context` problem**) we are currently patching the OpenDSU bundles with the change in this PR (and due to it, successfully running several concurrent operations simultaneously in our backend services)
 
 Please let us know if this PR can be approved (or if not, what is the reason for this opinionated hardcoded enclave write?) But regardless, this slight change is imperative for us in our current project, and will be ever more imperative should the production grade FGT move forward in the next few months as we expect it to

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant