New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(*): state management of contract instances #709

Closed

sanketshevkar wants to merge 42 commits into accordproject:poc-slc from sanketshevkar:sanketshevkar/state-management-slc

Member

sanketshevkar commented Dec 8, 2021 •

edited

Loading

Changes

added to new utility function added to cicero-core to add history of a contract instance to the slc file.
A new history.json file stores all the history/states of that contract instance.
New api endpoints added to cicero-server to handle contract instances.

Author Checklist

Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
Vital features and changes captured in unit and/or integration tests
Commits messages follow AP format
Extend the documentation, if necessary
Merging to master from fork:branchname

jeromesimeon and others added 30 commits

August 24, 2021 13:33


          refactor(core) Clearer separation between contract template and contr…

665402b

…act instance

Signed-off-by: jeromesimeon <[email protected]>


          refactor(core) Rename instance classes

81a9a43

Signed-off-by: jeromesimeon <[email protected]>


          feature(instance) Adjust API, passing metadata to the instance

5cb540f

Signed-off-by: Jerome Simeon <[email protected]>


          feature(slc) Initial support for creating a smart legal contract archive

18c9bab

Signed-off-by: Jerome Simeon <[email protected]>


          fix(slc) Add contract data to .slc archive

632650b

Signed-off-by: Jerome Simeon <[email protected]>


          feature(core) Distinguish between template and instance metadata

232ea73

Signed-off-by: Jerome Simeon <[email protected]>


          chore(slc) Some basic testing for slc archives creation

1e23cb7

Signed-off-by: Jerome Simeon <[email protected]>


          feature(slc) Allow triggers on smart legal contracts

18bfc3a

Signed-off-by: Jerome Simeon <[email protected]>


          feature(slc) Add invoke and initialization support for smart legal co…

1d4a3da

…ntracts

Signed-off-by: Jerome Simeon <[email protected]>


          feature(slc) Export capability for smart legal contracts

a4cda7e

Signed-off-by: jeromesimeon <[email protected]>


          chore(build) Remove lib from gitignore for new package

60eb0e0

Signed-off-by: jeromesimeon <[email protected]>


          feat(*): contract instance signing and verification

71ed62a

Signed-off-by: sanket shevkar <[email protected]>


          test(cli): test for signing contract

e77f74f

Signed-off-by: sanket shevkar <[email protected]>


          test(cli): contract signatures verification

abd7f37

Signed-off-by: sanket shevkar <[email protected]>


          refactor(*): signed slc name and contract instance error

Signed-off-by: sanket shevkar <[email protected]>


          test(core): failing test cases for contract signature verification

1be8869

Signed-off-by: sanket shevkar <[email protected]>


          refactor(*): removal of commented code and logger message correction …

4c1fc86

…(last gsoc commit)

Signed-off-by: sanket shevkar <[email protected]>


          fix(test) Fixes failing negative tests for verify signature

609b42c

Signed-off-by: jeromesimeon <[email protected]>


          test(cicero-cli): failing tests corrected

20cb3ab

Signed-off-by: sanket shevkar <[email protected]>


          feat(*): state management for contract instances

fe89b74

Signed-off-by: sanket shevkar <[email protected]>


          refactor(cli): removal of duplicate verify command

4564b46

Signed-off-by: sanket shevkar <[email protected]>


          feat(*): state management for contract instances

0a4d5ed

Signed-off-by: sanket shevkar <[email protected]>


          refactor(core): new utility function to add state

f548deb

Signed-off-by: sanket shevkar <[email protected]>


          refactor(core): new utility function to add state

97523ba

Signed-off-by: sanket shevkar <[email protected]>


          feat(cli): readline module imported for input

fdc24dc

Signed-off-by: sanket shevkar <[email protected]>


          feat(*): cli interactive signing

18784ad

Signed-off-by: sanket shevkar <[email protected]>


          test(core): update test of contract instances for state management

2c885a0

Signed-off-by: sanket shevkar <[email protected]>


          test(core): updated cta files with state management

00b4658

Signed-off-by: sanket shevkar <[email protected]>


          test(*): test correction of cli and new test for utils

6ea8253

Signed-off-by: sanket shevkar <[email protected]>


          test(cli): change test data slc file

04964b3

Signed-off-by: sanket shevkar <[email protected]>

sanket shevkar and others added 8 commits

September 30, 2021 17:00


          feat(cli): trigger invoke initialize added

da397ce

