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

Add a PCLI sub command to sign services stream files #6309

Merged
merged 25 commits into from May 5, 2023

Conversation

iwsimon
Copy link
Contributor

@iwsimon iwsimon commented Apr 28, 2023

Description:

Related issue(s):

Fixes #5710
Adds unit tests for #6264

To run pcli command as:

pcli record-stream sign keyFile keyFilePassword keyAlias -d=signedRecordStream -p=toBeSignedRecordStream -hv 0.37.0-allowance-SNAPSHOT

pcli -h, pcli record-stream sign -h, pcli account-balance sign -h to see the help messages

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
@iwsimon iwsimon added this to the v0.38 milestone Apr 28, 2023
@iwsimon iwsimon requested a review from a team as a code owner April 28, 2023 16:26
@iwsimon iwsimon self-assigned this Apr 28, 2023
@github-actions
Copy link

github-actions bot commented Apr 28, 2023

Node: Unit Test Results

  1 363 files    1 363 suites   52m 57s ⏱️
97 594 tests 97 586 ✔️ 8 💤 0
99 225 runs  99 217 ✔️ 8 💤 0

Results for commit cb5b0c9.

♻️ This comment has been updated with latest results.

Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
@github-actions
Copy link

github-actions bot commented Apr 28, 2023

Node: Integration Test Results

    3 files      3 suites   14m 57s ⏱️
151 tests 151 ✔️ 0 💤 0
152 runs  152 ✔️ 0 💤 0

Results for commit cb5b0c9.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Apr 28, 2023

Node: E2E Test Results

    1 files      1 suites   17m 27s ⏱️
310 tests 310 ✔️ 0 💤 0
328 runs  328 ✔️ 0 💤 0

Results for commit cb5b0c9.

♻️ This comment has been updated with latest results.

@nickpoorman nickpoorman modified the milestones: v0.38, v0.39 Apr 28, 2023
…mand

Signed-off-by: Lev Povolotsky <lev@swirldslabs.com>
@povolev15 povolev15 requested review from a team as code owners April 28, 2023 21:19
@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Patch coverage: 91.83% and project coverage change: +90.89 🎉

Comparison is base (238dc6c) 0.00% compared to head (cb5b0c9) 90.89%.

Additional details and impacted files
@@              Coverage Diff               @@
##             develop    #6309       +/-   ##
==============================================
+ Coverage           0   90.89%   +90.89%     
- Complexity         0    17413    +17413     
==============================================
  Files              0     1343     +1343     
  Lines              0    50126    +50126     
  Branches           0     5024     +5024     
==============================================
+ Hits               0    45561    +45561     
- Misses             0     3604     +3604     
- Partials           0      961      +961     
Impacted Files Coverage Δ
.../services/cli/sign/AccountBalanceSigningUtils.java 89.28% <62.50%> (ø)
...ra/services/cli/sign/RecordStreamSigningUtils.java 92.56% <92.56%> (ø)
...ices/cli/sign/InvalidProtobufVersionException.java 100.00% <100.00%> (ø)
...era/services/cli/sign/RecordStreamSignCommand.java 100.00% <100.00%> (ø)
...com/hedera/services/cli/sign/RecordStreamType.java 100.00% <100.00%> (ø)

... and 1338 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
@povolev15 povolev15 self-assigned this Apr 30, 2023
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
@povolev15 povolev15 requested a review from kimbor May 1, 2023 18:30
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
@JeffreyDallas
Copy link
Contributor

Is a test to compare generated signature files with the original signature files ?

Input is record/balance file with original signature file (obtained test net)
then compare recovered signature whether match the old signature file.

david-bakin-sl
david-bakin-sl previously approved these changes May 3, 2023
Copy link
Member

@david-bakin-sl david-bakin-sl left a comment

Choose a reason for hiding this comment

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

LGTM.

Following my standard practice - all comments are for information only, or suggestions that are up to you to judge. No bugs or other required changes that I need.

Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Signed-off-by: Iris Simon <iris.simon@swirldslabs.com>
Copy link
Member

@david-bakin-sl david-bakin-sl left a comment

Choose a reason for hiding this comment

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

LGTM!!

@sonarcloud
Copy link

sonarcloud bot commented May 5, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 23 Code Smells

92.6% 92.6% Coverage
0.0% 0.0% Duplication

Copy link
Member

@nathanklick nathanklick left a comment

Choose a reason for hiding this comment

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

Approved with respect to the Gradle and module changes only.

Copy link
Member

@nathanklick nathanklick left a comment

Choose a reason for hiding this comment

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

Approved with respect to the Gradle and module changes only.

@iwsimon iwsimon requested a review from a team May 5, 2023 20:51
Copy link
Contributor

@povolev15 povolev15 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@povolev15 povolev15 left a comment

Choose a reason for hiding this comment

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

LGTM

@iwsimon iwsimon merged commit c86c0fb into develop May 5, 2023
11 of 13 checks passed
@iwsimon iwsimon deleted the 5710-record-stream-sign branch May 5, 2023 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add a PCLI sub command to sign services stream files
7 participants