feat(connector-corda): support corda 5 RC via TS/HTTP (no JVM) #2814

adrianbatuto · 2023-10-19T09:30:50Z

Fixes #1480
Pull Request Requirements

Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

...cactus-plugin-ledger-connector-corda/src/main/typescript/web-services/get-cpi-endpoint-v1.ts

+      get: async () => ({
+        isProtected: true,
+        requiredRoles: [],
+        httpsAgent: new https.Agent({ rejectUnauthorized: false }),


...s/cactus-plugin-ledger-connector-corda/src/test/typescript/integration/corda-v5-flow-test.ts

+
+  const customHttpsAgent = new https.Agent({
+    // Configure your custom settings here
+    rejectUnauthorized: false, // Example: Allow self-signed certificates (use with caution)


Fixes hyperledger-cacti#1480 Signed-off-by: adrian.batuto <[email protected]>

petermetz

@adrianbatuto Preliminary change requests (I'll do a more thorough review after these initial points are addressed):

Please migrate the test case onto Jest instead of tape (and also remove the tape imports).
Please rename the test file corda-v5-flow-test.ts to end with .test.ts instead of -test.ts because that way VSCode will detect it as a test case instead of a code file that contains shippable code.
When I ran the test file locally, it failed on the last test case, could you please verify if this works on your machine and if yes please attach the exact command you used to run it and the complete log output of the test case as a file attachment.
Question: Does this change retain the v4 support in the connector or does it erase it? E.g., if we merged this diff, would it still be able to support v4.8 Corda ledgers as well as v5 ones or only v5?

adrianbatuto · 2023-10-25T05:37:35Z

@adrianbatuto Preliminary change requests (I'll do a more thorough review after these initial points are addressed):

Please migrate the test case onto Jest instead of tape (and also remove the tape imports).

Please rename the test file corda-v5-flow-test.ts to end with .test.ts instead of -test.ts because that way VSCode will detect it as a test case instead of a code file that contains shippable code.

When I ran the test file locally, it failed on the last test case, could you please verify if this works on your machine and if yes please attach the exact command you used to run it and the complete log output of the test case as a file attachment.

Question: Does this change retain the v4 support in the connector or does it erase it? E.g., if we merged this diff, would it still be able to support v4.8 Corda ledgers as well as v5 ones or only v5?

Hi @petermetz , I'll work on your requested changes for 1-2.

The testing does work on my machine locally, however for it to work you need to change the endpoints on the openapi.json file and then do a yarn run codegen. This is an example of the endpoint that works

The endpoints that I pushed here in this PR is using the corda connector.

Our expectation is that the test would pass after the connector has been published.

It should still retain the support for v4.

jagpreetsinghsasan · 2023-11-16T07:37:47Z

tools/docker/corda-all-in-one/corda-v5/healthcheck.sh

-curl -u earthling:password --insecure https://localhost:12112/api/v1/nodestatus/getnetworkreadinessstatus
-curl -u plutonian:password --insecure https://localhost:12119/api/v1/nodestatus/getnetworkreadinessstatus
+#Need to change since token is needed to check health status
+#curl -u admin:admin --insecure https://localhost:8888/api/v1/swagger#


Can we have a better healthcheck here?
For example, running the listVNodes gradle task will list all the 5 nodes (4 nodes and notary server) and then parsing the output till it shows those 5 nodes up and running

jagpreetsinghsasan · 2023-11-16T07:41:09Z

tools/docker/corda-all-in-one/corda-v5/start-services.sh

+  fi
+  sleep 5;
+done
+touch 5vNodeSetupdone.txt


This is not required I suppose (I added it as I was testing the next part of the script i.e. deploying the cordapps)

jagpreetsinghsasan · 2023-11-16T07:42:32Z

tools/docker/corda-all-in-one/corda-v5/start-services.sh

+cd /CSDE-cordapp-template-kotlin/
+while true; do
+  echo "Waiting for startCorda to execute..."
+  if ./gradlew listVnodes | grep "X500"; then


I think this is listVNodes (not sure if gradle tasks are case sensitive or not)

jagpreetsinghsasan · 2023-11-16T07:44:09Z

packages/cactus-plugin-persistence-ethereum/package.json

-    "copy-yarn-lock": "cp -af ../../yarn.lock ./dist/yarn.lock",
-    "generate-sdk": "openapi-generator-cli generate -i ./src/main/json/openapi.json -g typescript-axios -o ./src/main/typescript/generated/openapi/typescript-axios/ --reserved-words-mappings protected=protected"
+    "generate-sdk": "openapi-generator-cli generate -i ./src/main/json/openapi.json -g typescript-axios -o ./src/main/typescript/generated/openapi/typescript-axios/ --reserved-words-mappings protected=protected",
+    "copy-sql": "cp -r ./src/main/sql ./dist/lib/main/",


Why do we need this and the below copy-yarn-lock scripts?

jagpreetsinghsasan · 2023-11-16T07:47:34Z

...s/cactus-plugin-ledger-connector-corda/src/test/typescript/integration/corda-v5-flow-test.ts

+    cwd,
+  );
+
+  function extractShortHash(name: string) {


Can we have this function defined within the corda-v5-test-ledger.ts file? And then maybe return the shortHash of all the entities as a list (so that one can later write an equivalent function for production grade scenarios)

jagpreetsinghsasan · 2023-11-16T07:52:44Z

...s/cactus-plugin-ledger-connector-corda/src/test/typescript/integration/corda-v5-flow-test.ts

+  t.ok(startflow.status, "startFlowParameters endpoint OK");
+
+  await waitProcess(5);
+  await waitForStatusChange(shortHashCharlie, "test-1");


We can have this function also defined within the test ledgers (This helps in creating similar utilities for the production environment. For example, if the production environment is lets say Kubernetes, one can setup the corda network there and can simply look into our test-ledger class to create equivalent utilities for the kubernetes environment.)

jagpreetsinghsasan · 2023-11-16T07:58:13Z

...s/cactus-plugin-ledger-connector-corda/src/test/typescript/integration/corda-v5-flow-test.ts

@@ -0,0 +1,348 @@
+import test, { Test } from "tape-promise/tape";


We will need additional code & test cases on deploying the cordapps itself (and if possible, such functions should be independent of the environment where the network is setup which in this case is docker. This helps in re-usability of same functions for production environments)

jagpreetsinghsasan · 2023-11-16T08:12:25Z

...es/cactus-plugin-ledger-connector-corda/src/main/typescript/plugin-ledger-connector-corda.ts

@@ -232,6 +253,35 @@ export class PluginLedgerConnectorCorda
      endpoints.push(endpoint);
    }

+    {
+      const opts: IListCPIEndpointV1Options = {


We will also need to enable/disable these endpoints based on the version specified by CordaVersion var type

jagpreetsinghsasan · 2023-11-16T08:19:10Z

...cactus-plugin-ledger-connector-corda/src/main/typescript/web-services/get-cpi-endpoint-v1.ts

+
+    try {
+      if (this.apiUrl === undefined) throw "apiUrl option is necessary";
+      const body = await this.callInternalContainer(req.body);


Can we have connector class functions instead of calling internal container as all the nodes are exposed to the host network for now and then a single unified function within the connector class can handle GET/POST calls for all the flow related calls?

The same goes for all the endpoints for v5 and thus their implementation in the test case

jagpreetsinghsasan · 2023-11-16T08:20:43Z

...es/cactus-plugin-ledger-connector-corda/src/main/typescript/plugin-ledger-connector-corda.ts

+      };
+      const endpoint = new FlowStatusResponseEndpointV1(opts);
+      endpoints.push(endpoint);
+    }
    this.log.info(`Instantiated endpoints of ${pkgName}`);
    return endpoints;
  }


Here we should have the function to call all the flow rpc calls GET/POST.. (instead of having another server client layer for the same)

petermetz · 2024-01-24T09:04:59Z

@adrianbatuto How is this one going? Do you have an ETA?

adrianbatuto · 2024-01-25T06:18:27Z

@adrianbatuto How is this one going? Do you have an ETA?

Hi @petermetz, as I mentioned in the meeting earlier, I am done with the review points you have provided. I am now in progress with Jag's review points. I also think that there might be additional changes required since currently we are only able to run the test successfully using the endpoints that corda have provided, however when using the endpoints with the corda connector I am getting a 404 error. I am also expecting further review points since the reviews so far are only preliminary.

petermetz · 2024-01-26T01:20:36Z

@adrianbatuto How is this one going? Do you have an ETA?

Hi @petermetz, as I mentioned in the meeting earlier, I am done with the review points you have provided. I am now in progress with Jag's review points. I also think that there might be additional changes required since currently we are only able to run the test successfully using the endpoints that corda have provided, however when using the endpoints with the corda connector I am getting a 404 error. I am also expecting further review points since the reviews so far are only preliminary.

@adrianbatuto Thank you for the updates!

adrianbatuto · 2024-01-31T03:27:54Z

Closing this PR for now. As per discussions with @jagpreetsinghsasan there will be some significant changes to the code. Will reopen this PR or create a new one once those changes are ready for review.

github-advanced-security bot found potential problems Oct 19, 2023

View reviewed changes

adrianbatuto force-pushed the r3cordav5 branch 3 times, most recently from 84236a3 to 5a30cba Compare October 20, 2023 07:15

feat(connector-corda): support corda 5 RC via TS/HTTP (no JVM)

3cee879

Fixes hyperledger-cacti#1480 Signed-off-by: adrian.batuto <[email protected]>

adrianbatuto force-pushed the r3cordav5 branch from 5a30cba to 3cee879 Compare October 20, 2023 07:18

adrianbatuto marked this pull request as ready for review October 20, 2023 07:19

adrianbatuto requested review from petermetz, takeutak, izuru0, jagpreetsinghsasan, VRamakrishna and sandeepnRES as code owners October 20, 2023 07:19

petermetz requested changes Oct 23, 2023

View reviewed changes

jagpreetsinghsasan requested changes Nov 16, 2023

View reviewed changes

petermetz mentioned this pull request Dec 5, 2023

build: upgrade all test infrastructure container images to [email protected] #2853

Open

adrianbatuto closed this Jan 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(connector-corda): support corda 5 RC via TS/HTTP (no JVM) #2814

feat(connector-corda): support corda 5 RC via TS/HTTP (no JVM) #2814

adrianbatuto commented Oct 19, 2023 •

edited

Loading

petermetz left a comment

adrianbatuto commented Oct 25, 2023 •

edited

Loading

jagpreetsinghsasan Nov 16, 2023

jagpreetsinghsasan Nov 16, 2023

jagpreetsinghsasan Nov 16, 2023

jagpreetsinghsasan Nov 16, 2023

jagpreetsinghsasan Nov 16, 2023

jagpreetsinghsasan Nov 16, 2023

jagpreetsinghsasan Nov 16, 2023

jagpreetsinghsasan Nov 16, 2023

jagpreetsinghsasan Nov 16, 2023

jagpreetsinghsasan Nov 16, 2023

jagpreetsinghsasan Nov 16, 2023

petermetz commented Jan 24, 2024

adrianbatuto commented Jan 25, 2024 •

edited

Loading

petermetz commented Jan 26, 2024

adrianbatuto commented Jan 31, 2024

		@@ -0,0 +1,348 @@
		import test, { Test } from "tape-promise/tape";

feat(connector-corda): support corda 5 RC via TS/HTTP (no JVM) #2814

feat(connector-corda): support corda 5 RC via TS/HTTP (no JVM) #2814

Conversation

adrianbatuto commented Oct 19, 2023 • edited Loading

petermetz left a comment

Choose a reason for hiding this comment

adrianbatuto commented Oct 25, 2023 • edited Loading

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

petermetz commented Jan 24, 2024

adrianbatuto commented Jan 25, 2024 • edited Loading

petermetz commented Jan 26, 2024

adrianbatuto commented Jan 31, 2024

adrianbatuto commented Oct 19, 2023 •

edited

Loading

adrianbatuto commented Oct 25, 2023 •

edited

Loading

adrianbatuto commented Jan 25, 2024 •

edited

Loading