Skip to content
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.

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

feat(connector-iroha): adds connector plugin #1169

Merged
merged 2 commits into from
Aug 30, 2021

Conversation

hanxu12
Copy link
Contributor

@hanxu12 hanxu12 commented Jul 26, 2021

  1. Enables Cactus to manipulate Iroha ledger
  2. Supports 19/20 commands and 14/15 queries of the Iroha ledger. (removePeer and fetchCommits have not been fully implemented)
  3. Possibility of starting Iroha and Postgres with random ports.
  4. Possibility of passing arbitrary Iroha admin/node keypairs to the test ledger.
  5. Offers a comprehensive run transaction test suite to validate the support for Iroha ledger commands and queries.
  6. Provides an example of asset transfer between two separate Iroha nodes.

@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2021

Codecov Report

Merging #1169 (985dd14) into main (d9f6d5d) will decrease coverage by 0.13%.
The diff coverage is 67.32%.

❗ Current head 985dd14 differs from pull request most recent head 691bed8. Consider uploading reports for the commit 691bed8 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1169      +/-   ##
==========================================
- Coverage   71.83%   71.69%   -0.14%     
==========================================
  Files         278      292      +14     
  Lines       10019    10525     +506     
  Branches     1242     1282      +40     
==========================================
+ Hits         7197     7546     +349     
- Misses       2159     2302     +143     
- Partials      663      677      +14     
Impacted Files Coverage Δ
...enerated/openapi/typescript-axios/configuration.ts 9.09% <9.09%> (ø)
...cript/generated/openapi/typescript-axios/common.ts 43.47% <43.47%> (ø)
...ain/typescript/prometheus-exporter/data-fetcher.ts 50.00% <50.00%> (ø)
...escript/generated/openapi/typescript-axios/base.ts 58.33% <58.33%> (ø)
...pescript/generated/openapi/typescript-axios/api.ts 61.38% <61.38%> (ø)
...ces/get-prometheus-exporter-metrics-endpoint-v1.ts 62.06% <62.06%> (ø)
...c/main/typescript/plugin-ledger-connector-iroha.ts 71.10% <71.10%> (ø)
...escript/prometheus-exporter/prometheus-exporter.ts 73.33% <73.33%> (ø)
...-connector-iroha/src/main/typescript/public-api.ts 87.50% <87.50%> (ø)
...ypescript/web-services/run-transaction-endpoint.ts 88.37% <88.37%> (ø)
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9f6d5d...691bed8. Read the comment docs.

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hxlaf The contents under packages/cactus-test-tooling/src/main/typescript/iroha/example/ can be safely deleted I assume? Or if they are needed for something then we need to find them a different location because the ./src/main/ folder is designed to hold only code that is meant to be deployed to production (so the directory called example does not sound like it should be in there)

@hanxu12 hanxu12 closed this Jul 28, 2021
@hanxu12 hanxu12 reopened this Jul 28, 2021
@hanxu12 hanxu12 force-pushed the iroha_plugin2 branch 4 times, most recently from 43a4734 to 97b25a0 Compare July 30, 2021 23:35
Copy link

@baziorek baziorek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked all not generated automatically files with "iroha" in directory path. I see 100 files modified, rather not all of them by You so I suggest to remove files not modified by You from the PR - it would be easier for reviewers:). E.g. file rather not modified by You: docs/source/support/xdai.md

I'm not TypeScript specialist, so I've ignored many things connected with style (left commented code).

@petermetz petermetz added enhancement New feature or request Iroha Bugs/features related to the Iroha ledger connector plugin labels Aug 11, 2021
@petermetz petermetz added this to the v0.8.0 milestone Aug 11, 2021
@hanxu12 hanxu12 closed this Aug 11, 2021
@hanxu12 hanxu12 reopened this Aug 12, 2021
@petermetz petermetz changed the title Draft pull request for Iroha connector plugin feat(connector-iroha): adds connector plugin Aug 16, 2021
@hanxu12 hanxu12 closed this Aug 17, 2021
@hanxu12 hanxu12 reopened this Aug 17, 2021
@hanxu12 hanxu12 force-pushed the iroha_plugin2 branch 2 times, most recently from 65bfcd0 to 9a06146 Compare August 18, 2021 02:50
@hanxu12 hanxu12 reopened this Aug 21, 2021
@hanxu12 hanxu12 force-pushed the iroha_plugin2 branch 2 times, most recently from 5577cee to 1c0b9cf Compare August 21, 2021 03:28
Copy link

@baziorek baziorek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are few lines of commented code, but not much, but the PR looks good for me (Hyperledger Iroha's site).

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@petermetz petermetz marked this pull request as ready for review August 24, 2021 22:33
@petermetz petermetz requested review from takeutak and removed request for jonathan-m-hamilton August 24, 2021 22:34
@petermetz
Copy link
Contributor

@hxlaf If you fix the commit lint issue afterwards it should be good to go

@hanxu12
Copy link
Contributor Author

hanxu12 commented Aug 25, 2021

@hxlaf If you fix the commit lint issue afterwards it should be good to go

Thank you! Will talk to you tomorrow!

@takeutak
Copy link
Contributor

@petermetz Thanks for contributing!
We are also planning to release the iroha connector (socket.io typed) using python. Then we would like to develop it according to the following plans.

(1) Iroha connector using python can coexist with this product because of the difference of the communication type and language.
(2) I'd like to make some degree of consistency between the above two types of Iroha connector. Then I'm planning to make a request to enhance the function sets.

If you agree with the above plans, I can immediately approve these works because these works #1169 #1183 is independent of the works on the above plans.
Otherwise, I'd like to make some requests for adjustments to the function sets on these works before approving it.
So, would you mind if you agree with the above plans? @petermetz

@hanxu12 hanxu12 force-pushed the iroha_plugin2 branch 2 times, most recently from 71140ed to 257d84c Compare August 25, 2021 15:54
@hanxu12 hanxu12 force-pushed the iroha_plugin2 branch 4 times, most recently from 3651507 to c0f041f Compare August 29, 2021 19:58
@hanxu12 hanxu12 closed this Aug 29, 2021
@hanxu12 hanxu12 reopened this Aug 29, 2021
1. Enables Cactus to manipulate Iroha ledger
2. Supports 19/20 commands and 14/15 queries of the Iroha ledger
3. Possibility of starting Iroha and Postgres with random ports
4. Possibility of passing arbitrary Iroha admin/node keypairs to the test ledger
5. Offers a comprehensive transaction test suite to validate support for Iroha commands and queries
6. Provides an example of asset transfer between two separate Iroha nodes

Signed-off-by: im8a <[email protected]>
@petermetz petermetz merged commit 4745df0 into hyperledger-cacti:main Aug 30, 2021
@petermetz petermetz deleted the iroha_plugin2 branch August 30, 2021 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Iroha Bugs/features related to the Iroha ledger connector plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants