search

accordproject / template-archive

Smart Legal Contracts & Templating System
https://accordproject.org/projects/cicero/
Apache License 2.0
280 stars 119 forks source link

feat:(cicero-core) signing templates review #670

Closed sanketshevkar closed 3 years ago

sanketshevkar commented 3 years ago

Signed-off-by: Sanket Shevkar shevkar.sanket@gmail.com

Refer https://github.com/sanketshevkar/TemplateContractSigning for demo.

Changes

Screenshots or Video

https://www.youtube.com/watch?v=nvuJfN2Vkkw

Author Checklist

sanketshevkar commented 3 years ago
  1. How does the PR compare to the one here (in terms of functionality, cryptographic scheme, and general approach? #496

The dependencies are same, both approaches use PKCS12 keystores to store private keys and certs. Difference being, in my approach contract hash is signed and stored in an external json file and later picked from there for verification. Josh in #496 is using PKCS7 keystores to store the signatures, this needed some code in node-forge to be updated and that PR has been stuck for a long time.

  1. It definitely will require some testing (both unit tests and maybe some documentation as to how would someone use this)
  2. Should there be some command-line options for signing? What's the envisioned user interface?

This is just for code review of the basic workflow and functionality, I'll be writing unit tests, documentation and cli support in the branch you created. Apart from cli I was thinking of doing something similar to docusign.

  1. I'm wondering if the signature code between the contract class and the template class could be shared (or could be both based on a common utility library -- it's a lot of delicate code which look very similar).
  2. Quite a few linting errors, you can test locally with npm run test which I believe also runs the linter

I agree with you. I'll keep in mind to do these changes.

jeromesimeon commented 3 years ago
  1. How does the PR compare to the one here (in terms of functionality, cryptographic scheme, and general approach? #496

The dependencies are same, both approaches use PKCS12 keystores to store private keys and certs. Difference being, in my approach contract hash is signed and stored in an external json file and later picked from there for verification. Josh in #496 is using PKCS7 keystores to store the signatures, this needed some code in node-forge to be updated and that PR has been stuck for a long time.

Ok, that's great! I think a couple of things would help:

  1. It definitely will require some testing (both unit tests and maybe some documentation as to how would someone use this)
  2. Should there be some command-line options for signing? What's the envisioned user interface?

This is just for code review of the basic workflow and functionality, I'll be writing unit tests, documentation and cli support in the branch you created.

Sounds good!

Apart from cli I was thinking of doing something similar to docusign.

I'd love to hear more about that, could you explain? (similar to docusign in which sense?)

  1. I'm wondering if the signature code between the contract class and the template class could be shared (or could be both based on a common utility library -- it's a lot of delicate code which look very similar).
  2. Quite a few linting errors, you can test locally with npm run test which I believe also runs the linter

I agree with you. I'll keep in mind to do these changes.

👍

jeromesimeon commented 3 years ago

@sanketshevkar I believe this is superceded by #672 ? If yes, can we please close this PR.

sanketshevkar commented 3 years ago

@sanketshevkar I believe this is superceded by #672 ? If yes, can we please close this PR.

Yes, we can close this PR.