Signed-off-by: sanket shevkar <[email protected]>


          feat(cicero-server): endpoint creation for instantiating and signing …

fea21d6

…contract instances

Signed-off-by: sanketshevkar <[email protected]>


          refactor(cicero-server): access keystore from local storage of the sever

cac375c

Signed-off-by: sanketshevkar <[email protected]>


          refactor(*) states property changed to history, new timestamp format …

e32bd6b

…added, trigger result added to history

Signed-off-by: sanketshevkar <[email protected]>


          test(server): addition of new tests for all new apis

c9e2c9e

Signed-off-by: sanketshevkar <[email protected]>


          test(*): updating tests after code refactor

2fd9eee

Signed-off-by: sanketshevkar <[email protected]>


          refactor(server): adding cicero-transform dependency

28dca9f

Signed-off-by: sanketshevkar <[email protected]>


          refactor(core): resolve merge conflicts

b5fae6f

Signed-off-by: sanketshevkar <[email protected]>

coveralls commented Dec 8, 2021

Coverage decreased (-1.08%) to 96.198% when pulling b5fae6f on sanketshevkar:sanketshevkar/state-management-slc into 719594c on accordproject:master.

dselman requested changes

View reviewed changes

Contributor

dselman left a comment •

edited

Loading

I thought this was VERY impressive, but I have reservations about how maintainable it will be, in particular about how we have grafted instances into the code base for templates. I think it would be useful to take a step back and potentially to move the instance management code outside of core into a new cicero-contract package that uses cicero-core rather than extends and duplicates some of its logic. This will no doubt lead to some duplication in other areas (e.g. an instance zip file may have to be a zip of zip files (for the templates)) but I think the benefits in terms of modularity probably outweighs the costs. Let's discuss on a call.

We also need to work on the docs, which may also lead to some code improvements in terms of naming and conceptual clarity.

packages/cicero-core/src/contractinstance.js

+                          instance,
+                          instance.instantiator,
+                          'instantiate',
+                          'Instantiated Succesfully',

Contributor

dselman Dec 8, 2021

Localization?

Member

martinhalford Dec 10, 2021 •

edited

Loading

Not sure I understand the question. Are we talking about not having hard-coded strings in English? TBD.

packages/cicero-core/src/contractinstance.js

+                      if (this.contractSignatures.length !== 0 ) {
+                          this.verifySignatures();
+                      }
+                      const timestamp = Date.now();

Contributor

dselman Dec 8, 2021

We should allow the caller to pass in the timestamp, only using Date.now if not specified. E.g. to ensure determinism if this is running inside a blockchain node.

Member

martinhalford Dec 10, 2021

@sanketshevkar to fix.

packages/cicero-core/src/contractinstance.js Outdated

+                          'Signing in Progress'
+                      );
+                      return await this.toArchive('ergo');

Contributor

dselman Dec 8, 2021

I don't think you need to await the Promise as you are returning it.

Member

martinhalford Dec 10, 2021

@sanketshevkar to fix.

packages/cicero-core/src/contractinstance.js Outdated

+                  sign(p12File, passphrase, timestamp, signatory) {
+                      if (typeof(p12File) !== 'string') {throw new Error('p12File should be of type String!');}
+                      if (typeof(passphrase) !== 'string') {throw new Error('passphrase should be of type String!');}
+                      if (typeof(timestamp) !== 'number') {throw new Error('timeStamp should be of type Number!');}

Contributor

dselman Dec 8, 2021

Typo in timeStamp

Member

martinhalford Dec 10, 2021

@sanketshevkar to fix

packages/cicero-core/src/contractinstance.js Outdated

+                  /**
+                   * signs a string made up of contract hash and time stamp using private key derived
+                   * from the keystore
+                   * @param {String} p12File - encoded string of p12 keystore file

Contributor

dselman Dec 8, 2021

Encoded how?

Member

martinhalford Dec 10, 2021

@sanketshevkar to elaborate.

packages/cicero-core/src/util.js

+                  content.signatures = instance.contractSignatures;
+                  const hasher = crypto.createHash('sha256');
+                  hasher.update(stringify(content));

Contributor

dselman Dec 8, 2021

Use template hash

Member

martinhalford Dec 10, 2021

Let's move to some shared class/package. TBD.

packages/cicero-core/src/util.js

+                      currentState: currentState,
+                      currentHash: currentHash
+                  };
+                  instance.history.push(state);

Contributor

dselman Dec 8, 2021

How large do we expect history to get? It is ok that it lives in memory and just gets bigger and bigger?

Member

martinhalford Dec 10, 2021 •

edited

Loading

This is a good question. We may need a couple of different options here for large histories and/or some way to store on immutable blockchains. TBD.

packages/cicero-server/app.js

                   try {
-                      const clause = await initTemplateInstance(req);
+                      const instance = req.params.type === 'clause' ? await initTemplateInstance(req) : req.params.type === 'contract' ? await loadInstance(req) : null;

Contributor

dselman Dec 8, 2021

Why does the caller have to specify the type? We know the type from the template....

Member

martinhalford Dec 10, 2021

TBD.

packages/cicero-server/app.js

+                          );
+                          const archive = await instance.toArchive('ergo');
+                          const file = !req.body.output ? `${process.env.CICERO_CONTRACTS}/${req.params.instanceName}.slc` : `${process.env.CICERO_CONTRACTS}/${req.body.output}.slc`;
+                          fs.writeFileSync(file, archive);

Contributor

dselman Dec 8, 2021

Assumes we have write access to the mounted file system - which may not be true in Docker or Cloud.

Member

martinhalford Dec 10, 2021

Agreed. Let's go with this for the time being and fix in subsequent PR(s). TBD.

packages/cicero-server/app.js

+               * An object containing the data, instatiator's name, author's signatures, party signatures and the history
+               *
+               */
+              app.post('/instantiate/:instanceName', async function (req, httpResponse, next) {

Contributor

dselman Dec 8, 2021

I'm finding these "polymorphic" rest routes confusing. Would it not be easier to have something like:

/templates/foo/contract/bar/trigger to trigger a specific contract, or /contracts/bar/trigger if we don't want to nest contracts under templates?

Member

martinhalford Dec 10, 2021 •

edited

Loading

Agreed. Would be good to have different end points for templates vs. contract instances. TBD.

jeromesimeon self-requested a review

December 8, 2021 16:34

jeromesimeon suggested changes

View reviewed changes

Member

jeromesimeon left a comment

this is great work but this shouldn't be merged into master at this stage. I would suggest instead to merge into the slc branch where we can address the general question around "what is the slc format" along with all the questions that Dan already asked.

Member

jeromesimeon commented Dec 8, 2021 •

edited

Loading

this is great work but this shouldn't be merged into master at this stage. I would suggest instead to merge into the slc branch where we can address the general question around "what is the slc format" along with all the questions that Dan already asked.

Another option is to: make a push for a proper "slc" format in the slc branch and first merge that, then integrate the additional work which is in this PR. (instead of trying to merge both at the same time).

Also, it would be useful to know how this PR relates to #698 (is it just an extension, a rewrite, etc). The merge conflicts makes me wonder if it is based on a slightly outdated version of #698.

sanketshevkar changed the base branch from master to poc-slc

December 9, 2021 09:24

sanketshevkar changed the base branch from poc-slc to master

December 9, 2021 09:24


          conflict(*): resolved

72f29f9

Signed-off-by: sanketshevkar <[email protected]>

sanketshevkar changed the base branch from master to poc-slc

December 9, 2021 09:56

sanketshevkar added 3 commits

December 11, 2021 11:33


          test(server): coverage corection

7cf4233

Signed-off-by: sanketshevkar <[email protected]>


          refactor(server): test corrections

18c2d6a

Signed-off-by: sanketshevkar <[email protected]>


          refactor(*): bug and typo fixes

3c76826

Signed-off-by: sanketshevkar <[email protected]>

Member

jeromesimeon commented Dec 15, 2021

@martinhalford my suggestions to get Sanket's work available for further fixes in a short amount of time:

push the branch "as is" to the repo. I believe Sanket should have the permissions to do that?
have someone rebase Sanket's branch on top of poc-slc
do additional PRs as needed to address Dan's comments

Let me know what you think. The benefit of this is that after 1. anyone can contribute to the work.

Member

jeromesimeon commented Dec 15, 2021

I believe all of those commits are already in the poc-slc branch which seems to indicate that this PR hasn't been properly based on the latest poc-slc.

Member

martinhalford commented Dec 16, 2021

Hi @jeromesimeon, Ok. I'm in agreement if you and @dselman are happy to accept the PR "as is".

(Cc: @sanketshevkar - see above)

jeromesimeon suggested changes

View reviewed changes

Member

jeromesimeon left a comment •

edited

Loading

There are three major blocking issues with this PR:

Not properly based on the poc-slc branch
deletes the whole cicero-transform package, instead importing a fixed version (why?!)
tests are not passing (not sure why they aren't triggered on this PR). Below is a log

  1) ContractInstance
       #sign
         should sign the content hash and timestamp string using the keystore:
     Error: Failed to find history.json in archive file.
      at Function.loadZipFileContents (/Users/jerome.simeon/git/accordproject/cicero/node_modules/@accordproject/ergo-compiler/lib/fileloader.js:47:19)
      at /Users/jerome.simeon/git/accordproject/cicero/packages/cicero-core/lib/instanceloader.js:1417:69
      at Generator.next (<anonymous>)
      at asyncGeneratorStep (lib/instanceloader.js:1305:103)
      at _next (lib/instanceloader.js:1307:194)

  2) ContractInstance
       #verify
         should verify a contract signature:
     Error: Failed to find history.json in archive file.
      at Function.loadZipFileContents (/Users/jerome.simeon/git/accordproject/cicero/node_modules/@accordproject/ergo-compiler/lib/fileloader.js:47:19)
      at /Users/jerome.simeon/git/accordproject/cicero/packages/cicero-core/lib/instanceloader.js:1417:69
      at Generator.next (<anonymous>)
      at asyncGeneratorStep (lib/instanceloader.js:1305:103)
      at _next (lib/instanceloader.js:1307:194)

  3) ContractInstance
       #verify
         should throw error for failed signature verification:
     Error: Failed to find history.json in archive file.
      at Function.loadZipFileContents (/Users/jerome.simeon/git/accordproject/cicero/node_modules/@accordproject/ergo-compiler/lib/fileloader.js:47:19)
      at /Users/jerome.simeon/git/accordproject/cicero/packages/cicero-core/lib/instanceloader.js:1417:69
      at Generator.next (<anonymous>)
      at asyncGeneratorStep (lib/instanceloader.js:1305:103)
      at _next (lib/instanceloader.js:1307:194)

  4) ContractInstance
       #verifySignatures
         should verify all contract signatures:
     Error: Failed to find history.json in archive file.
      at Function.loadZipFileContents (/Users/jerome.simeon/git/accordproject/cicero/node_modules/@accordproject/ergo-compiler/lib/fileloader.js:47:19)
      at /Users/jerome.simeon/git/accordproject/cicero/packages/cicero-core/lib/instanceloader.js:1417:69
      at Generator.next (<anonymous>)
      at asyncGeneratorStep (lib/instanceloader.js:1305:103)
      at _next (lib/instanceloader.js:1307:194)

  5) ContractInstance
       #verifySignatures
         should throw error while verifying the contract signatures:
     Error: Failed to find history.json in archive file.
      at Function.loadZipFileContents (/Users/jerome.simeon/git/accordproject/cicero/node_modules/@accordproject/ergo-compiler/lib/fileloader.js:47:19)
      at /Users/jerome.simeon/git/accordproject/cicero/packages/cicero-core/lib/instanceloader.js:1417:69
      at Generator.next (<anonymous>)
      at asyncGeneratorStep (lib/instanceloader.js:1305:103)
      at _next (lib/instanceloader.js:1307:194)

  6) ContractInstance
       #signContract
         should verify all contract signatures:
     Error: Failed to find history.json in archive file.
      at Function.loadZipFileContents (/Users/jerome.simeon/git/accordproject/cicero/node_modules/@accordproject/ergo-compiler/lib/fileloader.js:47:19)
      at /Users/jerome.simeon/git/accordproject/cicero/packages/cicero-core/lib/instanceloader.js:1417:69
      at Generator.next (<anonymous>)
      at asyncGeneratorStep (lib/instanceloader.js:1305:103)
      at _next (lib/instanceloader.js:1307:194)

As discussed during the last Tech WG call, I'm looking into recreating this PR properly.

This was referenced Jan 24, 2022

Sanket slc history #712

Closed

Sanket's PR on adding history to contract archives + fixes #713

Open

Member

jeromesimeon commented Jan 24, 2022

@martinhalford @sanketshevkar A new PR has been open which replaces this PR at #713 . It would need review to make sure this properly reflect the original work.

Member

jeromesimeon commented Feb 23, 2022

This work is now included in #713

jeromesimeon closed this

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

Labels

None yet